* Re: [PATCH 1/9] bitfield: add FIELD_GET_SIGNED()
From: Yury Norov @ 2026-04-17 21:09 UTC (permalink / raw)
To: David Laight
Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
H. Peter Anvin, Andy Lutomirski, Peter Zijlstra, Jonathan Cameron,
David Lechner, Nuno Sá, Andy Shevchenko, Ping-Ke Shih,
Richard Cochran, Andrew Lunn, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Alexandre Belloni, Yury Norov,
Rasmus Villemoes, Hans de Goede, Linus Walleij, Sakari Ailus,
Salah Triki, Achim Gratz, Ben Collins, linux-kernel, linux-iio,
linux-wireless, netdev, linux-rtc
In-Reply-To: <20260417204355.37fd960d@pumpkin>
On Fri, Apr 17, 2026 at 08:43:55PM +0100, David Laight wrote:
> On Fri, 17 Apr 2026 13:36:12 -0400
> Yury Norov <ynorov@nvidia.com> wrote:
>
> > The bitfields are designed in assumption that fields contain unsigned
> > integer values, thus extracting the values from the field implies
> > zero-extending.
> >
> > Some drivers need to sign-extend their fields, and currently do it like:
> >
> > dc_re += sign_extend32(FIELD_GET(0xfff000, tmp), 11);
> > dc_im += sign_extend32(FIELD_GET(0xfff, tmp), 11);
> >
> > It's error-prone because it relies on user to provide the correct
> > index of the most significant bit and proper 32 vs 64 function flavor.
> >
> > Thus, introduce a FIELD_GET_SIGNED() macro, which is the more
> > convenient and compiles (on x86_64) to just a couple instructions:
> > shl and sar.
> >
> > Signed-off-by: Yury Norov <ynorov@nvidia.com>
> > ---
> > include/linux/bitfield.h | 16 ++++++++++++++++
> > 1 file changed, 16 insertions(+)
> >
> > diff --git a/include/linux/bitfield.h b/include/linux/bitfield.h
> > index 54aeeef1f0ec..35ef63972810 100644
> > --- a/include/linux/bitfield.h
> > +++ b/include/linux/bitfield.h
> > @@ -178,6 +178,22 @@
> > __FIELD_GET(_mask, _reg, "FIELD_GET: "); \
> > })
> >
> > +/**
> > + * FIELD_GET_SIGNED() - extract a signed bitfield element
> > + * @mask: shifted mask defining the field's length and position
> > + * @reg: value of entire bitfield
> > + *
> > + * Returns the sign-extended field specified by @_mask from the
> > + * bitfield passed in as @_reg by masking and shifting it down.
> > + */
> > +#define FIELD_GET_SIGNED(mask, reg) \
> > + ({ \
> > + __BF_FIELD_CHECK(mask, reg, 0U, "FIELD_GET_SIGNED: "); \
> > + ((__signed_scalar_typeof(mask))((long long)(reg) << \
> > + __builtin_clzll(mask) >> (__builtin_clzll(mask) + \
> > + __builtin_ctzll(mask))));\
>
> Have you looked at what that generates on a typical 32bit architecture?
Yes, for arm32:
#define FIELD_GET_SIGNED(mask, reg) \
((long long)(reg) << \
__builtin_clzll(mask) >> (__builtin_clzll(mask) + \
__builtin_ctzll(mask)))
long long foo(long long reg)
{
return FIELD_GET_SIGNED(0x00f00000ULL, reg);
}
generates:
foo(long long):
lsls r1, r0, #8
asrs r0, r1, #28
asrs r1, r1, #31
bx lr
Just as good as x86_64.
https://godbolt.org/z/eMnKrnocq
> It really a bad idea to use __signed_scalar_typeof() on anything that isn't
> a simple variable.
> The bloat from all this when 'mask' is an expansion of GENMASK() is horrid.
> Indeed both signed_scalar_typeof() and unsigned_scalar_typeof() should
> really not be used - there are generally much better ways.
David, it's not the first time you're throwing "bad idea, horrid bloat,
really not be used"-like rant with absolutely no evidence that people
do something wrong. Today I became another random victim of your style
of communication, and I don't think there's any benefit to tolerate it
for me or anybody else.
I encourage you to change your attitude, and use professional and
specific communication style in the kernel mailing list.
Starting from now, I'm not a free tester for your ideas anymore. If
you think that my patch is wrong, please prove it yourself. If you
think that 32-bit or whatever code generation is bad - please send
an example. If you believe that your implementation is any better -
please bother yourself to convince me.
I will continue receiving patches from you in my tree, but if your
patch is claimed to improve code generation, performance of any sort,
or similar things, and doesn't provide any numbers - I'll not waste
my time on it.
Thanks,
Yury
> In this case you can just write:
> ({
> auto _mask = mask;
> unsigned int __sl = __builtin_clzll(_mask);
> unsigned int __sr = __sl + __builtin_ctzll(_mask);
> __builtin_chose_expr(sizeof(_mask) <= 4,
> (int)(reg) << __sl - 32 >> __sr - 32,
> ((long long)(reg) << __sl >> __sr)
> })
> and let the compiler do any more integer promotions (etc).
>
> I'm also not convinced that the checks __BF_FIELD_CHECK() does
> on 'reg' are in any sense worth the effort.
>
> I have tried some simpler alternatives, eg:
> !__builtin_constant_p(reg) && statically_true((reg & mask) == 0)
> however that throws up some false positives due to some of weird ways
> people have used FIELD_GET() where it is nothing like the simplest
> (or most obvious) way to do things.
> That might have been the code that split a 32bit value into bytes
> in a printf with:
> FIELD_GET(GENMASK(7, 0), val), FIELD_GET(GENMASK(15, 8), val),
> FIELD_GET(GENMASK(23, 16), val), FIELD_GET(GENMASK(31, 24), val),
>
> David
>
> > + })
> > +
> > /**
> > * FIELD_MODIFY() - modify a bitfield element
> > * @_mask: shifted mask defining the field's length and position
^ permalink raw reply
* [PATCH net] sctp: fix sockets_allocated imbalance after sk_clone()
From: Xin Long @ 2026-04-17 21:09 UTC (permalink / raw)
To: network dev, linux-sctp
Cc: davem, kuba, Eric Dumazet, Paolo Abeni, Simon Horman,
Marcelo Ricardo Leitner, Kuniyuki Iwashima
sk_clone() increments sockets_allocated and sets the socket refcount to 2.
SCTP performs additional accounting in sctp_clone_sock(), so the clone-time
increment must be undone to avoid double counting.
Note we cannot simply remove the SCTP-side increment, because the SCTP
destroy path in sctp_destroy_sock() only decrements sockets_allocated when
sp->ep is set, which may not be true for all failure paths in
sctp_clone_sock().
Fixes: 16942cf4d3e3 ("sctp: Use sk_clone() in sctp_accept().")
Signed-off-by: Xin Long <lucien.xin@gmail.com>
---
net/sctp/socket.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/net/sctp/socket.c b/net/sctp/socket.c
index d2665bbd41a2..d0e7048230c0 100644
--- a/net/sctp/socket.c
+++ b/net/sctp/socket.c
@@ -4855,8 +4855,9 @@ static struct sock *sctp_clone_sock(struct sock *sk,
if (!newsk)
return ERR_PTR(err);
- /* sk_clone() sets refcnt to 2 */
+ /* sk_clone() sets refcnt to 2 and increments sockets_allocated */
sock_put(newsk);
+ sk_sockets_allocated_dec(newsk);
newinet = inet_sk(newsk);
newsp = sctp_sk(newsk);
--
2.47.1
^ permalink raw reply related
* Re: [PATCH] tcp: fix orphan count order in __tcp_close()
From: Eric Dumazet @ 2026-04-17 20:54 UTC (permalink / raw)
To: RubenKelevra
Cc: netdev, ncardwell, kuniyu, davem, dsahern, kuba, pabeni, horms
In-Reply-To: <20260417202518.1354891-1-rubenkelevra@gmail.com>
On Fri, Apr 17, 2026 at 1:25 PM RubenKelevra <rubenkelevra@gmail.com> wrote:
>
> __tcp_close() calls sock_orphan(sk) first and drains the backlog with
> __release_sock(sk), which might call tcp_done() which decrements the
> tcp_orphan_count. After which we will increment tcp_orphan_count again.
>
> Since tcp_orphan_count is an unsigned int, we underflow to uint_max if we
> started with a 0 - at least on all current supported platforms.
>
> I could not locate a direct user of this value, and in
> tcp_orphan_count_sum() this underflow is contained by adding the unsigned
> int value into a signed int sum, causing it to behave like -1 on current
> supported platforms and then get clamped by max(n, 0) to 0.
>
> The impact therefore is currently limited to e.g. tcp_too_many_orphans()
> checking an artificially low value, if the cached sum is refreshed within
> this timeframe.
>
> This fix mirrors the previous fix I found while investigating: commit
> 75c2d9077c63 ("[TCP]: Fix sock_orphan dead lock")
>
> Later commit eb4dea585304 ("net: Fix percpu counters deadlock") moved the
> increment down for old percpu_counter reasons. commit 19757cebf0c5 ("tcp:
> switch orphan_count to bare per-cpu counters") changed orphan accounting to
> plain per-cpu counters, so that old reason no longer applies the same way
> now.
I find this patch rather confusing. Have you used an LLM to generate it?
You are mentioning old patches that are not relevant (bh disable/enable).
Given we advise only increasing tcp_max_orphans value (default being
262144 on modern hosts),
your patch has really no effect.
^ permalink raw reply
* [PATCH] tcp: fix orphan count order in __tcp_close()
From: RubenKelevra @ 2026-04-17 20:25 UTC (permalink / raw)
To: netdev, edumazet, ncardwell
Cc: kuniyu, davem, dsahern, kuba, pabeni, horms, RubenKelevra
__tcp_close() calls sock_orphan(sk) first and drains the backlog with
__release_sock(sk), which might call tcp_done() which decrements the
tcp_orphan_count. After which we will increment tcp_orphan_count again.
Since tcp_orphan_count is an unsigned int, we underflow to uint_max if we
started with a 0 - at least on all current supported platforms.
I could not locate a direct user of this value, and in
tcp_orphan_count_sum() this underflow is contained by adding the unsigned
int value into a signed int sum, causing it to behave like -1 on current
supported platforms and then get clamped by max(n, 0) to 0.
The impact therefore is currently limited to e.g. tcp_too_many_orphans()
checking an artificially low value, if the cached sum is refreshed within
this timeframe.
This fix mirrors the previous fix I found while investigating: commit
75c2d9077c63 ("[TCP]: Fix sock_orphan dead lock")
Later commit eb4dea585304 ("net: Fix percpu counters deadlock") moved the
increment down for old percpu_counter reasons. commit 19757cebf0c5 ("tcp:
switch orphan_count to bare per-cpu counters") changed orphan accounting to
plain per-cpu counters, so that old reason no longer applies the same way
now.
Fixes: 19757cebf0c5 ("tcp: switch orphan_count to bare per-cpu counters")
Signed-off-by: RubenKelevra <rubenkelevra@gmail.com>
---
net/ipv4/tcp.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
index 2014a6408e93..1a91cb31b02f 100644
--- a/net/ipv4/tcp.c
+++ b/net/ipv4/tcp.c
@@ -3234,11 +3234,10 @@ void __tcp_close(struct sock *sk, long timeout)
local_bh_disable();
bh_lock_sock(sk);
+ tcp_orphan_count_inc();
/* remove backlog if any, without releasing ownership. */
__release_sock(sk);
- tcp_orphan_count_inc();
-
/* Have we already been destroyed by a softirq or backlog? */
if (state != TCP_CLOSE && sk->sk_state == TCP_CLOSE)
goto out;
--
2.53.0
^ permalink raw reply related
* Re: [PATCH] rds: zero per-item info buffer before handing it to visitors
From: Allison Henderson @ 2026-04-17 20:07 UTC (permalink / raw)
To: Michael Bommarito, David S . Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni
Cc: Simon Horman, netdev, linux-rdma, rds-devel, linux-kernel
In-Reply-To: <20260417141916.494761-1-michael.bommarito@gmail.com>
On Fri, 2026-04-17 at 10:19 -0400, Michael Bommarito wrote:
> Yet another from my "clanker." This only applies to people who
> don't use CONFIG_INIT_STACK_ALL_ZERO, but I presume that's
> still enough people that it's worth backporting since it can
> be chained through leaked addresses to defeat KASLR.
>
> rds_for_each_conn_info() and rds_walk_conn_path_info() both hand a
> caller-allocated on-stack u64 buffer to a per-connection visitor and
> then copy the full item_len bytes back to user space via
> rds_info_copy() regardless of how much of the buffer the visitor
> actually wrote.
>
> rds_ib_conn_info_visitor() and rds6_ib_conn_info_visitor() only
> write a subset of their output struct when the underlying
> rds_connection is not in state RDS_CONN_UP (src/dst addr, tos, sl
> and the two GIDs via explicit memsets). Several u32 fields
> (max_send_wr, max_recv_wr, max_send_sge, rdma_mr_max, rdma_mr_size,
> cache_allocs) and the 2-byte alignment hole between sl and
> cache_allocs remain as whatever stack contents preceded the visitor
> call and are then memcpy_to_user()'d out to user space.
>
> struct rds_info_rdma_connection and struct rds6_info_rdma_connection
> are the only rds_info_* structs in include/uapi/linux/rds.h that are
> not marked __attribute__((packed)), so they have a real alignment
> hole. The other info visitors (rds_conn_info_visitor,
> rds6_conn_info_visitor, rds_tcp_tc_info, ...) write all fields of
> their packed output struct today and are not known to be vulnerable,
> but a future visitor that adds a conditional write-path would have
> the same bug.
>
> Reproduction on a kernel built without CONFIG_INIT_STACK_ALL_ZERO=y:
> a local unprivileged user opens AF_RDS, sets SO_RDS_TRANSPORT=IB,
> binds to a local address on an RDMA-capable netdev (rxe soft-RoCE on
> any netdev is sufficient), sendto()'s any peer on the same subnet
> (fails cleanly but installs an rds_connection in the global hash in
> RDS_CONN_CONNECTING), then calls getsockopt(SOL_RDS,
> RDS_INFO_IB_CONNECTIONS). The returned 68-byte item contains 26
> bytes of stack garbage including kernel text/data pointers:
>
> 0..7 0a 63 00 01 0a 63 00 02 src=10.99.0.1 dst=10.99.0.2
> 8..39 00 ... gids (memset-zeroed)
> 40..47 e0 92 a3 81 ff ff ff ff kernel pointer (max_send_wr)
> 48..55 7f 37 b5 81 ff ff ff ff kernel pointer (rdma_mr_max)
> 56..59 01 00 08 00 rdma_mr_size (garbage)
> 60..61 00 00 tos, sl
> 62..63 00 00 alignment padding
> 64..67 18 00 00 00 cache_allocs (garbage)
>
> Fix by zeroing the per-item buffer in both rds_for_each_conn_info()
> and rds_walk_conn_path_info() before invoking the visitor. This
> covers the IPv4/IPv6 IB visitors and hardens all current and future
> visitors against the same class of bug.
>
> No functional change for visitors that fully populate their output.
>
> Fixes: ec16227e1414 ("RDS/IB: Infiniband transport")
> Signed-off-by: Michael Bommarito <michael.bommarito@gmail.com>
> Assisted-by: Claude:claude-opus-4-7
Hi Micheal,
The change looks fine to me. Since this is a bug fix, you'll want to cc stable
tree stable@vger.kernel.org, and note the target tree and component in the
subject line like this:
[PATCH net v2] net/rds: zero per-item info buffer before handing it to visitors
Other than that, the patch looks good to me. Thanks Micheal.
Reviewed-by: Allison Henderson <achender@kernel.org>
Allison
> ---
> net/rds/connection.c | 14 ++++++++++++++
> 1 file changed, 14 insertions(+)
>
> diff --git a/net/rds/connection.c b/net/rds/connection.c
> index 412441aaa298..c10b7ed06c49 100644
> --- a/net/rds/connection.c
> +++ b/net/rds/connection.c
> @@ -701,6 +701,13 @@ void rds_for_each_conn_info(struct socket *sock, unsigned int len,
> i++, head++) {
> hlist_for_each_entry_rcu(conn, head, c_hash_node) {
>
> + /* Zero the per-item buffer before handing it to the
> + * visitor so any field the visitor does not write -
> + * including implicit alignment padding - cannot leak
> + * stack contents to user space via rds_info_copy().
> + */
> + memset(buffer, 0, item_len);
> +
> /* XXX no c_lock usage.. */
> if (!visitor(conn, buffer))
> continue;
> @@ -750,6 +757,13 @@ static void rds_walk_conn_path_info(struct socket *sock, unsigned int len,
> */
> cp = conn->c_path;
>
> + /* Zero the per-item buffer for the same reason as
> + * rds_for_each_conn_info(): any byte the visitor
> + * does not write (including alignment padding) must
> + * not leak stack contents via rds_info_copy().
> + */
> + memset(buffer, 0, item_len);
> +
> /* XXX no cp_lock usage.. */
> if (!visitor(cp, buffer))
> continue;
^ permalink raw reply
* Re: [PATCH net] net/packet: fix TOCTOU race on mmap'd vnet_hdr in tpacket_snd()
From: Willem de Bruijn @ 2026-04-17 20:01 UTC (permalink / raw)
To: Zero Mark, Willem de Bruijn
Cc: security, David S . Miller, Jakub Kicinski, Eric Dumazet, netdev,
Zero Mark
In-Reply-To: <20260417133610.88158-1-patzilla007@gmail.com>
Zero Mark wrote:
> In tpacket_snd(), when PACKET_VNET_HDR is enabled, vnet_hdr points
> directly into the mmap'd TX ring buffer shared with userspace. The
> kernel validates the header via __packet_snd_vnet_parse() but then
> re-reads all fields later in virtio_net_hdr_to_skb(). A concurrent
> userspace thread can modify the vnet_hdr fields between validation
> and use, bypassing all safety checks.
>
> The non-TPACKET path (packet_snd()) already correctly copies vnet_hdr
> to a stack-local variable. All other vnet_hdr consumers in the kernel
> (tun.c, tap.c, virtio_net.c) also use stack copies. The TPACKET TX
> path is the only caller of virtio_net_hdr_to_skb() that reads directly
> from user-controlled shared memory.
>
> Fix this by copying vnet_hdr from the mmap'd ring buffer to a
> stack-local variable before validation and use, consistent with the
> approach used in packet_snd() and all other callers.
>
> Fixes: 1d036d25e560 ("packet: tpacket_snd gso and checksum offload")
> Signed-off-by: Zero Mark <patzilla007@gmail.com>
Reviewed-by: Willem de Bruijn <willemb@google.com>
^ permalink raw reply
* [BUG] some temporary IPv6 address don't get regenerated
From: Łukasz Stelmach @ 2026-04-17 15:23 UTC (permalink / raw)
To: netdev
Hi,
Apparently, something in addrconf.c can go wrong and a temporary
addresses may not get regenerated leaving users who wish to use them
with only the stable ones. Below, 2a01:110f:4321:1002:abcc:78d7:2055:94ec
while still valid is not preferred anymore. Even if it's not the only
global temporary address it is the only usable to contact hosts on the
Internet because the other temporary addresses are ULA.
Neither received RAs nor adding and removing an address manually (one
with a different prefix unrelated to these below) which as far as I
understand, should trigger address maintenance code.
I noticed this phenomenon once or twice before. It seems to be very
rare, yet quite undesirable I'd say. What might have triggered it today
is a reboot of my router and (possible?) change in lifetime values it
announced.
Even stranger is that there is a preferred fd89:: (ULA), but not 2a01::.
accept_ra as well as use_tempaddr are set to 2.
Of course, this ma be get fixed by a suspend/resume (no, it may not) or
manual ifdown/ifup (of course it helped), but nevertheless I thought it
was worth reporting.
--8<---------------cut here---------------start------------->8---
107: bond0: <BROADCAST,MULTICAST,MASTER,UP,LOWER_UP> mtu 1500 qdisc noqueue state UP group default qlen 1000
link/ether b8:ca:3a:d4:1e:97 brd ff:ff:ff:ff:ff:ff
inet 192.168.2.122/24 brd 192.168.2.255 scope global dynamic bond0
valid_lft 165585sec preferred_lft 165585sec
inet6 fd89:82bb:420:2:cab3:d2d6:aeeb:e250/64 scope global temporary dynamic
valid_lft 597079sec preferred_lft 78177sec
inet6 2a01:110f:4321:1002:abcc:78d7:2055:94ec/64 scope global temporary deprecated dynamic
valid_lft 60815sec preferred_lft 0sec
inet6 2a01:110f:4321:1002:abac:3aff:fed4:beef/64 scope global dynamic mngtmpaddr proto kernel_ra
valid_lft 60815sec preferred_lft 60815sec
inet6 fd89:82bb:420:2:24bf:fe8f:5c9c:c753/64 scope global temporary deprecated dynamic
valid_lft 511186sec preferred_lft 0sec
inet6 fd89:82bb:420:2:abac:3aff:fed4:beef/64 scope global dynamic mngtmpaddr proto kernel_ra
valid_lft 2591805sec preferred_lft 604605sec
inet6 fe80::abac:3aff:fed4:beef/64 scope link proto kernel_ll
valid_lft forever preferred_lft forever
--8<---------------cut here---------------end--------------->8---
--
Kind regards,
Łukasz Stelmach
^ permalink raw reply
* RE: [Intel-wired-lan] [PATCH iwl-net v1] igc: set tx buffer type for SMD frames
From: Loktionov, Aleksandr @ 2026-04-17 19:48 UTC (permalink / raw)
To: Kohei Enju, intel-wired-lan@lists.osuosl.org,
netdev@vger.kernel.org
Cc: Nguyen, Anthony L, Kitszel, Przemyslaw, Andrew Lunn,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Choong Yong Liang, Vladimir Oltean, Gomes, Vinicius,
Choong, Chwee Lin, Simon Horman
In-Reply-To: <20260417193223.291093-1-kohei@enjuk.jp>
> -----Original Message-----
> From: Intel-wired-lan <intel-wired-lan-bounces@osuosl.org> On Behalf
> Of Kohei Enju
> Sent: Friday, April 17, 2026 9:32 PM
> To: intel-wired-lan@lists.osuosl.org; netdev@vger.kernel.org
> Cc: Nguyen, Anthony L <anthony.l.nguyen@intel.com>; Kitszel,
> Przemyslaw <przemyslaw.kitszel@intel.com>; Andrew Lunn
> <andrew+netdev@lunn.ch>; David S. Miller <davem@davemloft.net>; Eric
> Dumazet <edumazet@google.com>; Jakub Kicinski <kuba@kernel.org>; Paolo
> Abeni <pabeni@redhat.com>; Choong Yong Liang
> <yong.liang.choong@linux.intel.com>; Vladimir Oltean
> <vladimir.oltean@nxp.com>; Gomes, Vinicius <vinicius.gomes@intel.com>;
> Choong, Chwee Lin <chwee.lin.choong@intel.com>; Simon Horman
> <horms@kernel.org>; Kohei Enju <kohei@enjuk.jp>
> Subject: [Intel-wired-lan] [PATCH iwl-net v1] igc: set tx buffer type
> for SMD frames
>
> Sashiko pointed out that igc_fpe_init_smd_frame() initializes
> igc_tx_buffer fields for an SMD skb, but does not set the buffer type:
> https://sashiko.dev/#/patchset/20260415025226.114115-1-
> kohei%40enjuk.jp
>
> Since igc_tx_buffer entries are reused, a stale XDP or XSK type can
> remain and make TX completion use the wrong cleanup path.
>
> Set the buffer type to IGC_TX_BUFFER_TYPE_SKB.
>
> Fixes: 5422570c0010 ("igc: add support for frame preemption
> verification")
> Signed-off-by: Kohei Enju <kohei@enjuk.jp>
> ---
> drivers/net/ethernet/intel/igc/igc_tsn.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/net/ethernet/intel/igc/igc_tsn.c
> b/drivers/net/ethernet/intel/igc/igc_tsn.c
> index 02dd9f0290a3..52de2bcbadbe 100644
> --- a/drivers/net/ethernet/intel/igc/igc_tsn.c
> +++ b/drivers/net/ethernet/intel/igc/igc_tsn.c
> @@ -34,6 +34,7 @@ static int igc_fpe_init_smd_frame(struct igc_ring
> *ring,
> return -ENOMEM;
> }
>
> + buffer->type = IGC_TX_BUFFER_TYPE_SKB;
> buffer->skb = skb;
> buffer->protocol = 0;
> buffer->bytecount = skb->len;
> --
> 2.53.0
Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>
^ permalink raw reply
* Re: [PATCH 1/9] bitfield: add FIELD_GET_SIGNED()
From: David Laight @ 2026-04-17 19:43 UTC (permalink / raw)
To: Yury Norov
Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
H. Peter Anvin, Andy Lutomirski, Peter Zijlstra, Jonathan Cameron,
David Lechner, Nuno Sá, Andy Shevchenko, Ping-Ke Shih,
Richard Cochran, Andrew Lunn, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Alexandre Belloni, Yury Norov,
Rasmus Villemoes, Hans de Goede, Linus Walleij, Sakari Ailus,
Salah Triki, Achim Gratz, Ben Collins, linux-kernel, linux-iio,
linux-wireless, netdev, linux-rtc
In-Reply-To: <20260417173621.368914-2-ynorov@nvidia.com>
On Fri, 17 Apr 2026 13:36:12 -0400
Yury Norov <ynorov@nvidia.com> wrote:
> The bitfields are designed in assumption that fields contain unsigned
> integer values, thus extracting the values from the field implies
> zero-extending.
>
> Some drivers need to sign-extend their fields, and currently do it like:
>
> dc_re += sign_extend32(FIELD_GET(0xfff000, tmp), 11);
> dc_im += sign_extend32(FIELD_GET(0xfff, tmp), 11);
>
> It's error-prone because it relies on user to provide the correct
> index of the most significant bit and proper 32 vs 64 function flavor.
>
> Thus, introduce a FIELD_GET_SIGNED() macro, which is the more
> convenient and compiles (on x86_64) to just a couple instructions:
> shl and sar.
>
> Signed-off-by: Yury Norov <ynorov@nvidia.com>
> ---
> include/linux/bitfield.h | 16 ++++++++++++++++
> 1 file changed, 16 insertions(+)
>
> diff --git a/include/linux/bitfield.h b/include/linux/bitfield.h
> index 54aeeef1f0ec..35ef63972810 100644
> --- a/include/linux/bitfield.h
> +++ b/include/linux/bitfield.h
> @@ -178,6 +178,22 @@
> __FIELD_GET(_mask, _reg, "FIELD_GET: "); \
> })
>
> +/**
> + * FIELD_GET_SIGNED() - extract a signed bitfield element
> + * @mask: shifted mask defining the field's length and position
> + * @reg: value of entire bitfield
> + *
> + * Returns the sign-extended field specified by @_mask from the
> + * bitfield passed in as @_reg by masking and shifting it down.
> + */
> +#define FIELD_GET_SIGNED(mask, reg) \
> + ({ \
> + __BF_FIELD_CHECK(mask, reg, 0U, "FIELD_GET_SIGNED: "); \
> + ((__signed_scalar_typeof(mask))((long long)(reg) << \
> + __builtin_clzll(mask) >> (__builtin_clzll(mask) + \
> + __builtin_ctzll(mask))));\
Have you looked at what that generates on a typical 32bit architecture?
It really a bad idea to use __signed_scalar_typeof() on anything that isn't
a simple variable.
The bloat from all this when 'mask' is an expansion of GENMASK() is horrid.
Indeed both signed_scalar_typeof() and unsigned_scalar_typeof() should
really not be used - there are generally much better ways.
In this case you can just write:
({
auto _mask = mask;
unsigned int __sl = __builtin_clzll(_mask);
unsigned int __sr = __sl + __builtin_ctzll(_mask);
__builtin_chose_expr(sizeof(_mask) <= 4,
(int)(reg) << __sl - 32 >> __sr - 32,
((long long)(reg) << __sl >> __sr)
})
and let the compiler do any more integer promotions (etc).
I'm also not convinced that the checks __BF_FIELD_CHECK() does
on 'reg' are in any sense worth the effort.
I have tried some simpler alternatives, eg:
!__builtin_constant_p(reg) && statically_true((reg & mask) == 0)
however that throws up some false positives due to some of weird ways
people have used FIELD_GET() where it is nothing like the simplest
(or most obvious) way to do things.
That might have been the code that split a 32bit value into bytes
in a printf with:
FIELD_GET(GENMASK(7, 0), val), FIELD_GET(GENMASK(15, 8), val),
FIELD_GET(GENMASK(23, 16), val), FIELD_GET(GENMASK(31, 24), val),
David
> + })
> +
> /**
> * FIELD_MODIFY() - modify a bitfield element
> * @_mask: shifted mask defining the field's length and position
^ permalink raw reply
* [PATCH iwl-net v1] igc: set tx buffer type for SMD frames
From: Kohei Enju @ 2026-04-17 19:31 UTC (permalink / raw)
To: intel-wired-lan, netdev
Cc: Tony Nguyen, Przemek Kitszel, Andrew Lunn, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Choong Yong Liang,
Vladimir Oltean, Vinicius Costa Gomes, Chwee-Lin Choong,
Simon Horman, Kohei Enju
Sashiko pointed out that igc_fpe_init_smd_frame() initializes
igc_tx_buffer fields for an SMD skb, but does not set the buffer type:
https://sashiko.dev/#/patchset/20260415025226.114115-1-kohei%40enjuk.jp
Since igc_tx_buffer entries are reused, a stale XDP or XSK type can
remain and make TX completion use the wrong cleanup path.
Set the buffer type to IGC_TX_BUFFER_TYPE_SKB.
Fixes: 5422570c0010 ("igc: add support for frame preemption verification")
Signed-off-by: Kohei Enju <kohei@enjuk.jp>
---
drivers/net/ethernet/intel/igc/igc_tsn.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/net/ethernet/intel/igc/igc_tsn.c b/drivers/net/ethernet/intel/igc/igc_tsn.c
index 02dd9f0290a3..52de2bcbadbe 100644
--- a/drivers/net/ethernet/intel/igc/igc_tsn.c
+++ b/drivers/net/ethernet/intel/igc/igc_tsn.c
@@ -34,6 +34,7 @@ static int igc_fpe_init_smd_frame(struct igc_ring *ring,
return -ENOMEM;
}
+ buffer->type = IGC_TX_BUFFER_TYPE_SKB;
buffer->skb = skb;
buffer->protocol = 0;
buffer->bytecount = skb->len;
--
2.53.0
^ permalink raw reply related
* Re: [PATCH net v4 4/4] selftests: bonding: add test for lacp_strict mode
From: Jakub Kicinski @ 2026-04-17 19:27 UTC (permalink / raw)
To: Louis Scalbert
Cc: netdev, stephen, andrew+netdev, jv, edumazet, pabeni, fbl, andy,
shemminger, maheshb
In-Reply-To: <20260417140505.3860237-5-louis.scalbert@6wind.com>
On Fri, 17 Apr 2026 16:05:05 +0200 Louis Scalbert wrote:
> +ip netns exec "${p_ns}" tc qdisc add dev eth0 root netem loss 100%
netem is not included the in the config for bonding tests, this fails
in the CI.
^ permalink raw reply
* Re: [PATCH 0/9] bitfield: add FIELD_GET_SIGNED()
From: Yury Norov @ 2026-04-17 19:21 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
H. Peter Anvin, Andy Lutomirski, Peter Zijlstra, Jonathan Cameron,
David Lechner, Nuno Sá, Andy Shevchenko, Ping-Ke Shih,
Richard Cochran, Andrew Lunn, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Alexandre Belloni, Yury Norov,
Rasmus Villemoes, Hans de Goede, Linus Walleij, Sakari Ailus,
Salah Triki, Achim Gratz, Ben Collins, linux-kernel, linux-iio,
linux-wireless, netdev, linux-rtc
In-Reply-To: <aeJ6hnZSbo2DrLpi@ashevche-desk.local>
On Fri, Apr 17, 2026 at 09:23:02PM +0300, Andy Shevchenko wrote:
> On Fri, Apr 17, 2026 at 01:36:11PM -0400, Yury Norov wrote:
> > The bitfields are designed in assumption that fields contain unsigned
> > integer values, thus extracting the values from the field implies
> > zero-extending.
> >
> > Some drivers need to sign-extend their fields, and currently do it like:
> >
> > dc_re += sign_extend32(FIELD_GET(0xfff000, tmp), 11);
> > dc_im += sign_extend32(FIELD_GET(0xfff, tmp), 11);
> >
> > It's error-prone because it relies on user to provide the correct
> > index of the most significant bit.
> >
> > This series adds a signed version of FIELD_GET(), which is the more
> > convenient and compiles (on x86_64) to just a couple instructions:
> > shl and sar.
> >
> > Patch #1 adds FIELD_GET_SIGNED(), and the rest of the series applies it
> > tree-wide.
>
> Here the example is missing.
This series is full of examples... I'll add one here if you prefer, if
it comes to v2.
> Nevertheless, I looked at the implementation a bit and wondering how would it
> work for 64-bit mask of say GENMASK_ULL(63, 60)? Wouldn't it give an overflow?
In that case, the '<< __builtin_clzll(mask)' part becomes a NOP, and
the compiler only emits a single sar:
long long foo(long long reg)
{
10: f3 0f 1e fa endbr64
return FIELD_GET_SIGNED(GENMASK_ULL(63, 60), reg);
14: 48 89 f8 mov %rdi,%rax
17: 48 c1 f8 3c sar $0x3c,%rax
}
Just tested it with a real kernel build with gcc-15.2, and it works as
intended.
^ permalink raw reply
* Re: [PATCH net] eth: fbnic: fix double-free of PCS on phylink creation failure
From: Bobby Eshleman @ 2026-04-17 18:44 UTC (permalink / raw)
To: Andrew Lunn
Cc: Alexander Duyck, Jakub Kicinski, kernel-team, Andrew Lunn,
David S. Miller, Eric Dumazet, Paolo Abeni, Russell King, netdev,
linux-kernel, Bobby Eshleman
In-Reply-To: <49138791-0726-4065-a772-56fea43070b7@lunn.ch>
On Fri, Apr 17, 2026 at 01:48:38AM +0200, Andrew Lunn wrote:
> > diff --git a/drivers/net/ethernet/meta/fbnic/fbnic_netdev.c b/drivers/net/ethernet/meta/fbnic/fbnic_netdev.c
> > index e3ca5fcfabef..2a6a73393732 100644
> > --- a/drivers/net/ethernet/meta/fbnic/fbnic_netdev.c
> > +++ b/drivers/net/ethernet/meta/fbnic/fbnic_netdev.c
> > @@ -818,7 +818,8 @@ struct net_device *fbnic_netdev_alloc(struct fbnic_dev *fbd)
> > netif_tx_stop_all_queues(netdev);
> >
> > if (fbnic_phylink_create(netdev)) {
> > - fbnic_netdev_free(fbd);
> > + free_netdev(netdev);
> > + fbd->netdev = NULL;
>
> Why set it to NULL? Setting pointers to NULL like this often suggests
> you are not confident the code is correct and you are being
> defensive. It is better to review the code and be sure it does the
> correct thing.
>
> > return NULL;
> > }
> >
> > diff --git a/drivers/net/ethernet/meta/fbnic/fbnic_phylink.c b/drivers/net/ethernet/meta/fbnic/fbnic_phylink.c
> > index 09c5225111be..50240e6c2ee9 100644
> > --- a/drivers/net/ethernet/meta/fbnic/fbnic_phylink.c
> > +++ b/drivers/net/ethernet/meta/fbnic/fbnic_phylink.c
> > @@ -237,6 +237,7 @@ int fbnic_phylink_create(struct net_device *netdev)
> > dev_err(netdev->dev.parent,
> > "Failed to create Phylink interface, err: %d\n", err);
> > xpcs_destroy_pcs(pcs);
> > + fbn->pcs = NULL;
>
> Why set it to NULL? If it failed, you are unwinding and about to fail
> the probe. Nothing should be using it.
>
> I would also say fbnic_phylink_destroy() is wrong or at least whoever
> wrote it is not confident in there own code. It should only be called
> if fbnic_phylink_create() was successful, so you know fbn->pcs is
> valid, so there is no need to test it. The same for fbn->phylink.
>
> Andrew
Fair points. I think it looks sound without resetting to NULL, but I'll
double check and remove if confident.
I'll look at the checks in _destroy() too.
Thanks,
Bobby
^ permalink raw reply
* [PATCH 4/4 nf] netfilter: xtables: fix L4 header parsing for non-first fragments
From: Fernando Fernandez Mancera @ 2026-04-17 18:34 UTC (permalink / raw)
To: netfilter-devel
Cc: netdev, coreteam, pablo, fw, phil, Fernando Fernandez Mancera
In-Reply-To: <20260417183433.4739-1-fmancera@suse.de>
The TPROXY target and osf match relies on L4 header to operate. For
fragmented packets, every fragment carries the transport protocol
identifier, but only the first fragment contains the L4 header.
As the 'raw' table can be configured to run at priority -450 (before
defragmentation at -400), the target/match can be reached before
reassembly. In this case, non-first fragments have their payload
incorrectly parsed as a TCP/UDP header.
Add a fragment check to ensure TPROXY/osf only evaluates unfragmented
packets or the first fragment in the stream.
Fixes: 902d6a4c2a4f ("netfilter: nf_defrag: Skip defrag if NOTRACK is set")
Signed-off-by: Fernando Fernandez Mancera <fmancera@suse.de>
---
net/netfilter/xt_TPROXY.c | 8 ++++++--
net/netfilter/xt_osf.c | 3 +++
2 files changed, 9 insertions(+), 2 deletions(-)
diff --git a/net/netfilter/xt_TPROXY.c b/net/netfilter/xt_TPROXY.c
index e4bea1d346cf..ac4b011ce48c 100644
--- a/net/netfilter/xt_TPROXY.c
+++ b/net/netfilter/xt_TPROXY.c
@@ -40,6 +40,9 @@ tproxy_tg4(struct net *net, struct sk_buff *skb, __be32 laddr, __be16 lport,
struct udphdr _hdr, *hp;
struct sock *sk;
+ if (ip_is_fragment(iph))
+ return NF_DROP;
+
hp = skb_header_pointer(skb, ip_hdrlen(skb), sizeof(_hdr), &_hdr);
if (hp == NULL)
return NF_DROP;
@@ -106,6 +109,7 @@ tproxy_tg6_v1(struct sk_buff *skb, const struct xt_action_param *par)
{
const struct ipv6hdr *iph = ipv6_hdr(skb);
const struct xt_tproxy_target_info_v1 *tgi = par->targinfo;
+ unsigned short fragoff = 0;
struct udphdr _hdr, *hp;
struct sock *sk;
const struct in6_addr *laddr;
@@ -113,8 +117,8 @@ tproxy_tg6_v1(struct sk_buff *skb, const struct xt_action_param *par)
int thoff = 0;
int tproto;
- tproto = ipv6_find_hdr(skb, &thoff, -1, NULL, NULL);
- if (tproto < 0)
+ tproto = ipv6_find_hdr(skb, &thoff, -1, &fragoff, NULL);
+ if (tproto < 0 || fragoff)
return NF_DROP;
hp = skb_header_pointer(skb, thoff, sizeof(_hdr), &_hdr);
diff --git a/net/netfilter/xt_osf.c b/net/netfilter/xt_osf.c
index dc9485854002..889dff4daff0 100644
--- a/net/netfilter/xt_osf.c
+++ b/net/netfilter/xt_osf.c
@@ -27,6 +27,9 @@
static bool
xt_osf_match_packet(const struct sk_buff *skb, struct xt_action_param *p)
{
+ if (ip_is_fragment(ip_hdr(skb)))
+ return false;
+
return nf_osf_match(skb, xt_family(p), xt_hooknum(p), xt_in(p),
xt_out(p), p->matchinfo, xt_net(p), nf_osf_fingers);
}
--
2.53.0
^ permalink raw reply related
* [PATCH 3/4 nf] netfilter: nft_osf: skip evaluation for non-first fragments
From: Fernando Fernandez Mancera @ 2026-04-17 18:34 UTC (permalink / raw)
To: netfilter-devel
Cc: netdev, coreteam, pablo, fw, phil, Fernando Fernandez Mancera
In-Reply-To: <20260417183433.4739-1-fmancera@suse.de>
The osf expression extracts TCP options to match them against
fingerprints. For fragmented packets, every fragment carries the
transport protocol used but only the first fragment contains the TCP
header.
As nftables is not evaluating chain priority, a osf expression could be
attached to a PREROUTING chain with a priority lower than -400. This
would bypass defragmentation. In addition, nft_osf should be able to
work in stateless environments, therefore it can be use in situation
when defragmentation is not being performed.
Add a check for pkt->fragoff to ensure osf only evaluates unfragmented
packets or the first fragment in the stream.
Fixes: b96af92d6eaf ("netfilter: nf_tables: implement Passive OS fingerprint module in nft_osf")
Signed-off-by: Fernando Fernandez Mancera <fmancera@suse.de>
---
net/netfilter/nft_osf.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/netfilter/nft_osf.c b/net/netfilter/nft_osf.c
index 1c0b493ef0a9..ceca87e405eb 100644
--- a/net/netfilter/nft_osf.c
+++ b/net/netfilter/nft_osf.c
@@ -28,7 +28,7 @@ static void nft_osf_eval(const struct nft_expr *expr, struct nft_regs *regs,
struct nf_osf_data data;
struct tcphdr _tcph;
- if (pkt->tprot != IPPROTO_TCP) {
+ if (pkt->tprot != IPPROTO_TCP || pkt->fragoff) {
regs->verdict.code = NFT_BREAK;
return;
}
--
2.53.0
^ permalink raw reply related
* [PATCH 2/4 nf] netfilter: nft_tproxy: skip evaluation for non-first fragments
From: Fernando Fernandez Mancera @ 2026-04-17 18:34 UTC (permalink / raw)
To: netfilter-devel
Cc: netdev, coreteam, pablo, fw, phil, Fernando Fernandez Mancera
In-Reply-To: <20260417183433.4739-1-fmancera@suse.de>
The tproxy expression relies on L4 ports to perform socke lookups. For
fragmented packets, every fragment carries the transport protocol used
but only the first fragment contains the L4 header.
As nftables is not evaluating chain priority, a tproxy expression could
be attached to a PREROUTING chain with a priority lower than -400. This
would bypass defragmentation.
Add a check for pkt->fragoff to ensure tproxy only evaluates
unfragmented packets or the first fragment in the stream.
Fixes: 4ed8eb6570a4 ("netfilter: nf_tables: Add native tproxy support")
Signed-off-by: Fernando Fernandez Mancera <fmancera@suse.de>
---
net/netfilter/nft_tproxy.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/net/netfilter/nft_tproxy.c b/net/netfilter/nft_tproxy.c
index 50481280abd2..8080cbd878cd 100644
--- a/net/netfilter/nft_tproxy.c
+++ b/net/netfilter/nft_tproxy.c
@@ -30,8 +30,8 @@ static void nft_tproxy_eval_v4(const struct nft_expr *expr,
__be16 tport = 0;
struct sock *sk;
- if (pkt->tprot != IPPROTO_TCP &&
- pkt->tprot != IPPROTO_UDP) {
+ if ((pkt->tprot != IPPROTO_TCP &&
+ pkt->tprot != IPPROTO_UDP) || pkt->fragoff) {
regs->verdict.code = NFT_BREAK;
return;
}
@@ -97,8 +97,8 @@ static void nft_tproxy_eval_v6(const struct nft_expr *expr,
memset(&taddr, 0, sizeof(taddr));
- if (pkt->tprot != IPPROTO_TCP &&
- pkt->tprot != IPPROTO_UDP) {
+ if ((pkt->tprot != IPPROTO_TCP &&
+ pkt->tprot != IPPROTO_UDP) || pkt->fragoff) {
regs->verdict.code = NFT_BREAK;
return;
}
--
2.53.0
^ permalink raw reply related
* [PATCH 1/4 nf] netfilter: nft_exthdr: skip SCTP chunk evaluation for non-first fragments
From: Fernando Fernandez Mancera @ 2026-04-17 18:34 UTC (permalink / raw)
To: netfilter-devel
Cc: netdev, coreteam, pablo, fw, phil, Fernando Fernandez Mancera
The SCTP chunk matching logic in nft_exthdr relies on SCTP common header
being present at the transport header offset. For fragmented packets at
IP level, only the first fragment would match this condition.
The nft_exthdr could be used in a PREROUTING chain with a priority lower
than -400. This would bypass defragmentation. In addition, it can be use
in stateless environments so it should work on a environment where
defragmentation is not being performed at all.
Add a check for pkt->fragoff to ensure exthdr SCTP only evaluates
unfragmented packets or the first fragment in the stream.
Fixes: 133dc203d77d ("netfilter: nft_exthdr: Support SCTP chunks")
Signed-off-by: Fernando Fernandez Mancera <fmancera@suse.de>
---
net/netfilter/nft_exthdr.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/netfilter/nft_exthdr.c b/net/netfilter/nft_exthdr.c
index 7eedf4e3ae9c..8eb708bb8cff 100644
--- a/net/netfilter/nft_exthdr.c
+++ b/net/netfilter/nft_exthdr.c
@@ -376,7 +376,7 @@ static void nft_exthdr_sctp_eval(const struct nft_expr *expr,
const struct sctp_chunkhdr *sch;
struct sctp_chunkhdr _sch;
- if (pkt->tprot != IPPROTO_SCTP)
+ if (pkt->tprot != IPPROTO_SCTP || pkt->fragoff)
goto err;
do {
--
2.53.0
^ permalink raw reply related
* Re: [PATCH 0/9] bitfield: add FIELD_GET_SIGNED()
From: Andy Shevchenko @ 2026-04-17 18:23 UTC (permalink / raw)
To: Yury Norov
Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
H. Peter Anvin, Andy Lutomirski, Peter Zijlstra, Jonathan Cameron,
David Lechner, Nuno Sá, Andy Shevchenko, Ping-Ke Shih,
Richard Cochran, Andrew Lunn, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Alexandre Belloni, Yury Norov,
Rasmus Villemoes, Hans de Goede, Linus Walleij, Sakari Ailus,
Salah Triki, Achim Gratz, Ben Collins, linux-kernel, linux-iio,
linux-wireless, netdev, linux-rtc
In-Reply-To: <20260417173621.368914-1-ynorov@nvidia.com>
On Fri, Apr 17, 2026 at 01:36:11PM -0400, Yury Norov wrote:
> The bitfields are designed in assumption that fields contain unsigned
> integer values, thus extracting the values from the field implies
> zero-extending.
>
> Some drivers need to sign-extend their fields, and currently do it like:
>
> dc_re += sign_extend32(FIELD_GET(0xfff000, tmp), 11);
> dc_im += sign_extend32(FIELD_GET(0xfff, tmp), 11);
>
> It's error-prone because it relies on user to provide the correct
> index of the most significant bit.
>
> This series adds a signed version of FIELD_GET(), which is the more
> convenient and compiles (on x86_64) to just a couple instructions:
> shl and sar.
>
> Patch #1 adds FIELD_GET_SIGNED(), and the rest of the series applies it
> tree-wide.
Here the example is missing.
Nevertheless, I looked at the implementation a bit and wondering how would it
work for 64-bit mask of say GENMASK_ULL(63, 60)? Wouldn't it give an overflow?
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: [PATCH] net: hsr: avoid synchronize_net() in hsr_del_port() under rtnl_mutex
From: Eric Dumazet @ 2026-04-17 18:18 UTC (permalink / raw)
To: Shardul Bankar
Cc: davem, kuba, pabeni, horms, netdev, liuhangbin, lukma, acsjakub,
kees, xiaoliang.yang_1, fmancera, linux-kernel, janak,
kalpan.jani, Shardul Bankar, syzbot+f2fbf7478a35a94c8b7c
In-Reply-To: <20260417175322.2701465-1-shardul.b@mpiricsoftware.com>
On Fri, Apr 17, 2026 at 10:53 AM Shardul Bankar <shardulsb08@gmail.com> wrote:
>
> hsr_del_port() calls netdev_rx_handler_unregister(), which calls
> synchronize_net() while rtnl_mutex is held. During netns teardown,
> cleanup_net() walks pernet_list and reaches default_device_exit_batch(),
> which takes rtnl_mutex and iterates devices calling
> rtnl_link_ops->dellink() for each. For HSR this resolves to
> hsr_dellink() -> hsr_del_ports() -> hsr_del_port(), so the
> synchronize_net() call runs under rtnl_mutex. Under contention this
> stalls other rtnl_mutex waiters long enough to trip the hung-task
> detector:
>
> kworker cleanup_net -> default_device_exit_batch -> hsr_del_port
> -> netdev_rx_handler_unregister -> synchronize_rcu_expedited
> [slow, holds rtnl_mutex]
synchronize_rcu_expedited() should be quite fast...
> syz-executor -> ksys_unshare -> setup_net -> ip_tunnel_init_net
> -> rtnl_lock [blocked on rtnl_mutex]
>
> Open-code netdev_rx_handler_unregister() in hsr_del_port() without
> the synchronize_net() step. This is safe because hsr_del_port()
> already defers the port free via kfree_rcu(port, rcu), so the
> rx_handler_data memory remains valid until an RCU grace period
> elapses naturally. hsr_handle_frame() additionally re-validates
> rx_handler via hsr_port_get_rcu() before dereferencing
> rx_handler_data, so a reader that observes rx_handler_data == NULL
> in the brief window between the two clears takes the NULL path and
> returns RX_HANDLER_PASS without touching the port.
>
> Reported-by: syzbot+f2fbf7478a35a94c8b7c@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?id=cb64c22a492202ca929e18262fdb8cb89e635c70
Signature looks like bug fixed recently in wireguard.
commit 60a25ef8dacb3566b1a8c4de00572a498e2a3bf9
Author: Shardul Bankar <shardul.b@mpiricsoftware.com>
Date: Tue Apr 14 17:39:44 2026 +0200
wireguard: device: use exit_rtnl callback instead of manual
rtnl_lock in pre_exit
> Signed-off-by: Shardul Bankar <shardul.b@mpiricsoftware.com>
> ---
> Testing status:
>
> The hang was originally observed on 7.0-rc6 (commit 7ca6d1cfec80) with
> the syzkaller reproducer and a KASAN+LOCKDEP config. Subsequent
> reproduction attempts with the same reproducer on current mainline did
> not retrigger this specific signature, so I could not verify the fix
> against a live reproducer. The patch is submitted on the basis of
> code analysis.
>
> net/hsr/hsr_slave.c | 13 ++++++++++++-
> 1 file changed, 12 insertions(+), 1 deletion(-)
>
> diff --git a/net/hsr/hsr_slave.c b/net/hsr/hsr_slave.c
> index d9af9e65f72f..5e92f23fa1a5 100644
> --- a/net/hsr/hsr_slave.c
> +++ b/net/hsr/hsr_slave.c
> @@ -239,7 +239,18 @@ void hsr_del_port(struct hsr_port *port)
> if (port != master) {
> netdev_update_features(master->dev);
> dev_set_mtu(master->dev, hsr_get_max_mtu(hsr));
> - netdev_rx_handler_unregister(port->dev);
> + /* Open-code netdev_rx_handler_unregister() without the
> + * synchronize_net() step: holding rtnl_mutex across the
> + * grace-period wait stalls other rtnl_mutex waiters long
> + * enough to trip the hung-task detector under load.
> + * Skipping synchronize_net() is safe because the port is
> + * freed via kfree_rcu() below (so rx_handler_data memory
> + * outlives any in-flight reader), and hsr_handle_frame()
> + * re-validates rx_handler via hsr_port_get_rcu() before
> + * dereferencing rx_handler_data.
> + */
> + RCU_INIT_POINTER(port->dev->rx_handler, NULL);
> + RCU_INIT_POINTER(port->dev->rx_handler_data, NULL);
> if (!port->hsr->fwd_offloaded)
> dev_set_promiscuity(port->dev, -1);
> netdev_upper_dev_unlink(port->dev, master->dev);
You are saying that all netdev_rx_handler_unregister() uses are
potentially a problem.
This can not be true.
^ permalink raw reply
* Re: [PATCH 1/9] bitfield: add FIELD_GET_SIGNED()
From: Andy Shevchenko @ 2026-04-17 18:12 UTC (permalink / raw)
To: Yury Norov
Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
H. Peter Anvin, Andy Lutomirski, Peter Zijlstra, Jonathan Cameron,
David Lechner, Nuno Sá, Andy Shevchenko, Ping-Ke Shih,
Richard Cochran, Andrew Lunn, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Alexandre Belloni, Yury Norov,
Rasmus Villemoes, Hans de Goede, Linus Walleij, Sakari Ailus,
Salah Triki, Achim Gratz, Ben Collins, linux-kernel, linux-iio,
linux-wireless, netdev, linux-rtc
In-Reply-To: <20260417173621.368914-2-ynorov@nvidia.com>
On Fri, Apr 17, 2026 at 01:36:12PM -0400, Yury Norov wrote:
> The bitfields are designed in assumption that fields contain unsigned
> integer values, thus extracting the values from the field implies
> zero-extending.
>
> Some drivers need to sign-extend their fields, and currently do it like:
>
> dc_re += sign_extend32(FIELD_GET(0xfff000, tmp), 11);
> dc_im += sign_extend32(FIELD_GET(0xfff, tmp), 11);
>
> It's error-prone because it relies on user to provide the correct
> index of the most significant bit and proper 32 vs 64 function flavor.
>
> Thus, introduce a FIELD_GET_SIGNED() macro, which is the more
> convenient and compiles (on x86_64) to just a couple instructions:
> shl and sar.
...
> +#define FIELD_GET_SIGNED(mask, reg) \
> + ({ \
> + __BF_FIELD_CHECK(mask, reg, 0U, "FIELD_GET_SIGNED: "); \
> + ((__signed_scalar_typeof(mask))((long long)(reg) << \
> + __builtin_clzll(mask) >> (__builtin_clzll(mask) + \
> + __builtin_ctzll(mask))));\
I would re-indent these lines as
((__signed_scalar_typeof(mask))
((long long)(reg) << __builtin_clzll(mask) >> \
(__builtin_clzll(mask) + __builtin_ctzll(mask)))); \
> + })
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: [PATCH net] net: dsa: mt7530: fix .get_stats64 sleeping in atomic context
From: Daniel Golle @ 2026-04-17 18:03 UTC (permalink / raw)
To: Breno Leitao
Cc: Chester A. Unal, Andrew Lunn, Vladimir Oltean, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Matthias Brugger,
AngeloGioacchino Del Regno, Russell King, Christian Marangi,
netdev, linux-kernel, linux-arm-kernel, linux-mediatek,
Frank Wunderlich, John Crispin
In-Reply-To: <aeJjcoHyezEgUG_Q@gmail.com>
On Fri, Apr 17, 2026 at 10:46:29AM -0700, Breno Leitao wrote:
> On Fri, Apr 17, 2026 at 04:55:57AM +0100, Daniel Golle wrote:
> > @@ -3404,6 +3449,9 @@ EXPORT_SYMBOL_GPL(mt7530_probe_common);
> > void
> > mt7530_remove_common(struct mt7530_priv *priv)
> > {
> > + if (priv->bus)
> > + cancel_delayed_work_sync(&priv->stats_work);
> > +
>
> Shouldn't you cancel the work later, after dsa_unregister_switch()?
>
> I am wondering if the following race cannot happen:
>
> mt7530_remove_common() someone reading /proc/net/dev
> cancel_delayed_work_sync()
> /* returns: work neither pending
> nor executing - true at this
> instant */
> mt7530_get_stats64()
> mod_delayed_work(...)
> /* work is queued again */
> dsa_unregister_switch()
> return
Thanks you for pointing this out.
cancel_delayed_work_sync() should be moved after dsa_unregister_switch()
to avoid this kind of race.
^ permalink raw reply
* [PATCH] net: hsr: avoid synchronize_net() in hsr_del_port() under rtnl_mutex
From: Shardul Bankar @ 2026-04-17 17:53 UTC (permalink / raw)
To: davem, edumazet, kuba, pabeni, horms, netdev
Cc: liuhangbin, lukma, acsjakub, kees, xiaoliang.yang_1, fmancera,
linux-kernel, janak, kalpan.jani, shardulsb08, Shardul Bankar,
syzbot+f2fbf7478a35a94c8b7c
hsr_del_port() calls netdev_rx_handler_unregister(), which calls
synchronize_net() while rtnl_mutex is held. During netns teardown,
cleanup_net() walks pernet_list and reaches default_device_exit_batch(),
which takes rtnl_mutex and iterates devices calling
rtnl_link_ops->dellink() for each. For HSR this resolves to
hsr_dellink() -> hsr_del_ports() -> hsr_del_port(), so the
synchronize_net() call runs under rtnl_mutex. Under contention this
stalls other rtnl_mutex waiters long enough to trip the hung-task
detector:
kworker cleanup_net -> default_device_exit_batch -> hsr_del_port
-> netdev_rx_handler_unregister -> synchronize_rcu_expedited
[slow, holds rtnl_mutex]
syz-executor -> ksys_unshare -> setup_net -> ip_tunnel_init_net
-> rtnl_lock [blocked on rtnl_mutex]
Open-code netdev_rx_handler_unregister() in hsr_del_port() without
the synchronize_net() step. This is safe because hsr_del_port()
already defers the port free via kfree_rcu(port, rcu), so the
rx_handler_data memory remains valid until an RCU grace period
elapses naturally. hsr_handle_frame() additionally re-validates
rx_handler via hsr_port_get_rcu() before dereferencing
rx_handler_data, so a reader that observes rx_handler_data == NULL
in the brief window between the two clears takes the NULL path and
returns RX_HANDLER_PASS without touching the port.
Reported-by: syzbot+f2fbf7478a35a94c8b7c@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?id=cb64c22a492202ca929e18262fdb8cb89e635c70
Signed-off-by: Shardul Bankar <shardul.b@mpiricsoftware.com>
---
Testing status:
The hang was originally observed on 7.0-rc6 (commit 7ca6d1cfec80) with
the syzkaller reproducer and a KASAN+LOCKDEP config. Subsequent
reproduction attempts with the same reproducer on current mainline did
not retrigger this specific signature, so I could not verify the fix
against a live reproducer. The patch is submitted on the basis of
code analysis.
net/hsr/hsr_slave.c | 13 ++++++++++++-
1 file changed, 12 insertions(+), 1 deletion(-)
diff --git a/net/hsr/hsr_slave.c b/net/hsr/hsr_slave.c
index d9af9e65f72f..5e92f23fa1a5 100644
--- a/net/hsr/hsr_slave.c
+++ b/net/hsr/hsr_slave.c
@@ -239,7 +239,18 @@ void hsr_del_port(struct hsr_port *port)
if (port != master) {
netdev_update_features(master->dev);
dev_set_mtu(master->dev, hsr_get_max_mtu(hsr));
- netdev_rx_handler_unregister(port->dev);
+ /* Open-code netdev_rx_handler_unregister() without the
+ * synchronize_net() step: holding rtnl_mutex across the
+ * grace-period wait stalls other rtnl_mutex waiters long
+ * enough to trip the hung-task detector under load.
+ * Skipping synchronize_net() is safe because the port is
+ * freed via kfree_rcu() below (so rx_handler_data memory
+ * outlives any in-flight reader), and hsr_handle_frame()
+ * re-validates rx_handler via hsr_port_get_rcu() before
+ * dereferencing rx_handler_data.
+ */
+ RCU_INIT_POINTER(port->dev->rx_handler, NULL);
+ RCU_INIT_POINTER(port->dev->rx_handler_data, NULL);
if (!port->hsr->fwd_offloaded)
dev_set_promiscuity(port->dev, -1);
netdev_upper_dev_unlink(port->dev, master->dev);
--
2.34.1
^ permalink raw reply related
* [PATCH v2 3/3] mISDN: cache stable device names outside the kobject
From: Shuvam Pandey @ 2026-04-17 17:49 UTC (permalink / raw)
To: netdev; +Cc: linux-kernel, Simon Horman
In-Reply-To: <cover.1776446840.git.shuvampandey1@gmail.com>
mISDN prints and exports device names from several paths that are not tied
to the kobject rename internals. device_rename() replaces the kobject name
string, so reading it directly leaves mISDN dependent on that storage
remaining stable while those paths run.
Keep an mISDN-owned copy of the device name, update it on registration and
successful rename, and use that cached name from the socket, sysfs, stack,
and debug paths.
Assisted-by: Codex:GPT-5.3-Codex
Signed-off-by: Shuvam Pandey <shuvampandey1@gmail.com>
---
drivers/isdn/mISDN/core.c | 18 +++++++++++++-----
drivers/isdn/mISDN/layer1.c | 2 +-
drivers/isdn/mISDN/socket.c | 12 ++++++++----
drivers/isdn/mISDN/stack.c | 36 ++++++++++++++++++------------------
drivers/isdn/mISDN/tei.c | 2 +-
include/linux/mISDNif.h | 1 +
6 files changed, 42 insertions(+), 29 deletions(-)
diff --git a/drivers/isdn/mISDN/core.c b/drivers/isdn/mISDN/core.c
index 4e2be8f03119b..d89c9e54cb5fa 100644
--- a/drivers/isdn/mISDN/core.c
+++ b/drivers/isdn/mISDN/core.c
@@ -89,8 +89,15 @@ static DEVICE_ATTR_RO(protocol);
static ssize_t name_show(struct device *dev,
struct device_attribute *attr, char *buf)
{
- strcpy(buf, dev_name(dev));
- return strlen(buf);
+ struct mISDNdevice *mdev = dev_to_mISDN(dev);
+ ssize_t len;
+
+ if (!mdev)
+ return -ENODEV;
+ device_lock(dev);
+ len = sysfs_emit(buf, "%s", mdev->name);
+ device_unlock(dev);
+ return len;
}
static DEVICE_ATTR_RO(name);
@@ -227,9 +234,10 @@ mISDN_register_device(struct mISDNdevice *dev,
dev_set_name(&dev->dev, "%s", name);
else
dev_set_name(&dev->dev, "mISDN%d", dev->id);
+ strscpy(dev->name, dev_name(&dev->dev), sizeof(dev->name));
if (debug & DEBUG_CORE)
printk(KERN_DEBUG "mISDN_register %s %d\n",
- dev_name(&dev->dev), dev->id);
+ dev->name, dev->id);
dev->dev.class = &mISDN_class;
err = create_stack(dev);
@@ -258,7 +266,7 @@ void
mISDN_unregister_device(struct mISDNdevice *dev) {
if (debug & DEBUG_CORE)
printk(KERN_DEBUG "mISDN_unregister %s %d\n",
- dev_name(&dev->dev), dev->id);
+ dev->name, dev->id);
/* sysfs_remove_link(&dev->dev.kobj, "device"); */
/*
* Remove the device from sysfs before taking dev->mutex so bind-side
@@ -358,7 +366,7 @@ const char *mISDNDevName4ch(struct mISDNchannel *ch)
return msg_no_stack;
if (!ch->st->dev)
return msg_no_stackdev;
- return dev_name(&ch->st->dev->dev);
+ return ch->st->dev->name;
};
EXPORT_SYMBOL(mISDNDevName4ch);
diff --git a/drivers/isdn/mISDN/layer1.c b/drivers/isdn/mISDN/layer1.c
index 3fbc170acf9ab..c5a2e9119e868 100644
--- a/drivers/isdn/mISDN/layer1.c
+++ b/drivers/isdn/mISDN/layer1.c
@@ -100,7 +100,7 @@ l1m_debug(struct FsmInst *fi, char *fmt, ...)
vaf.fmt = fmt;
vaf.va = &va;
- printk(KERN_DEBUG "%s: %pV\n", dev_name(&l1->dch->dev.dev), &vaf);
+ printk(KERN_DEBUG "%s: %pV\n", l1->dch->dev.name, &vaf);
va_end(va);
}
diff --git a/drivers/isdn/mISDN/socket.c b/drivers/isdn/mISDN/socket.c
index 42cda5b8bbe16..bce71ae5eb7d4 100644
--- a/drivers/isdn/mISDN/socket.c
+++ b/drivers/isdn/mISDN/socket.c
@@ -499,7 +499,7 @@ data_sock_ioctl(struct socket *sock, unsigned int cmd, unsigned long arg)
memcpy(di.channelmap, dev->channelmap,
sizeof(di.channelmap));
di.nrbchan = dev->nrbchan;
- strscpy(di.name, dev_name(&dev->dev), sizeof(di.name));
+ strscpy(di.name, dev->name, sizeof(di.name));
device_unlock(&dev->dev);
if (copy_to_user((void __user *)arg, &di, sizeof(di)))
err = -EFAULT;
@@ -826,7 +826,7 @@ base_sock_ioctl(struct socket *sock, unsigned int cmd, unsigned long arg)
memcpy(di.channelmap, dev->channelmap,
sizeof(di.channelmap));
di.nrbchan = dev->nrbchan;
- strscpy(di.name, dev_name(&dev->dev), sizeof(di.name));
+ strscpy(di.name, dev->name, sizeof(di.name));
device_unlock(&dev->dev);
if (copy_to_user((void __user *)arg, &di, sizeof(di)))
err = -EFAULT;
@@ -846,10 +846,14 @@ base_sock_ioctl(struct socket *sock, unsigned int cmd, unsigned long arg)
dev = get_mdevice(dn.id);
if (dev) {
device_lock(&dev->dev);
- if (!device_is_registered(&dev->dev))
+ if (!device_is_registered(&dev->dev)) {
err = -ENODEV;
- else
+ } else {
err = device_rename(&dev->dev, dn.name);
+ if (!err)
+ strscpy(dev->name, dev_name(&dev->dev),
+ sizeof(dev->name));
+ }
device_unlock(&dev->dev);
put_device(&dev->dev);
} else {
diff --git a/drivers/isdn/mISDN/stack.c b/drivers/isdn/mISDN/stack.c
index 4e96684af0aac..9b39ddb4a0944 100644
--- a/drivers/isdn/mISDN/stack.c
+++ b/drivers/isdn/mISDN/stack.c
@@ -165,7 +165,7 @@ send_msg_to_layer(struct mISDNstack *st, struct sk_buff *skb)
else
printk(KERN_WARNING
"%s: dev(%s) prim(%x) id(%x) no channel\n",
- __func__, dev_name(&st->dev->dev), hh->prim,
+ __func__, st->dev->name, hh->prim,
hh->id);
} else if (lm == 0x8) {
WARN_ON(lm == 0x8);
@@ -175,12 +175,12 @@ send_msg_to_layer(struct mISDNstack *st, struct sk_buff *skb)
else
printk(KERN_WARNING
"%s: dev(%s) prim(%x) id(%x) no channel\n",
- __func__, dev_name(&st->dev->dev), hh->prim,
+ __func__, st->dev->name, hh->prim,
hh->id);
} else {
/* broadcast not handled yet */
printk(KERN_WARNING "%s: dev(%s) prim %x not delivered\n",
- __func__, dev_name(&st->dev->dev), hh->prim);
+ __func__, st->dev->name, hh->prim);
}
return -ESRCH;
}
@@ -202,7 +202,7 @@ mISDNStackd(void *data)
sigfillset(¤t->blocked);
if (*debug & DEBUG_MSG_THREAD)
printk(KERN_DEBUG "mISDNStackd %s started\n",
- dev_name(&st->dev->dev));
+ st->dev->name);
if (st->notify != NULL) {
complete(st->notify);
@@ -238,7 +238,7 @@ mISDNStackd(void *data)
printk(KERN_DEBUG
"%s: %s prim(%x) id(%x) "
"send call(%d)\n",
- __func__, dev_name(&st->dev->dev),
+ __func__, st->dev->name,
mISDN_HEAD_PRIM(skb),
mISDN_HEAD_ID(skb), err);
dev_kfree_skb(skb);
@@ -281,7 +281,7 @@ mISDNStackd(void *data)
mISDN_STACK_ACTION_MASK));
if (*debug & DEBUG_MSG_THREAD)
printk(KERN_DEBUG "%s: %s wake status %08lx\n",
- __func__, dev_name(&st->dev->dev), st->status);
+ __func__, st->dev->name, st->status);
test_and_set_bit(mISDN_STACK_ACTIVE, &st->status);
test_and_clear_bit(mISDN_STACK_WAKEUP, &st->status);
@@ -296,17 +296,17 @@ mISDNStackd(void *data)
#ifdef MISDN_MSG_STATS
printk(KERN_DEBUG "mISDNStackd daemon for %s proceed %d "
"msg %d sleep %d stopped\n",
- dev_name(&st->dev->dev), st->msg_cnt, st->sleep_cnt,
+ st->dev->name, st->msg_cnt, st->sleep_cnt,
st->stopped_cnt);
task_cputime(st->thread, &utime, &stime);
printk(KERN_DEBUG
"mISDNStackd daemon for %s utime(%llu) stime(%llu)\n",
- dev_name(&st->dev->dev), utime, stime);
+ st->dev->name, utime, stime);
printk(KERN_DEBUG
"mISDNStackd daemon for %s nvcsw(%ld) nivcsw(%ld)\n",
- dev_name(&st->dev->dev), st->thread->nvcsw, st->thread->nivcsw);
+ st->dev->name, st->thread->nvcsw, st->thread->nivcsw);
printk(KERN_DEBUG "mISDNStackd daemon for %s killed now\n",
- dev_name(&st->dev->dev));
+ st->dev->name);
#endif
test_and_set_bit(mISDN_STACK_KILLED, &st->status);
test_and_clear_bit(mISDN_STACK_RUNNING, &st->status);
@@ -397,15 +397,15 @@ create_stack(struct mISDNdevice *dev)
newst->own.recv = mISDN_queue_message;
if (*debug & DEBUG_CORE_FUNC)
printk(KERN_DEBUG "%s: st(%s)\n", __func__,
- dev_name(&newst->dev->dev));
+ newst->dev->name);
newst->notify = &done;
newst->thread = kthread_run(mISDNStackd, (void *)newst, "mISDN_%s",
- dev_name(&newst->dev->dev));
+ newst->dev->name);
if (IS_ERR(newst->thread)) {
err = PTR_ERR(newst->thread);
printk(KERN_ERR
"mISDN:cannot create kernel thread for %s (%d)\n",
- dev_name(&newst->dev->dev), err);
+ newst->dev->name, err);
delete_teimanager(dev->teimgr);
kfree(newst);
} else
@@ -424,7 +424,7 @@ connect_layer1(struct mISDNdevice *dev, struct mISDNchannel *ch,
if (*debug & DEBUG_CORE_FUNC)
printk(KERN_DEBUG "%s: %s proto(%x) adr(%d %d %d %d)\n",
- __func__, dev_name(&dev->dev), protocol, adr->dev,
+ __func__, dev->name, protocol, adr->dev,
adr->channel, adr->sapi, adr->tei);
switch (protocol) {
case ISDN_P_NT_S0:
@@ -461,7 +461,7 @@ connect_Bstack(struct mISDNdevice *dev, struct mISDNchannel *ch,
if (*debug & DEBUG_CORE_FUNC)
printk(KERN_DEBUG "%s: %s proto(%x) adr(%d %d %d %d)\n",
- __func__, dev_name(&dev->dev), protocol,
+ __func__, dev->name, protocol,
adr->dev, adr->channel, adr->sapi,
adr->tei);
ch->st = dev->D.st;
@@ -517,7 +517,7 @@ create_l2entity(struct mISDNdevice *dev, struct mISDNchannel *ch,
if (*debug & DEBUG_CORE_FUNC)
printk(KERN_DEBUG "%s: %s proto(%x) adr(%d %d %d %d)\n",
- __func__, dev_name(&dev->dev), protocol,
+ __func__, dev->name, protocol,
adr->dev, adr->channel, adr->sapi,
adr->tei);
rq.protocol = ISDN_P_TE_S0;
@@ -570,7 +570,7 @@ delete_channel(struct mISDNchannel *ch)
}
if (*debug & DEBUG_CORE_FUNC)
printk(KERN_DEBUG "%s: st(%s) protocol(%x)\n", __func__,
- dev_name(&ch->st->dev->dev), ch->protocol);
+ ch->st->dev->name, ch->protocol);
if (ch->protocol >= ISDN_P_B_START) {
if (ch->peer) {
ch->peer->ctrl(ch->peer, CLOSE_CHANNEL, NULL);
@@ -623,7 +623,7 @@ delete_stack(struct mISDNdevice *dev)
if (*debug & DEBUG_CORE_FUNC)
printk(KERN_DEBUG "%s: st(%s)\n", __func__,
- dev_name(&st->dev->dev));
+ st->dev->name);
if (dev->teimgr)
delete_teimanager(dev->teimgr);
if (st->thread) {
diff --git a/drivers/isdn/mISDN/tei.c b/drivers/isdn/mISDN/tei.c
index 2bad3083be901..876c1194920ef 100644
--- a/drivers/isdn/mISDN/tei.c
+++ b/drivers/isdn/mISDN/tei.c
@@ -990,7 +990,7 @@ create_teimgr(struct manager *mgr, struct channel_req *crq)
if (*debug & DEBUG_L2_TEI)
printk(KERN_DEBUG "%s: %s proto(%x) adr(%d %d %d %d)\n",
- __func__, dev_name(&mgr->ch.st->dev->dev),
+ __func__, mgr->ch.st->dev->name,
crq->protocol, crq->adr.dev, crq->adr.channel,
crq->adr.sapi, crq->adr.tei);
if (crq->adr.tei > GROUP_TEI)
diff --git a/include/linux/mISDNif.h b/include/linux/mISDNif.h
index ce26d70c1ebfb..79f6d8f218b13 100644
--- a/include/linux/mISDNif.h
+++ b/include/linux/mISDNif.h
@@ -497,6 +497,7 @@ struct mISDNdevice {
u_int Bprotocols;
u_int nrbchan;
u_char channelmap[MISDN_CHMAP_SIZE];
+ char name[MISDN_MAX_IDLEN];
struct list_head bchannels;
struct mISDNchannel *teimgr;
struct completion released;
--
2.50.1 (Apple Git-155)
^ permalink raw reply related
* [PATCH v2 2/3] mISDN: socket: drop temporary references from get_mdevice()
From: Shuvam Pandey @ 2026-04-17 17:49 UTC (permalink / raw)
To: netdev; +Cc: linux-kernel, Simon Horman
In-Reply-To: <cover.1776446840.git.shuvampandey1@gmail.com>
IMGETDEVINFO and IMSETDEVNAME only use get_mdevice() for a temporary
lookup, but neither path drops the reference returned by
class_find_device().
Drop the temporary reference once the ioctl finishes. Serialize the name
read and rename paths with device_lock() while doing so, and reject
lookups that raced with unregister after device_del().
Fixes: b36b654a7e82 ("mISDN: Create /sys/class/mISDN")
Cc: stable@vger.kernel.org
Assisted-by: Codex:GPT-5.3-Codex
Signed-off-by: Shuvam Pandey <shuvampandey1@gmail.com>
---
drivers/isdn/mISDN/socket.c | 31 ++++++++++++++++++++++++++++---
1 file changed, 28 insertions(+), 3 deletions(-)
diff --git a/drivers/isdn/mISDN/socket.c b/drivers/isdn/mISDN/socket.c
index bf3ad0a2a42bc..42cda5b8bbe16 100644
--- a/drivers/isdn/mISDN/socket.c
+++ b/drivers/isdn/mISDN/socket.c
@@ -484,6 +484,13 @@ data_sock_ioctl(struct socket *sock, unsigned int cmd, unsigned long arg)
if (dev) {
struct mISDN_devinfo di;
+ device_lock(&dev->dev);
+ if (!device_is_registered(&dev->dev)) {
+ device_unlock(&dev->dev);
+ put_device(&dev->dev);
+ err = -ENODEV;
+ break;
+ }
memset(&di, 0, sizeof(di));
di.id = dev->id;
di.Dprotocols = dev->Dprotocols;
@@ -493,8 +500,10 @@ data_sock_ioctl(struct socket *sock, unsigned int cmd, unsigned long arg)
sizeof(di.channelmap));
di.nrbchan = dev->nrbchan;
strscpy(di.name, dev_name(&dev->dev), sizeof(di.name));
+ device_unlock(&dev->dev);
if (copy_to_user((void __user *)arg, &di, sizeof(di)))
err = -EFAULT;
+ put_device(&dev->dev);
} else
err = -ENODEV;
break;
@@ -802,6 +811,13 @@ base_sock_ioctl(struct socket *sock, unsigned int cmd, unsigned long arg)
if (dev) {
struct mISDN_devinfo di;
+ device_lock(&dev->dev);
+ if (!device_is_registered(&dev->dev)) {
+ device_unlock(&dev->dev);
+ put_device(&dev->dev);
+ err = -ENODEV;
+ break;
+ }
memset(&di, 0, sizeof(di));
di.id = dev->id;
di.Dprotocols = dev->Dprotocols;
@@ -811,8 +827,10 @@ base_sock_ioctl(struct socket *sock, unsigned int cmd, unsigned long arg)
sizeof(di.channelmap));
di.nrbchan = dev->nrbchan;
strscpy(di.name, dev_name(&dev->dev), sizeof(di.name));
+ device_unlock(&dev->dev);
if (copy_to_user((void __user *)arg, &di, sizeof(di)))
err = -EFAULT;
+ put_device(&dev->dev);
} else
err = -ENODEV;
break;
@@ -826,10 +844,17 @@ base_sock_ioctl(struct socket *sock, unsigned int cmd, unsigned long arg)
}
dn.name[sizeof(dn.name) - 1] = '\0';
dev = get_mdevice(dn.id);
- if (dev)
- err = device_rename(&dev->dev, dn.name);
- else
+ if (dev) {
+ device_lock(&dev->dev);
+ if (!device_is_registered(&dev->dev))
+ err = -ENODEV;
+ else
+ err = device_rename(&dev->dev, dn.name);
+ device_unlock(&dev->dev);
+ put_device(&dev->dev);
+ } else {
err = -ENODEV;
+ }
}
break;
default:
--
2.50.1 (Apple Git-155)
^ permalink raw reply related
* [PATCH v2 1/3] mISDN: serialize socket teardown against device unregister
From: Shuvam Pandey @ 2026-04-17 17:49 UTC (permalink / raw)
To: netdev; +Cc: linux-kernel, Simon Horman
In-Reply-To: <cover.1776446840.git.shuvampandey1@gmail.com>
get_mdevice() returns a referenced struct device, and mISDN sockets keep
that reference in _pms(sk)->dev after bind.
That means mISDN_unregister_device() cannot tear the device down as if no
socket can still reach it. Several teardown paths can otherwise run after
delete_stack() has freed the stack, or after the driver has freed the
embedding object once mISDN_unregister_device() returns.
Close sockets that still point at the device before delete_stack() runs,
wait for the final device release before returning from
mISDN_unregister_device(), and serialize bind against unregister with the
device lock so a new socket cannot attach after unregister has started.
While tightening the close path, reset channel state after CLOSE_CHANNEL so
later socket release does not try to tear the same B-channel down twice,
and make recvmsg/getname tolerate sockets whose device pointer was cleared
by unregister.
Fixes: b36b654a7e82 ("mISDN: Create /sys/class/mISDN")
Cc: stable@vger.kernel.org
Assisted-by: Codex:GPT-5.3-Codex
Signed-off-by: Shuvam Pandey <shuvampandey1@gmail.com>
---
drivers/isdn/mISDN/core.c | 19 ++-
drivers/isdn/mISDN/core.h | 1 +
drivers/isdn/mISDN/socket.c | 224 +++++++++++++++++++++++++++++++-----
include/linux/mISDNif.h | 1 +
4 files changed, 216 insertions(+), 29 deletions(-)
diff --git a/drivers/isdn/mISDN/core.c b/drivers/isdn/mISDN/core.c
index 8ec2d4d4f1352..4e2be8f03119b 100644
--- a/drivers/isdn/mISDN/core.c
+++ b/drivers/isdn/mISDN/core.c
@@ -26,7 +26,9 @@ static DEFINE_RWLOCK(bp_lock);
static void mISDN_dev_release(struct device *dev)
{
- /* nothing to do: the device is part of its parent's data structure */
+ struct mISDNdevice *mdev = container_of(dev, struct mISDNdevice, dev);
+
+ complete(&mdev->released);
}
static ssize_t id_show(struct device *dev,
@@ -219,6 +221,7 @@ mISDN_register_device(struct mISDNdevice *dev,
return err;
dev->id = err;
+ init_completion(&dev->released);
device_initialize(&dev->dev);
if (name && name[0])
dev_set_name(&dev->dev, "%s", name);
@@ -257,12 +260,24 @@ mISDN_unregister_device(struct mISDNdevice *dev) {
printk(KERN_DEBUG "mISDN_unregister %s %d\n",
dev_name(&dev->dev), dev->id);
/* sysfs_remove_link(&dev->dev.kobj, "device"); */
+ /*
+ * Remove the device from sysfs before taking dev->mutex so bind-side
+ * get_mdevice() users will fail the later device_is_registered()
+ * recheck after they acquire device_lock().
+ */
device_del(&dev->dev);
dev_set_drvdata(&dev->dev, NULL);
-
+ device_lock(&dev->dev);
+ misdn_sock_release_device(dev);
test_and_clear_bit(dev->id, (u_long *)&device_ids);
delete_stack(dev);
+ device_unlock(&dev->dev);
put_device(&dev->dev);
+ /*
+ * Drivers free the enclosing object after unregister returns, so wait
+ * until the last outstanding device reference is dropped.
+ */
+ wait_for_completion(&dev->released);
}
EXPORT_SYMBOL(mISDN_unregister_device);
diff --git a/drivers/isdn/mISDN/core.h b/drivers/isdn/mISDN/core.h
index 5617c06de8e4d..2cd89293bc211 100644
--- a/drivers/isdn/mISDN/core.h
+++ b/drivers/isdn/mISDN/core.h
@@ -41,6 +41,7 @@ extern int connect_layer1(struct mISDNdevice *, struct mISDNchannel *,
u_int, struct sockaddr_mISDN *);
extern int create_l2entity(struct mISDNdevice *, struct mISDNchannel *,
u_int, struct sockaddr_mISDN *);
+void misdn_sock_release_device(struct mISDNdevice *dev);
extern int create_stack(struct mISDNdevice *);
extern int create_teimanager(struct mISDNdevice *);
diff --git a/drivers/isdn/mISDN/socket.c b/drivers/isdn/mISDN/socket.c
index 77b900db1cac2..bf3ad0a2a42bc 100644
--- a/drivers/isdn/mISDN/socket.c
+++ b/drivers/isdn/mISDN/socket.c
@@ -7,6 +7,7 @@
*/
#include <linux/mISDNif.h>
+#include <linux/device.h>
#include <linux/slab.h>
#include <linux/export.h>
#include "core.h"
@@ -57,6 +58,97 @@ static void mISDN_sock_unlink(struct mISDN_sock_list *l, struct sock *sk)
write_unlock_bh(&l->lock);
}
+/*
+ * Socket teardown driven by unregister takes device_lock(dev) before
+ * lock_sock(sk). Bind paths take the same order so unregister can close a
+ * socket without racing a new bind onto the same device.
+ */
+static struct sock *
+misdn_sock_get(struct mISDN_sock_list *l, struct mISDNdevice *dev)
+{
+ struct sock *sk;
+
+ read_lock_bh(&l->lock);
+ sk_for_each(sk, &l->head) {
+ if (READ_ONCE(_pms(sk)->dev) != dev)
+ continue;
+ sock_hold(sk);
+ read_unlock_bh(&l->lock);
+ return sk;
+ }
+ read_unlock_bh(&l->lock);
+ return NULL;
+}
+
+static void data_sock_reset_channel(struct sock *sk)
+{
+ _pms(sk)->ch.protocol = ISDN_P_NONE;
+ _pms(sk)->ch.nr = 0;
+ _pms(sk)->ch.addr = 0;
+ _pms(sk)->ch.st = NULL;
+ _pms(sk)->ch.peer = NULL;
+ _pms(sk)->ch.recv = NULL;
+}
+
+static void data_sock_close(struct sock *sk)
+{
+ bool active = _pms(sk)->ch.protocol != ISDN_P_NONE;
+
+ sk->sk_state = MISDN_CLOSED;
+
+ if (active)
+ delete_channel(&_pms(sk)->ch);
+
+ data_sock_reset_channel(sk);
+
+ if (_pms(sk)->dev) {
+ put_device(&_pms(sk)->dev->dev);
+ _pms(sk)->dev = NULL;
+ }
+}
+
+static void base_sock_close(struct sock *sk)
+{
+ sk->sk_state = MISDN_CLOSED;
+
+ if (_pms(sk)->dev) {
+ put_device(&_pms(sk)->dev->dev);
+ _pms(sk)->dev = NULL;
+ }
+}
+
+void
+misdn_sock_release_device(struct mISDNdevice *dev)
+{
+ struct sock *sk;
+
+ if (dev->D.st) {
+ while ((sk = misdn_sock_get(&dev->D.st->l1sock, dev))) {
+ lock_sock(sk);
+ if (_pms(sk)->dev == dev)
+ data_sock_close(sk);
+ release_sock(sk);
+ sock_put(sk);
+ }
+ }
+
+ while ((sk = misdn_sock_get(&data_sockets, dev))) {
+ lock_sock(sk);
+ if (_pms(sk)->dev == dev)
+ data_sock_close(sk);
+ release_sock(sk);
+ sock_put(sk);
+ }
+
+ while ((sk = misdn_sock_get(&base_sockets, dev))) {
+ lock_sock(sk);
+ if (_pms(sk)->dev == dev)
+ base_sock_close(sk);
+ release_sock(sk);
+ sock_put(sk);
+ }
+}
+
static int
mISDN_send(struct mISDNchannel *ch, struct sk_buff *skb)
{
@@ -86,6 +178,14 @@ mISDN_ctrl(struct mISDNchannel *ch, u_int cmd, void *arg)
switch (cmd) {
case CLOSE_CHANNEL:
msk->sk.sk_state = MISDN_CLOSED;
+ if (msk->ch.protocol >= ISDN_P_B_START) {
+ msk->ch.protocol = ISDN_P_NONE;
+ msk->ch.nr = 0;
+ msk->ch.addr = 0;
+ msk->ch.st = NULL;
+ msk->ch.peer = NULL;
+ msk->ch.recv = NULL;
+ }
break;
}
return 0;
@@ -127,18 +227,30 @@ mISDN_sock_recvmsg(struct socket *sock, struct msghdr *msg, size_t len,
if (msg->msg_name) {
DECLARE_SOCKADDR(struct sockaddr_mISDN *, maddr, msg->msg_name);
+ int dev_id, ch_nr, ch_addr;
+
+ lock_sock(sk);
+ if (!_pms(sk)->dev) {
+ release_sock(sk);
+ skb_free_datagram(sk, skb);
+ return -EBADFD;
+ }
+ dev_id = _pms(sk)->dev->id;
+ ch_nr = _pms(sk)->ch.nr;
+ ch_addr = _pms(sk)->ch.addr;
+ release_sock(sk);
maddr->family = AF_ISDN;
- maddr->dev = _pms(sk)->dev->id;
+ maddr->dev = dev_id;
if ((sk->sk_protocol == ISDN_P_LAPD_TE) ||
(sk->sk_protocol == ISDN_P_LAPD_NT)) {
maddr->channel = (mISDN_HEAD_ID(skb) >> 16) & 0xff;
maddr->tei = (mISDN_HEAD_ID(skb) >> 8) & 0xff;
maddr->sapi = mISDN_HEAD_ID(skb) & 0xff;
} else {
- maddr->channel = _pms(sk)->ch.nr;
- maddr->sapi = _pms(sk)->ch.addr & 0xFF;
- maddr->tei = (_pms(sk)->ch.addr >> 8) & 0xFF;
+ maddr->channel = ch_nr;
+ maddr->sapi = ch_addr & 0xFF;
+ maddr->tei = (ch_addr >> 8) & 0xFF;
}
msg->msg_namelen = sizeof(*maddr);
}
@@ -241,16 +353,14 @@ data_sock_release(struct socket *sock)
printk(KERN_DEBUG "%s(%p) sk=%p\n", __func__, sock, sk);
if (!sk)
return 0;
+
+ lock_sock(sk);
+
switch (sk->sk_protocol) {
case ISDN_P_TE_S0:
case ISDN_P_NT_S0:
case ISDN_P_TE_E1:
case ISDN_P_NT_E1:
- if (sk->sk_state == MISDN_BOUND)
- delete_channel(&_pms(sk)->ch);
- else
- mISDN_sock_unlink(&data_sockets, sk);
- break;
case ISDN_P_LAPD_TE:
case ISDN_P_LAPD_NT:
case ISDN_P_B_RAW:
@@ -259,13 +369,11 @@ data_sock_release(struct socket *sock)
case ISDN_P_B_L2DTMF:
case ISDN_P_B_L2DSP:
case ISDN_P_B_L2DSPHDLC:
- delete_channel(&_pms(sk)->ch);
+ data_sock_close(sk);
mISDN_sock_unlink(&data_sockets, sk);
break;
}
- lock_sock(sk);
-
sock_orphan(sk);
skb_queue_purge(&sk->sk_receive_queue);
@@ -466,6 +574,7 @@ data_sock_bind(struct socket *sock, struct sockaddr_unsized *addr, int addr_len)
{
struct sockaddr_mISDN *maddr = (struct sockaddr_mISDN *) addr;
struct sock *sk = sock->sk;
+ struct mISDNdevice *dev, *lockdev;
struct sock *csk;
int err = 0;
@@ -477,13 +586,35 @@ data_sock_bind(struct socket *sock, struct sockaddr_unsized *addr, int addr_len)
return -EINVAL;
lock_sock(sk);
+ if (sk->sk_state == MISDN_CLOSED) {
+ release_sock(sk);
+ return -EBADFD;
+ }
+ if (_pms(sk)->dev) {
+ release_sock(sk);
+ return -EALREADY;
+ }
+ release_sock(sk);
+
+ dev = get_mdevice(maddr->dev);
+ if (!dev)
+ return -ENODEV;
+
+ lockdev = dev;
+ device_lock(&dev->dev);
+ lock_sock(sk);
+
+ if (sk->sk_state == MISDN_CLOSED) {
+ err = -EBADFD;
+ goto done;
+ }
if (_pms(sk)->dev) {
err = -EALREADY;
goto done;
}
- _pms(sk)->dev = get_mdevice(maddr->dev);
- if (!_pms(sk)->dev) {
+
+ if (!device_is_registered(&dev->dev)) {
err = -ENODEV;
goto done;
}
@@ -493,7 +624,7 @@ data_sock_bind(struct socket *sock, struct sockaddr_unsized *addr, int addr_len)
sk_for_each(csk, &data_sockets.head) {
if (sk == csk)
continue;
- if (_pms(csk)->dev != _pms(sk)->dev)
+ if (READ_ONCE(_pms(csk)->dev) != dev)
continue;
if (csk->sk_protocol >= ISDN_P_B_START)
continue;
@@ -516,15 +647,15 @@ data_sock_bind(struct socket *sock, struct sockaddr_unsized *addr, int addr_len)
case ISDN_P_TE_E1:
case ISDN_P_NT_E1:
mISDN_sock_unlink(&data_sockets, sk);
- err = connect_layer1(_pms(sk)->dev, &_pms(sk)->ch,
- sk->sk_protocol, maddr);
+ err = connect_layer1(dev, &_pms(sk)->ch, sk->sk_protocol,
+ maddr);
if (err)
mISDN_sock_link(&data_sockets, sk);
break;
case ISDN_P_LAPD_TE:
case ISDN_P_LAPD_NT:
- err = create_l2entity(_pms(sk)->dev, &_pms(sk)->ch,
- sk->sk_protocol, maddr);
+ err = create_l2entity(dev, &_pms(sk)->ch, sk->sk_protocol,
+ maddr);
break;
case ISDN_P_B_RAW:
case ISDN_P_B_HDLC:
@@ -532,19 +663,26 @@ data_sock_bind(struct socket *sock, struct sockaddr_unsized *addr, int addr_len)
case ISDN_P_B_L2DTMF:
case ISDN_P_B_L2DSP:
case ISDN_P_B_L2DSPHDLC:
- err = connect_Bstack(_pms(sk)->dev, &_pms(sk)->ch,
- sk->sk_protocol, maddr);
+ err = connect_Bstack(dev, &_pms(sk)->ch, sk->sk_protocol,
+ maddr);
break;
default:
err = -EPROTONOSUPPORT;
}
- if (err)
+ if (err) {
+ data_sock_reset_channel(sk);
goto done;
+ }
+ _pms(sk)->dev = dev;
+ dev = NULL;
sk->sk_state = MISDN_BOUND;
_pms(sk)->ch.protocol = sk->sk_protocol;
done:
release_sock(sk);
+ device_unlock(&lockdev->dev);
+ if (dev)
+ put_device(&dev->dev);
return err;
}
@@ -555,10 +693,11 @@ data_sock_getname(struct socket *sock, struct sockaddr *addr,
struct sockaddr_mISDN *maddr = (struct sockaddr_mISDN *) addr;
struct sock *sk = sock->sk;
- if (!_pms(sk)->dev)
- return -EBADFD;
-
lock_sock(sk);
+ if (!_pms(sk)->dev) {
+ release_sock(sk);
+ return -EBADFD;
+ }
maddr->family = AF_ISDN;
maddr->dev = _pms(sk)->dev->id;
@@ -623,6 +762,10 @@ base_sock_release(struct socket *sock)
if (!sk)
return 0;
+ lock_sock(sk);
+ base_sock_close(sk);
+ release_sock(sk);
+
mISDN_sock_unlink(&base_sockets, sk);
sock_orphan(sk);
sock_put(sk);
@@ -700,6 +843,7 @@ base_sock_bind(struct socket *sock, struct sockaddr_unsized *addr, int addr_len)
{
struct sockaddr_mISDN *maddr = (struct sockaddr_mISDN *) addr;
struct sock *sk = sock->sk;
+ struct mISDNdevice *dev, *lockdev;
int err = 0;
if (addr_len < sizeof(struct sockaddr_mISDN))
@@ -709,21 +853,47 @@ base_sock_bind(struct socket *sock, struct sockaddr_unsized *addr, int addr_len)
return -EINVAL;
lock_sock(sk);
+ if (sk->sk_state == MISDN_CLOSED) {
+ release_sock(sk);
+ return -EBADFD;
+ }
+ if (_pms(sk)->dev) {
+ release_sock(sk);
+ return -EALREADY;
+ }
+ release_sock(sk);
+
+ dev = get_mdevice(maddr->dev);
+ if (!dev)
+ return -ENODEV;
+
+ lockdev = dev;
+ device_lock(&dev->dev);
+ lock_sock(sk);
+
+ if (sk->sk_state == MISDN_CLOSED) {
+ err = -EBADFD;
+ goto done;
+ }
if (_pms(sk)->dev) {
err = -EALREADY;
goto done;
}
- _pms(sk)->dev = get_mdevice(maddr->dev);
- if (!_pms(sk)->dev) {
+ if (!device_is_registered(&dev->dev)) {
err = -ENODEV;
goto done;
}
+ _pms(sk)->dev = dev;
+ dev = NULL;
sk->sk_state = MISDN_BOUND;
done:
release_sock(sk);
+ device_unlock(&lockdev->dev);
+ if (dev)
+ put_device(&dev->dev);
return err;
}
diff --git a/include/linux/mISDNif.h b/include/linux/mISDNif.h
index 7aab4a7697369..ce26d70c1ebfb 100644
--- a/include/linux/mISDNif.h
+++ b/include/linux/mISDNif.h
@@ -499,6 +499,7 @@ struct mISDNdevice {
u_char channelmap[MISDN_CHMAP_SIZE];
struct list_head bchannels;
struct mISDNchannel *teimgr;
+ struct completion released;
struct device dev;
};
--
2.50.1 (Apple Git-155)
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox