Re: [PATCH net] fou: avoid using sk_user_data before it is initialised

[Simon Horman]

On Thu, May 19, 2016 at 10:17:40PM -0700, Cong Wang wrote:
> On Wed, May 18, 2016 at 10:16 AM, Tom Herbert  wrote:
> > On Wed, May 18, 2016 at 10:07 AM, Cong Wang  
> > wrote:
> >> On Wed, May 18, 2016 at 9:30 AM, Tom Herbert  wrote:
> >>> On Wed, May 18, 2016 at 1:30 AM, Simon Horman
> >>>  wrote:
>  During initialisation sk->sk_user_data should be initialised before
>  it is referenced via a call to gue_encap_init().
> 
> >>> I think this is should be fixed by proposed patch "fou:
> >>> Call-setup_udp_tunnel_sock".
> >>
> >> That is 6/16, do you have a fix for stable to backport?
> >
> > Your patch to use fou directly is good.
> 
> Note, I am waiting for Simon to update his patch if he agrees.

Thanks, I've sent an updated patch.

I'm also quite happy for it to be fixed another way if you prefer
e.g. by your own patch.

Re: [PATCH net] fou: avoid using sk_user_data before it is initialised

[Cong Wang]

On Wed, May 18, 2016 at 10:16 AM, Tom Herbert  wrote:
> On Wed, May 18, 2016 at 10:07 AM, Cong Wang  wrote:
>> On Wed, May 18, 2016 at 9:30 AM, Tom Herbert  wrote:
>>> On Wed, May 18, 2016 at 1:30 AM, Simon Horman
>>>  wrote:
 During initialisation sk->sk_user_data should be initialised before
 it is referenced via a call to gue_encap_init().

>>> I think this is should be fixed by proposed patch "fou:
>>> Call-setup_udp_tunnel_sock".
>>
>> That is 6/16, do you have a fix for stable to backport?
>
> Your patch to use fou directly is good.

Note, I am waiting for Simon to update his patch if he agrees.

Re: [PATCH net] fou: avoid using sk_user_data before it is initialised

[Tom Herbert]

On Wed, May 18, 2016 at 10:07 AM, Cong Wang  wrote:
> On Wed, May 18, 2016 at 9:30 AM, Tom Herbert  wrote:
>> On Wed, May 18, 2016 at 1:30 AM, Simon Horman
>>  wrote:
>>> During initialisation sk->sk_user_data should be initialised before
>>> it is referenced via a call to gue_encap_init().
>>>
>> I think this is should be fixed by proposed patch "fou:
>> Call-setup_udp_tunnel_sock".
>
> That is 6/16, do you have a fix for stable to backport?

Your patch to use fou directly is good.

Tom

Re: [PATCH net] fou: avoid using sk_user_data before it is initialised

[Cong Wang]

On Wed, May 18, 2016 at 9:30 AM, Tom Herbert  wrote:
> On Wed, May 18, 2016 at 1:30 AM, Simon Horman
>  wrote:
>> During initialisation sk->sk_user_data should be initialised before
>> it is referenced via a call to gue_encap_init().
>>
> I think this is should be fixed by proposed patch "fou:
> Call-setup_udp_tunnel_sock".

That is 6/16, do you have a fix for stable to backport?

Re: [PATCH net] fou: avoid using sk_user_data before it is initialised

[Cong Wang]

On Wed, May 18, 2016 at 1:30 AM, Simon Horman
 wrote:
> During initialisation sk->sk_user_data should be initialised before
> it is referenced via a call to gue_encap_init().

Or just use 'fou' directly?

diff --git a/net/ipv4/fou.c b/net/ipv4/fou.c
index eeec7d6..0b7a983 100644
--- a/net/ipv4/fou.c
+++ b/net/ipv4/fou.c
@@ -453,7 +453,7 @@ static int fou_encap_init(struct sock *sk, struct
fou *fou, struct fou_cfg *cfg)
udp_sk(sk)->encap_rcv = fou_udp_recv;
udp_sk(sk)->gro_receive = fou_gro_receive;
udp_sk(sk)->gro_complete = fou_gro_complete;
-   fou_from_sock(sk)->protocol = cfg->protocol;
+   fou->protocol = cfg->protocol;

return 0;
 }

Re: [PATCH net] fou: avoid using sk_user_data before it is initialised

[Tom Herbert]

On Wed, May 18, 2016 at 1:30 AM, Simon Horman
 wrote:
> During initialisation sk->sk_user_data should be initialised before
> it is referenced via a call to gue_encap_init().
>
I think this is should be fixed by proposed patch "fou:
Call-setup_udp_tunnel_sock".

Thanks,
Tom

