[PATCH net 0/2] net: don't strip zerocopy frag markers from a forwarded skb

[PATCH net 0/2] net: don't strip zerocopy frag markers from a forwarded skb

[Norbert Szetei]

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 af_packet's TX_RING
path, can.

Norbert Szetei (2):
  openvswitch: only skb_tx_error() a packet we are about to drop
  net: skbuff: don't skb_tx_error() the source skb in skb_zerocopy()

 net/core/skbuff.c          | 5 ++---
 net/openvswitch/datapath.c | 3 +--
 2 files changed, 3 insertions(+), 5 deletions(-)

-- 
2.55.0

[PATCH net 1/2] openvswitch: only skb_tx_error() a packet we are about to drop

[Norbert Szetei]

queue_userspace_packet() borrows the packet skb -- it only copies it into
a private netlink message (user_skb) and does not own it; on return
do_execute_actions() keeps forwarding it through the flow's remaining
actions. Its error path nevertheless calls skb_tx_error(skb), which via
skb_zcopy_clear() does skb_shinfo(skb)->flags &= ~SKBFL_ALL_ZEROCOPY,
stripping SKBFL_SHARED_FRAG from that live skb (skb_tx_error()'s kerneldoc
says "skb must be freed afterwards").

For a MSG_ZEROCOPY skb carrying page-cache frags, SKBFL_SHARED_FRAG is
what makes esp_input() skb_cow_data() before in-place AEAD; once it is
stripped a later local ESP-in-UDP delivery decrypts in place over pages
the sender does not own -- an unprivileged page-cache write (the
"Fragnesia" primitive).
do_execute_actions() ignores output_userspace()'s return value, so any
action after a failed USERSPACE upcall inherits the stripped skb.

Move the skb_tx_error() to the flow-miss drop path - the "default"
branch of ovs_dp_process_packet()'s switch(error), before kfree_skb().

The call has been here since commit 36d5fe6a0007 ("core, nfqueue,
openvswitch: Orphan frags in skb_zerocopy and handle errors") but was
harmless until esp_input() began relying on SKBFL_SHARED_FRAG to gate
in-place decrypt; only then did stripping it on a still-forwarded skb
become a page-cache write primitive.

Fixes: 36d5fe6a0007 ("core, nfqueue, openvswitch: Orphan frags in skb_zerocopy and handle errors")
Fixes: f4c50a4034e6 ("xfrm: esp: avoid in-place decrypt on shared skb frags")
Cc: stable@vger.kernel.org
Assisted-by: Claude:claude-opus-5
Signed-off-by: Norbert Szetei <norbert@doyensec.com>
---
 net/openvswitch/datapath.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/net/openvswitch/datapath.c b/net/openvswitch/datapath.c
index ae69b2cabab9..fff75c3eed11 100644
--- a/net/openvswitch/datapath.c
+++ b/net/openvswitch/datapath.c
@@ -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;
 		}
@@ -601,8 +602,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);
 
-- 
2.55.0

[PATCH net 2/2] net: skbuff: don't skb_tx_error() the source skb in skb_zerocopy()

[Norbert Szetei]

skb_zerocopy() copies frags from @from into @to. On an
skb_orphan_frags() failure it calls skb_tx_error(@from), a destructive
operation on the source skb the copy helper does not own. That completes
@from's zerocopy uarg and clears SKBFL_ALL_ZEROCOPY, including the
SKBFL_SHARED_FRAG page-ownership marker.

Both callers already report the failure on their own drop path.
nfnetlink_queue does it at nla_put_failure, and open vSwitch does it in
the flow-miss drop arm of ovs_dp_process_packet(), so nothing is lost by
dropping it here.

On Open vSwitch's OVS_ACTION_ATTR_USERSPACE path the skb is not freed on
this error: do_execute_actions() ignores output_userspace()'s return
value and, unless the upcall was the last action, keeps forwarding the
same skb through the flow's remaining actions. The uarg is completed
while that skb is still in flight, telling the producer its buffers are
free, and SKBFL_SHARED_FRAG is cleared on an skb the rest of the stack
still handles. That flag is what makes esp_input() call skb_cow_data()
instead of decrypting in place, so a later local ESP delivery can
decrypt over frags the skb does not own privately.

