All of lore.kernel.org
 help / color / mirror / Atom feed
From: Sabrina Dubroca <sd@queasysnail.net>
To: Haseeb Malik via B4 Relay <devnull+haseebulhaq55.gmail.com@kernel.org>
Cc: netdev@vger.kernel.org, Andrew Lunn <andrew+netdev@lunn.ch>,
	"David S. Miller" <davem@davemloft.net>,
	Eric Dumazet <edumazet@kernel.org>,
	Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>,
	Shuah Khan <shuah@kernel.org>,
	Hannes Frederic Sowa <hannes@stressinduktion.org>,
	linux-kselftest@vger.kernel.org, linux-kernel@vger.kernel.org,
	Haseeb Malik <haseebulhaq55@gmail.com>
Subject: Re: [PATCH net] macsec: check the resolved SCI for duplicates
Date: Thu, 1 Oct 2026 00:18:03 +0200	[thread overview]
Message-ID: <ar2KmwhtCMbOKLsG@krikkit> (raw)
In-Reply-To: <20260927-fix-macsec-duplicate-sci-v1-1-085bd742c8e9@gmail.com>

2026-09-27, 01:03:24 -0400, Haseeb Malik via B4 Relay wrote:
> From: Haseeb Malik <haseebulhaq55@gmail.com>
> 
> An all-ones IFLA_MACSEC_SCI selects the default SCI derived from the
> MACsec device's MAC address and port 1. macsec_init_secy() resolves this
> value and stores the result in secy.sci, but macsec_newlink() checks for
> duplicates using the unchanged local sci argument.
> 
> Consequently, an all-ones request can create a second MACsec device with
> the same transmit SCI on the same lower device, while requesting that
> SCI explicitly returns -EBUSY.
> 
> Check the initialized SecY's SCI so that duplicate detection uses the
> value that the new device will actually use. Preserve the all-ones
> fallback when the resulting SCI is available.

Or move that fallback from macsec_init_secy to macsec_newlink? That'd
be a bit cleaner than hiding a rewrite of the value in some other
function, and then having to go fetch it.

Something like:

macsec_newlink()
{
	sci_t sci = MACSEC_UNDEF_SCI;

...
	if (data && data[IFLA_MACSEC_SCI])
		sci = nla_get_sci(data[IFLA_MACSEC_SCI]);
	else if (data && data[IFLA_MACSEC_PORT])
		sci = dev_to_sci(dev, nla_get_be16(data[IFLA_MACSEC_PORT]));

	if (sci == MACSEC_UNDEF_SCI)
		sci = dev_to_sci(dev, MACSEC_PORT_ES);
...
}



And after your previous patch, we could also move the sci_exists
check up after determining the sci, there's no reason to do it after
registering anymore. since I didn't notice that while reviewing your
previous patch, that should be done in net-next once net and this new
fix gets merged into it.

> Add regression tests for duplicate rejection using default, explicit and
> all-ones SCI requests, and for valid fallback and reuse after deletion.
> The same tests reproduce two failures before the fix and pass afterward.

Nice.

-- 
Sabrina

  parent reply	other threads:[~2026-09-30 22:18 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-27  5:03 [PATCH net] macsec: check the resolved SCI for duplicates Haseeb Malik via B4 Relay
2026-09-27  5:03 ` Haseeb Malik
2026-09-30 12:06 ` netdev-bot+sashiko
2026-09-30 22:18 ` Sabrina Dubroca [this message]
2026-10-01 23:21 ` 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=ar2KmwhtCMbOKLsG@krikkit \
    --to=sd@queasysnail.net \
    --cc=andrew+netdev@lunn.ch \
    --cc=davem@davemloft.net \
    --cc=devnull+haseebulhaq55.gmail.com@kernel.org \
    --cc=edumazet@kernel.org \
    --cc=hannes@stressinduktion.org \
    --cc=haseebulhaq55@gmail.com \
    --cc=kuba@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-kselftest@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=shuah@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.