Re: [PATCH net] l2tp: do not accept arbitrary sockets

2018-03-06 Thread James Chapman
On 6 March 2018 at 15:54, Eric Dumazet <eric.duma...@gmail.com> wrote: > From: Eric Dumazet <eduma...@google.com> > > syzkaller found an issue caused by lack of sufficient checks > in l2tp_tunnel_create() > > RAW sockets can not be considered as UDP ones for inst

[PATCH net 5/5] l2tp: fix tunnel lookup use-after-free race

2018-02-23 Thread James Chapman
47bd5acde002e353...@syzkaller.appspotmail.com Reported-and-tested-by: syzbot+6e6a5ec8de31a94cd...@syzkaller.appspotmail.com Reported-and-tested-by: syzbot+9df43faf09bd400f2...@syzkaller.appspotmail.com Signed-off-by: James Chapman <jchap...@katalix.com> --- net/l2tp/l2tp_core.c | 14 +++--- 1 file chang

[PATCH net 2/5] l2tp: don't use inet_shutdown on ppp session destroy

2018-02-23 Thread James Chapman
patch into separate l2tp and ppp parts") Signed-off-by: James Chapman <jchap...@katalix.com> --- net/l2tp/l2tp_ppp.c | 10 -- 1 file changed, 10 deletions(-) diff --git a/net/l2tp/l2tp_ppp.c b/net/l2tp/l2tp_ppp.c index 59f246d7b290..2d2955e8f710 100644 --- a/net/l2tp/l2tp_ppp.

[PATCH net 1/5] l2tp: don't use inet_shutdown on tunnel destroy

2018-02-23 Thread James Chapman
00 00 00 <49> 81 3c 24 80 93 3f 83 b8 00 00 00 00 44 0f 44 c0 83 fe 01 0f RIP: __lock_acquire+0x263/0x1630 RSP: 88001a37fc70 CR2: 00a0 Fixes: 309795f4bec2d ("l2tp: Add netlink control API for L2TP") Signed-off-by: James Chapman <jchap...@katalix.com> --- net

[PATCH net 0/5] l2tp: fix API races discovered by syzbot

2018-02-23 Thread James Chapman
could be destroyed while its associated tunnel/session object still existed (patches 3, 4). Patch 5 addresses a problem with the way tunnels are removed from the tunnel list. Patch 5 is tagged that it addresses all four syzbot issues, though all 5 patches are needed. James Chapman (5): l2tp

[PATCH net 3/5] l2tp: fix races with tunnel socket close

2018-02-23 Thread James Chapman
fc 49 8d bc 24 28 02 00 00 48 b8 00 00 00 00 00 fc ff df 48 89 f a 48 c1 ea 03 <80> 3c 02 00 0f 85 ed 02 00 00 4d 8b a4 24 28 02 00 00 e8 13 16 Fixes: 80d84ef3ff1dd ("l2tp: prevent l2tp_tunnel_delete racing with userspace close") Signed-off-by: James Chapman <jchap...@k

[PATCH net 4/5] l2tp: fix race in pppol2tp_release with session object destroy

2018-02-23 Thread James Chapman
64 85 e 8 2a 55 14 ff <0f> 0b 83 05 ad 2a 68 04 01 48 83 c4 18 5b 41 5c 41 5d 41 5e 41 Fixes: ee40fb2e1eb5b ("l2tp: protect sock pointer of struct pppol2tp_session with RCU") Signed-off-by: James Chapman <jchap...@katalix.com> --- ne

[PATCH net-next v3 04/16] l2tp: refactor tunnel lifetime handling wrt its socket

2018-02-12 Thread James Chapman
used so is removed. Fixes: 80d84ef3ff1dd ("l2tp: prevent l2tp_tunnel_delete racing with userspace close") Signed-off-by: James Chapman <jchap...@katalix.com> --- net/l2tp/l2tp_core.c | 128 ++- net/l2tp/l2tp_core.h | 26 ++---

[PATCH net-next v3 00/16] l2tp: fix API races discovered by syzbot

2018-02-12 Thread James Chapman
fixes/reported-by tags to end of commit text and add my signoff which got dropped in v2. James Chapman (16): l2tp: update sk_user_data while holding sk_callback_lock l2tp: add RCU read lock to protect tunnel ptr in ip socket destroy l2tp: don't use inet_shutdown on tunnel destroy l2tp

[PATCH net-next v3 08/16] l2tp: hide session from pppol2tp_sock_to_session if it is closing

2018-02-12 Thread James Chapman
Signed-off-by: James Chapman <jchap...@katalix.com> --- net/l2tp/l2tp_ppp.c | 10 +- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/net/l2tp/l2tp_ppp.c b/net/l2tp/l2tp_ppp.c index ff95a4d4eac5..947066b3d6d8 100644 --- a/net/l2tp/l2tp_ppp.c +++ b/net/l2tp/l2tp_ppp.c @@

[PATCH net-next v3 01/16] l2tp: update sk_user_data while holding sk_callback_lock

2018-02-12 Thread James Chapman
orted-by: syzbot+8865eaff7f9acd593...@syzkaller.appspotmail.com Signed-off-by: James Chapman <jchap...@katalix.com> --- net/l2tp/l2tp_core.c | 21 ++--- net/l2tp/l2tp_ppp.c | 8 ++-- 2 files changed, 24 insertions(+), 5 deletions(-) diff --git a/net/l2tp/l2tp_core.c b/net/l2tp/l2tp_cor

[PATCH net-next v3 06/16] l2tp: refactor session lifetime handling

