KumiSystems/openwrtv3
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity

kernel: atm: pppoatm fix vc-mux connection failures

Browse source 
Backport a hot off the press upstream kernel ATM fix:

Preserve value of skb->truesize when accounting to vcc

"There's a hack in pskb_expand_head() to avoid adjusting skb->truesize
for certain skbs. Ideally it would cover ATM too. It doesn't. Just
stashing the accounted value and using it in atm_raw_pop() is probably
the easiest way to cope."

The issue was exposed by upstream with:

commit 14afee4b6092fde451ee17604e5f5c89da33e71e
Author: Reshetova, Elena <elena.reshetova@intel.com>
Date:   Fri Jun 30 13:08:00 2017 +0300

    net: convert sock.sk_wmem_alloc from atomic_t to refcount_t

But an earlier commit left the ticking timebomb:

158f323b9868 ("net: adjust skb->truesize in pskb_expand_head()

Sincerest thanks to Mathias Kresin <dev@kresin.me> for debugging
assistance and to David Woodhouse <dwmw2@infradead.org> for further
guidance, cajoling & patience in interpreting the debug I was giving him
and producing a fix!

Fixes FS#1567

Signed-off-by: Kevin Darbyshire-Bryant <ldir@darbyshire-bryant.me.uk>
This commit is contained in:
Kevin Darbyshire-Bryant 2018-06-18 09:08:05 +00:00 committed by John Crispin
parent 0276e1f760
commit d600de3ddd
1 changed files with 172 additions and 0 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

172
target/linux/generic/backport-4.14/080-net-convert-sock.sk_wmem_alloc-from-atomic_t-to-refc.patch Normal file
View file

@ -0,0 +1,172 @@
From 9bbe60a67be5a1c6f79b3c9be5003481a50529ff Mon Sep 17 00:00:00 2001
From: David Woodhouse <dwmw2@infradead.org>
Date: Sat, 16 Jun 2018 11:55:44 +0100
Subject: atm: Preserve value of skb->truesize when accounting to vcc
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
ATM accounts for in-flight TX packets in sk_wmem_alloc of the VCC on
which they are to be sent. But it doesn't take ownership of those
packets from the sock (if any) which originally owned them. They should
remain owned by their actual sender until they've left the box.
There's a hack in pskb_expand_head() to avoid adjusting skb->truesize
for certain skbs, precisely to avoid messing up sk_wmem_alloc
accounting. Ideally that hack would cover the ATM use case too, but it
doesn't — skbs which aren't owned by any sock, for example PPP control
frames, still get their truesize adjusted when the low-level ATM driver
adds headroom.
This has always been an issue, it seems. The truesize of a packet
increases, and sk_wmem_alloc on the VCC goes negative. But this wasn't
for normal traffic, only for control frames. So I think we just got away
with it, and we probably needed to send 2GiB of LCP echo frames before
the misaccounting would ever have caused a problem and caused
atm_may_send() to start refusing packets.
Commit 14afee4b609 ("net: convert sock.sk_wmem_alloc from atomic_t to
refcount_t") did exactly what it was intended to do, and turned this
mostly-theoretical problem into a real one, causing PPPoATM to fail
immediately as sk_wmem_alloc underflows and atm_may_send() *immediately*
starts refusing to allow new packets.
The least intrusive solution to this problem is to stash the value of
skb->truesize that was accounted to the VCC, in a new member of the
ATM_SKB(skb) structure. Then in atm_pop_raw() subtract precisely that
value instead of the then-current value of skb->truesize.
Fixes: 158f323b9868 ("net: adjust skb->truesize in pskb_expand_head()")
Signed-off-by: David Woodhouse <dwmw2@infradead.org>
Tested-by: Kevin Darbyshire-Bryant <ldir@darbyshire-bryant.me.uk>
Signed-off-by: David S. Miller <davem@davemloft.net>
---
include/linux/atmdev.h | 15 +++++++++++++++
net/atm/br2684.c | 3 +--
net/atm/clip.c | 3 +--
net/atm/common.c | 3 +--
net/atm/lec.c | 3 +--
net/atm/mpc.c | 3 +--
net/atm/pppoatm.c | 3 +--
net/atm/raw.c | 4 ++--
8 files changed, 23 insertions(+), 14 deletions(-)
--- a/include/linux/atmdev.h
+++ b/include/linux/atmdev.h
@@ -214,6 +214,7 @@ struct atmphy_ops {
struct atm_skb_data {
struct atm_vcc *vcc; /* ATM VCC */
unsigned long atm_options; /* ATM layer options */
+ unsigned int acct_truesize; /* truesize accounted to vcc */
};
#define VCC_HTABLE_SIZE 32
@@ -241,6 +242,20 @@ void vcc_insert_socket(struct sock *sk);
void atm_dev_release_vccs(struct atm_dev *dev);
+static inline void atm_account_tx(struct atm_vcc *vcc, struct sk_buff *skb)
+{
+ /*
+ * Because ATM skbs may not belong to a sock (and we don't
+ * necessarily want to), skb->truesize may be adjusted,
+ * escaping the hack in pskb_expand_head() which avoids
+ * doing so for some cases. So stash the value of truesize
+ * at the time we accounted it, and atm_pop_raw() can use
+ * that value later, in case it changes.
+ */
+ refcount_add(skb->truesize, &sk_atm(vcc)->sk_wmem_alloc);
+ ATM_SKB(skb)->acct_truesize = skb->truesize;
+ ATM_SKB(skb)->atm_options = vcc->atm_options;
+}
static inline void atm_force_charge(struct atm_vcc *vcc,int truesize)
{
--- a/net/atm/br2684.c
+++ b/net/atm/br2684.c
@@ -252,8 +252,7 @@ static int br2684_xmit_vcc(struct sk_buf
ATM_SKB(skb)->vcc = atmvcc = brvcc->atmvcc;
pr_debug("atm_skb(%p)->vcc(%p)->dev(%p)\n", skb, atmvcc, atmvcc->dev);
- refcount_add(skb->truesize, &sk_atm(atmvcc)->sk_wmem_alloc);
- ATM_SKB(skb)->atm_options = atmvcc->atm_options;
+ atm_account_tx(atmvcc, skb);
dev->stats.tx_packets++;
dev->stats.tx_bytes += skb->len;
--- a/net/atm/clip.c
+++ b/net/atm/clip.c
@@ -381,8 +381,7 @@ static netdev_tx_t clip_start_xmit(struc
memcpy(here, llc_oui, sizeof(llc_oui));
((__be16 *) here)[3] = skb->protocol;
}
- refcount_add(skb->truesize, &sk_atm(vcc)->sk_wmem_alloc);
- ATM_SKB(skb)->atm_options = vcc->atm_options;
+ atm_account_tx(vcc, skb);
entry->vccs->last_use = jiffies;
pr_debug("atm_skb(%p)->vcc(%p)->dev(%p)\n", skb, vcc, vcc->dev);
old = xchg(&entry->vccs->xoff, 1); /* assume XOFF ... */
--- a/net/atm/common.c
+++ b/net/atm/common.c
@@ -630,10 +630,9 @@ int vcc_sendmsg(struct socket *sock, str
goto out;
}
pr_debug("%d += %d\n", sk_wmem_alloc_get(sk), skb->truesize);
- refcount_add(skb->truesize, &sk->sk_wmem_alloc);
+ atm_account_tx(vcc, skb);
skb->dev = NULL; /* for paths shared with net_device interfaces */
- ATM_SKB(skb)->atm_options = vcc->atm_options;
if (!copy_from_iter_full(skb_put(skb, size), size, &m->msg_iter)) {
kfree_skb(skb);
error = -EFAULT;
--- a/net/atm/lec.c
+++ b/net/atm/lec.c
@@ -182,9 +182,8 @@ lec_send(struct atm_vcc *vcc, struct sk_
struct net_device *dev = skb->dev;
ATM_SKB(skb)->vcc = vcc;
- ATM_SKB(skb)->atm_options = vcc->atm_options;
+ atm_account_tx(vcc, skb);
- refcount_add(skb->truesize, &sk_atm(vcc)->sk_wmem_alloc);
if (vcc->send(vcc, skb) < 0) {
dev->stats.tx_dropped++;
return;
--- a/net/atm/mpc.c
+++ b/net/atm/mpc.c
@@ -555,8 +555,7 @@ static int send_via_shortcut(struct sk_b
sizeof(struct llc_snap_hdr));
}
- refcount_add(skb->truesize, &sk_atm(entry->shortcut)->sk_wmem_alloc);
- ATM_SKB(skb)->atm_options = entry->shortcut->atm_options;
+ atm_account_tx(entry->shortcut, skb);
entry->shortcut->send(entry->shortcut, skb);
entry->packets_fwded++;
mpc->in_ops->put(entry);
--- a/net/atm/pppoatm.c
+++ b/net/atm/pppoatm.c
@@ -350,8 +350,7 @@ static int pppoatm_send(struct ppp_chann
return 1;
}
- refcount_add(skb->truesize, &sk_atm(ATM_SKB(skb)->vcc)->sk_wmem_alloc);
- ATM_SKB(skb)->atm_options = ATM_SKB(skb)->vcc->atm_options;
+ atm_account_tx(vcc, skb);
pr_debug("atm_skb(%p)->vcc(%p)->dev(%p)\n",
skb, ATM_SKB(skb)->vcc, ATM_SKB(skb)->vcc->dev);
ret = ATM_SKB(skb)->vcc->send(ATM_SKB(skb)->vcc, skb)
--- a/net/atm/raw.c
+++ b/net/atm/raw.c
@@ -35,8 +35,8 @@ static void atm_pop_raw(struct atm_vcc *
struct sock *sk = sk_atm(vcc);
pr_debug("(%d) %d -= %d\n",
- vcc->vci, sk_wmem_alloc_get(sk), skb->truesize);
- WARN_ON(refcount_sub_and_test(skb->truesize, &sk->sk_wmem_alloc));
+ vcc->vci, sk_wmem_alloc_get(sk), ATM_SKB(skb)->acct_truesize);
+ WARN_ON(refcount_sub_and_test(ATM_SKB(skb)->acct_truesize, &sk->sk_wmem_alloc));
dev_kfree_skb_any(skb);
sk->sk_write_space(sk);
}