[PATCH 1/1] tipc: check return value of nlmsg_new

[PATCH 1/1] tipc: check return value of nlmsg_new

[Pan Bian]

Function nlmsg_new() will return a NULL pointer if there is no enough
memory, and its return value should be checked before it is used.
However, in function tipc_nl_node_get_monitor(), the validation of the
return value of function nlmsg_new() is missed. This patch fixes the
bug.

Signed-off-by: Pan Bian <bianpan2016@163.com>
---
 net/tipc/node.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/net/tipc/node.c b/net/tipc/node.c
index 4512e83..568e48d 100644
--- a/net/tipc/node.c
+++ b/net/tipc/node.c
@@ -2098,6 +2098,8 @@ int tipc_nl_node_get_monitor(struct sk_buff *skb, struct genl_info *info)
 	int err;
 
 	msg.skb = nlmsg_new(NLMSG_GOODSIZE, GFP_KERNEL);
+	if (!msg.skb)
+		return -ENOMEM;
 	msg.portid = info->snd_portid;
 	msg.seq = info->snd_seq;
 
-- 
1.9.1

Re: [PATCH 1/1] tipc: check return value of nlmsg_new

[Joe Perches]

On Sun, 2017-04-23 at 15:09 +0800, Pan Bian wrote:
> Function nlmsg_new() will return a NULL pointer if there is no enough
> memory, and its return value should be checked before it is used.
> However, in function tipc_nl_node_get_monitor(), the validation of the
> return value of function nlmsg_new() is missed. This patch fixes the
> bug.

Hello.

Thanks for the patches.

Are you finding these via a tool or inspection?

If a tool is being used, could you please describe it?

Re: [PATCH 1/1] tipc: check return value of nlmsg_new

[PanBian]

On Sun, Apr 23, 2017 at 12:17:16AM -0700, Joe Perches wrote:
> On Sun, 2017-04-23 at 15:09 +0800, Pan Bian wrote:
> > Function nlmsg_new() will return a NULL pointer if there is no enough
> > memory, and its return value should be checked before it is used.
> > However, in function tipc_nl_node_get_monitor(), the validation of the
> > return value of function nlmsg_new() is missed. This patch fixes the
> > bug.
> 
> Hello.
> 
> Thanks for the patches.
> 
> Are you finding these via a tool or inspection?
> 
> If a tool is being used, could you please describe it?
> 

Yes. I developed a tool to find this kind of bugs.

The detecting idea is simple. In large systems like the Linux kernel, 
most implementations are correct, and incorrect ones are rare. Based on
this observation, we take programs that have different implementations
with others as bugs. For example, in most cases, the return vlaue of
nlmsg_new() is validated and it will not be passed to genlmsg_reply() if
its value is NULL. However, in function tipc_nl_node_get_monitor(), the
validation is missing. The abnormal behavior leads us to believe that
there is a bug.


Thanks for your attention.

Re: [PATCH 1/1] tipc: check return value of nlmsg_new

[Joe Perches]

On Sun, 2017-04-23 at 16:00 +0800, PanBian wrote:
> On Sun, Apr 23, 2017 at 12:17:16AM -0700, Joe Perches wrote:
> > On Sun, 2017-04-23 at 15:09 +0800, Pan Bian wrote:
> > > Function nlmsg_new() will return a NULL pointer if there is no enough
> > > memory, and its return value should be checked before it is used.
> > > However, in function tipc_nl_node_get_monitor(), the validation of the
> > > return value of function nlmsg_new() is missed. This patch fixes the
> > > bug.
> > 
> > Hello.
> > 
> > Thanks for the patches.
> > 
> > Are you finding these via a tool or inspection?
> > 
> > If a tool is being used, could you please describe it?
> > 
> 
> Yes. I developed a tool to find this kind of bugs.
> 
> The detecting idea is simple. In large systems like the Linux kernel, 
> most implementations are correct, and incorrect ones are rare. Based on
> this observation, we take programs that have different implementations
> with others as bugs. For example, in most cases, the return vlaue of
> nlmsg_new() is validated and it will not be passed to genlmsg_reply() if
> its value is NULL. However, in function tipc_nl_node_get_monitor(), the
> validation is missing. The abnormal behavior leads us to believe that
> there is a bug.