2018-02-12 Thread James Chapman
inated tunnels") Signed-off-by: James Chapman <jchap...@katalix.com> --- net/l2tp/l2tp_core.c | 7 ++- net/l2tp/l2tp_ppp.c | 36 ++-- 2 files changed, 20 insertions(+), 23 deletions(-) diff --git a/net/l2tp/l2tp_core.c b/net/l2tp/l2tp_core.c index 691fe93

[PATCH net-next v3 05/16] l2tp: use tunnel closing flag

2018-02-12 Thread James Chapman
The tunnel's closing flag is set when the tunnel is being destroyed. Use it to reject new sessions and remove acpt_newsess which was doing the same thing. Also prevent the tunnel being seen in l2tp_tunnel_get lookups. Signed-off-by: James Chapman <jchap...@katalix.com> --- net/l2tp/l2tp_

[PATCH net-next v3 02/16] l2tp: add RCU read lock to protect tunnel ptr in ip socket destroy

2018-02-12 Thread James Chapman
v3 IP encapsulation (no UDP) support") Signed-off-by: James Chapman <jchap...@katalix.com> --- net/l2tp/l2tp_ip.c | 5 - net/l2tp/l2tp_ip6.c | 5 - 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/net/l2tp/l2tp_ip.c b/net/l2tp/l2tp_ip.c index ff61124fdf59..42f3c2f72bf4 10064

[PATCH net-next v3 07/16] l2tp: hide sessions if they are closing

2018-02-12 Thread James Chapman
Replace the dead flag in the session context with a closing flag and spinlock. Check it in session lookup functions such that we don't try to access session data while it is being destroyed. Signed-off-by: James Chapman <jchap...@katalix.com> --- net/l2tp/l2tp_core.

[PATCH net-next v3 09/16] l2tp: refactor pppol2tp_connect

2018-02-12 Thread James Chapman
It's hard to understand pppol2tp_connect so split it up into separate functions and document it better. Fixes: fd558d186d ("l2tp: Split pppol2tp patch into separate l2tp and ppp parts") Signed-off-by: James Chapman <jchap...@katalix.com> --- net/l2tp/

[PATCH net-next v3 11/16] l2tp: do session destroy using a workqueue

2018-02-12 Thread James Chapman
Handle session destroy in the same way as we handle tunnel destroy - through a workqueue. Sessions can be destroyed either because its socket is closed (if it has a socket) or by netlink request. A workqueue synchronises these. Signed-off-by: James Chapman <jchap...@katalix.com> --- ne

[PATCH net-next v3 03/16] l2tp: don't use inet_shutdown on tunnel destroy

2018-02-12 Thread James Chapman
00 00 00 <49> 81 3c 24 80 93 3f 83 b8 00 00 00 00 44 0f 44 c0 83 fe 01 0f RIP: __lock_acquire+0x263/0x1630 RSP: 88001a37fc70 CR2: 00a0 Fixes: 309795f4be ("l2tp: Add netlink control API for L2TP") Signed-off-by: James Chapman <jchap...@katalix.com> --- net

[PATCH net-next v3 12/16] l2tp: simplify l2tp_tunnel_closeall

2018-02-12 Thread James Chapman
is removed. Signed-off-by: James Chapman <jchap...@katalix.com> --- net/l2tp/l2tp_core.c | 32 net/l2tp/l2tp_core.h | 1 - 2 files changed, 4 insertions(+), 29 deletions(-) diff --git a/net/l2tp/l2tp_core.c b/net/l2tp/l2tp_core.c index 55b1f312fedc..c909fe9273c9

[PATCH net-next v3 15/16] l2tp: remove unwanted error message

2018-02-12 Thread James Chapman
If when creating a new tunnel, the indicated fd is closed by another thread, we emit an error message about it. e.g. l2tp_core: tunl 4: sockfd_lookup(fd=3) returned -9 It's not useful so remove it. Signed-off-by: James Chapman <jchap...@katalix.com> --- net/l2tp/l2tp_core.c | 2 --

[PATCH net-next v3 10/16] l2tp: add session_free callback

2018-02-12 Thread James Chapman
the session. Signed-off-by: James Chapman <jchap...@katalix.com> --- net/l2tp/l2tp_core.c | 7 +-- net/l2tp/l2tp_core.h | 1 + 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/net/l2tp/l2tp_core.c b/net/l2tp/l2tp_core.c index 869dec89ff0f..d6306ba2d78e 100644 --- a/net/l2tp/l2tp_

[PATCH net-next v3 14/16] l2tp: remove redundant sk_user_data check when creating tunnels

2018-02-12 Thread James Chapman
l2tp_tunnel_create now checks sk_user_data so this check is redundant Signed-off-by: James Chapman <jchap...@katalix.com> --- net/l2tp/l2tp_core.c | 8 1 file changed, 8 deletions(-) diff --git a/net/l2tp/l2tp_core.c b/net/l2tp/l2tp_core.c index c909fe9273c9..a91cd384e397

[PATCH net-next v3 13/16] l2tp: refactor ppp session cleanup paths

2018-02-12 Thread James Chapman
...@syzkaller.appspotmail.com Reported-by: syzbot+6e6a5ec8de31a94cd...@syzkaller.appspotmail.com Reported-by: syzbot+19c09769f14b48810...@syzkaller.appspotmail.com Reported-by: syzbot+347bd5acde002e353...@syzkaller.appspotmail.com Signed-off-by: James Chapman <jchap...@katalix.com> --- net/l2tp/l2tp

[PATCH net-next v3 16/16] l2tp: make __l2tp_session_unhash internal