> Found by bisection after noticing the following:
>
> $ ip fou add port  ipproto 47
> [0.383417] BUG: unable to handle kernel NULL pointer dereference at 
> 0008
> [0.384132] IP: [] fou_nl_cmd_add_port+0x1e1/0x230
> [0.384707] PGD 1fafc067 PUD 1fb72067 PMD 0
> [0.385110] Oops: 0002 [#1] SMP
> [0.385387] Modules linked in:
> [0.385667] CPU: 0 PID: 55 Comm: ip Not tainted 4.6.0-03623-g0b7962a6c4a3 
> #430
> [0.386244] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 
> 1.7.5-20140531_083030-gandalf 04/01/2014
> [0.386244] task: 88001fb9cac0 ti: 88001fbc8000 task.ti: 
> 88001fbc8000
> [0.386244] RIP: 0010:[]  [] 
> fou_nl_cmd_add_port+0x1e1/0x230
> [0.386244] RSP: 0018:88001fbcbb78  EFLAGS: 00010246
> [0.386244] RAX: 0001 RBX: 88001fb8eb40 RCX: 
> 002f
> [0.386244] RDX:  RSI:  RDI: 
> 880019fcafc0
> [0.386244] RBP: 880019fcaf80 R08: 8130c370 R09: 
> 880019fcaf80
> [0.386244] R10: 880019e12b8c R11:  R12: 
> 0004
> [0.386244] R13: 0014 R14: 88001fb1a300 R15: 
> 816634c0
> [0.386244] FS:  7f016eb4d700() GS:88001a20() 
> knlGS:
> [0.386244] CS:  0010 DS:  ES:  CR0: 80050033
> [0.386244] CR2: 0008 CR3: 1fb69000 CR4: 
> 06b0
> [0.386244] Stack:
> [0.386244]  88001faaea24 8800192426c0 0002002f0001 
> 
> [0.386244]     
> b822
> [0.386244]  81461480 88001faaea14 0004 
> 812b0e17
> [0.386244] Call Trace:
> [0.386244]  [] ? genl_family_rcv_msg+0x197/0x320
> [0.386244]  [] ? genl_family_rcv_msg+0x320/0x320
> [0.386244]  [] ? genl_rcv_msg+0x70/0xb0
> [0.386244]  [] ? netlink_rcv_skb+0xa1/0xc0
> [0.386244]  [] ? genl_rcv+0x24/0x40
> [0.386244]  [] ? netlink_unicast+0x143/0x1d0
> [0.386244]  [] ? netlink_sendmsg+0x366/0x390
> [0.386244]  [] ? rw_copy_check_uvector+0x68/0x110
> [0.386244]  [] ? sock_sendmsg+0x10/0x20
> [0.386244]  [] ? ___sys_sendmsg+0x1f1/0x200
> [0.386244]  [] ? pipe_write+0x1a0/0x420
> [0.386244]  [] ? do_filp_open+0x92/0xe0
> [0.386244]  [] ? __sys_sendmsg+0x41/0x70
> [0.386244]  [] ? entry_SYSCALL_64_fastpath+0x13/0x8f
> [0.386244] Code: 4c 24 12 48 8b 93 28 02 00 00 48 c7 83 68 03 00 00 e0 76 
> 32 81 48 c7 83 78 03 00 00 50 61 32 81 48 c7 83 80 03 00 00 e0 64 32 81 <88> 
> 4a 08 e9 20 ff ff ff 4c 89 e7 bb 8e ff ff ff e8 1a 34 07 00
> [0.386244] RIP  [] fou_nl_cmd_add_port+0x1e1/0x230
> [0.386244]  RSP 
> [0.386244] CR2: 0008
> [0.407176] ---[ end trace 13bf0d24a4b7f9c3 ]---
>
> Fixes: d92283e338f6 ("fou: change to use UDP socket GRO")
> Signed-off-by: Simon Horman 
> ---
>  net/ipv4/fou.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/net/ipv4/fou.c b/net/ipv4/fou.c
> index eeec7d60e5fd..5f9634915cf2 100644
> --- a/net/ipv4/fou.c
> +++ b/net/ipv4/fou.c
> @@ -488,6 +488,7 @@ static int fou_create(struct net *net, struct fou_cfg 
> *cfg,
> }
>
> sk = sock->sk;
> +   sk->sk_user_data = fou;
>
> fou->flags = cfg->flags;
> fou->port = cfg->udp_config.local_udp_port;
> @@ -514,7 +515,6 @@ static int fou_create(struct net *net, struct fou_cfg 
> *cfg,
> udp_sk(sk)->encap_type = 1;
> udp_encap_enable();
>
> -   sk->sk_user_data = fou;
> fou->sock = sock;
>
> inet_inc_convert_csum(sk);
> --
> 2.1.4
>