On Friday, July 31, 2020, 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. > > > > > Cc: stable@vger.kernel.org > > Fixes: f037590fff30 ("rds: fix a leak of kernel memory") > > Fixes: bdbe6fbc6a2f ("RDS: recv.c") > > Suggested-by: Dan Carpenter > > Signed-off-by: Peilin Ye > > --- > > Note: the "real" copy_to_user() happens in put_cmsg(), where > > `cmlen - sizeof(*cm)` equals to `sizeof(cmsg)`. > > > > Reference: https://lwn.net/Articles/417989/ > > > > $ 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 */ > > }; > > > > net/rds/recv.c | 3 ++- > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > diff --git a/net/rds/recv.c b/net/rds/recv.c > > index c8404971d5ab..aba4afe4dfed 100644 > > --- a/net/rds/recv.c > > +++ b/net/rds/recv.c > > @@ -450,12 +450,13 @@ static int rds_still_queued(struct rds_sock *rs, > struct rds_incoming *inc, > > int rds_notify_queue_get(struct rds_sock *rs, struct msghdr *msghdr) > > { > > struct rds_notifier *notifier; > > - struct rds_rdma_notify cmsg = { 0 }; /* fill holes with zero */ > > + struct rds_rdma_notify cmsg; > > unsigned int count = 0, max_messages = ~0U; > > unsigned long flags; > > LIST_HEAD(copy); > > int err = 0; > > > > + memset(&cmsg, 0, sizeof(cmsg)); /* fill holes with zero */ > > It works, but the right solution is to drop 0 from cmsg initialization > and write "struct rds_rdma_notify cmsg = {};" without any memset. No, the right solution is to use memset() over gcc extension. > > Thanks > -- With Best Regards, Andy Shevchenko