2018-02-12 Thread James Chapman
__l2tp_session_unhash is now only used internally so there is no reason to expose it to other l2tp modules. Rename it l2tp_session_unhash while we're at it. Signed-off-by: James Chapman <jchap...@katalix.com> --- net/l2tp/l2tp_core.c | 5 ++--- net/l2tp/l2tp_core.h | 1 - 2 files chan

Re: [PATCH net-next v2 03/16] l2tp: don't use inet_shutdown on tunnel destroy

2018-02-12 Thread James Chapman
On 12/02/18 16:22, David Miller wrote: > From: James Chapman <jchap...@katalix.com> > Date: Mon, 12 Feb 2018 10:11:07 + > >> Previously, if a tunnel was closed, we called inet_shutdown to mark >> the socket as unconnected such that userspace would get errors a

[PATCH net-next v2 06/16] l2tp: refactor session lifetime handling

2018-02-12 Thread James Chapman
Simplify relationship with tunnel such that the session holds a ref on the tunnel, not its socket. This guarantees that the tunnel is always extant if one or more sessions exists on the tunnel. If the session has a socket (ppp), have it hold a ref on the socket until the session is destroyed.

[PATCH net-next v2 12/16] l2tp: simplify l2tp_tunnel_closeall

2018-02-12 Thread James Chapman
Since session destroy now uses a workqueue, let l2tp_session_delete handle all the work of destroying a session. Don't remove the session from the tunnel's list immediately. The tunnel will remain extant until all of its sessions are gone anyway. The session's dead flag is now unused so is

[PATCH net-next v2 03/16] l2tp: don't use inet_shutdown on tunnel destroy

2018-02-12 Thread James Chapman
Previously, if a tunnel was closed, we called inet_shutdown to mark the socket as unconnected such that userspace would get errors and then close the socket. This could race with userspace closing the socket. Instead, leave userspace to close the socket in its own time (our tunnel will be detached

[PATCH net-next v2 09/16] l2tp: refactor pppol2tp_connect

2018-02-12 Thread James Chapman
It's hard to understand pppol2tp_connect so split it up into separate functions and document it better. Fixes: fd558d186d ("l2tp: Split pppol2tp patch into separate l2tp and ppp parts") --- net/l2tp/l2tp_ppp.c | 307 +++- 1 file changed, 185

[PATCH net-next v2 02/16] l2tp: add RCU read lock to protect tunnel ptr in ip socket destroy

2018-02-12 Thread James Chapman
If an L2TPIP socket is closed, add RCU protection when we deref sk_user_data to prevent races with another thread closing the same tunnel. Fixes: 0d76751fad ("l2tp: Add L2TPv3 IP encapsulation (no UDP) support") refcount_t: increment on 0; use-after-free. WARNING: CPU: 2 PID: 2892 at

[PATCH net-next v2 01/16] l2tp: update sk_user_data while holding sk_callback_lock

2018-02-12 Thread James Chapman
Since L2TP hooks on sockets opened by userspace using sk_user_data, we may race with other socket families that attempt to use the same socket. This problem was discovered by syzbot using AF_KCM. KCM has since been modified to use only TCP sockets to avoid hitting this issue but we should prevent

[PATCH net-next v2 11/16] l2tp: do session destroy using a workqueue

2018-02-12 Thread James Chapman
Handle session destroy in the same way as we handle tunnel destroy - through a workqueue. Sessions can be destroyed either because its socket is closed (if it has a socket) or by netlink request. A workqueue synchronises these. --- net/l2tp/l2tp_core.c | 30 +++---

[PATCH net-next v2 16/16] l2tp: make __l2tp_session_unhash internal

2018-02-12 Thread James Chapman
__l2tp_session_unhash is now only used internally so there is no reason to expose it to other l2tp modules. Rename it l2tp_session_unhash while we're at it. --- net/l2tp/l2tp_core.c | 5 ++--- net/l2tp/l2tp_core.h | 1 - 2 files changed, 2 insertions(+), 4 deletions(-) diff --git

[PATCH net-next v2 14/16] l2tp: remove redundant sk_user_data check when creating tunnels

2018-02-12 Thread James Chapman
l2tp_tunnel_create now checks sk_user_data so this check is redundant --- net/l2tp/l2tp_core.c | 8 1 file changed, 8 deletions(-) diff --git a/net/l2tp/l2tp_core.c b/net/l2tp/l2tp_core.c index c909fe9273c9..a91cd384e397 100644 --- a/net/l2tp/l2tp_core.c +++ b/net/l2tp/l2tp_core.c @@

[PATCH net-next v2 13/16] l2tp: refactor ppp session cleanup paths

2018-02-12 Thread James Chapman
Use l2tp core's session_free callback to drive the ppp session cleanup. PPP sessions are cleaned up by RCU. The PPP session socket is allowed to close only when the session is freed. With this patch, the following syzbot bug reports are finally fixed. Reported-by:

[PATCH net-next v2 04/16] l2tp: refactor tunnel lifetime handling wrt its socket

2018-02-12 Thread James Chapman
Ensure that the tunnel's socket is always extant while the tunnel object exists. Hold a ref on the socket until the tunnel is destroyed and ensure that all tunnel destroy paths go through a common function (l2tp_tunnel_delete). Since the tunnel's socket is now guaranteed to exist if the tunnel

[PATCH net-next v2 08/16] l2tp: hide session from pppol2tp_sock_to_session if it is closing

2018-02-12 Thread James Chapman
--- net/l2tp/l2tp_ppp.c | 10 +- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/net/l2tp/l2tp_ppp.c b/net/l2tp/l2tp_ppp.c index ff95a4d4eac5..947066b3d6d8 100644 --- a/net/l2tp/l2tp_ppp.c +++ b/net/l2tp/l2tp_ppp.c @@ -172,8 +172,16 @@ static inline struct l2tp_session

[PATCH net-next v2 07/16] l2tp: hide sessions if they are closing

2018-02-12 Thread James Chapman
Replace the dead flag in the session context with a closing flag and spinlock. Check it in session lookup functions such that we don't try to access session data while it is being destroyed. --- net/l2tp/l2tp_core.c | 34 +- net/l2tp/l2tp_core.h | 2 ++ 2 files

[PATCH net-next v2 15/16] l2tp: remove unwanted error message

2018-02-12 Thread James Chapman
If when creating a new tunnel, the indicated fd is closed by another thread, we emit an error message about it. e.g. l2tp_core: tunl 4: sockfd_lookup(fd=3) returned -9 It's not useful so remove it. --- net/l2tp/l2tp_core.c | 2 -- 1 file changed, 2 deletions(-) diff --git

[PATCH net-next v2 05/16] l2tp: use tunnel closing flag

2018-02-12 Thread James Chapman
The tunnel's closing flag is set when the tunnel is being destroyed. Use it to reject new sessions and remove acpt_newsess which was doing the same thing. Also prevent the tunnel being seen in l2tp_tunnel_get lookups. --- net/l2tp/l2tp_core.c | 27 +-- net/l2tp/l2tp_core.h

[PATCH net-next v2 00/16] l2tp: fix API races discovered by syzbot

2018-02-12 Thread James Chapman
) its socket closed. (patches 10-13) 6. Misc cleanups made possible by the refactoring done in this series. (patches 14-16) Changes in v2:- Fix compile error that would have broken bisect. James Chapman (16): l2tp: update sk_user_data while holding sk_callback_lock l2tp: add RCU

