| Message ID | 20210325073928.12211-2-po-hsu.lin@canonical.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [OEM-5.6,SRU,PATCHv2,1/1] l2tp: remove skb_dst_set() from l2tp_xmit_skb() | expand |
Acked-by: Thadeu Lima de Souza Cascardo <cascardo@canonical.com>
Acked-by: Tim Gardner <tim.gardner@canonical.com> On 3/25/21 1:39 AM, Po-Hsu Lin wrote: > From: Xin Long <lucien.xin@gmail.com> > > BugLink: https://bugs.launchpad.net/bugs/1919277 > > In the tx path of l2tp, l2tp_xmit_skb() calls skb_dst_set() to set > skb's dst. However, it will eventually call inet6_csk_xmit() or > ip_queue_xmit() where skb's dst will be overwritten by: > > skb_dst_set_noref(skb, dst); > > without releasing the old dst in skb. Then it causes dst/dev refcnt leak: > > unregister_netdevice: waiting for eth0 to become free. Usage count = 1 > > This can be reproduced by simply running: > > # modprobe l2tp_eth && modprobe l2tp_ip > # sh ./tools/testing/selftests/net/l2tp.sh > > So before going to inet6_csk_xmit() or ip_queue_xmit(), skb's dst > should be dropped. This patch is to fix it by removing skb_dst_set() > from l2tp_xmit_skb() and moving skb_dst_drop() into l2tp_xmit_core(). > > Fixes: 3557baabf280 ("[L2TP]: PPP over L2TP driver core") > Reported-by: Hangbin Liu <liuhangbin@gmail.com> > Signed-off-by: Xin Long <lucien.xin@gmail.com> > Acked-by: James Chapman <jchapman@katalix.com> > Tested-by: James Chapman <jchapman@katalix.com> > Signed-off-by: David S. Miller <davem@davemloft.net> > (cherry picked from commit 27d53323664c549b5bb2dfaaf6f7ad6e0376a64e) > Signed-off-by: Po-Hsu Lin <po-hsu.lin@canonical.com> > --- > net/l2tp/l2tp_core.c | 5 +---- > 1 file changed, 1 insertion(+), 4 deletions(-) > > diff --git a/net/l2tp/l2tp_core.c b/net/l2tp/l2tp_core.c > index 6d7ef78..6434d17 100644 > --- a/net/l2tp/l2tp_core.c > +++ b/net/l2tp/l2tp_core.c > @@ -1028,6 +1028,7 @@ static void l2tp_xmit_core(struct l2tp_session *session, struct sk_buff *skb, > > /* Queue the packet to IP for output */ > skb->ignore_df = 1; > + skb_dst_drop(skb); > #if IS_ENABLED(CONFIG_IPV6) > if (l2tp_sk_is_v6(tunnel->sock)) > error = inet6_csk_xmit(tunnel->sock, skb, NULL); > @@ -1099,10 +1100,6 @@ int l2tp_xmit_skb(struct l2tp_session *session, struct sk_buff *skb, int hdr_len > goto out_unlock; > } > > - /* Get routing info from the tunnel socket */ > - skb_dst_drop(skb); > - skb_dst_set(skb, sk_dst_check(sk, 0)); > - > inet = inet_sk(sk); > fl = &inet->cork.fl; > switch (tunnel->encap) { >
Applied to focal:linux-oem-5.6/oem-5.6-next. Thanks. -rtg On 3/31/21 8:03 AM, Tim Gardner wrote: > Acked-by: Tim Gardner <tim.gardner@canonical.com> > > On 3/25/21 1:39 AM, Po-Hsu Lin wrote: >> From: Xin Long <lucien.xin@gmail.com> >> >> BugLink: https://bugs.launchpad.net/bugs/1919277 >> >> In the tx path of l2tp, l2tp_xmit_skb() calls skb_dst_set() to set >> skb's dst. However, it will eventually call inet6_csk_xmit() or >> ip_queue_xmit() where skb's dst will be overwritten by: >> >> skb_dst_set_noref(skb, dst); >> >> without releasing the old dst in skb. Then it causes dst/dev refcnt leak: >> >> unregister_netdevice: waiting for eth0 to become free. Usage count = 1 >> >> This can be reproduced by simply running: >> >> # modprobe l2tp_eth && modprobe l2tp_ip >> # sh ./tools/testing/selftests/net/l2tp.sh >> >> So before going to inet6_csk_xmit() or ip_queue_xmit(), skb's dst >> should be dropped. This patch is to fix it by removing skb_dst_set() >> from l2tp_xmit_skb() and moving skb_dst_drop() into l2tp_xmit_core(). >> >> Fixes: 3557baabf280 ("[L2TP]: PPP over L2TP driver core") >> Reported-by: Hangbin Liu <liuhangbin@gmail.com> >> Signed-off-by: Xin Long <lucien.xin@gmail.com> >> Acked-by: James Chapman <jchapman@katalix.com> >> Tested-by: James Chapman <jchapman@katalix.com> >> Signed-off-by: David S. Miller <davem@davemloft.net> >> (cherry picked from commit 27d53323664c549b5bb2dfaaf6f7ad6e0376a64e) >> Signed-off-by: Po-Hsu Lin <po-hsu.lin@canonical.com> >> --- >> net/l2tp/l2tp_core.c | 5 +---- >> 1 file changed, 1 insertion(+), 4 deletions(-) >> >> diff --git a/net/l2tp/l2tp_core.c b/net/l2tp/l2tp_core.c >> index 6d7ef78..6434d17 100644 >> --- a/net/l2tp/l2tp_core.c >> +++ b/net/l2tp/l2tp_core.c >> @@ -1028,6 +1028,7 @@ static void l2tp_xmit_core(struct l2tp_session >> *session, struct sk_buff *skb, >> /* Queue the packet to IP for output */ >> skb->ignore_df = 1; >> + skb_dst_drop(skb); >> #if IS_ENABLED(CONFIG_IPV6) >> if (l2tp_sk_is_v6(tunnel->sock)) >> error = inet6_csk_xmit(tunnel->sock, skb, NULL); >> @@ -1099,10 +1100,6 @@ int l2tp_xmit_skb(struct l2tp_session *session, >> struct sk_buff *skb, int hdr_len >> goto out_unlock; >> } >> - /* Get routing info from the tunnel socket */ >> - skb_dst_drop(skb); >> - skb_dst_set(skb, sk_dst_check(sk, 0)); >> - >> inet = inet_sk(sk); >> fl = &inet->cork.fl; >> switch (tunnel->encap) { >> >
diff --git a/net/l2tp/l2tp_core.c b/net/l2tp/l2tp_core.c index 6d7ef78..6434d17 100644 --- a/net/l2tp/l2tp_core.c +++ b/net/l2tp/l2tp_core.c @@ -1028,6 +1028,7 @@ static void l2tp_xmit_core(struct l2tp_session *session, struct sk_buff *skb, /* Queue the packet to IP for output */ skb->ignore_df = 1; + skb_dst_drop(skb); #if IS_ENABLED(CONFIG_IPV6) if (l2tp_sk_is_v6(tunnel->sock)) error = inet6_csk_xmit(tunnel->sock, skb, NULL); @@ -1099,10 +1100,6 @@ int l2tp_xmit_skb(struct l2tp_session *session, struct sk_buff *skb, int hdr_len goto out_unlock; } - /* Get routing info from the tunnel socket */ - skb_dst_drop(skb); - skb_dst_set(skb, sk_dst_check(sk, 0)); - inet = inet_sk(sk); fl = &inet->cork.fl; switch (tunnel->encap) {