All of lore.kernel.org
 help / color / mirror / Atom feed
From: Simon Horman <horms@kernel.org>
To: Jamal Hadi Salim <jhs@mojatatu.com>
Cc: netdev@vger.kernel.org, Victor Nogueira <victor@mojatatu.com>,
	Jiri Pirko <jiri@resnulli.us>,
	"David S . Miller" <davem@davemloft.net>,
	Eric Dumazet <edumazet@kernel.org>,
	Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>,
	stable@vger.kernel.org, sashiko-bot@kernel.org
Subject: Re: [PATCH net v3 1/2] net/sched: sch_teql: unshare the header before writing and fix retry strip
Date: Mon, 9 Nov 2026 17:15:39 +0000	[thread overview]
Message-ID: <20261109171539.GU18859@horms.kernel.org> (raw)
In-Reply-To: <QDISC-6HS9.v3.r0.20261103073017-1@mojatatu.com>

On Tue, Nov 03, 2026 at 07:39:15AM -0500, Jamal Hadi Salim wrote:
> The issue:
> teql_master_xmit() writes the slave's link header into the skb in
> teql_resolve() and on retry pulls it back with __skb_pull().  Two bugs:
> 
> 1. When the skb's header is cloned (e.g. a tc-mirred action on a
>    physical device's clsact egress redirects to the teql master), the
>    write corrupts the other clone.  Call skb_cow_head() first.
> 
> 2. The pull always removes skb_network_offset() bytes.  For an
>    AF_PACKET/SOCK_RAW frame with no dst entry, teql_resolve() adds no
>    header, so the retry strips the user-supplied one.  Pull back only
>    the length this slave added.
> 
> The Fix:
> Track the added length per iteration; a header-cloned skb is unshared
> before the write and an AF_PACKET frame retries with its header intact.
> 
> Conditions to recreate the bug:
> CONFIG_NET_SCH_TEQL=y and CONFIG_VETH=y, run as root. A teql master with
> two veth slaves, the first slave returning NETDEV_TX_BUSY without
> consuming the skb so that teql retries the frame on the second one:
> 
>   ip link add vethBAD type veth peer name vethBADp
>   ip link add vethGOOD type veth peer name vethGOODp
>   ip link set vethBAD up;  ip link set vethBADp up
>   ip link set vethGOOD up; ip link set vethGOODp up
>   tc qdisc add dev vethBAD root teql0
>   tc qdisc add dev vethGOOD root teql0
>   ip link set teql0 up
> 
> Send a SOCK_RAW frame that carries its own link header on teql0 and
> capture it on the second slave's peer. A veth transmit does not return
> NETDEV_TX_BUSY on its own; to hold the skb pending this reproduction used
> a test-only IFF_DEBUG accelerator in veth_xmit(), which is not part of
> this patch and was applied to both the unfixed and the fixed build.
> 
> On the unfixed tree the captured frame arrives with its header stripped
> (176 of the 208 bytes sent, starting at the payload) because the retry
> pulls skb_network_offset() bytes while this slave added none. With the
> patch the frame arrives intact.
> 
> The shared headroom case needs a cloned skb: a PF_PACKET tap on the
> master whose socket is never drained keeps the clone alive across the
> transmit. A kprobe on eth_header then reports cloned=1 on the unfixed
> tree and cloned=0 with the patch.
> 
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> Reported-by: Sashiko (gemini) <sashiko-bot@kernel.org>
> Closes: https://sashiko.dev/#/patchset/20260824115928.4099988-1-victor@mojatatu.com
> Link: https://lore.kernel.org/netdev/20260824115928.4099988-1-victor@mojatatu.com/
> Cc: stable@vger.kernel.org
> Reviewed-by: Victor Nogueira <victor@mojatatu.com>
> Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>

Reviewed-by: Simon Horman <horms@kernel.org>


  reply	other threads:[~2026-11-09 17:15 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-11-03 12:39 [PATCH net v3 0/2] net/sched: sch_teql: fix header-cloned write and retry strip Jamal Hadi Salim
2026-11-03 12:39 ` [PATCH net v3 1/2] net/sched: sch_teql: unshare the header before writing and fix " Jamal Hadi Salim
2026-11-09 17:15   ` Simon Horman [this message]
2026-11-03 12:39 ` [PATCH net v3 2/2] net/sched: sch_teql: keep skb->dev consistent on the arp-queue path Jamal Hadi Salim
2026-11-07 15:16   ` netdev-bot+sashiko
2026-11-09 11:44     ` Jamal Hadi Salim
2026-11-09 17:15       ` Simon Horman
2026-11-10 18:19       ` Jakub Kicinski
2026-11-10 19:35         ` Jamal Hadi Salim
2026-11-10 18:19   ` Jakub Kicinski

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=20261109171539.GU18859@horms.kernel.org \
    --to=horms@kernel.org \
    --cc=davem@davemloft.net \
    --cc=edumazet@kernel.org \
    --cc=jhs@mojatatu.com \
    --cc=jiri@resnulli.us \
    --cc=kuba@kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=sashiko-bot@kernel.org \
    --cc=stable@vger.kernel.org \
    --cc=victor@mojatatu.com \
    /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.