[PATCH net-next v2 10/16] l2tp: add session_free callback

2018-02-12 Thread James Chapman
When a session refcount hits 0, the session is freed via l2tp_session_free. Some pseudowires (ppp, eth) may have additional resources to free when this happens. Add a session_free callback that can be used by pseudowires to override the default kfree. The callback is responsible for freeing the

[PATCH net-next 02/16] l2tp: add RCU read lock to protect tunnel ptr in ip socket destroy

2018-02-09 Thread James Chapman
4 02 5d c3 80 3d 97 87 bb 01 00 75 f5 48 c7 c7 58 3e cc 82 c6 05 87 87 bb 01 Signed-off-by: James Chapman <jchap...@katalix.com> --- net/l2tp/l2tp_ip.c | 5 - net/l2tp/l2tp_ip6.c | 5 - 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/net/l2tp/l2tp_ip.c b/net/l2tp

[PATCH net-next 01/16] l2tp: update sk_user_data while holding sk_callback_lock

2018-02-09 Thread James Chapman
2/0x670 net/l2tp/l2tp_ppp.c:304 RSP: 8801d4887438 Signed-off-by: James Chapman <jchap...@katalix.com> --- net/l2tp/l2tp_core.c | 21 ++--- net/l2tp/l2tp_ppp.c | 8 ++-- 2 files changed, 24 insertions(+), 5 deletions(-) diff --git a/net/l2tp/l2tp_core.c b/net/

[PATCH net-next 15/16] l2tp: remove unwanted error message

2018-02-09 Thread James Chapman
If when creating a new tunnel, the indicated fd is closed by another thread, we emit an error message about it. e.g. l2tp_core: tunl 4: sockfd_lookup(fd=3) returned -9 It's not useful so remove it. Signed-off-by: James Chapman <jchap...@katalix.com> --- net/l2tp/l2tp_core.c | 2 --

[PATCH net-next 09/16] l2tp: refactor pppol2tp_connect

2018-02-09 Thread James Chapman
It's hard to understand pppol2tp_connect so split it up into separate functions and document it better. Fixes: fd558d186d ("l2tp: Split pppol2tp patch into separate l2tp and ppp parts") Signed-off-by: James Chapman <jchap...@katalix.com> --- net/l2tp/

[PATCH net-next 04/16] l2tp: refactor tunnel lifetime handling wrt its socket

2018-02-09 Thread James Chapman
used so is removed. Fixes: 80d84ef3ff1dd ("l2tp: prevent l2tp_tunnel_delete racing with userspace close") Signed-off-by: James Chapman <jchap...@katalix.com> --- net/l2tp/l2tp_core.c | 128 ++- net/l2tp/l2tp_core.h | 26 ++---

[PATCH net-next 05/16] l2tp: use tunnel closing flag

2018-02-09 Thread James Chapman
The tunnel's closing flag is set when the tunnel is being destroyed. Use it to reject new sessions and remove acpt_newsess which was doing the same thing. Also prevent the tunnel being seen in l2tp_tunnel_get lookups. Signed-off-by: James Chapman <jchap...@katalix.com> --- net/l2tp/l2tp_

[PATCH net-next 10/16] l2tp: add session_free callback

2018-02-09 Thread James Chapman
the session. Signed-off-by: James Chapman <jchap...@katalix.com> --- net/l2tp/l2tp_core.c | 7 +-- net/l2tp/l2tp_core.h | 1 + 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/net/l2tp/l2tp_core.c b/net/l2tp/l2tp_core.c index 869dec89ff0f..d6306ba2d78e 100644 --- a/net/l2tp/l2tp_

[PATCH net-next 13/16] l2tp: refactor ppp session cleanup paths

2018-02-09 Thread James Chapman
...@syzkaller.appspotmail.com Reported-by: syzbot+6e6a5ec8de31a94cd...@syzkaller.appspotmail.com Reported-by: syzbot+19c09769f14b48810...@syzkaller.appspotmail.com Reported-by: syzbot+347bd5acde002e353...@syzkaller.appspotmail.com Signed-off-by: James Chapman <jchap...@katalix.com> --- net/l2tp/l2tp

