Re: [PATCH] staging: gdm724x: add error handling for nlmsg_put
Also I think the email list is rejecting your replies because they're in html? regards, dan carpenter
Re: [PATCH] staging: gdm724x: add error handling for nlmsg_put
Also I think the email list is rejecting your replies because they're in html? regards, dan carpenter
Re: [PATCH] staging: gdm724x: add error handling for nlmsg_put
On Thu, Jun 14, 2018 at 11:27:26AM +0800, Zhouyang Jia wrote: > Hi, > > I reported this bug since more than 95% callsites of nlmsg_put are > well handled in kernel, and many of them also allocated skb before. > That's probably true, but the NULL check is not required *here*. Just mark it as a false positive in your tool and move on... It's not like we are close to running out of real bugs to deal with. regards, dan carpenter
Re: [PATCH] staging: gdm724x: add error handling for nlmsg_put
On Thu, Jun 14, 2018 at 11:27:26AM +0800, Zhouyang Jia wrote: > Hi, > > I reported this bug since more than 95% callsites of nlmsg_put are > well handled in kernel, and many of them also allocated skb before. > That's probably true, but the NULL check is not required *here*. Just mark it as a false positive in your tool and move on... It's not like we are close to running out of real bugs to deal with. regards, dan carpenter
Re: [PATCH] staging: gdm724x: add error handling for nlmsg_put
On Mon, Jun 11, 2018 at 01:09:24PM +0800, Zhouyang Jia wrote: > When nlmsg_put fails, the lack of error-handling code may > cause unexpected results. > > This patch adds error-handling code after calling nlmsg_put. > > Signed-off-by: Zhouyang Jia > --- > drivers/staging/gdm724x/netlink_k.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/drivers/staging/gdm724x/netlink_k.c > b/drivers/staging/gdm724x/netlink_k.c > index abe2425..4623140 100644 > --- a/drivers/staging/gdm724x/netlink_k.c > +++ b/drivers/staging/gdm724x/netlink_k.c > @@ -119,6 +119,9 @@ int netlink_send(struct sock *sock, int group, u16 type, > void *msg, int len) > seq++; > > nlh = nlmsg_put(skb, 0, seq, type, len, 0); > + if (!nlh) > + return -EMSGSIZE; This can't fail. We just allocated skb on the line before and we allocated enough space for "len". Also if we *did* hit a bug here then we would need to do some more cleanup instead of a direct return. regards, dan carpenter
Re: [PATCH] staging: gdm724x: add error handling for nlmsg_put
On Mon, Jun 11, 2018 at 01:09:24PM +0800, Zhouyang Jia wrote: > When nlmsg_put fails, the lack of error-handling code may > cause unexpected results. > > This patch adds error-handling code after calling nlmsg_put. > > Signed-off-by: Zhouyang Jia > --- > drivers/staging/gdm724x/netlink_k.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/drivers/staging/gdm724x/netlink_k.c > b/drivers/staging/gdm724x/netlink_k.c > index abe2425..4623140 100644 > --- a/drivers/staging/gdm724x/netlink_k.c > +++ b/drivers/staging/gdm724x/netlink_k.c > @@ -119,6 +119,9 @@ int netlink_send(struct sock *sock, int group, u16 type, > void *msg, int len) > seq++; > > nlh = nlmsg_put(skb, 0, seq, type, len, 0); > + if (!nlh) > + return -EMSGSIZE; This can't fail. We just allocated skb on the line before and we allocated enough space for "len". Also if we *did* hit a bug here then we would need to do some more cleanup instead of a direct return. regards, dan carpenter