Leave error reporting to the callers.

Fixes: 36d5fe6a0007 ("core, nfqueue, openvswitch: Orphan frags in skb_zerocopy and handle errors")
Cc: stable@vger.kernel.org
Suggested-by: Ilya Maximets <i.maximets@ovn.org>
Signed-off-by: Norbert Szetei <norbert@doyensec.com>
---
 net/core/skbuff.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index ba3dbac80fb4..db62ed6e04b9 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -3907,10 +3907,9 @@ 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 (unlikely(skb_orphan_frags(from, GFP_ATOMIC)))
 		return -ENOMEM;
-	}
+
 	skb_zerocopy_clone(to, from, GFP_ATOMIC);
 
 	for (i = 0; i < skb_shinfo(from)->nr_frags; i++) {
-- 
2.55.0

Re: [PATCH net 1/2] openvswitch: only skb_tx_error() a packet we are about to drop

[Ilya Maximets]

On 8/13/26 7:47 AM, Norbert Szetei wrote:
> queue_userspace_packet() borrows the packet skb -- it only copies it into
> a private netlink message (user_skb) and does not own it; on return
> do_execute_actions() keeps forwarding it through the flow's remaining
> actions. Its error path nevertheless calls skb_tx_error(skb), which via
> skb_zcopy_clear() does skb_shinfo(skb)->flags &= ~SKBFL_ALL_ZEROCOPY,
> stripping SKBFL_SHARED_FRAG from that live skb (skb_tx_error()'s kerneldoc
> says "skb must be freed afterwards").
> 
> For a MSG_ZEROCOPY skb carrying page-cache frags, SKBFL_SHARED_FRAG is
> what makes esp_input() skb_cow_data() before in-place AEAD; once it is
> stripped a later local ESP-in-UDP delivery decrypts in place over pages
> the sender does not own -- an unprivileged page-cache write (the
> "Fragnesia" primitive).
> do_execute_actions() ignores output_userspace()'s return value, so any
> action after a failed USERSPACE upcall inherits the stripped skb.
> 
> Move the skb_tx_error() to the flow-miss drop path - the "default"
> branch of ovs_dp_process_packet()'s switch(error), before kfree_skb().
> 
> The call has been here since commit 36d5fe6a0007 ("core, nfqueue,
> openvswitch: Orphan frags in skb_zerocopy and handle errors") but was
> harmless until esp_input() began relying on SKBFL_SHARED_FRAG to gate
> in-place decrypt; only then did stripping it on a still-forwarded skb
> become a page-cache write primitive.
> 
> Fixes: 36d5fe6a0007 ("core, nfqueue, openvswitch: Orphan frags in skb_zerocopy and handle errors")
> Fixes: f4c50a4034e6 ("xfrm: esp: avoid in-place decrypt on shared skb frags")
> Cc: stable@vger.kernel.org
> Assisted-by: Claude:claude-opus-5
> Signed-off-by: Norbert Szetei <norbert@doyensec.com>
> ---
Reviewed-by: Ilya Maximets <i.maximets@ovn.org>

Re: [PATCH net 2/2] net: skbuff: don't skb_tx_error() the source skb in skb_zerocopy()

[Ilya Maximets]

On 8/13/26 7:49 AM, Norbert Szetei wrote:
> skb_zerocopy() copies frags from @from into @to. On an
> skb_orphan_frags() failure it calls skb_tx_error(@from), a destructive
> operation on the source skb the copy helper does not own. That completes
> @from's zerocopy uarg and clears SKBFL_ALL_ZEROCOPY, including the
> SKBFL_SHARED_FRAG page-ownership marker.
> 
> Both callers already report the failure on their own drop path.
> nfnetlink_queue does it at nla_put_failure, and open vSwitch does it in
> the flow-miss drop arm of ovs_dp_process_packet(), so nothing is lost by
> dropping it here.
> 
> On Open vSwitch's OVS_ACTION_ATTR_USERSPACE path the skb is not freed on
> this error: do_execute_actions() ignores output_userspace()'s return
> value and, unless the upcall was the last action, keeps forwarding the
> same skb through the flow's remaining actions. The uarg is completed
> while that skb is still in flight, telling the producer its buffers are
> free, and SKBFL_SHARED_FRAG is cleared on an skb the rest of the stack
> still handles. That flag is what makes esp_input() call skb_cow_data()
> instead of decrypting in place, so a later local ESP delivery can
> decrypt over frags the skb does not own privately.
> 
> Leave error reporting to the callers.
> 
> Fixes: 36d5fe6a0007 ("core, nfqueue, openvswitch: Orphan frags in skb_zerocopy and handle errors")
> Cc: stable@vger.kernel.org
> Suggested-by: Ilya Maximets <i.maximets@ovn.org>
> Signed-off-by: Norbert Szetei <norbert@doyensec.com>
> ---

Reviewed-by: Ilya Maximets <i.maximets@ovn.org>

Re: [PATCH net 1/2] openvswitch: only skb_tx_error() a packet we are about to drop

[Norbert Szetei]

Thanks for the review. Sashiko flagged that the moved call may still be
reachable through the RECIRC action, and I confirmed dynamically that it is.
With 1/2 applied, a flow matching recirc_id 0 exactly, with actions
RECIRC(1), OUTPUT(0) instead of USERSPACE(unbound), OUTPUT(0), reproduces
the same issue. So please hold off on 1/2.

Moving the call to the "default" branch assumes that branch only sees a
packet the datapath owns. For a non-last OVS_ACTION_ATTR_RECIRC,
clone_execute() does

	skb = last ? skb : skb_clone(skb, GFP_ATOMIC);
	...
	ovs_dp_process_packet(skb, clone);

so a clone lands there while do_execute_actions() carries on with the
original. The clone shares skb_shinfo() exactly for the skbs this series is
about, since skb_clone() -> skb_orphan_frags() returns early on
SKBFL_DONT_ORPHAN and does not copy the frags - so settling the uarg
through the clone clears SKBFL_SHARED_FRAG for the skb still being
forwarded.

Removing the call, as I originally suggested, does fix this in my testing.
If you would still rather keep it, how would you prefer to solve this?

Thanks,
Norbert

> On Aug 13, 2026, at 12:00, Ilya Maximets <i.maximets@ovn.org> wrote:
> 
> On 8/13/26 7:47 AM, Norbert Szetei wrote:
>> queue_userspace_packet() borrows the packet skb -- it only copies it into
>> a private netlink message (user_skb) and does not own it; on return
>> do_execute_actions() keeps forwarding it through the flow's remaining
>> actions. Its error path nevertheless calls skb_tx_error(skb), which via
>> skb_zcopy_clear() does skb_shinfo(skb)->flags &= ~SKBFL_ALL_ZEROCOPY,
>> stripping SKBFL_SHARED_FRAG from that live skb (skb_tx_error()'s kerneldoc
>> says "skb must be freed afterwards").
>> 
>> For a MSG_ZEROCOPY skb carrying page-cache frags, SKBFL_SHARED_FRAG is
>> what makes esp_input() skb_cow_data() before in-place AEAD; once it is
>> stripped a later local ESP-in-UDP delivery decrypts in place over pages
>> the sender does not own -- an unprivileged page-cache write (the
>> "Fragnesia" primitive).
>> do_execute_actions() ignores output_userspace()'s return value, so any
>> action after a failed USERSPACE upcall inherits the stripped skb.
>> 
>> Move the skb_tx_error() to the flow-miss drop path - the "default"
>> branch of ovs_dp_process_packet()'s switch(error), before kfree_skb().
>> 
>> The call has been here since commit 36d5fe6a0007 ("core, nfqueue,
>> openvswitch: Orphan frags in skb_zerocopy and handle errors") but was
>> harmless until esp_input() began relying on SKBFL_SHARED_FRAG to gate
>> in-place decrypt; only then did stripping it on a still-forwarded skb
>> become a page-cache write primitive.
>> 
>> Fixes: 36d5fe6a0007 ("core, nfqueue, openvswitch: Orphan frags in skb_zerocopy and handle errors")
>> Fixes: f4c50a4034e6 ("xfrm: esp: avoid in-place decrypt on shared skb frags")
>> Cc: stable@vger.kernel.org
>> Assisted-by: Claude:claude-opus-5
>> Signed-off-by: Norbert Szetei <norbert@doyensec.com>
>> ---
> Reviewed-by: Ilya Maximets <i.maximets@ovn.org>

Re: [PATCH net 1/2] openvswitch: only skb_tx_error() a packet we are about to drop

[Ilya Maximets]

On 8/14/26 1:01 PM, Norbert Szetei wrote:
> Thanks for the review. Sashiko flagged

Hmm.  I do not see any reports in either of the instances.  Do you have a link?

> that the moved call may still be
> reachable through the RECIRC action, and I confirmed dynamically that it is.
> With 1/2 applied, a flow matching recirc_id 0 exactly, with actions
> RECIRC(1), OUTPUT(0) instead of USERSPACE(unbound), OUTPUT(0), reproduces
> the same issue. So please hold off on 1/2.
> 
> Moving the call to the "default" branch assumes that branch only sees a
> packet the datapath owns. For a non-last OVS_ACTION_ATTR_RECIRC,
> clone_execute() does
> 
> 	skb = last ? skb : skb_clone(skb, GFP_ATOMIC);
> 	...
> 	ovs_dp_process_packet(skb, clone);
> 
> so a clone lands there while do_execute_actions() carries on with the
> original. The clone shares skb_shinfo() exactly for the skbs this series is
> about, since skb_clone() -> skb_orphan_frags() returns early on
> SKBFL_DONT_ORPHAN and does not copy the frags - so settling the uarg
> through the clone clears SKBFL_SHARED_FRAG for the skb still being
> forwarded.

AFAIU, operations on a cloned skb performed via proper skb helpers must
not affect the original.  That's the whole point of the clone.  However,
in this case indeed it looks like the skb_tx_copy() just modifies the
shared info not checking if it is shared or not.  And this sounds like
a bug in skb_tx_copy().

> 
> Removing the call, as I originally suggested, does fix this in my testing.
> If you would still rather keep it, how would you prefer to solve this?

Just removing the call from openvswitch module doesn't solve the problem.
Packet may enter OVS already cloned somewhere else in the stack, and at
any other point in the kernel where skb_tx_copy() is called it may be
operating on a clone of some other skb causing the exact same issue.  So,
it needs to be addressed inside the skb_tx_copy() itself.

On the other hand, reading the history of this function, it seems like it
lost of its meaning with commit 1f8b977ab32d ("sock: enable MSG_ZEROCOPY")
from Willem that changed it to just call skb_zcopy_clear(skb, true);  This
changed the "false" signaling to "true".  So it doesn't even signal an error
anymore.

Later, in commit 753f1ca4e1e5 ("net: introduce managed frags infrastructure")
Pavel added skb_zcopy_downgrade_managed(skb); call that takes extra frag
references.  Though it seems pointless for an skb that must be freed right
after.

So, I'm not sure if this function is useful in general.  Feels like it is
only harmful as it directly modifies shared data with no regards to clones.

We have two options here:

1. Minimal fix: add something like skb_cloned() guard into skb_tx_copy().

2. Remove skb_tx_copy() entirely (all calls and the definition) as it
   seems pointless after 1f8b977ab32d.

Any thoughts?  Willem, Pavel, others?

> 
> Thanks,
> Norbert
> 
>> On Aug 13, 2026, at 12:00, Ilya Maximets <i.maximets@ovn.org> wrote:
>>
>> On 8/13/26 7:47 AM, Norbert Szetei wrote:
>>> queue_userspace_packet() borrows the packet skb -- it only copies it into
>>> a private netlink message (user_skb) and does not own it; on return
>>> do_execute_actions() keeps forwarding it through the flow's remaining
>>> actions. Its error path nevertheless calls skb_tx_error(skb), which via
>>> skb_zcopy_clear() does skb_shinfo(skb)->flags &= ~SKBFL_ALL_ZEROCOPY,
>>> stripping SKBFL_SHARED_FRAG from that live skb (skb_tx_error()'s kerneldoc
>>> says "skb must be freed afterwards").
>>>
>>> For a MSG_ZEROCOPY skb carrying page-cache frags, SKBFL_SHARED_FRAG is
>>> what makes esp_input() skb_cow_data() before in-place AEAD; once it is
>>> stripped a later local ESP-in-UDP delivery decrypts in place over pages
>>> the sender does not own -- an unprivileged page-cache write (the
>>> "Fragnesia" primitive).
>>> do_execute_actions() ignores output_userspace()'s return value, so any
>>> action after a failed USERSPACE upcall inherits the stripped skb.
>>>
>>> Move the skb_tx_error() to the flow-miss drop path - the "default"
>>> branch of ovs_dp_process_packet()'s switch(error), before kfree_skb().
>>>
>>> The call has been here since commit 36d5fe6a0007 ("core, nfqueue,
>>> openvswitch: Orphan frags in skb_zerocopy and handle errors") but was
>>> harmless until esp_input() began relying on SKBFL_SHARED_FRAG to gate
>>> in-place decrypt; only then did stripping it on a still-forwarded skb
>>> become a page-cache write primitive.
>>>
>>> Fixes: 36d5fe6a0007 ("core, nfqueue, openvswitch: Orphan frags in skb_zerocopy and handle errors")
>>> Fixes: f4c50a4034e6 ("xfrm: esp: avoid in-place decrypt on shared skb frags")
>>> Cc: stable@vger.kernel.org
>>> Assisted-by: Claude:claude-opus-5
>>> Signed-off-by: Norbert Szetei <norbert@doyensec.com>
>>> ---
>> Reviewed-by: Ilya Maximets <i.maximets@ovn.org>
> 

Re: [PATCH net 1/2] openvswitch: only skb_tx_error() a packet we are about to drop

[Norbert Szetei]

> On Aug 14, 2026, at 14:31, Ilya Maximets <i.maximets@ovn.org> wrote:
> 
> On 8/14/26 1:01 PM, Norbert Szetei wrote:
>> Thanks for the review. Sashiko flagged
> 
> Hmm.  I do not see any reports in either of the instances.  Do you have a link?

https://sashiko.dev/#/patchset/C35992B1-7740-4886-94FF-F85DE8B0106F@doyensec.com

>> that the moved call may still be
>> reachable through the RECIRC action, and I confirmed dynamically that it is.
>> With 1/2 applied, a flow matching recirc_id 0 exactly, with actions
>> RECIRC(1), OUTPUT(0) instead of USERSPACE(unbound), OUTPUT(0), reproduces
>> the same issue. So please hold off on 1/2.
>> 
>> Moving the call to the "default" branch assumes that branch only sees a
>> packet the datapath owns. For a non-last OVS_ACTION_ATTR_RECIRC,
>> clone_execute() does
>> 
>> skb = last ? skb : skb_clone(skb, GFP_ATOMIC);
>> ...
>> ovs_dp_process_packet(skb, clone);
>> 
>> so a clone lands there while do_execute_actions() carries on with the
>> original. The clone shares skb_shinfo() exactly for the skbs this series is
>> about, since skb_clone() -> skb_orphan_frags() returns early on
>> SKBFL_DONT_ORPHAN and does not copy the frags - so settling the uarg
>> through the clone clears SKBFL_SHARED_FRAG for the skb still being
>> forwarded.
> 
> AFAIU, operations on a cloned skb performed via proper skb helpers must
> not affect the original.  That's the whole point of the clone.  However,
> in this case indeed it looks like the skb_tx_copy() just modifies the
> shared info not checking if it is shared or not.  And this sounds like
> a bug in skb_tx_copy().
> 
>> 
>> Removing the call, as I originally suggested, does fix this in my testing.
>> If you would still rather keep it, how would you prefer to solve this?
> 
> Just removing the call from openvswitch module doesn't solve the problem.
> Packet may enter OVS already cloned somewhere else in the stack, and at
> any other point in the kernel where skb_tx_copy() is called it may be
> operating on a clone of some other skb causing the exact same issue.  So,
> it needs to be addressed inside the skb_tx_copy() itself.
> 
> On the other hand, reading the history of this function, it seems like it
> lost of its meaning with commit 1f8b977ab32d ("sock: enable MSG_ZEROCOPY")
> from Willem that changed it to just call skb_zcopy_clear(skb, true);  This
> changed the "false" signaling to "true".  So it doesn't even signal an error
> anymore.
> 
> Later, in commit 753f1ca4e1e5 ("net: introduce managed frags infrastructure")
> Pavel added skb_zcopy_downgrade_managed(skb); call that takes extra frag
> references.  Though it seems pointless for an skb that must be freed right
> after.
> 
> So, I'm not sure if this function is useful in general.  Feels like it is
> only harmful as it directly modifies shared data with no regards to clones.
> 
> We have two options here:
> 
> 1. Minimal fix: add something like skb_cloned() guard into skb_tx_copy().
> 
> 2. Remove skb_tx_copy() entirely (all calls and the definition) as it
>   seems pointless after 1f8b977ab32d.

Thanks for digging out 1f8b977ab32d.

Option 2 sounds cleaner to me, though it touches tun, ovpn and nfnetlink_queue
as well. Option 1 would not cover the reported case on its own, since there is
no clone on the OVS_ACTION_ATTR_USERSPACE path, but it should work with this 
patch 1/2.

Curious what the others think. I can write whichever you settle on.

N.

> 
> Any thoughts?  Willem, Pavel, others?
> 
>> 
>> Thanks,
>> Norbert
>> 
>>> On Aug 13, 2026, at 12:00, Ilya Maximets <i.maximets@ovn.org> wrote:
>>> 
>>> On 8/13/26 7:47 AM, Norbert Szetei wrote:
>>>> queue_userspace_packet() borrows the packet skb -- it only copies it into
>>>> a private netlink message (user_skb) and does not own it; on return
>>>> do_execute_actions() keeps forwarding it through the flow's remaining
>>>> actions. Its error path nevertheless calls skb_tx_error(skb), which via
>>>> skb_zcopy_clear() does skb_shinfo(skb)->flags &= ~SKBFL_ALL_ZEROCOPY,
>>>> stripping SKBFL_SHARED_FRAG from that live skb (skb_tx_error()'s kerneldoc
>>>> says "skb must be freed afterwards").
>>>> 
>>>> For a MSG_ZEROCOPY skb carrying page-cache frags, SKBFL_SHARED_FRAG is
>>>> what makes esp_input() skb_cow_data() before in-place AEAD; once it is
>>>> stripped a later local ESP-in-UDP delivery decrypts in place over pages
>>>> the sender does not own -- an unprivileged page-cache write (the
>>>> "Fragnesia" primitive).
>>>> do_execute_actions() ignores output_userspace()'s return value, so any
>>>> action after a failed USERSPACE upcall inherits the stripped skb.
>>>> 
>>>> Move the skb_tx_error() to the flow-miss drop path - the "default"
>>>> branch of ovs_dp_process_packet()'s switch(error), before kfree_skb().
>>>> 
>>>> The call has been here since commit 36d5fe6a0007 ("core, nfqueue,
>>>> openvswitch: Orphan frags in skb_zerocopy and handle errors") but was
>>>> harmless until esp_input() began relying on SKBFL_SHARED_FRAG to gate
>>>> in-place decrypt; only then did stripping it on a still-forwarded skb
>>>> become a page-cache write primitive.
>>>> 
>>>> Fixes: 36d5fe6a0007 ("core, nfqueue, openvswitch: Orphan frags in skb_zerocopy and handle errors")
>>>> Fixes: f4c50a4034e6 ("xfrm: esp: avoid in-place decrypt on shared skb frags")
>>>> Cc: stable@vger.kernel.org
>>>> Assisted-by: Claude:claude-opus-5
>>>> Signed-off-by: Norbert Szetei <norbert@doyensec.com>
>>>> ---
>>> Reviewed-by: Ilya Maximets <i.maximets@ovn.org>

Re: [PATCH net 1/2] openvswitch: only skb_tx_error() a packet we are about to drop

[Ilya Maximets]

On 8/14/26 3:40 PM, Norbert Szetei wrote:
>> On Aug 14, 2026, at 14:31, Ilya Maximets <i.maximets@ovn.org> wrote:
>>
>> On 8/14/26 1:01 PM, Norbert Szetei wrote:
>>> Thanks for the review. Sashiko flagged
>>
>> Hmm.  I do not see any reports in either of the instances.  Do you have a link?
> 
> https://sashiko.dev/#/patchset/C35992B1-7740-4886-94FF-F85DE8B0106F@doyensec.com

Thanks, looks like I was trying to search using the patch name and it
only searches sets.

> 
>>> that the moved call may still be
>>> reachable through the RECIRC action, and I confirmed dynamically that it is.
>>> With 1/2 applied, a flow matching recirc_id 0 exactly, with actions
>>> RECIRC(1), OUTPUT(0) instead of USERSPACE(unbound), OUTPUT(0), reproduces
>>> the same issue. So please hold off on 1/2.
>>>
>>> Moving the call to the "default" branch assumes that branch only sees a
>>> packet the datapath owns. For a non-last OVS_ACTION_ATTR_RECIRC,
>>> clone_execute() does
>>>
>>> skb = last ? skb : skb_clone(skb, GFP_ATOMIC);
>>> ...
>>> ovs_dp_process_packet(skb, clone);
>>>
>>> so a clone lands there while do_execute_actions() carries on with the
>>> original. The clone shares skb_shinfo() exactly for the skbs this series is
>>> about, since skb_clone() -> skb_orphan_frags() returns early on
>>> SKBFL_DONT_ORPHAN and does not copy the frags - so settling the uarg
>>> through the clone clears SKBFL_SHARED_FRAG for the skb still being
>>> forwarded.
>>
>> AFAIU, operations on a cloned skb performed via proper skb helpers must
>> not affect the original.  That's the whole point of the clone.  However,
>> in this case indeed it looks like the skb_tx_copy() just modifies the
>> shared info not checking if it is shared or not.  And this sounds like
>> a bug in skb_tx_copy().
>>
>>>
>>> Removing the call, as I originally suggested, does fix this in my testing.
>>> If you would still rather keep it, how would you prefer to solve this?
>>
>> Just removing the call from openvswitch module doesn't solve the problem.
>> Packet may enter OVS already cloned somewhere else in the stack, and at
>> any other point in the kernel where skb_tx_copy() is called it may be
>> operating on a clone of some other skb causing the exact same issue.  So,
>> it needs to be addressed inside the skb_tx_copy() itself.
>>
>> On the other hand, reading the history of this function, it seems like it
>> lost of its meaning with commit 1f8b977ab32d ("sock: enable MSG_ZEROCOPY")
>> from Willem that changed it to just call skb_zcopy_clear(skb, true);  This
>> changed the "false" signaling to "true".  So it doesn't even signal an error
>> anymore.
>>
>> Later, in commit 753f1ca4e1e5 ("net: introduce managed frags infrastructure")
>> Pavel added skb_zcopy_downgrade_managed(skb); call that takes extra frag
>> references.  Though it seems pointless for an skb that must be freed right
>> after.
>>
>> So, I'm not sure if this function is useful in general.  Feels like it is
>> only harmful as it directly modifies shared data with no regards to clones.
>>
>> We have two options here:
>>
>> 1. Minimal fix: add something like skb_cloned() guard into skb_tx_copy().
>>
>> 2. Remove skb_tx_copy() entirely (all calls and the definition) as it

* I meant skb_tx_error(), of course, everywhere above in place of skb_tx_copy()
  that is not a real function...

>>   seems pointless after 1f8b977ab32d.
> 
> Thanks for digging out 1f8b977ab32d.
> 
> Option 2 sounds cleaner to me, though it touches tun, ovpn and nfnetlink_queue
> as well. Option 1 would not cover the reported case on its own, since there is
> no clone on the OVS_ACTION_ATTR_USERSPACE path, but it should work with this 
> patch 1/2.
> 
> Curious what the others think. I can write whichever you settle on.

If there will be no other suggestions, I'd say what we can do is to have
a minimal fix for net and stable, i.e., a 3-patch set with 2 current patches
plus the new skb_cloned() guard inside skb_tx_error().  These should be
simple enough to backport.

Once those are accepted, we could remove the skb_tx_error() from net-next as
a follow up, so it doesn't muddy the waters moving forward.

> 
> N.
> 
>>
>> Any thoughts?  Willem, Pavel, others?
>>
>>>
>>> Thanks,
>>> Norbert
>>>
>>>> On Aug 13, 2026, at 12:00, Ilya Maximets <i.maximets@ovn.org> wrote:
>>>>
>>>> On 8/13/26 7:47 AM, Norbert Szetei wrote:
>>>>> queue_userspace_packet() borrows the packet skb -- it only copies it into
>>>>> a private netlink message (user_skb) and does not own it; on return
>>>>> do_execute_actions() keeps forwarding it through the flow's remaining
>>>>> actions. Its error path nevertheless calls skb_tx_error(skb), which via
>>>>> skb_zcopy_clear() does skb_shinfo(skb)->flags &= ~SKBFL_ALL_ZEROCOPY,
>>>>> stripping SKBFL_SHARED_FRAG from that live skb (skb_tx_error()'s kerneldoc
>>>>> says "skb must be freed afterwards").
>>>>>
>>>>> For a MSG_ZEROCOPY skb carrying page-cache frags, SKBFL_SHARED_FRAG is
>>>>> what makes esp_input() skb_cow_data() before in-place AEAD; once it is
>>>>> stripped a later local ESP-in-UDP delivery decrypts in place over pages
>>>>> the sender does not own -- an unprivileged page-cache write (the
>>>>> "Fragnesia" primitive).
>>>>> do_execute_actions() ignores output_userspace()'s return value, so any
>>>>> action after a failed USERSPACE upcall inherits the stripped skb.
>>>>>
>>>>> Move the skb_tx_error() to the flow-miss drop path - the "default"
>>>>> branch of ovs_dp_process_packet()'s switch(error), before kfree_skb().
>>>>>
>>>>> The call has been here since commit 36d5fe6a0007 ("core, nfqueue,
>>>>> openvswitch: Orphan frags in skb_zerocopy and handle errors") but was
>>>>> harmless until esp_input() began relying on SKBFL_SHARED_FRAG to gate
>>>>> in-place decrypt; only then did stripping it on a still-forwarded skb
>>>>> become a page-cache write primitive.
>>>>>
>>>>> Fixes: 36d5fe6a0007 ("core, nfqueue, openvswitch: Orphan frags in skb_zerocopy and handle errors")
>>>>> Fixes: f4c50a4034e6 ("xfrm: esp: avoid in-place decrypt on shared skb frags")
>>>>> Cc: stable@vger.kernel.org
>>>>> Assisted-by: Claude:claude-opus-5
>>>>> Signed-off-by: Norbert Szetei <norbert@doyensec.com>
>>>>> ---
>>>> Reviewed-by: Ilya Maximets <i.maximets@ovn.org>
> 
>