When a file descriptor of pppol2tp socket is passed as file descriptor
of UDP socket, a recursive deadlock occurs in l2tp_tunnel_register().
This situation is reproduced by the following program:
int main(void)
{
int sock;
struct sockaddr_pppol2tp addr;
sock = socket(AF_PPPOX, SOCK_DGRAM, PX_PROTO_OL2TP);
if (sock < 0) {
perror("socket");
return 1;
}
addr.sa_family = AF_PPPOX;
addr.sa_protocol = PX_PROTO_OL2TP;
addr.pppol2tp.pid = 0;
addr.pppol2tp.fd = sock;
addr.pppol2tp.addr.sin_family = PF_INET;
addr.pppol2tp.addr.sin_port = htons(0);
addr.pppol2tp.addr.sin_addr.s_addr = inet_addr("192.168.0.1");
addr.pppol2tp.s_tunnel = 1;
addr.pppol2tp.s_session = 0;
addr.pppol2tp.d_tunnel = 0;
addr.pppol2tp.d_session = 0;
if (connect(sock, (const struct sockaddr *)&addr, sizeof(addr)) < 0) {
perror("connect");
return 1;
}
return 0;
}
This program causes the following lockdep warning:
============================================
WARNING: possible recursive locking detected
6.2.0-rc5-00205-gc96618275234 #56 Not tainted
--------------------------------------------
repro/8607 is trying to acquire lock:
ffff8880213c8130 (sk_lock-AF_PPPOX){+.+.}-{0:0}, at: l2tp_tunnel_register+0x2b7/0x11c0
but task is already holding lock:
ffff8880213c8130 (sk_lock-AF_PPPOX){+.+.}-{0:0}, at: pppol2tp_connect+0xa82/0x1a30
other info that might help us debug this:
Possible unsafe locking scenario:
CPU0
----
lock(sk_lock-AF_PPPOX);
lock(sk_lock-AF_PPPOX);
*** DEADLOCK ***
May be due to missing lock nesting notation
1 lock held by repro/8607:
#0: ffff8880213c8130 (sk_lock-AF_PPPOX){+.+.}-{0:0}, at: pppol2tp_connect+0xa82/0x1a30
stack backtrace:
CPU: 0 PID: 8607 Comm: repro Not tainted 6.2.0-rc5-00205-gc96618275234 #56
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.16.1-2.fc37 04/01/2014
Call Trace:
<TASK>
dump_stack_lvl+0x100/0x178
__lock_acquire.cold+0x119/0x3b9
? lockdep_hardirqs_on_prepare+0x410/0x410
lock_acquire+0x1e0/0x610
? l2tp_tunnel_register+0x2b7/0x11c0
? lock_downgrade+0x710/0x710
? __fget_files+0x283/0x3e0
lock_sock_nested+0x3a/0xf0
? l2tp_tunnel_register+0x2b7/0x11c0
l2tp_tunnel_register+0x2b7/0x11c0
? sprintf+0xc4/0x100
? l2tp_tunnel_del_work+0x6b0/0x6b0
? debug_object_deactivate+0x320/0x320
? lockdep_init_map_type+0x16d/0x7a0
? lockdep_init_map_type+0x16d/0x7a0
? l2tp_tunnel_create+0x2bf/0x4b0
? l2tp_tunnel_create+0x3c6/0x4b0
pppol2tp_connect+0x14e1/0x1a30
? pppol2tp_put_sk+0xd0/0xd0
? aa_sk_perm+0x2b7/0xa80
? aa_af_perm+0x260/0x260
? bpf_lsm_socket_connect+0x9/0x10
? pppol2tp_put_sk+0xd0/0xd0
__sys_connect_file+0x14f/0x190
__sys_connect+0x133/0x160
? __sys_connect_file+0x190/0x190
? lockdep_hardirqs_on+0x7d/0x100
? ktime_get_coarse_real_ts64+0x1b7/0x200
? ktime_get_coarse_real_ts64+0x147/0x200
? __audit_syscall_entry+0x396/0x500
__x64_sys_connect+0x72/0xb0
do_syscall_64+0x38/0xb0
entry_SYSCALL_64_after_hwframe+0x63/0xcd
This patch fixes the issue by getting/creating the tunnel before
locking the pppol2tp socket.
Fixes: 0b2c59720e65 ("l2tp: close all race conditions in l2tp_tunnel_register()")
Signed-off-by: Shigeru Yoshida <[email protected]>
---
v2: Get/create the tunnel before locking the pppol2tp socket suggested
by Guillaume Nault.
v1: https://lore.kernel.org/all/[email protected]/
net/l2tp/l2tp_ppp.c | 118 ++++++++++++++++++++++++--------------------
1 file changed, 64 insertions(+), 54 deletions(-)
diff --git a/net/l2tp/l2tp_ppp.c b/net/l2tp/l2tp_ppp.c
index db2e584c625e..68d02e217ca3 100644
--- a/net/l2tp/l2tp_ppp.c
+++ b/net/l2tp/l2tp_ppp.c
@@ -650,6 +650,65 @@ static int pppol2tp_tunnel_mtu(const struct l2tp_tunnel *tunnel)
return mtu - PPPOL2TP_HEADER_OVERHEAD;
}
+static struct l2tp_tunnel *pppol2tp_tunnel_get(struct net *net,
+ struct l2tp_connect_info *info,
+ bool *new_tunnel)
+{
+ struct l2tp_tunnel *tunnel;
+ int error;
+
+ *new_tunnel = false;
+
+ tunnel = l2tp_tunnel_get(net, info->tunnel_id);
+
+ /* Special case: create tunnel context if session_id and
+ * peer_session_id is 0. Otherwise look up tunnel using supplied
+ * tunnel id.
+ */
+ if (!info->session_id && !info->peer_session_id) {
+ if (!tunnel) {
+ struct l2tp_tunnel_cfg tcfg = {
+ .encap = L2TP_ENCAPTYPE_UDP,
+ };
+
+ /* Prevent l2tp_tunnel_register() from trying to set up
+ * a kernel socket.
+ */
+ if (info->fd < 0)
+ return ERR_PTR(-EBADF);
+
+ error = l2tp_tunnel_create(info->fd,
+ info->version,
+ info->tunnel_id,
+ info->peer_tunnel_id, &tcfg,
+ &tunnel);
+ if (error < 0)
+ return ERR_PTR(error);
+
+ l2tp_tunnel_inc_refcount(tunnel);
+ error = l2tp_tunnel_register(tunnel, net, &tcfg);
+ if (error < 0) {
+ kfree(tunnel);
+ return ERR_PTR(error);
+ }
+
+ *new_tunnel = true;
+ }
+ } else {
+ /* Error if we can't find the tunnel */
+ if (!tunnel)
+ return ERR_PTR(-ENOENT);
+
+ /* Error if socket is not prepped */
+ if (!tunnel->sock) {
+ l2tp_tunnel_dec_refcount(tunnel);
+ return ERR_PTR(-ENOENT);
+ }
+ }
+
+ return tunnel;
+}
+
/* connect() handler. Attach a PPPoX socket to a tunnel UDP socket
*/
static int pppol2tp_connect(struct socket *sock, struct sockaddr *uservaddr,
@@ -663,7 +722,6 @@ static int pppol2tp_connect(struct socket *sock, struct sockaddr *uservaddr,
struct pppol2tp_session *ps;
struct l2tp_session_cfg cfg = { 0, };
bool drop_refcnt = false;
- bool drop_tunnel = false;
bool new_session = false;
bool new_tunnel = false;
int error;
@@ -672,6 +730,10 @@ static int pppol2tp_connect(struct socket *sock, struct sockaddr *uservaddr,
if (error < 0)
return error;
+ tunnel = pppol2tp_tunnel_get(sock_net(sk), &info, &new_tunnel);
+ if (IS_ERR(tunnel))
+ return PTR_ERR(tunnel);
+
lock_sock(sk);
/* Check for already bound sockets */
@@ -689,57 +751,6 @@ static int pppol2tp_connect(struct socket *sock, struct sockaddr *uservaddr,
if (!info.tunnel_id)
goto end;
- tunnel = l2tp_tunnel_get(sock_net(sk), info.tunnel_id);
- if (tunnel)
- drop_tunnel = true;
-
- /* Special case: create tunnel context if session_id and
- * peer_session_id is 0. Otherwise look up tunnel using supplied
- * tunnel id.
- */
- if (!info.session_id && !info.peer_session_id) {
- if (!tunnel) {
- struct l2tp_tunnel_cfg tcfg = {
- .encap = L2TP_ENCAPTYPE_UDP,
- };
-
- /* Prevent l2tp_tunnel_register() from trying to set up
- * a kernel socket.
- */
- if (info.fd < 0) {
- error = -EBADF;
- goto end;
- }
-
- error = l2tp_tunnel_create(info.fd,
- info.version,
- info.tunnel_id,
- info.peer_tunnel_id, &tcfg,
- &tunnel);
- if (error < 0)
- goto end;
-
- l2tp_tunnel_inc_refcount(tunnel);
- error = l2tp_tunnel_register(tunnel, sock_net(sk),
- &tcfg);
- if (error < 0) {
- kfree(tunnel);
- goto end;
- }
- drop_tunnel = true;
- new_tunnel = true;
- }
- } else {
- /* Error if we can't find the tunnel */
- error = -ENOENT;
- if (!tunnel)
- goto end;
-
- /* Error if socket is not prepped */
- if (!tunnel->sock)
- goto end;
- }
-
if (tunnel->peer_tunnel_id == 0)
tunnel->peer_tunnel_id = info.peer_tunnel_id;
@@ -840,8 +851,7 @@ static int pppol2tp_connect(struct socket *sock, struct sockaddr *uservaddr,
}
if (drop_refcnt)
l2tp_session_dec_refcount(session);
- if (drop_tunnel)
- l2tp_tunnel_dec_refcount(tunnel);
+ l2tp_tunnel_dec_refcount(tunnel);
release_sock(sk);
return error;
--
2.39.0
On Mon, Feb 13, 2023 at 01:26:23AM +0900, Shigeru Yoshida wrote:
> +static struct l2tp_tunnel *pppol2tp_tunnel_get(struct net *net,
> + struct l2tp_connect_info *info,
Please make "*info" const.
> + bool *new_tunnel)
> +{
> + struct l2tp_tunnel *tunnel;
> + int error;
> +
> + *new_tunnel = false;
> +
> + tunnel = l2tp_tunnel_get(net, info->tunnel_id);
> +
> + /* Special case: create tunnel context if session_id and
> + * peer_session_id is 0. Otherwise look up tunnel using supplied
> + * tunnel id.
> + */
> + if (!info->session_id && !info->peer_session_id) {
> + if (!tunnel) {
> + struct l2tp_tunnel_cfg tcfg = {
> + .encap = L2TP_ENCAPTYPE_UDP,
> + };
> +
> + /* Prevent l2tp_tunnel_register() from trying to set up
> + * a kernel socket.
> + */
> + if (info->fd < 0)
> + return ERR_PTR(-EBADF);
> +
> + error = l2tp_tunnel_create(info->fd,
> + info->version,
> + info->tunnel_id,
> + info->peer_tunnel_id, &tcfg,
> + &tunnel);
> + if (error < 0)
> + return ERR_PTR(error);
> +
> + l2tp_tunnel_inc_refcount(tunnel);
> + error = l2tp_tunnel_register(tunnel, net, &tcfg);
> + if (error < 0) {
> + kfree(tunnel);
> + return ERR_PTR(error);
> + }
> +
> + *new_tunnel = true;
> + }
> + } else {
> + /* Error if we can't find the tunnel */
> + if (!tunnel)
> + return ERR_PTR(-ENOENT);
> +
> + /* Error if socket is not prepped */
> + if (!tunnel->sock) {
> + l2tp_tunnel_dec_refcount(tunnel);
> + return ERR_PTR(-ENOENT);
> + }
> + }
> +
> + return tunnel;
> +}
> +
> /* connect() handler. Attach a PPPoX socket to a tunnel UDP socket
> */
> static int pppol2tp_connect(struct socket *sock, struct sockaddr *uservaddr,
> @@ -663,7 +722,6 @@ static int pppol2tp_connect(struct socket *sock, struct sockaddr *uservaddr,
> struct pppol2tp_session *ps;
> struct l2tp_session_cfg cfg = { 0, };
> bool drop_refcnt = false;
> - bool drop_tunnel = false;
> bool new_session = false;
> bool new_tunnel = false;
> int error;
> @@ -672,6 +730,10 @@ static int pppol2tp_connect(struct socket *sock, struct sockaddr *uservaddr,
> if (error < 0)
> return error;
>
> + tunnel = pppol2tp_tunnel_get(sock_net(sk), &info, &new_tunnel);
> + if (IS_ERR(tunnel))
> + return PTR_ERR(tunnel);
> +
> lock_sock(sk);
>
> /* Check for already bound sockets */
> @@ -689,57 +751,6 @@ static int pppol2tp_connect(struct socket *sock, struct sockaddr *uservaddr,
> if (!info.tunnel_id)
> goto end;
The original code did test info.tunnel_id before trying to get or
create the tunnel (as it doesn't make sense to work on a tunnel whose
ID is 0). So we need move this test before the pppol2tp_tunnel_get()
call.
> - tunnel = l2tp_tunnel_get(sock_net(sk), info.tunnel_id);
> - if (tunnel)
> - drop_tunnel = true;
> -
> - /* Special case: create tunnel context if session_id and
> - * peer_session_id is 0. Otherwise look up tunnel using supplied
> - * tunnel id.
> - */
Just a note for your future submissions: for networking patches, we
normally indicate which tree the patch is targetted to in the mail
subject (for example "[PATCH net v2]"). Also, you should Cc:
the author of the patch listed in the Fixes tag.
From: Shigeru Yoshida <[email protected]>
Date: Mon, 13 Feb 2023 01:26:23 +0900
> When a file descriptor of pppol2tp socket is passed as file descriptor
> of UDP socket, a recursive deadlock occurs in l2tp_tunnel_register().
> This situation is reproduced by the following program:
[...]
> +static struct l2tp_tunnel *pppol2tp_tunnel_get(struct net *net,
> + struct l2tp_connect_info *info,
> + bool *new_tunnel)
> +{
> + struct l2tp_tunnel *tunnel;
> + int error;
> +
> + *new_tunnel = false;
> +
> + tunnel = l2tp_tunnel_get(net, info->tunnel_id);
> +
> + /* Special case: create tunnel context if session_id and
> + * peer_session_id is 0. Otherwise look up tunnel using supplied
> + * tunnel id.
> + */
> + if (!info->session_id && !info->peer_session_id) {
> + if (!tunnel) {
This `if` is the sole thing the outer `if` contains, could we combine them?
> + struct l2tp_tunnel_cfg tcfg = {
> + .encap = L2TP_ENCAPTYPE_UDP,
> + };
> +
> + /* Prevent l2tp_tunnel_register() from trying to set up
> + * a kernel socket.
> + */
> + if (info->fd < 0)
> + return ERR_PTR(-EBADF);
> +
> + error = l2tp_tunnel_create(info->fd,
> + info->version,
This fits into the prev line.
> + info->tunnel_id,
> + info->peer_tunnel_id, &tcfg,
> + &tunnel);
> + if (error < 0)
> + return ERR_PTR(error);
> +
> + l2tp_tunnel_inc_refcount(tunnel);
> + error = l2tp_tunnel_register(tunnel, net, &tcfg);
> + if (error < 0) {
> + kfree(tunnel);
> + return ERR_PTR(error);
> + }
> +
> + *new_tunnel = true;
> + }
> + } else {
> + /* Error if we can't find the tunnel */
> + if (!tunnel)
> + return ERR_PTR(-ENOENT);
[...]
Thanks,
Olek
On Mon, Feb 13, 2023 at 04:05:59PM +0100, Alexander Lobakin wrote:
> From: Shigeru Yoshida <[email protected]>
> Date: Mon, 13 Feb 2023 01:26:23 +0900
>
> > When a file descriptor of pppol2tp socket is passed as file descriptor
> > of UDP socket, a recursive deadlock occurs in l2tp_tunnel_register().
> > This situation is reproduced by the following program:
>
> [...]
>
> > +static struct l2tp_tunnel *pppol2tp_tunnel_get(struct net *net,
> > + struct l2tp_connect_info *info,
> > + bool *new_tunnel)
> > +{
> > + struct l2tp_tunnel *tunnel;
> > + int error;
> > +
> > + *new_tunnel = false;
> > +
> > + tunnel = l2tp_tunnel_get(net, info->tunnel_id);
> > +
> > + /* Special case: create tunnel context if session_id and
> > + * peer_session_id is 0. Otherwise look up tunnel using supplied
> > + * tunnel id.
> > + */
> > + if (!info->session_id && !info->peer_session_id) {
> > + if (!tunnel) {
>
> This `if` is the sole thing the outer `if` contains, could we combine them?
The logic of this code is a bit convoluted, sure, but if we want to
rework it, let's simplify it for real:
tunnel = l2tp_tunnel_get(...)
if (tunnel)
return tunnel; /* the original !tunnel->sock test is useless I believe */
/* Tunnel not found. Try to create one if both session_id and
* peer_session_id are 0. Fail otherwise.
*/
if (info->session_id || info->peer_session_id)
return ERR_PTR(-ENOENT);
[...] /* Tunnel creation code */
However, I'd prefer to keep such refactoring for later. Keeping the
same structure in pppol2tp_tunnel_get() as in the original code helps
reviewing the patch.
> > + struct l2tp_tunnel_cfg tcfg = {
> > + .encap = L2TP_ENCAPTYPE_UDP,
> > + };
> > +
> > + /* Prevent l2tp_tunnel_register() from trying to set up
> > + * a kernel socket.
> > + */
> > + if (info->fd < 0)
> > + return ERR_PTR(-EBADF);
> > +
> > + error = l2tp_tunnel_create(info->fd,
> > + info->version,
>
> This fits into the prev line.
>
> > + info->tunnel_id,
> > + info->peer_tunnel_id, &tcfg,
> > + &tunnel);
> > + if (error < 0)
> > + return ERR_PTR(error);
> > +
> > + l2tp_tunnel_inc_refcount(tunnel);
> > + error = l2tp_tunnel_register(tunnel, net, &tcfg);
> > + if (error < 0) {
> > + kfree(tunnel);
> > + return ERR_PTR(error);
> > + }
> > +
> > + *new_tunnel = true;
> > + }
> > + } else {
> > + /* Error if we can't find the tunnel */
> > + if (!tunnel)
> > + return ERR_PTR(-ENOENT);
>
> [...]
>
> Thanks,
> Olek
>
Hi Guillaume,
On Mon, Feb 13, 2023 at 03:55:24PM +0100, Guillaume Nault wrote:
> On Mon, Feb 13, 2023 at 01:26:23AM +0900, Shigeru Yoshida wrote:
> > +static struct l2tp_tunnel *pppol2tp_tunnel_get(struct net *net,
> > + struct l2tp_connect_info *info,
>
> Please make "*info" const.
Thank you so much for your comment. I got it.
> > + bool *new_tunnel)
> > +{
> > + struct l2tp_tunnel *tunnel;
> > + int error;
> > +
> > + *new_tunnel = false;
> > +
> > + tunnel = l2tp_tunnel_get(net, info->tunnel_id);
> > +
> > + /* Special case: create tunnel context if session_id and
> > + * peer_session_id is 0. Otherwise look up tunnel using supplied
> > + * tunnel id.
> > + */
> > + if (!info->session_id && !info->peer_session_id) {
> > + if (!tunnel) {
> > + struct l2tp_tunnel_cfg tcfg = {
> > + .encap = L2TP_ENCAPTYPE_UDP,
> > + };
> > +
> > + /* Prevent l2tp_tunnel_register() from trying to set up
> > + * a kernel socket.
> > + */
> > + if (info->fd < 0)
> > + return ERR_PTR(-EBADF);
> > +
> > + error = l2tp_tunnel_create(info->fd,
> > + info->version,
> > + info->tunnel_id,
> > + info->peer_tunnel_id, &tcfg,
> > + &tunnel);
> > + if (error < 0)
> > + return ERR_PTR(error);
> > +
> > + l2tp_tunnel_inc_refcount(tunnel);
> > + error = l2tp_tunnel_register(tunnel, net, &tcfg);
> > + if (error < 0) {
> > + kfree(tunnel);
> > + return ERR_PTR(error);
> > + }
> > +
> > + *new_tunnel = true;
> > + }
> > + } else {
> > + /* Error if we can't find the tunnel */
> > + if (!tunnel)
> > + return ERR_PTR(-ENOENT);
> > +
> > + /* Error if socket is not prepped */
> > + if (!tunnel->sock) {
> > + l2tp_tunnel_dec_refcount(tunnel);
> > + return ERR_PTR(-ENOENT);
> > + }
> > + }
> > +
> > + return tunnel;
> > +}
> > +
> > /* connect() handler. Attach a PPPoX socket to a tunnel UDP socket
> > */
> > static int pppol2tp_connect(struct socket *sock, struct sockaddr *uservaddr,
> > @@ -663,7 +722,6 @@ static int pppol2tp_connect(struct socket *sock, struct sockaddr *uservaddr,
> > struct pppol2tp_session *ps;
> > struct l2tp_session_cfg cfg = { 0, };
> > bool drop_refcnt = false;
> > - bool drop_tunnel = false;
> > bool new_session = false;
> > bool new_tunnel = false;
> > int error;
> > @@ -672,6 +730,10 @@ static int pppol2tp_connect(struct socket *sock, struct sockaddr *uservaddr,
> > if (error < 0)
> > return error;
> >
> > + tunnel = pppol2tp_tunnel_get(sock_net(sk), &info, &new_tunnel);
> > + if (IS_ERR(tunnel))
> > + return PTR_ERR(tunnel);
> > +
> > lock_sock(sk);
> >
> > /* Check for already bound sockets */
> > @@ -689,57 +751,6 @@ static int pppol2tp_connect(struct socket *sock, struct sockaddr *uservaddr,
> > if (!info.tunnel_id)
> > goto end;
>
> The original code did test info.tunnel_id before trying to get or
> create the tunnel (as it doesn't make sense to work on a tunnel whose
> ID is 0). So we need move this test before the pppol2tp_tunnel_get()
> call.
Got it.
> > - tunnel = l2tp_tunnel_get(sock_net(sk), info.tunnel_id);
> > - if (tunnel)
> > - drop_tunnel = true;
> > -
> > - /* Special case: create tunnel context if session_id and
> > - * peer_session_id is 0. Otherwise look up tunnel using supplied
> > - * tunnel id.
> > - */
>
> Just a note for your future submissions: for networking patches, we
> normally indicate which tree the patch is targetted to in the mail
> subject (for example "[PATCH net v2]"). Also, you should Cc:
> the author of the patch listed in the Fixes tag.
Thanks for the helpful advice.
Just one more thing. I created this patch based on the mainline linux
tree, but networking subsystem has own tree, net. Is it preferable to
create a patch based on net tree for networking patches?
Thanks,
Shigeru
>
Hi Olek,
On Mon, Feb 13, 2023 at 04:05:59PM +0100, Alexander Lobakin wrote:
> From: Shigeru Yoshida <[email protected]>
> Date: Mon, 13 Feb 2023 01:26:23 +0900
>
> > When a file descriptor of pppol2tp socket is passed as file descriptor
> > of UDP socket, a recursive deadlock occurs in l2tp_tunnel_register().
> > This situation is reproduced by the following program:
>
> [...]
>
> > +static struct l2tp_tunnel *pppol2tp_tunnel_get(struct net *net,
> > + struct l2tp_connect_info *info,
> > + bool *new_tunnel)
> > +{
> > + struct l2tp_tunnel *tunnel;
> > + int error;
> > +
> > + *new_tunnel = false;
> > +
> > + tunnel = l2tp_tunnel_get(net, info->tunnel_id);
> > +
> > + /* Special case: create tunnel context if session_id and
> > + * peer_session_id is 0. Otherwise look up tunnel using supplied
> > + * tunnel id.
> > + */
> > + if (!info->session_id && !info->peer_session_id) {
> > + if (!tunnel) {
>
> This `if` is the sole thing the outer `if` contains, could we combine them?
Thank you so much for your comment. Yes, I agree with you. But I'd
like to keep the original structure as Guillaume suggested.
Thanks,
Shigeru
>
> > + struct l2tp_tunnel_cfg tcfg = {
> > + .encap = L2TP_ENCAPTYPE_UDP,
> > + };
> > +
> > + /* Prevent l2tp_tunnel_register() from trying to set up
> > + * a kernel socket.
> > + */
> > + if (info->fd < 0)
> > + return ERR_PTR(-EBADF);
> > +
> > + error = l2tp_tunnel_create(info->fd,
> > + info->version,
>
> This fits into the prev line.
>
> > + info->tunnel_id,
> > + info->peer_tunnel_id, &tcfg,
> > + &tunnel);
> > + if (error < 0)
> > + return ERR_PTR(error);
> > +
> > + l2tp_tunnel_inc_refcount(tunnel);
> > + error = l2tp_tunnel_register(tunnel, net, &tcfg);
> > + if (error < 0) {
> > + kfree(tunnel);
> > + return ERR_PTR(error);
> > + }
> > +
> > + *new_tunnel = true;
> > + }
> > + } else {
> > + /* Error if we can't find the tunnel */
> > + if (!tunnel)
> > + return ERR_PTR(-ENOENT);
>
> [...]
>
> Thanks,
> Olek
>
On Wed, Feb 15, 2023 at 01:49:08AM +0900, Shigeru Yoshida wrote:
> Just one more thing. I created this patch based on the mainline linux
> tree, but networking subsystem has own tree, net. Is it preferable to
> create a patch based on net tree for networking patches?
Yes. Networking fixes should be based on the "net" tree:
https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net.git
For more details about posting patches to netdev, you can check
Documentation/process/maintainer-netdev.rst
Or the online version:
https://kernel.org/doc/html/latest/process/maintainer-netdev.html
> Thanks,
> Shigeru
>
> >
>
Hi Guillaume,
On Tue, Feb 14, 2023 at 06:09:33PM +0100, Guillaume Nault wrote:
> On Wed, Feb 15, 2023 at 01:49:08AM +0900, Shigeru Yoshida wrote:
> > Just one more thing. I created this patch based on the mainline linux
> > tree, but networking subsystem has own tree, net. Is it preferable to
> > create a patch based on net tree for networking patches?
>
> Yes. Networking fixes should be based on the "net" tree:
> https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net.git
Thanks. I'll use the "net" tree for v3 patch.
> For more details about posting patches to netdev, you can check
> Documentation/process/maintainer-netdev.rst
>
> Or the online version:
> https://kernel.org/doc/html/latest/process/maintainer-netdev.html
Got it. I'll check that.
Anyway, I'll make v3 patch based on your feedback.
Thanks,
Shigeru
>
> > Thanks,
> > Shigeru
> >
> > >
> >
>