From: Aaron Conole <aconole@redhat.com>
To: 侯敏熙 <houminxi@gmail.com>
Cc: netdev@vger.kernel.org, Ilya Maximets <i.maximets@ovn.org>,
Kyle Zeng <kylebot@openai.com>,
Eelco Chaudron <echaudro@redhat.com>,
Outbound Disclosures <outbounddisclosures@openai.com>,
security@kernel.org
Subject: Re: [Security] Vulnerability in: Linux kernel, openvswitch zerocopy underflow
Date: Mon, 06 Jul 2026 10:21:52 -0400 [thread overview]
Message-ID: <f7tcxx02o73.fsf@redhat.com> (raw)
In-Reply-To: <CAJ0BgHeKoF7nnwA1_V0PSVETA+T9z9EX=OtSM4vMxQJv=nZAPg@mail.gmail.com> ("侯敏熙"'s message of "Mon, 6 Jul 2026 08:25:51 -0400")
侯敏熙 <houminxi@gmail.com> writes:
>> Just an FYI - this vulnerability is now public after Sashiko scanned
>> a proposed patch:
>
> Thanks for the heads-up, Aaron.
>
> My test_trunc() only exercises the non-GSO forwarding path (veth +
> ping), so neither the GSO segmentation underflow nor the POP_ETH +
> TRUNC combination is covered.
>
> Once a fix lands on net-next, I'll extend the TRUNC test to verify
> the new cutlen semantics.
No need. Let's keep it as is. We can look at doing complex 'fuzzing'
stuff in the future, but for now just giving a heads up.
> happy to help with testing if needed.
>
> Minxi
>
> Aaron Conole <aconole@redhat.com> 于2026年7月6日周一 08:05写道：
>
> Ilya Maximets <i.maximets@ovn.org> writes:
>
> > On 6/26/26 5:12 AM, Kyle Zeng wrote:
> >> Hi Ilya,
> >>
> >> I just tested the patch. It prevents underflow. However, it leads to a
> >> BUG_ON crash, the PoC and crash splash are attached.
> >>
> >> The patch changes `OVS_CB(skb)->cutlen` from "number of bytes to
> >> remove" to "number of bytes to preserve." That also changes the
> >> no-truncation sentinel from 0 to U32_MAX.
> >> Most of the action paths were updated to use U32_MAX, but the
> >> miss-upcall path was not. It still does `error = ovs_dp_upcall(dp,
> >> skb, key, &upcall, 0);` in `ovs_dp_process_packet()`.
> >
> > This is a good point, I missed that part.
>
> Just an FYI - this vulnerability is now public after Sashiko scanned a
> proposed patch:
>
> https://sashiko.dev/#/patchset/20260702074926.1174810-1-houminxi%40gmail.com
>
> > This is a pre-existing issue, but while looking at how TRUNC is handled,
> > there appears to be an integer underflow vulnerability...
>
> Minxi has been adding tests for different actions, and proposed a TRUNC
> action test. When reviewing it, I looked at the AI comments from the two
> different Sashiko instances and saw this. I've CC'd Minxi as well,
> since he proposed the selftest.
>
> Further discussion probably doesn't need to be on the security only
> list, and I think it is best if we go to the public mailing list.
>
> >>
> >> Under the new semantics, that 0 is no longer "no truncation" It means
> >> "preserve zero bytes". As a result, `queue_userspace_packet()`
> >> computes:
> >> `skb_len = min(skb->len, cutlen); /* becomes 0 */` and then: `hlen =
> >> skb_len; /* also becomes 0 */` before calling `skb_zerocopy(user_skb,
> >> skb, skb_len, hlen);`.
> >>
> >> I adapted your patch and the new patch is attached.
> >
> > See some comments below.
> >
> >> From 5b2b6f328b339a22c8a96bdacda4b62884640696 Mon Sep 17 00:00:00 2001
> >> From: Kyle Zeng <kylebot@openai.com>
> >> Date: Fri, 26 Jun 2026 02:48:14 +0000
> >> Subject: [PATCH] openvswitch: fix GSO userspace truncation underflow
> >>
> >> OVS_ACTION_ATTR_TRUNC currently stores a delta from the original skb
> >> length in OVS_CB(skb)->cutlen. When a later userspace action segments a
> >> GSO skb, queue_gso_packets() reuses that delta for each smaller segment.
> >> A segment can then reach queue_userspace_packet() with cutlen greater
> >> than skb->len, underflowing the length passed to skb_zerocopy().
> >>
> >> Store the maximum preserved length instead and derive the effective
> >> length from each current skb with min(). Use U32_MAX as the
> >> no-truncation sentinel so the value remains valid if skb geometry
> >> changes before a consumer handles it.
> >>
> >> Fixes: f2a4d086ed4c ("openvswitch: Add packet truncation support.")
> >> Cc: stable@vger.kernel.org
> >> Assisted-by: Codex:gpt-5.5
> >> Signed-off-by: Kyle Zeng <kylebot@openai.com>
> >> ---
> >> net/openvswitch/actions.c | 20 ++++++++------------
> >> net/openvswitch/datapath.c | 19 +++++++++++--------
> >> net/openvswitch/datapath.h | 3 ++-
> >> net/openvswitch/vport.c | 2 +-
> >> 4 files changed, 22 insertions(+), 22 deletions(-)
> >>
> >> diff --git a/net/openvswitch/actions.c b/net/openvswitch/actions.c
> >> index 140388a18ae0..596ad1be8b6b 100644
> >> --- a/net/openvswitch/actions.c
> >> +++ b/net/openvswitch/actions.c
> >> @@ -837,12 +837,9 @@ static void do_output(struct datapath *dp, struct sk_buff
> *skb, int out_port,
> >> u16 mru = OVS_CB(skb)->mru;
> >> u32 cutlen = OVS_CB(skb)->cutlen;
> >>
> >> - if (unlikely(cutlen > 0)) {
> >> - if (skb->len - cutlen > ovs_mac_header_len(key))
> >> - pskb_trim(skb, skb->len - cutlen);
> >> - else
> >> - pskb_trim(skb, ovs_mac_header_len(key));
> >> - }
> >> + if (unlikely(cutlen != U32_MAX))
> >
> > There is no need to trim if cutlen >= skb->len. Is there some problem with
> > the approach I had in my diff:
> >
> > + if (unlikely(cutlen < skb->len))
> > + pskb_trim(skb, max(cutlen, ovs_mac_header_len(key)));
> >
> > ?
> >
> >> + pskb_trim(skb, max(min(skb->len, cutlen),
> >> + ovs_mac_header_len(key)));
> >>
> >> if (likely(!mru ||
> >> (skb->len <= mru + vport->dev->hard_header_len))) {
> >> @@ -1234,7 +1231,7 @@ static void execute_psample(struct datapath *dp, struct
> sk_buff *skb,
> >>
> >> psample_group.net⚠️ = ovs_dp_get_net(dp);
> >> md.in_ifindex = OVS_CB(skb)->input_vport->dev->ifindex;
> >> - md.trunc_size = skb->len - OVS_CB(skb)->cutlen;
> >> + md.trunc_size = min(skb->len, OVS_CB(skb)->cutlen);
> >> md.rate_as_probability = 1;
> >>
> >> rate = OVS_CB(skb)->probability ? OVS_CB(skb)->probability : U32_MAX;
> >> @@ -1284,22 +1281,21 @@ static int do_execute_actions(struct datapath *dp, struct
> sk_buff *skb,
> >> clone = skb_clone(skb, GFP_ATOMIC);
> >> if (clone)
> >> do_output(dp, clone, port, key);
> >> - OVS_CB(skb)->cutlen = 0;
> >> + OVS_CB(skb)->cutlen = U32_MAX;
> >> break;
> >> }
> >>
> >> case OVS_ACTION_ATTR_TRUNC: {
> >> struct ovs_action_trunc *trunc = nla_data(a);
> >>
> >> - if (skb->len > trunc->max_len)
> >> - OVS_CB(skb)->cutlen = skb->len - trunc->max_len;
> >> + OVS_CB(skb)->cutlen = trunc->max_len;
> >> break;
> >> }
> >>
> >> case OVS_ACTION_ATTR_USERSPACE:
> >> output_userspace(dp, skb, key, a, attr,
> >> len, OVS_CB(skb)->cutlen);
> >> - OVS_CB(skb)->cutlen = 0;
> >> + OVS_CB(skb)->cutlen = U32_MAX;
> >> if (nla_is_last(a, rem)) {
> >> consume_skb(skb);
> >> return 0;
> >> @@ -1453,7 +1449,7 @@ static int do_execute_actions(struct datapath *dp, struct
> sk_buff *skb,
> >>
> >> case OVS_ACTION_ATTR_PSAMPLE:
> >> execute_psample(dp, skb, a);
> >> - OVS_CB(skb)->cutlen = 0;
> >> + OVS_CB(skb)->cutlen = U32_MAX;
> >> if (nla_is_last(a, rem)) {
> >> consume_skb(skb);
> >> return 0;
> >> diff --git a/net/openvswitch/datapath.c b/net/openvswitch/datapath.c
> >> index f0164817d9b7..66c621a611b7 100644
> >> --- a/net/openvswitch/datapath.c
> >> +++ b/net/openvswitch/datapath.c
> >> @@ -276,7 +276,7 @@ void ovs_dp_process_packet(struct sk_buff *skb, struct
> sw_flow_key *key)
> >> upcall.portid = ovs_vport_find_upcall_portid(p, skb);
> >>
> >> upcall.mru = OVS_CB(skb)->mru;
> >> - error = ovs_dp_upcall(dp, skb, key, &upcall, 0);
> >> + error = ovs_dp_upcall(dp, skb, key, &upcall, U32_MAX);
> >> switch (error) {
> >> case 0:
> >> case -EAGAIN:
> >> @@ -458,6 +458,7 @@ static int queue_userspace_packet(struct datapath *dp, struct
> sk_buff *skb,
> >> struct sk_buff *user_skb = NULL; /* to be queued to userspace */
> >> struct nlattr *nla;
> >> size_t len;
> >
> > I would still rename this into msg_size. Having 'len' and 'skb_len'
> > in the same scope is confusing.
> >
> >> + unsigned int skb_len;
> >> unsigned int hlen;
> >> int err, dp_ifindex;
> >> u64 hash;
> >> @@ -478,7 +479,8 @@ static int queue_userspace_packet(struct datapath *dp, struct
> sk_buff *skb,
> >> skb = nskb;
> >> }
> >>
> >> - if (nla_attr_size(skb->len) > USHRT_MAX) {
> >> + skb_len = min(skb->len, cutlen);
> >> + if (nla_attr_size(skb_len) > USHRT_MAX) {
> >> err = -EFBIG;
> >> goto out;
> >> }
> >> @@ -493,11 +495,11 @@ static int queue_userspace_packet(struct datapath *dp,
> struct sk_buff *skb,
> >> * padding logic. Only perform zerocopy if padding is not required.
> >> */
> >> if (dp->user_features & OVS_DP_F_UNALIGNED)
> >> - hlen = skb_zerocopy_headlen(skb);
> >> + hlen = min(skb_zerocopy_headlen(skb), skb_len);
> >> else
> >> - hlen = skb->len;
> >> + hlen = skb_len;
> >>
> >> - len = upcall_msg_size(upcall_info, hlen - cutlen,
> >> + len = upcall_msg_size(upcall_info, hlen,
> >> OVS_CB(skb)->acts_origlen);
> >> user_skb = genlmsg_new(len, GFP_ATOMIC);
> >> if (!user_skb) {
> >> @@ -560,7 +562,7 @@ static int queue_userspace_packet(struct datapath *dp, struct
> sk_buff *skb,
> >> }
> >>
> >> /* Add OVS_PACKET_ATTR_LEN when packet is truncated */
> >> - if (cutlen > 0 &&
> >> + if (cutlen != U32_MAX &&
> >
> > This changes the logic. The attribute should be included only if the
> > packet was actually truncated, i.e., if skb_len < skb->len.
> >
> >> nla_put_u32(user_skb, OVS_PACKET_ATTR_LEN, skb->len)) {
> >> err = -ENOBUFS;
> >> goto out;
> >> @@ -585,9 +587,9 @@ static int queue_userspace_packet(struct datapath *dp, struct
> sk_buff *skb,
> >> err = -ENOBUFS;
> >> goto out;
> >> }
> >> - nla->nla_len = nla_attr_size(skb->len - cutlen);
> >> + nla->nla_len = nla_attr_size(skb_len);
> >>
> >> - err = skb_zerocopy(user_skb, skb, skb->len - cutlen, hlen);
> >> + err = skb_zerocopy(user_skb, skb, skb_len, hlen);
> >> if (err)
> >> goto out;
> >>
> >> @@ -644,6 +646,7 @@ static int ovs_packet_cmd_execute(struct sk_buff *skb, struct
> genl_info *info)
> >> packet->ignore_df = 1;
> >> }
> >> OVS_CB(packet)->mru = mru;
> >> + OVS_CB(packet)->cutlen = U32_MAX;
> >>
> >> if (a[OVS_PACKET_ATTR_HASH]) {
> >> hash = nla_get_u64(a[OVS_PACKET_ATTR_HASH]);
> >> diff --git a/net/openvswitch/datapath.h b/net/openvswitch/datapath.h
> >> index db0c3e69d66c..11fa104b6172 100644
> >> --- a/net/openvswitch/datapath.h
> >> +++ b/net/openvswitch/datapath.h
> >> @@ -118,7 +118,8 @@ struct datapath {
> >> * @mru: The maximum received fragement size; 0 if the packet is not
> >> * fragmented.
> >> * @acts_origlen: The netlink size of the flow actions applied to this skb.
> >> - * @cutlen: The number of bytes from the packet end to be removed.
> >> + * @cutlen: The maximum number of bytes to preserve on output. U32_MAX means
> >> + * no truncation.
> >> * @probability: The sampling probability that was applied to this skb; 0 means
> >> * no sampling has occurred; U32_MAX means 100% probability.
> >> * @upcall_pid: Netlink socket PID to use for sending this packet to userspace;
> >> diff --git a/net/openvswitch/vport.c b/net/openvswitch/vport.c
> >> index 56b2e2d1a749..12741485c939 100644
> >> --- a/net/openvswitch/vport.c
> >> +++ b/net/openvswitch/vport.c
> >> @@ -502,7 +502,7 @@ int ovs_vport_receive(struct vport *vport, struct sk_buff *skb,
> >>
> >> OVS_CB(skb)->input_vport = vport;
> >> OVS_CB(skb)->mru = 0;
> >> - OVS_CB(skb)->cutlen = 0;
> >> + OVS_CB(skb)->cutlen = U32_MAX;
> >> OVS_CB(skb)->probability = 0;
> >> OVS_CB(skb)->upcall_pid = 0;
> >> if (unlikely(dev_net(skb->dev) != ovs_dp_get_net(vport->dp))) {
> >> --
> >> 2.54.0
> >>
parent reply other threads:[~2026-07-06 14:22 UTC|newest]
Thread overview: expand[flat|nested] mbox.gz Atom feed
[parent not found: <CAJ0BgHeKoF7nnwA1_V0PSVETA+T9z9EX=OtSM4vMxQJv=nZAPg@mail.gmail.com>]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=f7tcxx02o73.fsf@redhat.com \
--to=aconole@redhat.com \
--cc=echaudro@redhat.com \
--cc=houminxi@gmail.com \
--cc=i.maximets@ovn.org \
--cc=kylebot@openai.com \
--cc=netdev@vger.kernel.org \
--cc=outbounddisclosures@openai.com \
--cc=security@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.