| Message ID | 1452597043-4298-1-git-send-email-sploving1@gmail.com |
|---|---|
| State | Rejected, archived |
| Delegated to: | David Miller |
| Headers | show |
On 1/12/16, Baozeng Ding <sploving1@gmail.com> wrote: > If groups is not 0 and nlk->groups is NULL, it will not return > immediately and cause a null pointer dereference later. > > Signed-off-by: Baozeng Ding <sploving1@gmail.com> > --- > The v3 version adds WARN_ON, suggested by David Miller. Thanks for > David's feedback. But can you explain how can we get to this situation? > --- > net/netlink/af_netlink.c | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/net/netlink/af_netlink.c b/net/netlink/af_netlink.c > index 59651af..f93d579 100644 > --- a/net/netlink/af_netlink.c > +++ b/net/netlink/af_netlink.c > @@ -1576,7 +1576,10 @@ static int netlink_bind(struct socket *sock, struct > sockaddr *addr, > } > } > > - if (!groups && (nlk->groups == NULL || !(u32)nlk->groups[0])) > + if (WARN_ON(!nlk->groups)) > + return 0; > + > + if (!groups && !(u32)nlk->groups[0]) > return 0; > > netlink_table_grab(); > -- > 1.9.1 > >
Denis Kirjanov <kda@linux-powerpc.org> wrote: > On 1/12/16, Baozeng Ding <sploving1@gmail.com> wrote: >> If groups is not 0 and nlk->groups is NULL, it will not return >> immediately and cause a null pointer dereference later. >> >> Signed-off-by: Baozeng Ding <sploving1@gmail.com> >> --- >> The v3 version adds WARN_ON, suggested by David Miller. Thanks for >> David's feedback. > > But can you explain how can we get to this situation? Indeed, this patch makes no sense as it stands. If groups is not NULL, then we should have allocated nlk->groups prior to reaching this point. If that allocation failed, then we should have already bailed out long ago. Perhaps we have a race condition that needs to be closed with a lock? Cheers,
From: Baozeng Ding <sploving1@gmail.com> Date: Tue, 12 Jan 2016 19:10:43 +0800 > If groups is not 0 and nlk->groups is NULL, it will not return > immediately and cause a null pointer dereference later. > > Signed-off-by: Baozeng Ding <sploving1@gmail.com> > --- > The v3 version adds WARN_ON, suggested by David Miller. Thanks for > David's feedback. My feedback was also that the situation is illegal, and therefore you have to explain to us how this can actually occur.
diff --git a/net/netlink/af_netlink.c b/net/netlink/af_netlink.c index 59651af..f93d579 100644 --- a/net/netlink/af_netlink.c +++ b/net/netlink/af_netlink.c @@ -1576,7 +1576,10 @@ static int netlink_bind(struct socket *sock, struct sockaddr *addr, } } - if (!groups && (nlk->groups == NULL || !(u32)nlk->groups[0])) + if (WARN_ON(!nlk->groups)) + return 0; + + if (!groups && !(u32)nlk->groups[0]) return 0; netlink_table_grab();
If groups is not 0 and nlk->groups is NULL, it will not return immediately and cause a null pointer dereference later. Signed-off-by: Baozeng Ding <sploving1@gmail.com> --- The v3 version adds WARN_ON, suggested by David Miller. Thanks for David's feedback. --- net/netlink/af_netlink.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)