[PATCH net-next 07/16] l2tp: hide sessions if they are closing

2018-02-09 Thread James Chapman
Replace the dead flag in the session context with a closing flag and spinlock. Check it in session lookup functions such that we don't try to access session data while it is being destroyed. Signed-off-by: James Chapman <jchap...@katalix.com> --- net/l2tp/l2tp_core.

[PATCH net-next 16/16] l2tp: make __l2tp_session_unhash internal

2018-02-09 Thread James Chapman
__l2tp_session_unhash is now only used internally so there is no reason to expose it to other l2tp modules. Rename it l2tp_session_unhash while we're at it. Signed-off-by: James Chapman <jchap...@katalix.com> --- net/l2tp/l2tp_core.c | 5 ++--- net/l2tp/l2tp_core.h | 1 - 2 files chan

[PATCH net-next 11/16] l2tp: do session destroy using a workqueue

2018-02-09 Thread James Chapman
Handle session destroy in the same way as we handle tunnel destroy - through a workqueue. Sessions can be destroyed either because its socket is closed (if it has a socket) or by netlink request. A workqueue synchronises these. Signed-off-by: James Chapman <jchap...@katalix.com> --- ne

[PATCH net-next 14/16] l2tp: remove redundant sk_user_data check when creating tunnels

2018-02-09 Thread James Chapman
l2tp_tunnel_create now checks sk_user_data so this check is redundant Signed-off-by: James Chapman <jchap...@katalix.com> --- net/l2tp/l2tp_core.c | 8 1 file changed, 8 deletions(-) diff --git a/net/l2tp/l2tp_core.c b/net/l2tp/l2tp_core.c index c909fe9273c9..a91cd384e397

[PATCH net-next 03/16] l2tp: don't use inet_shutdown on tunnel destroy

2018-02-09 Thread James Chapman
f ff 1f 00 00 0f 87 7a 13 00 00 45 85 f6 49 8b 85 68 08 00 00 0f 84 ae 03 00 00 c7 44 24 18 00 00 00 00 e9 f0 00 00 00 <49> 81 3c 24 80 93 3f 83 b8 00 00 00 00 44 0f 44 c0 83 fe 01 0f RIP: __lock_acquire+0x263/0x1630 RSP: 88001a37fc70 CR2: 00a0 Signed-off-by: Jame

[PATCH net-next 00/16] l2tp: fix API races discovered by syzbot

2018-02-09 Thread James Chapman
) its socket closed. (patches 10-13) 6. Misc cleanups made possible by the refactoring done in this series. (patches 14-16) James Chapman (16): l2tp: update sk_user_data while holding sk_callback_lock l2tp: add RCU read lock to protect tunnel ptr in ip socket destroy l2tp: don't use

[PATCH net-next 06/16] l2tp: refactor session lifetime handling

2018-02-09 Thread James Chapman
inated tunnels") Signed-off-by: James Chapman <jchap...@katalix.com> --- net/l2tp/l2tp_core.c | 7 ++- net/l2tp/l2tp_ppp.c | 36 ++-- 2 files changed, 20 insertions(+), 23 deletions(-) diff --git a/net/l2tp/l2tp_core.c b/net/l2tp/l2tp_core.c index 691fe93

[PATCH net-next 12/16] l2tp: simplify l2tp_tunnel_closeall

2018-02-09 Thread James Chapman
Since session destroy now uses a workqueue, let l2tp_session_delete handle all the work of destroying a session. Don't remove the session from the tunnel's list immediately. The tunnel will remain extant until all of its sessions are gone anyway. Signed-off-by: James Chapman <jchap...@katalix.

[PATCH net-next 08/16] l2tp: hide session from pppol2tp_sock_to_session if it is closing

2018-02-09 Thread James Chapman
Signed-off-by: James Chapman <jchap...@katalix.com> --- net/l2tp/l2tp_ppp.c | 10 +- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/net/l2tp/l2tp_ppp.c b/net/l2tp/l2tp_ppp.c index ff95a4d4eac5..947066b3d6d8 100644 --- a/net/l2tp/l2tp_ppp.c +++ b/net/l2tp/l2tp_ppp.c @@

l2tp: fixes for syzbot reports - in progress

2018-02-02 Thread James Chapman
I'm currently working on fixes for various l2tp problems reported by syzbot. If anyone else is also working on this, please get back to me so that we can avoid duplicate effort. The syzbot reports are: 9df43faf0 KASAN: use-after-free Read in pppol2tp_connect 6e6a5ec8d general protection fault in

Re: [PATCH net-next] kcm: do not attach sockets if sk_user_data is already used

2018-01-18 Thread James Chapman
On 18 January 2018 at 16:29, Guillaume Nault <g.na...@alphalink.fr> wrote: > On Thu, Jan 18, 2018 at 03:40:52PM +0000, James Chapman wrote: >> On 18 January 2018 at 15:18, Guillaume Nault <g.na...@alphalink.fr> wrote: >> > On Wed, Jan 17, 2018 at 02:25:38PM -0500, Da

Re: [PATCH net-next] kcm: do not attach sockets if sk_user_data is already used

2018-01-18 Thread James Chapman
On 18 January 2018 at 15:18, Guillaume Nault <g.na...@alphalink.fr> wrote: > On Wed, Jan 17, 2018 at 02:25:38PM -0500, David Miller wrote: >> From: James Chapman <jchap...@katalix.com> >> Date: Wed, 17 Jan 2018 11:13:33 + >> >> > On 16 January 2018 a

