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

Håkon Bugge
2020-07-31T12:05:29Z

> On 31 Jul 2020, at 13:59, Greg Kroah-Hartman <gregkh@linuxfoundation.org> wrote:
>
> On Fri, Jul 31, 2020 at 01:14:09PM +0200, Håkon Bugge wrote:
>>
>>
>>> On 31 Jul 2020, at 11:59, Dan Carpenter <dan.carpenter@oracle.com> wrote:
>>>
>>> On Fri, Jul 31, 2020 at 07:53:01AM +0300, Leon Romanovsky wrote:
>>>> On Thu, Jul 30, 2020 at 03:20:26PM -0400, Peilin Ye wrote:
>>>>> 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.
>>>>
>>>
>>> No, there is no difference. Even struct assignments like:
>>>
>>> foo = *bar;
>>>
>>> can leave struct holes uninitialized. Depending on the compiler the
>>> assignment can be implemented as a memset() or as a series of struct
>>> member assignments.
>>
>> What about:
>>
>> struct rds_rdma_notify {
>> __u64 user_token;
>> __s32 status;
>> } __attribute__((packed));
>
> Why is this still a discussion at all?
>
> Try it and see, run pahole and see if there are holes in this structure
> (odds are no), you don't need us to say what is happening here...

An older posting had this:

$ pahole -C "rds_rdma_notify" net/rds/recv.o
struct rds_rdma_notify {
__u64 user_token; /* 0 8 */
__s32 status; /* 8 4 */

/* size: 16, cachelines: 1, members: 2 */
/* padding: 4 */
/* last cacheline: 16 bytes */
};

Thxs, Håkon