Thread (28 messages) 28 messages, 10 authors, 2020-08-14

Re: [Linux-kernel-mentees] [PATCH net] rds: Prevent kernel-infoleak in rds_notify_queue_get()

From: Jason Gunthorpe <jgg@ziepe.ca>
Date: 2020-07-31 14:04:57
Also in: linux-kernel-mentees, linux-rdma, lkml

On Fri, Jul 31, 2020 at 07:33:33AM +0200, Greg Kroah-Hartman wrote:
On Fri, Jul 31, 2020 at 07:33:06AM +0200, Greg Kroah-Hartman wrote:
quoted
On Fri, Jul 31, 2020 at 07:53:01AM +0300, Leon Romanovsky wrote:
quoted
On Thu, Jul 30, 2020 at 03:20:26PM -0400, Peilin Ye wrote:
quoted
rds_notify_queue_get() is potentially copying uninitialized kernel stack
memory to userspace since the compiler may leave a 4-byte hole at the end
of `cmsg`.

In 2016 we tried to fix this issue by doing `= { 0 };` on `cmsg`, which
unfortunately does not always initialize that 4-byte hole. Fix it by using
memset() instead.
Of course, this is the difference between "{ 0 }" and "{}" initializations.
Really?  Neither will handle structures with holes in it, try it and
see.
And if true, where in the C spec does it say that?
The spec was updated in C11 to require zero'ing padding when doing
partial initialization of aggregates (eg = {})

"""if it is an aggregate, every member is initialized (recursively)
according to these rules, and any padding is initialized to zero
bits;"""

The difference between {0} and the {} extension is only that {}
reliably triggers partial initialization for all kinds of aggregates,
while {0} has a number of edge cases where it can fail to compile.

IIRC gcc has cleared the padding during aggregate initialization for a
long time. Considering we have thousands of aggregate initializers it
seems likely to me Linux also requires a compiler with this C11
behavior to operate correctly.

Does this patch actually fix anything? My compiler generates identical
assembly code in either case.

Jason
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help