| Message ID | Zva71Yf3F94uxi5A@iZbp1asjb3cy8ks0srf007Z (mailing list archive) |
|---|---|
| State | Accepted |
| Delegated to: | Jason Gunthorpe |
| Headers | show |
| Series | [v2] RDMA/nldev: Fix NULL pointer dereferences issue in rdma_nl_notify_event | expand |
On Fri, Sep 27, 2024 at 10:06:13PM +0800, Qianqiang Liu wrote: > nlmsg_put() may return a NULL pointer assigned to nlh, which will later > be dereferenced in nlmsg_end(). > > Fixes: 9cbed5aab5ae ("RDMA/nldev: Add support for RDMA monitoring") > Signed-off-by: Qianqiang Liu <qianqiang.liu@163.com> > --- > Changes since v1: > - Add Fixes tag > --- > drivers/infiniband/core/nldev.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/drivers/infiniband/core/nldev.c b/drivers/infiniband/core/nldev.c > index 39f89a4b86498..7dc8e2ec62cc8 100644 > --- a/drivers/infiniband/core/nldev.c > +++ b/drivers/infiniband/core/nldev.c > @@ -2816,6 +2816,8 @@ int rdma_nl_notify_event(struct ib_device *device, u32 port_num, > nlh = nlmsg_put(skb, 0, 0, > RDMA_NL_GET_TYPE(RDMA_NL_NLDEV, RDMA_NLDEV_CMD_MONITOR), > 0, 0); > + if (!nlh) > + goto err_free; It doesn't look to me like nlmsg_put can fail in this usage, but we should probbaly put the if to avoid getting static checkers warning on it. Applied to for-rc, thanks Jason
On Fri, Oct 04, 2024 at 05:13:02PM -0300, Jason Gunthorpe wrote: > On Fri, Sep 27, 2024 at 10:06:13PM +0800, Qianqiang Liu wrote: > > nlmsg_put() may return a NULL pointer assigned to nlh, which will later > > be dereferenced in nlmsg_end(). > > > > Fixes: 9cbed5aab5ae ("RDMA/nldev: Add support for RDMA monitoring") > > Signed-off-by: Qianqiang Liu <qianqiang.liu@163.com> > > --- > > Changes since v1: > > - Add Fixes tag > > --- > > drivers/infiniband/core/nldev.c | 2 ++ > > 1 file changed, 2 insertions(+) > > > > diff --git a/drivers/infiniband/core/nldev.c b/drivers/infiniband/core/nldev.c > > index 39f89a4b86498..7dc8e2ec62cc8 100644 > > --- a/drivers/infiniband/core/nldev.c > > +++ b/drivers/infiniband/core/nldev.c > > @@ -2816,6 +2816,8 @@ int rdma_nl_notify_event(struct ib_device *device, u32 port_num, > > nlh = nlmsg_put(skb, 0, 0, > > RDMA_NL_GET_TYPE(RDMA_NL_NLDEV, RDMA_NLDEV_CMD_MONITOR), > > 0, 0); > > + if (!nlh) > > + goto err_free; > > It doesn't look to me like nlmsg_put can fail in this usage, but we > should probbaly put the if to avoid getting static checkers warning on > it. > > Applied to for-rc, thanks It's difficult for static checkers to predict when a function can *really* return NULL or not. Smatch checks allocation functions and that's basically it. I suspect there is a heuristic here where it warns if the percent of callers that check is over 70% or something. regards, dan carpenter
diff --git a/drivers/infiniband/core/nldev.c b/drivers/infiniband/core/nldev.c index 39f89a4b86498..7dc8e2ec62cc8 100644 --- a/drivers/infiniband/core/nldev.c +++ b/drivers/infiniband/core/nldev.c @@ -2816,6 +2816,8 @@ int rdma_nl_notify_event(struct ib_device *device, u32 port_num, nlh = nlmsg_put(skb, 0, 0, RDMA_NL_GET_TYPE(RDMA_NL_NLDEV, RDMA_NLDEV_CMD_MONITOR), 0, 0); + if (!nlh) + goto err_free; switch (type) { case RDMA_REGISTER_EVENT:
nlmsg_put() may return a NULL pointer assigned to nlh, which will later be dereferenced in nlmsg_end(). Fixes: 9cbed5aab5ae ("RDMA/nldev: Add support for RDMA monitoring") Signed-off-by: Qianqiang Liu <qianqiang.liu@163.com> --- Changes since v1: - Add Fixes tag --- drivers/infiniband/core/nldev.c | 2 ++ 1 file changed, 2 insertions(+)