Message ID | 20200725133031.a5uxkpikopntgu4c@pesu.pes.edu |
---|---|
State | Rejected |
Delegated to: | David Miller |
Headers | show |
Series | net: ipv6: fix slab-out-of-bounds Read in __xfrm6_tunnel_spi_check | expand |
Hi K, Thank you for the patch! Perhaps something to improve: [auto build test WARNING on ipsec/master] [also build test WARNING on ipsec-next/master net-next/master net/master v5.8-rc6 next-20200724] [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] url: https://github.com/0day-ci/linux/commits/B-K-Karthik/net-ipv6-fix-slab-out-of-bounds-Read-in-__xfrm6_tunnel_spi_check/20200725-213142 base: https://git.kernel.org/pub/scm/linux/kernel/git/klassert/ipsec.git master config: parisc-allyesconfig (attached as .config) compiler: hppa-linux-gcc (GCC) 9.3.0 reproduce (this is a W=1 build): wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # save the attached .config to linux build tree COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=parisc If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot <lkp@intel.com> All warnings (new ones prefixed by >>): net/ipv6/xfrm6_tunnel.c: In function '__xfrm6_tunnel_spi_check': >> net/ipv6/xfrm6_tunnel.c:106:43: warning: passing argument 1 of 'xfrm6_tunnel_spi_hash_byaddr' makes pointer from integer without a cast [-Wint-conversion] 106 | int index = xfrm6_tunnel_spi_hash_byaddr(spi); | ^~~ | | | u32 {aka unsigned int} net/ipv6/xfrm6_tunnel.c:57:79: note: expected 'const xfrm_address_t *' {aka 'const union <anonymous> *'} but argument is of type 'u32' {aka 'unsigned int'} 57 | static inline unsigned int xfrm6_tunnel_spi_hash_byaddr(const xfrm_address_t *addr) | ~~~~~~~~~~~~~~~~~~~~~~^~~~ vim +/xfrm6_tunnel_spi_hash_byaddr +106 net/ipv6/xfrm6_tunnel.c 101 102 static int __xfrm6_tunnel_spi_check(struct net *net, u32 spi) 103 { 104 struct xfrm6_tunnel_net *xfrm6_tn = xfrm6_tunnel_pernet(net); 105 struct xfrm6_tunnel_spi *x6spi; > 106 int index = xfrm6_tunnel_spi_hash_byaddr(spi); 107 108 hlist_for_each_entry(x6spi, 109 &xfrm6_tn->spi_byaddr[index], 110 list_byspi) { 111 if (x6spi->spi == spi) 112 return -1; 113 } 114 return index; 115 } 116 --- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
Hi K, Thank you for the patch! Perhaps something to improve: [auto build test WARNING on ipsec/master] [also build test WARNING on ipsec-next/master net-next/master net/master v5.8-rc6 next-20200724] [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] url: https://github.com/0day-ci/linux/commits/B-K-Karthik/net-ipv6-fix-slab-out-of-bounds-Read-in-__xfrm6_tunnel_spi_check/20200725-213142 base: https://git.kernel.org/pub/scm/linux/kernel/git/klassert/ipsec.git master config: x86_64-randconfig-r032-20200726 (attached as .config) compiler: clang version 12.0.0 (https://github.com/llvm/llvm-project 8bf4c1f4fb257774f66c8cda07adc6c5e8668326) reproduce (this is a W=1 build): wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # install x86_64 cross compiling tool for clang build # apt-get install binutils-x86-64-linux-gnu # save the attached .config to linux build tree COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=x86_64 If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot <lkp@intel.com> All warnings (new ones prefixed by >>): >> net/ipv6/xfrm6_tunnel.c:106:43: warning: incompatible integer to pointer conversion passing 'u32' (aka 'unsigned int') to parameter of type 'const xfrm_address_t *' [-Wint-conversion] int index = xfrm6_tunnel_spi_hash_byaddr(spi); ^~~ net/ipv6/xfrm6_tunnel.c:57:79: note: passing argument to parameter 'addr' here static inline unsigned int xfrm6_tunnel_spi_hash_byaddr(const xfrm_address_t *addr) ^ net/ipv6/xfrm6_tunnel.c:69:28: warning: unused function 'xfrm6_tunnel_spi_hash_byspi' [-Wunused-function] static inline unsigned int xfrm6_tunnel_spi_hash_byspi(u32 spi) ^ 2 warnings generated. vim +106 net/ipv6/xfrm6_tunnel.c 101 102 static int __xfrm6_tunnel_spi_check(struct net *net, u32 spi) 103 { 104 struct xfrm6_tunnel_net *xfrm6_tn = xfrm6_tunnel_pernet(net); 105 struct xfrm6_tunnel_spi *x6spi; > 106 int index = xfrm6_tunnel_spi_hash_byaddr(spi); 107 108 hlist_for_each_entry(x6spi, 109 &xfrm6_tn->spi_byaddr[index], 110 list_byspi) { 111 if (x6spi->spi == spi) 112 return -1; 113 } 114 return index; 115 } 116 --- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
From: B K Karthik <bkkarthik@pesu.pes.edu> Date: Sat, 25 Jul 2020 19:00:31 +0530 > use spi_byaddr instead of spi_byspi ... > diff --git a/net/ipv6/xfrm6_tunnel.c b/net/ipv6/xfrm6_tunnel.c > index 25b7ebda2fab..cab7693ccfe3 100644 > --- a/net/ipv6/xfrm6_tunnel.c > +++ b/net/ipv6/xfrm6_tunnel.c > @@ -103,10 +103,10 @@ static int __xfrm6_tunnel_spi_check(struct net *net, u32 spi) > { > struct xfrm6_tunnel_net *xfrm6_tn = xfrm6_tunnel_pernet(net); > struct xfrm6_tunnel_spi *x6spi; > - int index = xfrm6_tunnel_spi_hash_byspi(spi); > + int index = xfrm6_tunnel_spi_hash_byaddr(spi); You are passing a u32 integer into a function that expects a pointer as an argument. This change isn't even compile tested properly, let alone run tested. Please stop making such careless submissions, this takes up valuable developer patch review resources. Thank you.
diff --git a/net/ipv6/xfrm6_tunnel.c b/net/ipv6/xfrm6_tunnel.c index 25b7ebda2fab..cab7693ccfe3 100644 --- a/net/ipv6/xfrm6_tunnel.c +++ b/net/ipv6/xfrm6_tunnel.c @@ -103,10 +103,10 @@ static int __xfrm6_tunnel_spi_check(struct net *net, u32 spi) { struct xfrm6_tunnel_net *xfrm6_tn = xfrm6_tunnel_pernet(net); struct xfrm6_tunnel_spi *x6spi; - int index = xfrm6_tunnel_spi_hash_byspi(spi); + int index = xfrm6_tunnel_spi_hash_byaddr(spi); hlist_for_each_entry(x6spi, - &xfrm6_tn->spi_byspi[index], + &xfrm6_tn->spi_byaddr[index], list_byspi) { if (x6spi->spi == spi) return -1;