Messages in this thread | | | Date | Wed, 15 Feb 2023 01:52:44 +0900 | From | Shigeru Yoshida <> | Subject | Re: [PATCH v2] l2tp: Avoid possible recursive deadlock in l2tp_tunnel_register() |
| |
Hi Olek,
On Mon, Feb 13, 2023 at 04:05:59PM +0100, Alexander Lobakin wrote: > From: Shigeru Yoshida <syoshida@redhat.com> > 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 >
| |