mirrors/linux
2
0
Fork 0
mirror of https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git synced 2026-08-31 12:52:29 -04:00
Code Issues Packages Projects Releases Wiki Activity

Merge branch 'net-don-t-strip-zerocopy-frag-markers-from-a-forwarded-skb'

Browse Source 
Norbert Szetei says:

====================
net: don't strip zerocopy frag markers from a forwarded skb

queue_userspace_packet() calls skb_tx_error() on the packet skb in its
error path, but it only borrows that skb: on the OVS_ACTION_ATTR_USERSPACE
action path do_execute_actions() ignores output_userspace()'s return value
and keeps forwarding the same skb through the flow's remaining actions.
skb_tx_error() completes the zerocopy uarg and clears SKBFL_ALL_ZEROCOPY,
and with it SKBFL_SHARED_FRAG.

For a MSG_ZEROCOPY skb carrying page-cache frags, SKBFL_SHARED_FRAG is
what makes esp_input() skb_cow_data() instead of taking the in-place AEAD
path. Once it is stripped, a later local ESP delivery decrypts in place
over pages the sender still shares with the page cache.

Patch 1 moves the skb_tx_error() into the one path that does drop the
packet, the "default" arm of ovs_dp_process_packet()'s switch(error).

Patch 2 removes a second such strip, in skb_zerocopy(), which calls
skb_tx_error() on its source when skb_orphan_frags() fails. A copy helper
should not perform a destructive action on its source, and both callers
already report the error on their own drop path. MSG_ZEROCOPY skbs cannot
reach that one -- SKBFL_DONT_ORPHAN makes skb_orphan_frags() return early
-- but producers that do not set that flag, such as vhost-net, can.
Patch 3 is new in v2. It stops skb_tx_error() from touching skb_shinfo()
state that is shared with clones, so patch 1's new call site cannot reach
a live skb either. For a non-last OVS_ACTION_ATTR_RECIRC action
clone_execute() sends a skb_clone() into ovs_dp_process_packet() while
do_execute_actions() keeps forwarding the original, and skb_clone() does
not privatise the frags for these skbs -- skb_orphan_frags() returns early
on SKBFL_DONT_ORPHAN -- so a flow miss on the clone strips
SKBFL_SHARED_FRAG from the packet still in flight. Confirmed on a KASAN
build with a flow matching recirc_id 0 and actions RECIRC(1),OUTPUT(0):
with patches 1 and 2 applied it still reproduces the page-cache write,
with patch 3 on top it no longer does (5/5 runs). A kprobe on
skb_tx_error() shows the datapath drop path is still reached in both
cases, so the difference is the guard and not the reproducer.

As Ilya noted, that makes patch 3 the general fix -- an skb can enter any
skb_tx_error() caller already cloned elsewhere in the stack -- while
patches 1 and 2 keep the callers from acting on an skb they do not own.
Removing skb_tx_error() altogether looks like the right long-term cleanup
and is planned as a net-next follow-up.

v3: https://lore.kernel.org/netdev/F3B9E5BA-0AC1-4AD1-A7D9-F38033304270@doyensec.com/
v2: https://lore.kernel.org/netdev/AD1B7BEE-C04C-4A1B-982C-8385F1908911@doyensec.com/
v1: https://lore.kernel.org/netdev/8063260C-05C9-4997-B9B6-2135063C4858@doyensec.com/
====================

Link: https://patch.msgid.link/4B5CCA6E-2C49-4F86-8C4E-E1BE15C16C0A@doyensec.com
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
This commit is contained in:
Paolo Abeni
2026-08-25 09:36:49 +02:00
parent b70b7b7156 f66bdb1cc0
commit c5f2613e69
2 changed files with 5 additions and 4 deletions
Download Patch File Download Diff File Expand all files Collapse all files

6
net/core/skbuff.c
View File

@@ -1417,10 +1417,13 @@ EXPORT_SYMBOL(skb_dump);
*
* Report xmit error if a device callback is tracking this skb.
* skb must be freed afterwards.
*
* Does nothing for a cloned skb: the zerocopy state lives in
* skb_shinfo(), which the clones share.
*/
void skb_tx_error(struct sk_buff *skb)
{
if (skb) {
if (skb && !skb_cloned(skb)) {
skb_zcopy_downgrade_managed(skb);
skb_zcopy_clear(skb, true);
}
@@ -3914,7 +3917,6 @@ skb_zerocopy(struct sk_buff *to, struct sk_buff *from, int len, int hlen)
skb_len_add(to, len + plen);
if (unlikely(skb_orphan_frags(from, GFP_ATOMIC))) {
skb_tx_error(from);
if (j > 0)
put_page(virt_to_head_page(from->head));
return -ENOMEM;

3
net/openvswitch/datapath.c
View File

@@ -285,6 +285,7 @@ void ovs_dp_process_packet(struct sk_buff *skb, struct sw_flow_key *key)
consume_skb(skb);
break;
default:
skb_tx_error(skb);
kfree_skb(skb);
break;
}
@@ -604,8 +605,6 @@ static int queue_userspace_packet(struct datapath *dp, struct sk_buff *skb,
err = genlmsg_unicast(ovs_dp_get_net(dp), user_skb, upcall_info->portid);
user_skb = NULL;
out:
if (err)
skb_tx_error(skb);
consume_skb(user_skb);
consume_skb(nskb);