Re: [PATCH v3 net-next 0/4] l2tp: set l2specific_len based on l2specific_type

2018-01-17 Thread James Chapman
; Mark L2TP_ATTR_L2SPEC_LEN attribute as not used >> > Nice. Thanks for doing this work Lorenzo. Acked-by: James Chapman <jchap...@katalix.com>

Re: [PATCH net-next] kcm: do not attach sockets if sk_user_data is already used

2018-01-17 Thread James Chapman
On 16 January 2018 at 19:00, David Miller wrote: > From: Tom Herbert > Date: Tue, 16 Jan 2018 09:36:41 -0800 > >> sk_user_data is set with the sk_callback lock held in code below. >> Should be able to take the lock earlier can do this check under the >>

Re: [PATCH v2 net-next 1/5] l2tp: fix switch default error handling in l2tp_nl_cmd_session_create()

2018-01-15 Thread James Chapman
On 15 January 2018 at 21:18, Lorenzo Bianconi wrote: >> On Sun, Jan 14, 2018 at 03:50:54PM +0100, Lorenzo Bianconi wrote: >>> Although this issue is harmless since that code path is protected by the >>> check on l2tp_nl_cmd_ops[]/l2tp_nl_cmd_ops[]->session_create(),

[PATCH net-next] kcm: do not attach sockets if sk_user_data is already used

2018-01-14 Thread James Chapman
inline] RSP: 8801d4887438 RIP: pppol2tp_sendmsg+0x512/0x670 net/l2tp/l2tp_ppp.c:304 RSP: 8801d4887438 Reported-by: syzbot+114b15f2be420a888...@syzkaller.appspotmail.com Fixes: ab7ac4eb9832 ("kcm: Kernel Connection Multiplexor module") Signed-off-by: James Chapman <jchap...@ka

Re: [PATCH net-next] l2tp: adjust comments about L2TPv3 offsets

2018-01-07 Thread James Chapman
are > - * present. > + * Cookie value and sublayer format are negotiated with the peer when > + * the session is set up. Unlike L2TPv2, we do not need to parse the > + * packet header to determine if optional fields are present. > * > * Caller must already have parsed the frame and determined that it is > * a data (not control) frame before coming here. Fields up to the Acked-by: James Chapman <jchap...@katalix.com>

Re: [PATCH net-next 3/4] l2tp: remove configurable payload offset

