Netdev List
 help / color / mirror / Atom feed
From: Vladimir Oltean <olteanv@gmail.com>
To: Andrew Lunn <andrew@lunn.ch>, Jakob Unterwurzacher <jakobunt@gmail.com>
Cc: Woojung Huh <woojung.huh@microchip.com>,
	UNGLinuxDriver@microchip.com,
	"David S. Miller" <davem@davemloft.net>,
	Eric Dumazet <edumazet@google.com>,
	Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>,
	Simon Horman <horms@kernel.org>,
	quentin.schulz@cherry.de,
	Jakob Unterwurzacher <jakob.unterwurzacher@cherry.de>,
	netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
	George McCollister <george.mccollister@gmail.com>
Subject: Re: [PATCH] net: dsa: microchip: linearize skb for tail-tagging switches
Date: Fri, 9 May 2025 15:56:31 +0300	[thread overview]
Message-ID: <20250509125631.cckfc2ychkyobqqo@skbuf> (raw)
In-Reply-To: <e76f230c-a513-4185-ae3f-72c033aeeb1e@lunn.ch>

On Fri, May 09, 2025 at 02:31:00PM +0200, Andrew Lunn wrote:
> On Fri, May 09, 2025 at 09:18:19AM +0200, Jakob Unterwurzacher wrote:
> > The pointer arithmentic for accessing the tail tag does not
> > seem to handle nonlinear skbs.
> > 
> > For nonlinear skbs, it reads uninitialized memory inside the
> > skb headroom, essentially randomizing the tag, breaking user
> > traffic.
> 
> Both tag_rtl8_4.c & tag_trailer.c also linearize, so i would say this
> is correct.
> 
> What is interesting is that both xrs700x_rcv() and
> sja1110_rcv_inband_control_extension() also don't call
> skb_linearize().
> 
> Vladimir? George?

Yes, it should be a more widespread problem.

Have non-zero needed_tailroom:
trailer
ksz8795
ksz9477
ksz9893
lan937x
hellcreek
sja1110
xrs700x

Call skb_linearize():
trailer
rtl8_4t

It should be only a matter of chance that the other taggers haven't come
across non-linear skbs.

My opinion is that we should let taggers linearize when and if it is
necessary, rather than doing so in the core. For example, sja1110 only
needs to do so if (rx_header & SJA1110_RX_HEADER_HAS_TRAILER), which the
core obviously does not know. Thus, I agree with the proposed fix.

Jakob, when you resend v2 retargeted to "net" and with the Fixes: tag
added, could you also address xrs700x and sja1110, or should I?

  reply	other threads:[~2025-05-09 12:56 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-05-09  7:18 [PATCH] net: dsa: microchip: linearize skb for tail-tagging switches Jakob Unterwurzacher
2025-05-09 12:31 ` Andrew Lunn
2025-05-09 12:56   ` Vladimir Oltean [this message]
2025-05-10  7:54     ` Jakob Unterwurzacher
2025-05-09 22:18 ` Tristram.Ha
2025-05-10 20:08   ` Jakob Unterwurzacher

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=20250509125631.cckfc2ychkyobqqo@skbuf \
    --to=olteanv@gmail.com \
    --cc=UNGLinuxDriver@microchip.com \
    --cc=andrew@lunn.ch \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=george.mccollister@gmail.com \
    --cc=horms@kernel.org \
    --cc=jakob.unterwurzacher@cherry.de \
    --cc=jakobunt@gmail.com \
    --cc=kuba@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=quentin.schulz@cherry.de \
    --cc=woojung.huh@microchip.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox