diff mbox series

[2/4] netlink: Move an assignment for the variable “sk” in __netlink_kernel_create()

Message ID 223a61e9-f826-4f37-b514-ca6ed53b1269@web.de (mailing list archive)
State Changes Requested
Headers show
Series netlink: Adjustments for __netlink_kernel_create() | expand

Checks

Context Check Description
netdev/series_format warning Target tree name not specified in the subject
netdev/tree_selection success Guessed tree name to be net-next
netdev/ynl success Generated files up to date; no warnings/errors; no diff in generated;
netdev/fixes_present success Fixes tag not required for -next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 1118 this patch: 1118
netdev/cc_maintainers success CCed 4 of 4 maintainers
netdev/build_clang fail Errors and warnings before: 1141 this patch: 1144
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 1145 this patch: 1145
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 15 lines checked
netdev/build_clang_rust success No Rust files in patch. Skipping build
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Markus Elfring Dec. 31, 2023, 5:44 p.m. UTC 
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sun, 31 Dec 2023 17:36:50 +0100

Move one assignment for the variable “sk” closer to the place
where this pointer is used.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 net/netlink/af_netlink.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

--
2.43.0

Comments

kernel test robot Jan. 1, 2024, 12:54 a.m. UTC | #1 
Hi Markus,

kernel test robot noticed the following build warnings:

[auto build test WARNING on net/main]
[also build test WARNING on net-next/main linus/master horms-ipvs/master v6.7-rc8 next-20231222]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Markus-Elfring/netlink-Improve-exception-handling-in-__netlink_kernel_create/20240101-015025
base:   net/main
patch link:    https://lore.kernel.org/r/223a61e9-f826-4f37-b514-ca6ed53b1269%40web.de
patch subject: [PATCH 2/4] netlink: Move an assignment for the variable “sk” in __netlink_kernel_create()
config: arm-randconfig-001-20240101 (https://download.01.org/0day-ci/archive/20240101/202401010855.4iU6im1B-lkp@intel.com/config)
compiler: clang version 18.0.0git (https://github.com/llvm/llvm-project 8a4266a626914765c0c69839e8a51be383013c1a)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240101/202401010855.4iU6im1B-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202401010855.4iU6im1B-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> net/netlink/af_netlink.c:2044:6: warning: variable 'sk' is used uninitialized whenever 'if' condition is true [-Wsometimes-uninitialized]
    2044 |         if (!listeners)
         |             ^~~~~~~~~~
   net/netlink/af_netlink.c:2081:25: note: uninitialized use occurs here
    2081 |         netlink_kernel_release(sk);
         |                                ^~
   net/netlink/af_netlink.c:2044:2: note: remove the 'if' if its condition is always false
    2044 |         if (!listeners)
         |         ^~~~~~~~~~~~~~~
    2045 |                 goto out_netlink_release_sock;
         |                 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   net/netlink/af_netlink.c:2021:17: note: initialize the variable 'sk' to silence this warning
    2021 |         struct sock *sk;
         |                        ^
         |                         = NULL
   1 warning generated.


vim +2044 net/netlink/af_netlink.c

^1da177e4c3f41 Linus Torvalds    2005-04-16  2009  
^1da177e4c3f41 Linus Torvalds    2005-04-16  2010  /*
^1da177e4c3f41 Linus Torvalds    2005-04-16  2011   *	We export these functions to other modules. They provide a
^1da177e4c3f41 Linus Torvalds    2005-04-16  2012   *	complete set of kernel non-blocking support for message
^1da177e4c3f41 Linus Torvalds    2005-04-16  2013   *	queueing.
^1da177e4c3f41 Linus Torvalds    2005-04-16  2014   */
^1da177e4c3f41 Linus Torvalds    2005-04-16  2015  
^1da177e4c3f41 Linus Torvalds    2005-04-16  2016  struct sock *
9f00d9776bc5be Pablo Neira Ayuso 2012-09-08  2017  __netlink_kernel_create(struct net *net, int unit, struct module *module,
a31f2d17b331db Pablo Neira Ayuso 2012-06-29  2018  			struct netlink_kernel_cfg *cfg)
^1da177e4c3f41 Linus Torvalds    2005-04-16  2019  {
^1da177e4c3f41 Linus Torvalds    2005-04-16  2020  	struct socket *sock;
^1da177e4c3f41 Linus Torvalds    2005-04-16  2021  	struct sock *sk;
77247bbb309424 Patrick McHardy   2005-08-14  2022  	struct netlink_sock *nlk;
5c398dc8f5a58b Eric Dumazet      2010-10-24  2023  	struct listeners *listeners = NULL;
a31f2d17b331db Pablo Neira Ayuso 2012-06-29  2024  	struct mutex *cb_mutex = cfg ? cfg->cb_mutex : NULL;
a31f2d17b331db Pablo Neira Ayuso 2012-06-29  2025  	unsigned int groups;
^1da177e4c3f41 Linus Torvalds    2005-04-16  2026  
fab2caf62ed03d Akinobu Mita      2006-08-29  2027  	BUG_ON(!nl_table);
^1da177e4c3f41 Linus Torvalds    2005-04-16  2028  
^1da177e4c3f41 Linus Torvalds    2005-04-16  2029  	if (unit < 0 || unit >= MAX_LINKS)
^1da177e4c3f41 Linus Torvalds    2005-04-16  2030  		return NULL;
^1da177e4c3f41 Linus Torvalds    2005-04-16  2031  
^1da177e4c3f41 Linus Torvalds    2005-04-16  2032  	if (sock_create_lite(PF_NETLINK, SOCK_DGRAM, unit, &sock))
^1da177e4c3f41 Linus Torvalds    2005-04-16  2033  		return NULL;
13d3078e22f565 Eric W. Biederman 2015-05-08  2034  
13d3078e22f565 Eric W. Biederman 2015-05-08  2035  	if (__netlink_create(net, sock, cb_mutex, unit, 1) < 0)
23fe18669e7fda Pavel Emelyanov   2008-01-30  2036  		goto out_sock_release_nosk;
23fe18669e7fda Pavel Emelyanov   2008-01-30  2037  
a31f2d17b331db Pablo Neira Ayuso 2012-06-29  2038  	if (!cfg || cfg->groups < 32)
4277a083ecd2c8 Patrick McHardy   2006-03-20  2039  		groups = 32;
a31f2d17b331db Pablo Neira Ayuso 2012-06-29  2040  	else
a31f2d17b331db Pablo Neira Ayuso 2012-06-29  2041  		groups = cfg->groups;
4277a083ecd2c8 Patrick McHardy   2006-03-20  2042  
5c398dc8f5a58b Eric Dumazet      2010-10-24  2043  	listeners = kzalloc(sizeof(*listeners) + NLGRPSZ(groups), GFP_KERNEL);
4277a083ecd2c8 Patrick McHardy   2006-03-20 @2044  	if (!listeners)
90ae404765d263 Markus Elfring    2023-12-31  2045  		goto out_netlink_release_sock;
4277a083ecd2c8 Patrick McHardy   2006-03-20  2046  
e547df909ec09d Markus Elfring    2023-12-31  2047  	sk = sock->sk;
^1da177e4c3f41 Linus Torvalds    2005-04-16  2048  	sk->sk_data_ready = netlink_data_ready;
a31f2d17b331db Pablo Neira Ayuso 2012-06-29  2049  	if (cfg && cfg->input)
a31f2d17b331db Pablo Neira Ayuso 2012-06-29  2050  		nlk_sk(sk)->netlink_rcv = cfg->input;
^1da177e4c3f41 Linus Torvalds    2005-04-16  2051  
8ea65f4a2dfaaf Herbert Xu        2015-01-26  2052  	if (netlink_insert(sk, 0))
77247bbb309424 Patrick McHardy   2005-08-14  2053  		goto out_sock_release;
4fdb3bb723db46 Harald Welte      2005-08-09  2054  
77247bbb309424 Patrick McHardy   2005-08-14  2055  	nlk = nlk_sk(sk);
8fe08d70a2b61b Eric Dumazet      2023-08-11  2056  	set_bit(NETLINK_F_KERNEL_SOCKET, &nlk->flags);
4fdb3bb723db46 Harald Welte      2005-08-09  2057  
4fdb3bb723db46 Harald Welte      2005-08-09  2058  	netlink_table_grab();
b4b510290b056b Eric W. Biederman 2007-09-12  2059  	if (!nl_table[unit].registered) {
4277a083ecd2c8 Patrick McHardy   2006-03-20  2060  		nl_table[unit].groups = groups;
5c398dc8f5a58b Eric Dumazet      2010-10-24  2061  		rcu_assign_pointer(nl_table[unit].listeners, listeners);
af65bdfce98d79 Patrick McHardy   2007-04-20  2062  		nl_table[unit].cb_mutex = cb_mutex;
77247bbb309424 Patrick McHardy   2005-08-14  2063  		nl_table[unit].module = module;
9785e10aedfa0f Pablo Neira Ayuso 2012-09-08  2064  		if (cfg) {
9785e10aedfa0f Pablo Neira Ayuso 2012-09-08  2065  			nl_table[unit].bind = cfg->bind;
6251edd932ce3f Hiroaki SHIMODA   2014-11-13  2066  			nl_table[unit].unbind = cfg->unbind;
a4c9a56e6a2cde Anjali Kulkarni   2023-07-19  2067  			nl_table[unit].release = cfg->release;
9785e10aedfa0f Pablo Neira Ayuso 2012-09-08  2068  			nl_table[unit].flags = cfg->flags;
9785e10aedfa0f Pablo Neira Ayuso 2012-09-08  2069  		}
ab33a1711cf60b Patrick McHardy   2005-08-14  2070  		nl_table[unit].registered = 1;
f937f1f46b6d2f Jesper Juhl       2007-10-15  2071  	} else {
f937f1f46b6d2f Jesper Juhl       2007-10-15  2072  		kfree(listeners);
869e58f87094b1 Denis V. Lunev    2008-01-18  2073  		nl_table[unit].registered++;
b4b510290b056b Eric W. Biederman 2007-09-12  2074  	}
4fdb3bb723db46 Harald Welte      2005-08-09  2075  	netlink_table_ungrab();
77247bbb309424 Patrick McHardy   2005-08-14  2076  	return sk;
77247bbb309424 Patrick McHardy   2005-08-14  2077  
4fdb3bb723db46 Harald Welte      2005-08-09  2078  out_sock_release:
4277a083ecd2c8 Patrick McHardy   2006-03-20  2079  	kfree(listeners);
90ae404765d263 Markus Elfring    2023-12-31  2080  out_netlink_release_sock:
9dfbec1fb2bedf Denis V. Lunev    2008-02-29  2081  	netlink_kernel_release(sk);
23fe18669e7fda Pavel Emelyanov   2008-01-30  2082  	return NULL;
23fe18669e7fda Pavel Emelyanov   2008-01-30  2083  
23fe18669e7fda Pavel Emelyanov   2008-01-30  2084  out_sock_release_nosk:
4fdb3bb723db46 Harald Welte      2005-08-09  2085  	sock_release(sock);
77247bbb309424 Patrick McHardy   2005-08-14  2086  	return NULL;
^1da177e4c3f41 Linus Torvalds    2005-04-16  2087  }
9f00d9776bc5be Pablo Neira Ayuso 2012-09-08  2088  EXPORT_SYMBOL(__netlink_kernel_create);
b7c6ba6eb1234e Denis V. Lunev    2008-01-28  2089
Markus Elfring Jan. 1, 2024, 10:22 a.m. UTC | #2 
…
> url:    https://github.com/intel-lab-lkp/linux/commits/Markus-Elfring/netlink-Improve-exception-handling-in-__netlink_kernel_create/20240101-015025
…
> patch link:    https://lore.kernel.org/r/223a61e9-f826-4f37-b514-ca6ed53b1269%40web.de
…
> All warnings (new ones prefixed by >>):
>
>>> net/netlink/af_netlink.c:2044:6: warning: variable 'sk' is used uninitialized whenever 'if' condition is true [-Wsometimes-uninitialized]
…
>
…
> ^1da177e4c3f41 Linus Torvalds    2005-04-16  2016  struct sock *
> 9f00d9776bc5be Pablo Neira Ayuso 2012-09-08  2017  __netlink_kernel_create(struct net *net, int unit, struct module *module,
> a31f2d17b331db Pablo Neira Ayuso 2012-06-29  2018  			struct netlink_kernel_cfg *cfg)
> ^1da177e4c3f41 Linus Torvalds    2005-04-16  2019  {
> ^1da177e4c3f41 Linus Torvalds    2005-04-16  2020  	struct socket *sock;
> ^1da177e4c3f41 Linus Torvalds    2005-04-16  2021  	struct sock *sk;
…
> 5c398dc8f5a58b Eric Dumazet      2010-10-24  2023  	struct listeners *listeners = NULL;
…
> ^1da177e4c3f41 Linus Torvalds    2005-04-16  2032  	if (sock_create_lite(PF_NETLINK, SOCK_DGRAM, unit, &sock))
> ^1da177e4c3f41 Linus Torvalds    2005-04-16  2033  		return NULL;
…
> 5c398dc8f5a58b Eric Dumazet      2010-10-24  2043  	listeners = kzalloc(sizeof(*listeners) + NLGRPSZ(groups), GFP_KERNEL);
> 4277a083ecd2c8 Patrick McHardy   2006-03-20 @2044  	if (!listeners)
> 90ae404765d263 Markus Elfring    2023-12-31  2045  		goto out_netlink_release_sock;
> 4277a083ecd2c8 Patrick McHardy   2006-03-20  2046
> e547df909ec09d Markus Elfring    2023-12-31  2047  	sk = sock->sk;
> ^1da177e4c3f41 Linus Torvalds    2005-04-16  2048  	sk->sk_data_ready = netlink_data_ready;
…
> 4fdb3bb723db46 Harald Welte      2005-08-09  2075  	netlink_table_ungrab();
> 77247bbb309424 Patrick McHardy   2005-08-14  2076  	return sk;
> 77247bbb309424 Patrick McHardy   2005-08-14  2077
> 4fdb3bb723db46 Harald Welte      2005-08-09  2078  out_sock_release:
> 4277a083ecd2c8 Patrick McHardy   2006-03-20  2079  	kfree(listeners);
> 90ae404765d263 Markus Elfring    2023-12-31  2080  out_netlink_release_sock:
> 9dfbec1fb2bedf Denis V. Lunev    2008-02-29  2081  	netlink_kernel_release(sk);
> 23fe18669e7fda Pavel Emelyanov   2008-01-30  2082  	return NULL;

How do you think about to use the function call “netlink_kernel_release(sock->sk)”?

Regards,
Markus
Stephen Hemminger Jan. 1, 2024, 6:14 p.m. UTC | #3 
On Sun, 31 Dec 2023 18:44:13 +0100
Markus Elfring <Markus.Elfring@web.de> wrote:

> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Sun, 31 Dec 2023 17:36:50 +0100
> 
> Move one assignment for the variable “sk” closer to the place
> where this pointer is used.
> 
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>

NAK
Useless churn of source code.
If compiler will do this kind of optimization itself.
diff mbox series

Patch

diff --git a/net/netlink/af_netlink.c b/net/netlink/af_netlink.c
index c3f88015cacf..b71d9c21d15b 100644
--- a/net/netlink/af_netlink.c
+++ b/net/netlink/af_netlink.c
@@ -2033,8 +2033,6 @@  __netlink_kernel_create(struct net *net, int unit, struct module *module,
 	if (__netlink_create(net, sock, cb_mutex, unit, 1) < 0)
 		goto out_sock_release_nosk;

-	sk = sock->sk;
-
 	if (!cfg || cfg->groups < 32)
 		groups = 32;
 	else
@@ -2044,6 +2042,7 @@  __netlink_kernel_create(struct net *net, int unit, struct module *module,
 	if (!listeners)
 		goto out_netlink_release_sock;

+	sk = sock->sk;
 	sk->sk_data_ready = netlink_data_ready;
 	if (cfg && cfg->input)
 		nlk_sk(sk)->netlink_rcv = cfg->input;