Re: [PATCH v3 10/10] RDMA: Replace memset with = {} pattern for ib_respond_udata()

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Removal of `memset` in `qedr_copy_qp_uresp` introduces a kernel stack information leak to user-space due to uninitialized struct padding bytes.
--

commit 5d1e218c098790b95c443563dc2cc0974927c81a
Author: Jason Gunthorpe <jgg@ziepe.ca>

RDMA: Replace memset with = {} pattern for ib_respond_udata()

This patch replaces multiple instances of memset() used for zeroing
response structures with the aggregate initialization pattern = {}. It
also removes a redundant memset() in qedr_copy_qp_uresp().

> diff --git a/drivers/infiniband/hw/qedr/verbs.c b/drivers/infiniband/hw/qedr/verbs.c
> index 79190c5b8b50b..1af908275ca72 100644
> --- a/drivers/infiniband/hw/qedr/verbs.c
> +++ b/drivers/infiniband/hw/qedr/verbs.c
[ ... ]
> @@ -1283,8 +1281,6 @@ static int qedr_copy_qp_uresp(struct qedr_dev *dev,
>  			      struct qedr_qp *qp, struct ib_udata *udata,
>  			      struct qedr_create_qp_uresp *uresp)
>  {
> -	memset(uresp, 0, sizeof(*uresp));
> -
>  	if (qedr_qp_has_sq(qp))
>  		qedr_copy_sq_uresp(dev, uresp, qp);

Does this change expose kernel stack memory to userspace?

The commit message notes that qedr_copy_qp_uresp() is already called with
zeroed memory. If the caller is using aggregate initialization (= {}) on
the stack, this may not completely zero the memory.

Unlike memset(), the C standard does not guarantee that = {} will zero out
implicit padding bytes. The compiler might optimize the initialization by
only zeroing the named fields.

Looking at struct qedr_create_qp_uresp, there appear to be 4 bytes of
implicit padding due to field alignment:

include/uapi/rdma/qedr-abi.h:struct qedr_create_qp_uresp {
	__u32 qp_id;
	__u32 atomic_supported;

	/* SQ */
	__u32 sq_db_offset;
	__u16 sq_icid;
        <--- 2 bytes of padding here to align rq_db_offset

	/* RQ */
	__u32 rq_db_offset;
	__u16 rq_icid;
        <--- 2 bytes of padding here to align rq_db2_offset

	__u32 rq_db2_offset;
	__u32 reserved;
...

Since ib_respond_udata() copies sizeof(*uresp) to userspace, could the
removal of memset() cause residual stack data in these padding holes to be
leaked?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/0-v3-4effdebad75a+e1-rdma_udata_rep_jgg@nvidia.com?part=10