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 <[email protected]>
---
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;
+
memcpy(NLMSG_DATA(nlh), msg, len);
NETLINK_CB(skb).portid = 0;
NETLINK_CB(skb).dst_group = 0;
--
2.7.4
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 <[email protected]>
> ---
> 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
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 <[email protected]>
---
v1->v2:
- Add some cleanup
---
drivers/staging/gdm724x/netlink_k.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/drivers/staging/gdm724x/netlink_k.c b/drivers/staging/gdm724x/netlink_k.c
index abe2425..16da03b 100644
--- a/drivers/staging/gdm724x/netlink_k.c
+++ b/drivers/staging/gdm724x/netlink_k.c
@@ -119,6 +119,11 @@ 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) {
+ kfree_skb(skb);
+ return -EMSGSIZE;
+ }
+
memcpy(NLMSG_DATA(nlh), msg, len);
NETLINK_CB(skb).portid = 0;
NETLINK_CB(skb).dst_group = 0;
--
2.7.4
On Thu, Jun 14, 2018 at 07:31:51PM +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 <[email protected]>
> ---
> v1->v2:
> - Add some cleanup
> ---
> drivers/staging/gdm724x/netlink_k.c | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/drivers/staging/gdm724x/netlink_k.c b/drivers/staging/gdm724x/netlink_k.c
> index abe2425..16da03b 100644
> --- a/drivers/staging/gdm724x/netlink_k.c
> +++ b/drivers/staging/gdm724x/netlink_k.c
> @@ -119,6 +119,11 @@ 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) {
> + kfree_skb(skb);
> + return -EMSGSIZE;
> + }
As I mentioned earlier, this patch is not required because the we just
allocated the skb and we know that it is large enough.
regards,
dan carpenter
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
Also I think the email list is rejecting your replies because they're
in html?
regards,
dan carpenter