2018-01-04 Thread James Chapman
On 04/01/18 10:25, Guillaume Nault wrote: >> diff --git a/net/l2tp/l2tp_debugfs.c b/net/l2tp/l2tp_debugfs.c >> index eb69411..2c30587 100644 >> --- a/net/l2tp/l2tp_debugfs.c >> +++ b/net/l2tp/l2tp_debugfs.c >> @@ -180,8 +180,8 @@ static void l2tp_dfs_seq_session_show(struct seq_file >> *m, void

[PATCH net-next 0/4] l2tp: remove configurable offset parameters

2018-01-03 Thread James Chapman
r of bytes padding between L2TPv3 header and payload. This is not compliant with L2TPv3 RFC3931. So this change removes the configurable offset altogether while retaining L2TP_ATTR_OFFSET in the API for backwards compatibility. If L2TP_ATTR_OFFSET is given, its value is now silently ignored. Jame

[PATCH net-next 4/4] l2tp: add comment in API header that L2TP_ATTR_OFFSET is not used

2018-01-03 Thread James Chapman
Signed-off-by: James Chapman <jchap...@katalix.com> --- include/uapi/linux/l2tp.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/uapi/linux/l2tp.h b/include/uapi/linux/l2tp.h index d84ce5c..f78eef4 100644 --- a/include/uapi/linux/l2tp.h +++ b/include/uapi/linux/

[PATCH net-next 2/4] l2tp: revert "l2tp: fix missing print session offset info"

2018-01-03 Thread James Chapman
Revert commit 820da5357572 ("l2tp: fix missing print session offset info"). The peer_offset parameter is removed. Signed-off-by: James Chapman <jchap...@katalix.com> --- net/l2tp/l2tp_netlink.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/net/l2tp/l2tp_netl

[PATCH net-next 3/4] l2tp: remove configurable payload offset

2018-01-03 Thread James Chapman
FSET value is ignored. Signed-off-by: James Chapman <jchap...@katalix.com> --- net/l2tp/l2tp_core.c| 14 -- net/l2tp/l2tp_core.h| 3 --- net/l2tp/l2tp_debugfs.c | 4 ++-- net/l2tp/l2tp_netlink.c | 3 --- 4 files changed, 6 insertions(+), 18 deletions(-) diff

[PATCH net-next 1/4] l2tp: revert "l2tp: add peer_offset parameter"

2018-01-03 Thread James Chapman
Revert commit f15bc54eeecd ("l2tp: add peer_offset parameter"). This is removed because it is adding another configurable offset and configurable offsets are being removed. Signed-off-by: James Chapman <jchap...@katalix.com> --- include/uapi/linux/l2tp.h | 1 - net/l2tp/l2tp_

Re: [PATCH net-next 2/2] l2tp: add peer_offset parameter

2018-01-02 Thread James Chapman
On 02/01/18 20:08, James Chapman wrote: On 02/01/18 18:05, Guillaume Nault wrote: Lorenzo, is this being added to fix interoperability with another L2TPv3 implementation? If so, can you share more details? Hi James, I introduced peer_offset parameter to fix a specific setup where tunnel

Re: [PATCH net-next 2/2] l2tp: add peer_offset parameter

2018-01-02 Thread James Chapman
On 02/01/18 17:50, Guillaume Nault wrote: On Fri, Dec 29, 2017 at 06:53:56PM +, James Chapman wrote: On 28/12/17 19:45, Guillaume Nault wrote: Here we have an option that: * creates invalid packets (AFAIK), * is buggy and leaks memory on the network, * doesn't seem to have any

Re: [PATCH net-next 2/2] l2tp: add peer_offset parameter

2018-01-02 Thread James Chapman
On 02/01/18 19:28, Lorenzo Bianconi wrote: Lorenzo, is this being added to fix interoperability with another L2TPv3 implementation? If so, can you share more details? Hi James, I introduced peer_offset parameter to fix a specific setup where tunnel endpoints running L2TPv3 would use different

Re: [PATCH net-next 2/2] l2tp: add peer_offset parameter

2018-01-02 Thread James Chapman
On 02/01/18 18:05, Guillaume Nault wrote: Lorenzo, is this being added to fix interoperability with another L2TPv3 implementation? If so, can you share more details? Hi James, I introduced peer_offset parameter to fix a specific setup where tunnel endpoints running L2TPv3 would use different

Re: [PATCH net-next 2/2] l2tp: add peer_offset parameter

2017-12-29 Thread James Chapman
Sorry for only just seeing this (vacation). On 28/12/17 19:45, Guillaume Nault wrote: On Thu, Dec 28, 2017 at 07:23:48PM +0100, Lorenzo Bianconi wrote: On Dec 28, Guillaume Nault wrote: After a quick review of L2TPv3 and pseudowires RFCs, I still don't see how adding some padding between the

Re: Fw: [Bug 197099] New: Kernel panic in interrupt [l2tp_ppp]

2017-10-06 Thread James Chapman
On 6 October 2017 at 05:45, SviMik <svi...@gmail.com> wrote: > 2017-10-04 10:49 GMT+03:00 James Chapman <jchap...@katalix.com>: >> On 3 October 2017 at 08:27, James Chapman <jchap...@katalix.com> wrote: >>> For capturing complete oops messages, have you tried

Re: Fw: [Bug 197099] New: Kernel panic in interrupt [l2tp_ppp]

2017-10-04 Thread James Chapman
On 3 October 2017 at 08:27, James Chapman <jchap...@katalix.com> wrote: > On 2 October 2017 at 19:35, SviMik <svi...@gmail.com> wrote: >> Hi, James! >> >> No, I'm suffering from kernel panics since I started using 4.x >> kernels. > It's interesting that

Re: Fw: [Bug 197099] New: Kernel panic in interrupt [l2tp_ppp]

2017-10-03 Thread James Chapman
slog on reboot. > 2017-10-02 16:56 GMT+03:00 Eric Dumazet <eric.duma...@gmail.com>: >> CC svi...@gmail.com so that he is aware of this netdev thread. >> >> On Mon, 2017-10-02 at 14:32 +0100, James Chapman wrote: >>> This seems to be a NULL pointer exception caused

Re: Fw: [Bug 197099] New: Kernel panic in interrupt [l2tp_ppp]

2017-10-02 Thread James Chapman
This seems to be a NULL pointer exception caused by tunnel->sock being NULL at the call to bh_lock_sock() in l2tp_xmit_skb() at l2tp_core.c:1135. tunnel->sock is set NULL in l2tp_core's tunnel socket destructor. At the moment, I don't understand how this happens because pppol2tp_xmit() does a

Re: [PATCH net-next 2/2] l2tp: define "l2tpeth" device type

2017-04-25 Thread James Chapman
On 24/04/17 13:16, Guillaume Nault wrote: > Export type of l2tpeth interfaces to userspace > (/sys/class/net//uevent). > > Signed-off-by: Guillaume Nault <g.na...@alphalink.fr> Acked-by: James Chapman <jchap...@katalix.com>

Re: [PATCH net-next 1/2] l2tp: set name_assign_type for devices created by l2tp_eth.c

2017-04-25 Thread James Chapman
na...@alphalink.fr> Acked-by: James Chapman <jchap...@katalix.com>

Re: [PATCH net-next v4 1/2] New kernel function to get IP overhead on a socket.

2017-03-24 Thread James Chapman
4 or IPv6 address families. >>> >>> Signed-off-by: R. Parameswaran <rpara...@brocade.com> >> Just use the IPv4/IPv6 header size for now, just like the VXLAN >> driver does. >> > Actually, that's how the original posting was - it was changed in >

Re: [PATCH net-next v4 1/2]L2TP:Adjust intf MTU, add underlay L3, L2 hdrs

2017-03-20 Thread James Chapman
np->opt, > + owned_by_user); > + if (optv6) > + overhead += (optv6->opt_flen + optv6->opt_nflen); > + return overhead; > +#endif /* IS_ENABLED(CONFIG_IPV6) */ > + default: /* Returns 0 overhead if the socket is not ipv4 or ipv6 */ > + return overhead; > + } > +} > +EXPORT_SYMBOL(kernel_sock_ip_overhead); -- James Chapman Katalix Systems Ltd http://www.katalix.com Catalysts for your Embedded Linux software development

Re: [PATCH net-next v4 2/2]L2TP:Adjust intf MTU, add underlay L3, L2 hdrs

2017-03-20 Thread James Chapman
_id, u32 p > } > > dev_net_set(dev, net); > - if (session->mtu == 0) > - session->mtu = dev->mtu - session->hdr_len; > - dev->mtu = session->mtu; > - dev->needed_headroom += session->hdr_len; > dev->min_mtu = 0; > dev->max_mtu = ETH_MAX_MTU; > > + l2tp_eth_adjust_mtu(tunnel, session, dev); > priv = netdev_priv(dev); > priv->dev = dev; > priv->session = session; -- James Chapman Katalix Systems Ltd http://www.katalix.com Catalysts for your Embedded Linux software development

