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 <[email protected]> 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/[email protected]?part=10
