> -----Original Message-----
> From: Dan Carpenter <dan.carpenter@xxxxxxxxxx>
> Sent: Wednesday, October 25, 2023 1:57 PM
> To: Bernard Metzler <BMT@xxxxxxxxxxxxxx>
> Cc: linux-rdma@xxxxxxxxxxxxxxx
> Subject: [EXTERNAL] [bug report] rdma/siw: connection management
>
> Hello Bernard Metzler,
>
> The patch 6c52fdc244b5: "rdma/siw: connection management" from Jun
> 20, 2019 (linux-next), leads to the following Smatch static checker
> warning:
>
> drivers/infiniband/sw/siw/siw_cm.c:1560 siw_accept()
> error: double free of 'cep->mpa.pdata'
>
> drivers/infiniband/sw/siw/siw_cm.c
> 1545 int siw_accept(struct iw_cm_id *id, struct iw_cm_conn_param
> *params)
> 1546 {
> 1547 struct siw_device *sdev = to_siw_dev(id->device);
> 1548 struct siw_cep *cep = (struct siw_cep *)id->provider_data;
> 1549 struct siw_qp *qp;
> 1550 struct siw_qp_attrs qp_attrs;
> 1551 int rv, max_priv_data = MPA_MAX_PRIVDATA;
> 1552 bool wait_for_peer_rts = false;
> 1553
> 1554 siw_cep_set_inuse(cep);
> 1555 siw_cep_put(cep);
> ^^^^^^^^^^^^^^^^^
>
> This potentially calls __siw_cep_dealloc() which frees cep->mpa.pdata.
>
> 1556
> 1557 /* Free lingering inbound private data */
> 1558 if (cep->mpa.hdr.params.pd_len) {
> 1559 cep->mpa.hdr.params.pd_len = 0;
> --> 1560 kfree(cep->mpa.pdata);
> ^^^^^^^^^^^^^^
> Double free?
>
> 1561 cep->mpa.pdata = NULL;
> 1562 }
> 1563 siw_cancel_mpatimer(cep);
>
> See also:
> drivers/infiniband/hw/erdma/erdma_cm.c:1141 erdma_accept() error: double
> free of 'cep->mpa.pdata'
>
> regards,
> dan carpenter
Thanks Dan.
siw_cep_put() only calls kfree() on cep->mpa.pdata
if cep was on its last reference. It then frees cep as well and
no further reference to cep is allowed. This cannot be the case here.
To satisfy Smatch, without changing functionality we can
reorder and first explicitly free any mpa.pdata and put it NULL
before calling siw_cep_put(). I don't like it though, because
it just satisfies Smatch but sacrifies readability.
Bernard.