Perhaps adding __must_check to some of the appropriate function
declarations/prototypes would help avoid new future misuses.

RE: [PATCH 1/1] tipc: check return value of nlmsg_new

[Jon Maloy]

Acknowledged. Thank you for doing this job.

///jon


> -----Original Message-----
> From: Pan Bian [mailto:bianpan2016@163.com]
> Sent: Sunday, April 23, 2017 03:09 AM
> To: Jon Maloy <jon.maloy@ericsson.com>; Ying Xue
> <ying.xue@windriver.com>; David S. Miller <davem@davemloft.net>
> Cc: netdev@vger.kernel.org; tipc-discussion@lists.sourceforge.net; linux-
> kernel@vger.kernel.org; Pan Bian <bianpan2016@163.com>
> Subject: [PATCH 1/1] tipc: check return value of nlmsg_new
> 
> Function nlmsg_new() will return a NULL pointer if there is no enough
> memory, and its return value should be checked before it is used.
> However, in function tipc_nl_node_get_monitor(), the validation of the
> return value of function nlmsg_new() is missed. This patch fixes the bug.
> 
> Signed-off-by: Pan Bian <bianpan2016@163.com>
> ---
>  net/tipc/node.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/net/tipc/node.c b/net/tipc/node.c index 4512e83..568e48d 100644
> --- a/net/tipc/node.c
> +++ b/net/tipc/node.c
> @@ -2098,6 +2098,8 @@ int tipc_nl_node_get_monitor(struct sk_buff *skb,
> struct genl_info *info)
>  	int err;
> 
>  	msg.skb = nlmsg_new(NLMSG_GOODSIZE, GFP_KERNEL);
> +	if (!msg.skb)
> +		return -ENOMEM;
>  	msg.portid = info->snd_portid;
>  	msg.seq = info->snd_seq;
> 
> --
> 1.9.1
> 

Re: [PATCH 1/1] tipc: check return value of nlmsg_new

[Jon Maloy]

Acknowledged. Thank you for doing this job.

///jon


> -----Original Message-----
> From: Pan Bian [mailto:bianpan2016@163.com]
> Sent: Sunday, April 23, 2017 03:09 AM
> To: Jon Maloy <jon.maloy@ericsson.com>; Ying Xue
> <ying.xue@windriver.com>; David S. Miller <davem@davemloft.net>
> Cc: netdev@vger.kernel.org; tipc-discussion@lists.sourceforge.net; linux-
> kernel@vger.kernel.org; Pan Bian <bianpan2016@163.com>
> Subject: [PATCH 1/1] tipc: check return value of nlmsg_new
> 
> Function nlmsg_new() will return a NULL pointer if there is no enough
> memory, and its return value should be checked before it is used.
> However, in function tipc_nl_node_get_monitor(), the validation of the
> return value of function nlmsg_new() is missed. This patch fixes the bug.
> 
> Signed-off-by: Pan Bian <bianpan2016@163.com>
> ---
>  net/tipc/node.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/net/tipc/node.c b/net/tipc/node.c index 4512e83..568e48d 100644
> --- a/net/tipc/node.c
> +++ b/net/tipc/node.c
> @@ -2098,6 +2098,8 @@ int tipc_nl_node_get_monitor(struct sk_buff *skb,
> struct genl_info *info)
>  	int err;
> 
>  	msg.skb = nlmsg_new(NLMSG_GOODSIZE, GFP_KERNEL);
> +	if (!msg.skb)
> +		return -ENOMEM;
>  	msg.portid = info->snd_portid;
>  	msg.seq = info->snd_seq;
> 
> --
> 1.9.1
> 


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot

Re: [PATCH 1/1] tipc: check return value of nlmsg_new

[David Miller]

From: Pan Bian <bianpan2016@163.com>
Date: Sun, 23 Apr 2017 15:09:19 +0800

> Function nlmsg_new() will return a NULL pointer if there is no enough
> memory, and its return value should be checked before it is used.
> However, in function tipc_nl_node_get_monitor(), the validation of the
> return value of function nlmsg_new() is missed. This patch fixes the
> bug.
> 
> Signed-off-by: Pan Bian <bianpan2016@163.com>

Applied.