Re: [PATCH] RDMA/ucma: fix a kernel-infoleak in ucma_init_qp_attr()

[Jason Gunthorpe <jgg@ziepe.ca>]

On Thu, Feb 03, 2022 at 07:30:57PM +0100, Greg KH wrote:
> On Thu, Feb 03, 2022 at 08:26:16PM +0200, Leon Romanovsky wrote:
> > On Thu, Feb 03, 2022 at 09:14:47PM +0300, Dan Carpenter wrote:
> > > From: Haimin Zhang <tcs.kernel@gmail.com>
> > > 
> > > The ib_copy_ah_attr_to_user() function only initializes "resp.grh" if
> > > the "resp.is_global" flag is set.  Unfortunately, this data is copied to
> > > the user and copying uninitialized stack data to the user is an
> > > information leak.  Zero out the whole struct to be safe.
> > > 
> > > Fixes: 4ba66093bdc6 ("IB/core: Check for global flag when using ah_attr")
> > > Reported-by: TCS Robot <tcs_robot@tencent.com>
> > > Signed-off-by: Haimin Zhang <tcs.kernel@gmail.com>
> > > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> > > Resending through the regular lists.
> > > 
> > > I added parentheses around the sizeof to make checkpatch happy.
> > > s/sizeof resp/sizeof(resp)/.
> > > 
> > >  drivers/infiniband/core/ucma.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > The change is ok, but I prefer to initialize to zero as early as possible.
> > 
> > diff --git a/drivers/infiniband/core/ucma.c b/drivers/infiniband/core/ucma.c
> > index 2b72c4fa9550..6d801ed2e46b 100644
> > +++ b/drivers/infiniband/core/ucma.c
> > @@ -1211,9 +1211,9 @@ static ssize_t ucma_init_qp_attr(struct ucma_file *file,
> >                                  int in_len, int out_len)
> >  {
> >         struct rdma_ucm_init_qp_attr cmd;
> > -       struct ib_uverbs_qp_attr resp;
> > +       struct ib_uverbs_qp_attr resp = {};
> >         struct ucma_context *ctx;
> > -       struct ib_qp_attr qp_attr;
> > +       struct ib_qp_attr qp_attr = {};
> 
> Will that catch all of the padding in the structure?  This seems to come
> up a lot and I never remember...

Yes, last time you asked we went over it.

Jason