Re: [PATCH v2 net 0/5] l2tp: fixes for l2tp_ip and l2tp_ip6 socket handling

2016-11-29 Thread James Chapman
l2tp/l2tp_ip.c | 63 ++ > net/l2tp/l2tp_ip6.c | 79 > --------- > 4 files changed, 81 insertions(+), 67 deletions(-) > Looks good. Acked-by: James Chapman <jchap...@katalix.com>

Re: [PATCH net v2] L2TP:Adjust intf MTU,factor underlay L3,overlay L2

2016-10-11 Thread James Chapman
On 11/10/16 02:54, R Parameswaran wrote: > > > Hi James, > > Please see inline: > > On Tue, Oct 4, 2016 at 12:53 AM, James Chapman <jchap...@katalix.com > <mailto:jchap...@katalix.com>> wrote: > > On 04/10/16 04:12, R. Parameswaran wrote: > &

Re: [PATCH net v2] L2TP:Adjust intf MTU,factor underlay L3,overlay L2

2016-10-04 Thread James Chapman
On 04/10/16 04:12, R. Parameswaran wrote: > > Hi James, > > Please see inline, thanks for the reply: > > On Sat, 1 Oct 2016, James Chapman wrote: > >> On 30/09/16 03:39, R. Parameswaran wrote: >>>>> + /* Adjust MTU, factor overhead - underlay L3 hdr,

Re: [PATCH net v2] L2TP:Adjust intf MTU,factor underlay L3,overlay L2

2016-10-01 Thread James Chapman
On 30/09/16 03:39, R. Parameswaran wrote: > >>> + /* Adjust MTU, factor overhead - underlay L3 hdr, overlay L2 hdr*/ >>> + if (tunnel->sock->sk_family == AF_INET) >>> + overhead += (ETH_HLEN + sizeof(struct iphdr)); >>> + else if (tunnel->sock->sk_family == AF_INET6) >>> +

Re: [PATCH net v2] L2TP:Adjust intf MTU,factor underlay L3,overlay L2

2016-09-29 Thread James Chapman
On 29/09/16 03:36, R. Parameswaran wrote: > I agree that something like 2. below would be needed in the long run (it > will need some effort and redesign -e.g. how do I lookup the parent tunnel > from the socket when receiving a PMTU update, existing pointer chain runs > from tunnel to socket).

Re: [PATCH net v2] L2TP:Adjust intf MTU,factor underlay L3,overlay L2

2016-09-29 Thread James Chapman
On 22/09/16 21:52, R. Parameswaran wrote: > From ed585bdd6d3d2b3dec58d414f514cd764d89159d Mon Sep 17 00:00:00 2001 > From: "R. Parameswaran" > Date: Thu, 22 Sep 2016 13:19:25 -0700 > Subject: [PATCH] L2TP:Adjust intf MTU,factor underlay L3,overlay L2 > > Take into account

Re: [PATCH] net: l2tp: fix reversed udp6 checksum flags

2016-04-28 Thread James Chapman
L2TP peers. One for stable? Acked-by: James Chapman <jchap...@katalix.com> On 28 April 2016 at 18:29, Wang Shanker <shankerwangm...@gmail.com> wrote: > This patch fixes a bug which causes the behavior of whether to ignore > udp6 checksum of udp6 encapsulated l2tp tunnel contrary

Re: [PATCH] ip: add udp_csum, udp6_csum_tx, udp6_csum_rx control flags to ip l2tp add tunnel

2016-04-28 Thread James Chapman
: >> >> >> >>> 在 2016年4月27日,20:21,James Chapman <jchap...@katalix.com> 写道: >>> >>> On 26 April 2016 at 15:15, Wang Shanker <shankerwangm...@gmail.com> wrote: >>>> Hi, all >>>> >>>> It’s my first time to contr

Re: [PATCH] ip: add udp_csum, udp6_csum_tx, udp6_csum_rx control flags to ip l2tp add tunnel

2016-04-27 Thread James Chapman
On 26 April 2016 at 15:15, Wang Shanker wrote: > Hi, all > > It’s my first time to contribute to such an important open source project. > Things began when I upgraded my server, called "Server A", form ubuntu 14.04 > to 16.04, which is shipped with new kernel version,

Re: [PATCH] l2tp: protect tunnel->del_work by ref_count

2015-09-28 Thread James Chapman
Acked-by: James Chapman <jchap...@katalix.com> On 28 September 2015 at 10:32, Alexander Couzens <lyn...@fe80.eu> wrote: > There is a small chance that tunnel_free() is called before tunnel->del_work > scheduled > resulting in a zero pointer dereference. > > Signed-

Re: [PATCH net-next 0/3] l2tp: module autoloading

2015-09-24 Thread James Chapman
good. Acked-By: James Chapman <jchap...@katalix.com> -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html

Re: problems with L2TP

2015-07-09 Thread James Chapman
. This is a control protocol implementation issue, not a kernel issue. -- James Chapman Katalix Systems Ltd http://www.katalix.com Catalysts for your Embedded Linux software development -- To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to majord

Re: problems with L2TP

2015-07-08 Thread James Chapman
driver so these might be useful as a reference: [a] pppd's pppol2tp plugin - http://download.samba.org/pub/ppp/ [b] accel-ppp - http://accel-ppp.org -- James Chapman Katalix Systems Ltd http://www.katalix.com Catalysts for your Embedded Linux software development -- To unsubscribe from this list

  1   2   >