On 2/21/23 8:00 AM, Dennis Dalessandro wrote: > On 2/18/23 1:57 PM, Kees Cook wrote: >> The "cplen" result used by the memcpy() into struct rvt_swqe "wqe" may >> be sized to 80 for struct rvt_ud_wr (which is member "ud_wr", not "wr" >> which is only 40 bytes in size). Change the destination union member so >> the compiler can use the correct bounds check. >> >> struct rvt_swqe { >> union { >> struct ib_send_wr wr; /* don't use wr.sg_list */ >> struct rvt_ud_wr ud_wr; >> ... >> }; >> ... >> }; >> >> Silences false positive memcpy() run-time warning: >> >> memcpy: detected field-spanning write (size 80) of single field "&wqe->wr" at drivers/infiniband/sw/rdmavt/qp.c:2043 (size 40) >> >> Reported-by: Zhang Yi <yizhan@xxxxxxxxxx> >> Link: https://bugzilla.kernel.org/show_bug.cgi?id=216561 >> Cc: Dennis Dalessandro <dennis.dalessandro@xxxxxxxxxxxxxxxxxxxx> >> Cc: Jason Gunthorpe <jgg@xxxxxxxx> >> Cc: Leon Romanovsky <leon@xxxxxxxxxx> >> Cc: linux-rdma@xxxxxxxxxxxxxxx >> Signed-off-by: Kees Cook <keescook@xxxxxxxxxxxx> >> --- >> drivers/infiniband/sw/rdmavt/qp.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/drivers/infiniband/sw/rdmavt/qp.c b/drivers/infiniband/sw/rdmavt/qp.c >> index 3acab569fbb9..3f707e1fa517 100644 >> --- a/drivers/infiniband/sw/rdmavt/qp.c >> +++ b/drivers/infiniband/sw/rdmavt/qp.c >> @@ -2040,7 +2040,7 @@ static int rvt_post_one_wr(struct rvt_qp *qp, >> wqe = rvt_get_swqe_ptr(qp, qp->s_head); >> >> /* cplen has length from above */ >> - memcpy(&wqe->wr, wr, cplen); >> + memcpy(&wqe->ud_wr, wr, cplen); >> >> wqe->length = 0; >> j = 0; > > Thanks for the patch. We've been debating this issue internally since last week. > The problem I have is this makes it look like everything is a "UD" req when it > could be a different QP type. Maybe we just need a comment explaining. For what it's worth I did test this patch. If you could add the comment that'd be great. If not I can touch it up later. Reviewed-by: Dennis Dalessandro <dennis.dalessandro@xxxxxxxxxxxxxxxxxxxx>