diff mbox

[net] udp: ipv6: reset daddr and dport in sk if connect() fails

Message ID 20170621213631.145600-1-tracywwnj@gmail.com
State Changes Requested, archived
Delegated to: David Miller
Headers show

Commit Message

Wei Wang June 21, 2017, 9:36 p.m. UTC 
From: Wei Wang <weiwan@google.com>

In __ip6_datagram_connect(), reset sk->sk_v6_daddr and inet->dport if
error occurs so that udp_v6_early_demux() won't be able to do exact match on
this socket and hence, won't consider this socket as a valid candidate for
early demux.

Signed-off-by: Wei Wang <weiwan@google.com>
Acked-by: Maciej Żenczykowski <maze@google.com>
---
 net/ipv6/datagram.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

Comments

David Miller June 22, 2017, 5:25 p.m. UTC | #1 
From: Wei Wang <tracywwnj@gmail.com>
Date: Wed, 21 Jun 2017 14:36:31 -0700

> +		memset(&sk->sk_v6_daddr, 0 sizeof(sk->sk_v6_daddr);

Tell me how that compiles.
kernel test robot June 22, 2017, 5:46 p.m. UTC | #2 
Hi Wei,

[auto build test ERROR on net/master]

url:    https://github.com/0day-ci/linux/commits/Wei-Wang/udp-ipv6-reset-daddr-and-dport-in-sk-if-connect-fails/20170623-011149
config: xtensa-allyesconfig (attached as .config)
compiler: xtensa-linux-gcc (GCC) 4.9.0
reproduce:
        wget https://raw.githubusercontent.com/01org/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=xtensa 

All errors (new ones prefixed by >>):

   net//ipv6/datagram.c: In function '__ip6_datagram_connect':
>> net//ipv6/datagram.c:257:30: error: expected ')' before 'sizeof'
      memset(&sk->sk_v6_daddr, 0 sizeof(sk->sk_v6_daddr);
                                 ^
>> net//ipv6/datagram.c:257:3: error: too few arguments to function 'memset'
      memset(&sk->sk_v6_daddr, 0 sizeof(sk->sk_v6_daddr);
      ^
   In file included from include/linux/string.h:18:0,
                    from include/linux/bitmap.h:8,
                    from include/linux/cpumask.h:11,
                    from include/linux/interrupt.h:9,
                    from net//ipv6/datagram.c:18:
   arch/xtensa/include/asm/string.h:110:14: note: declared here
    extern void *memset(void *__s, int __c, size_t __count);
                 ^
>> net//ipv6/datagram.c:260:2: error: expected ';' before '}' token
     }
     ^

vim +257 net//ipv6/datagram.c

   251	
   252		err = ip6_datagram_dst_update(sk, true);
   253		if (err) {
   254			/* Reset daddr and dport so that udp_v6_early_demux()
   255			 * fails to find this socket
   256			 */
 > 257			memset(&sk->sk_v6_daddr, 0 sizeof(sk->sk_v6_daddr);
   258			inet->inet_dport = 0;
   259			goto out;
 > 260		}
   261	
   262		sk->sk_state = TCP_ESTABLISHED;
   263		sk_set_txhash(sk);

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
Wei Wang June 22, 2017, 5:49 p.m. UTC | #3 
On Thu, Jun 22, 2017 at 10:25 AM, David Miller <davem@davemloft.net> wrote:
> From: Wei Wang <tracywwnj@gmail.com>
> Date: Wed, 21 Jun 2017 14:36:31 -0700
>
>> +             memset(&sk->sk_v6_daddr, 0 sizeof(sk->sk_v6_daddr);
>
> Tell me how that compiles.

Really sorry about that. I ported over the change manually from
another branch and made the stupid mistake...
Will send out v2 shortly.
kernel test robot June 22, 2017, 5:50 p.m. UTC | #4 
Hi Wei,

[auto build test ERROR on net/master]

url:    https://github.com/0day-ci/linux/commits/Wei-Wang/udp-ipv6-reset-daddr-and-dport-in-sk-if-connect-fails/20170623-011149
config: alpha-allyesconfig (attached as .config)
compiler: alpha-linux-gnu-gcc (Debian 6.1.1-9) 6.1.1 20160705
reproduce:
        wget https://raw.githubusercontent.com/01org/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=alpha 

All error/warnings (new ones prefixed by >>):

   net//ipv6/datagram.c: In function '__ip6_datagram_connect':
   net//ipv6/datagram.c:257:30: error: expected ')' before 'sizeof'
      memset(&sk->sk_v6_daddr, 0 sizeof(sk->sk_v6_daddr);
                                 ^~~~~~
   In file included from include/linux/string.h:18:0,
                    from include/linux/bitmap.h:8,
                    from include/linux/cpumask.h:11,
                    from include/linux/interrupt.h:9,
                    from net//ipv6/datagram.c:18:
>> arch/alpha/include/asm/string.h:45:16: error: too few arguments to function '__memset'
    #define memset __memset
                   ^
>> net//ipv6/datagram.c:257:3: note: in expansion of macro 'memset'
      memset(&sk->sk_v6_daddr, 0 sizeof(sk->sk_v6_daddr);
      ^~~~~~
   arch/alpha/include/asm/string.h:32:21: note: declared here
    extern inline void *__memset(void *s, int c, size_t n)
                        ^~~~~~~~
   net//ipv6/datagram.c:260:2: error: expected ';' before '}' token
     }
     ^
--
   net/ipv6/datagram.c: In function '__ip6_datagram_connect':
   net/ipv6/datagram.c:257:30: error: expected ')' before 'sizeof'
      memset(&sk->sk_v6_daddr, 0 sizeof(sk->sk_v6_daddr);
                                 ^~~~~~
   In file included from include/linux/string.h:18:0,
                    from include/linux/bitmap.h:8,
                    from include/linux/cpumask.h:11,
                    from include/linux/interrupt.h:9,
                    from net/ipv6/datagram.c:18:
>> arch/alpha/include/asm/string.h:45:16: error: too few arguments to function '__memset'
    #define memset __memset
                   ^
   net/ipv6/datagram.c:257:3: note: in expansion of macro 'memset'
      memset(&sk->sk_v6_daddr, 0 sizeof(sk->sk_v6_daddr);
      ^~~~~~
   arch/alpha/include/asm/string.h:32:21: note: declared here
    extern inline void *__memset(void *s, int c, size_t n)
                        ^~~~~~~~
   net/ipv6/datagram.c:260:2: error: expected ';' before '}' token
     }
     ^

vim +/__memset +45 arch/alpha/include/asm/string.h

a47e5bb5 arch/alpha/include/asm/string.h Richard Henderson 2013-07-11  29  /* For gcc 3.x, we cannot have the inline function named "memset" because
a47e5bb5 arch/alpha/include/asm/string.h Richard Henderson 2013-07-11  30     the __builtin_memset will attempt to resolve to the inline as well,
a47e5bb5 arch/alpha/include/asm/string.h Richard Henderson 2013-07-11  31     leading to a "sorry" about unimplemented recursive inlining.  */
a47e5bb5 arch/alpha/include/asm/string.h Richard Henderson 2013-07-11  32  extern inline void *__memset(void *s, int c, size_t n)
a47e5bb5 arch/alpha/include/asm/string.h Richard Henderson 2013-07-11  33  {
a47e5bb5 arch/alpha/include/asm/string.h Richard Henderson 2013-07-11  34  	if (__builtin_constant_p(c)) {
a47e5bb5 arch/alpha/include/asm/string.h Richard Henderson 2013-07-11  35  		if (__builtin_constant_p(n)) {
a47e5bb5 arch/alpha/include/asm/string.h Richard Henderson 2013-07-11  36  			return __builtin_memset(s, c, n);
a47e5bb5 arch/alpha/include/asm/string.h Richard Henderson 2013-07-11  37  		} else {
a47e5bb5 arch/alpha/include/asm/string.h Richard Henderson 2013-07-11  38  			unsigned long c8 = (c & 0xff) * 0x0101010101010101UL;
a47e5bb5 arch/alpha/include/asm/string.h Richard Henderson 2013-07-11  39  			return __constant_c_memset(s, c8, n);
a47e5bb5 arch/alpha/include/asm/string.h Richard Henderson 2013-07-11  40  		}
a47e5bb5 arch/alpha/include/asm/string.h Richard Henderson 2013-07-11  41  	}
a47e5bb5 arch/alpha/include/asm/string.h Richard Henderson 2013-07-11  42  	return ___memset(s, c, n);
a47e5bb5 arch/alpha/include/asm/string.h Richard Henderson 2013-07-11  43  }
a47e5bb5 arch/alpha/include/asm/string.h Richard Henderson 2013-07-11  44  
a47e5bb5 arch/alpha/include/asm/string.h Richard Henderson 2013-07-11 @45  #define memset __memset
^1da177e include/asm-alpha/string.h      Linus Torvalds    2005-04-16  46  
^1da177e include/asm-alpha/string.h      Linus Torvalds    2005-04-16  47  #define __HAVE_ARCH_STRCPY
^1da177e include/asm-alpha/string.h      Linus Torvalds    2005-04-16  48  extern char * strcpy(char *,const char *);
^1da177e include/asm-alpha/string.h      Linus Torvalds    2005-04-16  49  #define __HAVE_ARCH_STRNCPY
^1da177e include/asm-alpha/string.h      Linus Torvalds    2005-04-16  50  extern char * strncpy(char *, const char *, size_t);
^1da177e include/asm-alpha/string.h      Linus Torvalds    2005-04-16  51  #define __HAVE_ARCH_STRCAT
^1da177e include/asm-alpha/string.h      Linus Torvalds    2005-04-16  52  extern char * strcat(char *, const char *);
^1da177e include/asm-alpha/string.h      Linus Torvalds    2005-04-16  53  #define __HAVE_ARCH_STRNCAT

:::::: The code at line 45 was first introduced by commit
:::::: a47e5bb5764f029f989a182b0dd2d4cce69f8b14 alpha: Eliminate compiler warning from memset macro

:::::: TO: Richard Henderson <rth@twiddle.net>
:::::: CC: Matt Turner <mattst88@gmail.com>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
David Miller June 22, 2017, 5:54 p.m. UTC | #5 
From: Wei Wang <weiwan@google.com>
Date: Thu, 22 Jun 2017 10:49:24 -0700

> On Thu, Jun 22, 2017 at 10:25 AM, David Miller <davem@davemloft.net> wrote:
>> From: Wei Wang <tracywwnj@gmail.com>
>> Date: Wed, 21 Jun 2017 14:36:31 -0700
>>
>>> +             memset(&sk->sk_v6_daddr, 0 sizeof(sk->sk_v6_daddr);
>>
>> Tell me how that compiles.
> 
> Really sorry about that. I ported over the change manually from
> another branch and made the stupid mistake...
> Will send out v2 shortly.

Misporting the change isn't stupid.

Not build and functionally testing your change is.
Wei Wang June 22, 2017, 6 p.m. UTC | #6 
On Thu, Jun 22, 2017 at 10:54 AM, David Miller <davem@davemloft.net> wrote:
> From: Wei Wang <weiwan@google.com>
> Date: Thu, 22 Jun 2017 10:49:24 -0700
>
>> On Thu, Jun 22, 2017 at 10:25 AM, David Miller <davem@davemloft.net> wrote:
>>> From: Wei Wang <tracywwnj@gmail.com>
>>> Date: Wed, 21 Jun 2017 14:36:31 -0700
>>>
>>>> +             memset(&sk->sk_v6_daddr, 0 sizeof(sk->sk_v6_daddr);
>>>
>>> Tell me how that compiles.
>>
>> Really sorry about that. I ported over the change manually from
>> another branch and made the stupid mistake...
>> Will send out v2 shortly.
>
> Misporting the change isn't stupid.
>
> Not build and functionally testing your change is.

Yes. Agree...
I did the testing on the other branch but did not do it again after
porting the change over.
Will definitely remember to do it next time. Really sorry about it...
diff mbox

Patch

diff --git a/net/ipv6/datagram.c b/net/ipv6/datagram.c
index e011122ebd43..f43f4f1e69dd 100644
--- a/net/ipv6/datagram.c
+++ b/net/ipv6/datagram.c
@@ -250,8 +250,14 @@  int __ip6_datagram_connect(struct sock *sk, struct sockaddr *uaddr,
 	 */
 
 	err = ip6_datagram_dst_update(sk, true);
-	if (err)
+	if (err) {
+		/* Reset daddr and dport so that udp_v6_early_demux()
+		 * fails to find this socket
+		 */
+		memset(&sk->sk_v6_daddr, 0 sizeof(sk->sk_v6_daddr);
+		inet->inet_dport = 0;
 		goto out;
+	}
 
 	sk->sk_state = TCP_ESTABLISHED;
 	sk_set_txhash(sk);