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

[Leon Romanovsky <leon@kernel.org>]

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
--- a/drivers/infiniband/core/ucma.c
+++ 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 = {};
        int ret;

        if (out_len < sizeof(resp))
@@ -1229,8 +1229,6 @@ static ssize_t ucma_init_qp_attr(struct ucma_file *file,
        if (IS_ERR(ctx))
                return PTR_ERR(ctx);

-       resp.qp_attr_mask = 0;
-       memset(&qp_attr, 0, sizeof qp_attr);
        qp_attr.qp_state = cmd.qp_state;
        mutex_lock(&ctx->mutex);
        ret = rdma_init_qp_attr(ctx->cm_id, &qp_attr, &resp.qp_attr_mask);


> 
> diff --git a/drivers/infiniband/core/ucma.c b/drivers/infiniband/core/ucma.c
> index 9d6ac9dff39a..91485f13d842 100644
> --- a/drivers/infiniband/core/ucma.c
> +++ b/drivers/infiniband/core/ucma.c
> @@ -1232,7 +1232,7 @@ static ssize_t ucma_init_qp_attr(struct ucma_file *file,
>  	if (IS_ERR(ctx))
>  		return PTR_ERR(ctx);
>  
> -	resp.qp_attr_mask = 0;
> +	memset(&resp, 0, sizeof(resp));
>  	memset(&qp_attr, 0, sizeof qp_attr);
>  	qp_attr.qp_state = cmd.qp_state;
>  	mutex_lock(&ctx->mutex);
> -- 
> 2.20.1
>