| Message ID | AANLkTikp0m8cQYfEBHP_E8XpGZqdMj6Cr4M=aWCnei=X@mail.gmail.com |
|---|---|
| State | Rejected, archived |
| Delegated to: | David Miller |
| Headers | show |
On Wed, Sep 8, 2010 at 1:19 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote: > Le mercredi 08 septembre 2010 à 13:13 +0800, jovi zhang a écrit : >> It will kfree NULL pointer if listeners is NULL. fix it. >> >> Signed-off-by: bookjovi@gmail.com >> net/netlink/af_netlink.c | 5 +++-- >> 1 files changed, 3 insertions(+), 2 deletions(-) >> >> diff --git a/net/netlink/af_netlink.c b/net/netlink/af_netlink.c >> index 980fe4a..1c7bf48 100644 >> --- a/net/netlink/af_netlink.c >> +++ b/net/netlink/af_netlink.c >> @@ -1532,8 +1532,10 @@ netlink_kernel_create(struct net *net, int >> unit, unsigned int groups, >> if (input) >> nlk_sk(sk)->netlink_rcv = input; >> >> - if (netlink_insert(sk, net, 0)) >> + if (netlink_insert(sk, net, 0)) { >> + kfree(listeners); >> goto out_sock_release; >> + } >> >> nlk = nlk_sk(sk); >> nlk->flags |= NETLINK_KERNEL_SOCKET; >> @@ -1553,7 +1555,6 @@ netlink_kernel_create(struct net *net, int unit, >> unsigned int groups, >> return sk; >> >> out_sock_release: >> - kfree(listeners); >> netlink_kernel_release(sk); >> return NULL; > > > This patch is not needed > > kfree(NULL) is legal > > > > YES, maybe kfree(NULL) is legal, but I cannot see there have any need to invoke kfree(NULL) in this function. Also I check kfree usage in other code, I havn't find any kfree(NULL) usage. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
From: jovi zhang <bookjovi@gmail.com> Date: Wed, 8 Sep 2010 13:33:28 +0800 > Also I check kfree usage in other code, I havn't find any kfree(NULL) usage. You aren't looking hard enough. Please leave this code alone, it's functionally correct and fine. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
jovi zhang <bookjovi@gmail.com> writes: > On Wed, Sep 8, 2010 at 1:19 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote: >> >> >> This patch is not needed >> >> kfree(NULL) is legal >> >> >> >> > > YES, maybe kfree(NULL) is legal, but I cannot see there have any need > to invoke kfree(NULL) in this function. > Also I check kfree usage in other code, I havn't find any kfree(NULL) > usage. Usually kfree(NULL) is not explicit, but "var = NULL; kfree(var)" is common in error handling paths to reduce the complexity of error handling, making bugs less likely. Eric -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Le mercredi 08 septembre 2010 à 13:33 +0800, jovi zhang a écrit : > YES, maybe kfree(NULL) is legal, but I cannot see there have any need > to invoke kfree(NULL) in this function. > Also I check kfree usage in other code, I havn't find any kfree(NULL) usage. We have hundred call sites doing kfree(NULL), if it makes code shorter. Its usually code path handling exceptions (errors if you prefer), and we prefer to use ugly gotos, and even kfree(NULL), to make it as short as possible. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Sep 8, 2010 at 1:55 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote: > Le mercredi 08 septembre 2010 à 13:33 +0800, jovi zhang a écrit : > >> YES, maybe kfree(NULL) is legal, but I cannot see there have any need >> to invoke kfree(NULL) in this function. >> Also I check kfree usage in other code, I havn't find any kfree(NULL) usage. > > We have hundred call sites doing kfree(NULL), if it makes code shorter. > > Its usually code path handling exceptions (errors if you prefer), and we > prefer to use ugly gotos, and even kfree(NULL), to make it as short as > possible. OK, I understand. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/net/netlink/af_netlink.c b/net/netlink/af_netlink.c index 980fe4a..1c7bf48 100644 --- a/net/netlink/af_netlink.c +++ b/net/netlink/af_netlink.c @@ -1532,8 +1532,10 @@ netlink_kernel_create(struct net *net, int unit, unsigned int groups, if (input) nlk_sk(sk)->netlink_rcv = input; - if (netlink_insert(sk, net, 0)) + if (netlink_insert(sk, net, 0)) { + kfree(listeners); goto out_sock_release; + } nlk = nlk_sk(sk);
It will kfree NULL pointer if listeners is NULL. fix it. Signed-off-by: bookjovi@gmail.com net/netlink/af_netlink.c | 5 +++-- 1 files changed, 3 insertions(+), 2 deletions(-) nlk->flags |= NETLINK_KERNEL_SOCKET; @@ -1553,7 +1555,6 @@ netlink_kernel_create(struct net *net, int unit, unsigned int groups, return sk; out_sock_release: - kfree(listeners); netlink_kernel_release(sk); return NULL; -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html