Message ID | 156708405310.26102.7954021163316252673.stgit@warthog.procyon.org.uk |
---|---|
Headers | show |
Series | rxrpc: Fix use of skb_cow_data() | expand |
From: David Howells <dhowells@redhat.com> Date: Thu, 29 Aug 2019 14:07:33 +0100 > Here's a series of patches that replaces the use of skb_cow_data() in rxrpc > with skb_unshare() early on in the input process. The problem that is > being seen is that skb_cow_data() indirectly requires that the maximum > usage count on an sk_buff be 1, and it may generate an assertion failure in > pskb_expand_head() if not. > > This can occur because rxrpc_input_data() may be still holding a ref when > it has just attached the sk_buff to the rx ring and given that attachment > its own ref. If recvmsg happens fast enough, skb_cow_data() can see the > ref still held by the softirq handler. > > Further, a packet may contain multiple subpackets, each of which gets its > own attachment to the ring and its own ref - also making skb_cow_data() go > bang. > > Fix this by: ... > There are also patches to improve the rxrpc_skb tracepoint to make sure > that Tx-derived buffers are identified separately from Rx-derived buffers > in the trace. This looks great, thanks for reimplementing this using skb_unshare(). > The patches are tagged here: > > git://git.kernel.org/pub/scm/linux/kernel/git/dhowells/linux-fs.git > rxrpc-fixes-20190827 Pulled, thanks again.
Hillf Danton <hdanton@sina.com> wrote: > > + /* Unshare the packet so that it can be modified for in-place > > + * decryption. > > + */ > > + if (sp->hdr.securityIndex != 0) { > > + struct sk_buff *nskb = skb_unshare(skb, GFP_ATOMIC); > > + if (!nskb) { > > + rxrpc_eaten_skb(skb, rxrpc_skb_unshared_nomem); > > + goto out; > > + } > > + > > + if (nskb != skb) { > > + rxrpc_eaten_skb(skb, rxrpc_skb_received); > > + rxrpc_new_skb(skb, rxrpc_skb_unshared); > > + skb = nskb; > > + sp = rxrpc_skb(skb); > > + } > > + } > > Unsharing skb makes it perilous to take a peep at it afterwards. Ah, good point. rxrpc_new_skb() should be after the assignment. David