On Wed, Mar 12, 2025 at 09:53:17AM +0300, Dan Carpenter wrote:
> Hello Chiara Meiohas,
>
> Commit 61e51682816d ("RDMA/uverbs: Introduce UCAP (User CAPabilities)
> API") from Mar 6, 2025 (linux-next), leads to the following Smatch
> static checker warning:
>
> drivers/infiniband/core/ucaps.c:209 ib_release_ucap()
> error: buffer overflow 'ucaps_list' 2 <= 2 (assuming for loop doesn't break)
The thing is that we must have "break", so writing if(WARN_ON(type ==
"RDMA_UCAP_MAX)) return;" instead of existing WARN_ON is very
misleading.
Thanks
>
> drivers/infiniband/core/ucaps.c
> 198 static void ib_release_ucap(struct kref *ref)
> 199 {
> 200 struct ib_ucap *ucap = container_of(ref, struct ib_ucap, ref);
> 201 enum rdma_user_cap type;
> 202
> 203 for (type = RDMA_UCAP_FIRST; type < RDMA_UCAP_MAX; type++) {
> 204 if (ucaps_list[type] == ucap)
> 205 break;
> 206 }
> 207 WARN_ON(type == RDMA_UCAP_MAX);
>
> This prints a warning if we're out of bounds, but it doesn't handle the
> error. This is called from kref_put() and with kref_put() this could
> actually be done in a different thread with a delay from when
> ib_remove_ucap() is called. I wouldn't advise that for production systems
> but it's supposed to work.
>
> So this code makes me quite nervous.
>
> 208
> --> 209 ucaps_list[type] = NULL;
> 210 cdev_device_del(&ucap->cdev, &ucap->dev);
> 211 put_device(&ucap->dev);
> 212 }
>
> regards,
> dan carpenter
>
