* [PATCH net v4] raw: annotate lockless match fields in raw_v4_match()
From: Runyu Xiao @ 2026-07-16 14:29 UTC (permalink / raw)
To: netdev
Cc: kuniyu, edumazet, davem, dsahern, kuba, pabeni, horms,
linux-kernel, jianhao.xu, runyu.xiao
raw_v4_match() is a lockless match helper under sk_for_each_rcu(). It
still reads inet->inet_daddr, inet->inet_rcv_saddr and
sk->sk_bound_dev_if with plain loads while bind, connect and
bind-to-device paths can update the same match fields concurrently.
Annotate only those mutable match fields in raw_v4_match(), and do so
at the point of use instead of hoisting the bound-device read before
the earlier short-circuit tests.
Also annotate the raw bind writer and the shared IPv4 datagram connect
writer used by raw sockets, so the address fields updated on bind and
connect match explicit WRITE_ONCE() updates.
This version intentionally leaves the shared disconnect-side IPv4
writers to follow-up cleanup and limits the writer changes here to the
raw bind path and the datagram connect path directly exercised by raw
sockets.
Fixes: 0daf07e52709 ("raw: convert raw sockets to RCU")
Signed-off-by: Runyu Xiao <runyu.xiao@seu.edu.cn>
---
v4:
- drop the __udp_disconnect() annotation
- drop the inet_reset_saddr() annotation
- keep writer changes limited to raw_bind() and __ip4_datagram_connect()
- previous version: https://lore.kernel.org/r/20260611035318.1091442-1-runyu.xiao@seu.edu.cn
v3:
- drop the inet_num annotation for raw sockets
- cover inet_daddr as well
- avoid hoisting sk_bound_dev_if into a temporary variable
- annotate the matching IPv4 writer paths
v2:
- note that inet_num and sk_bound_dev_if already have WRITE_ONCE() writers
- add WRITE_ONCE() in raw_bind() for inet_rcv_saddr
- previous version: https://lore.kernel.org/r/20260601073937.1137673-1-runyu.xiao@seu.edu.cn
net/ipv4/datagram.c | 4 ++--
net/ipv4/raw.c | 23 ++++++++++++++++-------
2 files changed, 18 insertions(+), 9 deletions(-)
diff --git a/net/ipv4/datagram.c b/net/ipv4/datagram.c
index 1614593b6d72..7d25519a6cdd 100644
--- a/net/ipv4/datagram.c
+++ b/net/ipv4/datagram.c
@@ -63,12 +63,12 @@ int __ip4_datagram_connect(struct sock *sk, struct sockaddr_unsized *uaddr, int
}
/* Update addresses before rehashing */
- inet->inet_daddr = fl4->daddr;
+ WRITE_ONCE(inet->inet_daddr, fl4->daddr);
inet->inet_dport = usin->sin_port;
if (!inet->inet_saddr)
inet->inet_saddr = fl4->saddr;
if (!inet->inet_rcv_saddr) {
- inet->inet_rcv_saddr = fl4->saddr;
+ WRITE_ONCE(inet->inet_rcv_saddr, fl4->saddr);
if (sk->sk_prot->rehash)
sk->sk_prot->rehash(sk);
}
diff --git a/net/ipv4/raw.c b/net/ipv4/raw.c
index 5aaf9c62c8e1..dfb294b5c794 100644
--- a/net/ipv4/raw.c
+++ b/net/ipv4/raw.c
@@ -120,13 +120,21 @@ bool raw_v4_match(struct net *net, const struct sock *sk, unsigned short num,
__be32 raddr, __be32 laddr, int dif, int sdif)
{
const struct inet_sock *inet = inet_sk(sk);
+ __be32 daddr, rcv_saddr;
- if (net_eq(sock_net(sk), net) && inet->inet_num == num &&
- !(inet->inet_daddr && inet->inet_daddr != raddr) &&
- !(inet->inet_rcv_saddr && inet->inet_rcv_saddr != laddr) &&
- raw_sk_bound_dev_eq(net, sk->sk_bound_dev_if, dif, sdif))
- return true;
- return false;
+ if (!net_eq(sock_net(sk), net) || inet->inet_num != num)
+ return false;
+
+ daddr = READ_ONCE(inet->inet_daddr);
+ if (daddr && daddr != raddr)
+ return false;
+
+ rcv_saddr = READ_ONCE(inet->inet_rcv_saddr);
+ if (rcv_saddr && rcv_saddr != laddr)
+ return false;
+
+ return raw_sk_bound_dev_eq(net, READ_ONCE(sk->sk_bound_dev_if),
+ dif, sdif);
}
EXPORT_SYMBOL_GPL(raw_v4_match);
@@ -724,7 +732,8 @@ static int raw_bind(struct sock *sk, struct sockaddr_unsized *uaddr,
chk_addr_ret))
goto out;
- inet->inet_rcv_saddr = inet->inet_saddr = addr->sin_addr.s_addr;
+ inet->inet_saddr = addr->sin_addr.s_addr;
+ WRITE_ONCE(inet->inet_rcv_saddr, addr->sin_addr.s_addr);
if (chk_addr_ret == RTN_MULTICAST || chk_addr_ret == RTN_BROADCAST)
inet->inet_saddr = 0; /* Use device */
sk_dst_reset(sk);
--
2.34.1
^ permalink raw reply related
* Re: [PATCH net-next v2 1/4] net: hsr: add PRP interlink (RedBox) datapath and duplicate discard
From: Xin Xie @ 2026-07-16 14:38 UTC (permalink / raw)
To: Fernando Fernandez Mancera
Cc: netdev, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Simon Horman, Shuah Khan, Sebastian Andrzej Siewior,
Felix Maurer, Luka Gejak, linux-kselftest, linux-kernel
In-Reply-To: <178420323080.16062.7813915035618214520.b4-review@b4>
On Thu, 16 Jul 2026 14:00:30 +0200, Fernando Fernandez Mancera wrote:
> Since we already have rx (enum hsr_port_type) here, can we pass this
> directly to prp_is_lan_dup() function instead of passing frame?
>
> Of course these calls will need to pass rx too.
Yes, that is cleaner. Will change it in v4.
Thanks for the review!
Xin
^ permalink raw reply
* Re: [PATCH net-next v2 2/4] net: hsr: emit RedBox-MAC TLV in PRP RedBox supervision frames
From: Xin Xie @ 2026-07-16 14:38 UTC (permalink / raw)
To: Fernando Fernandez Mancera
Cc: netdev, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Simon Horman, Shuah Khan, Sebastian Andrzej Siewior,
Felix Maurer, Luka Gejak, linux-kselftest, linux-kernel
In-Reply-To: <178420323080.16062.11665494757602926366.b4-review@b4>
On Thu, 16 Jul 2026 13:59:54 +0200, Fernando Fernandez Mancera wrote:
> The indentation is wrong here, could you keep at the same level it was
> before?
Sure, will restore the original indentation in v4.
Thanks,
Xin
^ permalink raw reply
* Re: [PATCH net-next v9 1/9] net: Use helpers to get/set UDP len tree-wide
From: Simon Horman @ 2026-07-16 14:46 UTC (permalink / raw)
To: alice.kernel
Cc: Simon Horman, daniel, davem, edumazet, kuba, pabeni, lucien.xin,
willemdebruijn.kernel, willemb, dsahern, razor, shuah, stfomichev,
andrew+netdev, fw, netdev, alice
In-Reply-To: <20260710134242.216538-2-alice.kernel@fastmail.im>
This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
Full review at: https://sashiko.dev
---
--- Patch [1]: [PATCH net-next v9 1/9] net: Use helpers to get/set UDP len tree-wide ---
commit 113d28557bcedccc492f330fafdaaf1377eb2e79
Author: Alice Mikityanska <alice@isovalent.com>
net: Use helpers to get/set UDP len tree-wide
This commit introduces helpers for setting and getting the UDP header length
tree-wide. It prepares the codebase to support BIG TCP for UDP tunnels by
explicitly handling cases where UDP length might be 0 for packets larger
than 65535 bytes.
> diff --git a/net/6lowpan/nhc_udp.c b/net/6lowpan/nhc_udp.c
> index 0a506c77283dd..ed4227e6db744 100644
> --- a/net/6lowpan/nhc_udp.c
> +++ b/net/6lowpan/nhc_udp.c
> @@ -88,16 +88,16 @@ static int udp_uncompress(struct sk_buff *skb, size_t needed)
> switch (lowpan_dev(skb->dev)->lltype) {
> case LOWPAN_LLTYPE_IEEE802154:
> if (lowpan_802154_cb(skb)->d_size)
> - uh.len = htons(lowpan_802154_cb(skb)->d_size -
> - sizeof(struct ipv6hdr));
> + udp_set_len_short(&uh, lowpan_802154_cb(skb)->d_size -
> + sizeof(struct ipv6hdr));
[Severity: High]
If a crafted packet specifies a datagram size smaller than
sizeof(struct ipv6hdr), won't this subtraction underflow?
The resulting massive unsigned value passed to udp_set_len_short() would
then trigger the DEBUG_NET_WARN_ON_ONCE, potentially causing a panic if
panic_on_warn is set.
> [ ... ]
> diff --git a/net/ipv4/udp_offload.c b/net/ipv4/udp_offload.c
> index 29651b1a0bc70..493e2b9e16fbc 100644
> --- a/net/ipv4/udp_offload.c
> +++ b/net/ipv4/udp_offload.c
> [ ... ]
> @@ -915,12 +914,12 @@ static int udp_gro_complete_segment(struct sk_buff *skb)
> int udp_gro_complete(struct sk_buff *skb, int nhoff,
> udp_lookup_t lookup)
> {
> - __be16 newlen = htons(skb->len - nhoff);
> struct udphdr *uh = (struct udphdr *)(skb->data + nhoff);
> + unsigned int newlen = skb->len - nhoff;
> struct sock *sk;
> int err;
>
> - uh->len = newlen;
> + udp_set_len_short(uh, newlen);
[Severity: High]
If gro_max_size is configured larger than 65535 (which is supported and used
for BIG TCP), can't UDP GRO aggregate packets such that newlen > 65535?
This would instantly trigger the DEBUG_NET_WARN_ON_ONCE inside
udp_set_len_short() for legitimate large packets.
> [ ... ]
> diff --git a/net/ipv4/udp_tunnel_core.c b/net/ipv4/udp_tunnel_core.c
> index 9ab3728f9630b..0fccb38f074d2 100644
> --- a/net/ipv4/udp_tunnel_core.c
> +++ b/net/ipv4/udp_tunnel_core.c
> @@ -178,7 +178,7 @@ void udp_tunnel_xmit_skb(struct rtable *rt, struct sock *sk, struct sk_buff *skb
>
> uh->dest = dst_port;
> uh->source = src_port;
> - uh->len = htons(skb->len);
> + udp_set_len_short(uh, skb->len);
[Severity: High]
When transmitting a TCP BIG TCP packet (which can be > 64K) over a UDP
tunnel like VXLAN, skb->len will exceed 65535.
Does using udp_set_len_short() here incorrectly trigger the warning for
valid large GSO payloads?
> [ ... ]
> diff --git a/net/ipv6/ip6_udp_tunnel.c b/net/ipv6/ip6_udp_tunnel.c
> index 9adb5775487fa..dcff7fb16ff64 100644
> --- a/net/ipv6/ip6_udp_tunnel.c
> +++ b/net/ipv6/ip6_udp_tunnel.c
> @@ -93,7 +93,7 @@ void udp_tunnel6_xmit_skb(struct dst_entry *dst, struct sock *sk,
> uh->dest = dst_port;
> uh->source = src_port;
>
> - uh->len = htons(skb->len);
> + udp_set_len_short(uh, skb->len);
[Severity: High]
This appears to have the same issue as IPv4 tunnels. If a large GSO payload
is sent through an IPv6 UDP tunnel, won't skb->len exceed 65535 and trigger
the warning?
^ permalink raw reply
* Re: [PATCH net-next v9 2/9] net: Enable BIG TCP with partial GSO
From: Simon Horman @ 2026-07-16 14:47 UTC (permalink / raw)
To: alice.kernel
Cc: Simon Horman, daniel, davem, edumazet, kuba, pabeni, lucien.xin,
willemdebruijn.kernel, willemb, dsahern, razor, shuah, stfomichev,
andrew+netdev, fw, netdev, alice
In-Reply-To: <20260710134242.216538-3-alice.kernel@fastmail.im>
This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
Full review at: https://sashiko.dev
---
--- Patch [2]: [PATCH net-next v9 2/9] net: Enable BIG TCP with partial GSO ---
commit b8199f77774400021642eb53bd60ec1a062a54ef
Author: Alice Mikityanska <alice@isovalent.com>
net: Enable BIG TCP with partial GSO
This commit removes the 65534-byte clamp on segment length in skb_segment
for partial GSO. It stores the gso_by_frags flag early to allow BIG TCP
packets to use larger lengths without accidental resegmentation.
> diff --git a/net/core/skbuff.c b/net/core/skbuff.c
> index 18dabb4e9cfa0..6ae4c2b205f2e 100644
> --- a/net/core/skbuff.c
> +++ b/net/core/skbuff.c
[ ... ]
> @@ -4852,9 +4853,8 @@ struct sk_buff *skb_segment(struct sk_buff *head_skb,
> /* GSO partial only requires that we trim off any excess that
> * doesn't fit into an MSS sized block, so take care of that
> * now.
> - * Cap len to not accidentally hit GSO_BY_FRAGS.
> */
> - partial_segs = min(len, GSO_BY_FRAGS - 1) / mss;
> + partial_segs = len / mss;
[Severity: High]
When removing the clamp on partial GSO segments, mss is scaled up without any
upper bound. For BIG TCP packets, could this construct segments that exceed
MAX_SKB_FRAGS?
If a 512KB packet is passed to skb_segment() for partial GSO, mss is scaled
up to the full packet length. When attempting to build this single large
segment by flattening the frag_list into a new frags array, it appears it
would hit the fragment limit and abort:
net/core/skbuff.c:skb_segment() {
...
if (unlikely(skb_shinfo(nskb)->nr_frags >=
MAX_SKB_FRAGS)) {
net_warn_ratelimited(
"skb_segment: too many frags: %u %u\n",
pos, mss);
err = -EINVAL;
goto err;
}
...
}
Does this deterministically drop BIG TCP traffic over tunnels that require
partial GSO?
[Severity: Medium]
Can the partial_segs calculation exceed 65535 for malicious inputs and cause
a 16-bit truncation?
If a crafted packet with a very small gso_size (for example, mss=1 via an
untrusted packet socket) and a large payload is segmented, len / mss could
produce a value greater than 65535.
When this 32-bit value is later assigned to the 16-bit field in skb_segment():
net/core/skbuff.c:skb_segment() {
...
skb_shinfo(iter)->gso_segs = partial_segs;
...
}
Could this silent truncation corrupt hardware engine programming or Byte Queue
Limits?
> if (partial_segs > 1)
> mss *= partial_segs;
> else
[ ... ]
^ permalink raw reply
* Re: [PATCH net-next v9 4/9] udp: Support gro_ipv4_max_size > 65536
From: Simon Horman @ 2026-07-16 14:47 UTC (permalink / raw)
To: alice.kernel
Cc: Simon Horman, daniel, davem, edumazet, kuba, pabeni, lucien.xin,
willemdebruijn.kernel, willemb, dsahern, razor, shuah, stfomichev,
andrew+netdev, fw, netdev, alice
In-Reply-To: <20260710134242.216538-5-alice.kernel@fastmail.im>
This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
Full review at: https://sashiko.dev
---
--- Patch [4]: [PATCH net-next v9 4/9] udp: Support gro_ipv4_max_size > 65536 ---
commit 6dfda0f76318408b2e2b33603d561940b926529c
Author: Alice Mikityanska <alice@isovalent.com>
Subject: udp: Support gro_ipv4_max_size > 65536
This commit fixes UDP GRO aggregation for packets larger than 65536 bytes
by setting the UDP length field to 0 during GRO and restoring the true length
from skb->len in udp_rcv().
> diff --git a/net/ipv4/udp_offload.c b/net/ipv4/udp_offload.c
> index 493e2b9e16fbc..4f9a3922937cc 100644
> --- a/net/ipv4/udp_offload.c
> +++ b/net/ipv4/udp_offload.c
> @@ -919,7 +919,7 @@ int udp_gro_complete(struct sk_buff *skb, int nhoff,
> struct sock *sk;
> int err;
>
> - udp_set_len_short(uh, newlen);
> + udp_set_len(uh, newlen);
[Severity: High]
Does setting the UDP length to 0 here (for packets > 65535 bytes) break
software segmentation?
In net/ipv4/udp_offload.c:__udp_gso_segment(), the code adjusts the
pseudo-header checksum by subtracting uh->len:
check = csum16_add(csum16_sub(uh->check, uh->len), htons(newlen));
If uh->len is 0 for BIG TCP packets, this subtraction will fail to remove
the true 32-bit original length from the checksum.
Similarly, in net/ipv4/udp_offload.c:__skb_udp_tunnel_segment(), partial
GSO relies on uh->len to negate the old length:
if (skb_shinfo(skb)->gso_type & SKB_GSO_PARTIAL)
partial = (__force __wsum)uh->len;
else
partial = (__force __wsum)htonl(skb->len);
partial = csum_sub(csum_unfold(uh->check), partial);
Will forwarding BIG TCP over UDP packets that fall back to software
segmentation result in corrupted checksums for all generated segments?
^ permalink raw reply
* Re: [PATCH net-next v9 5/9] udp: Validate UDP length in udp_gro_receive
From: Simon Horman @ 2026-07-16 14:48 UTC (permalink / raw)
To: alice.kernel
Cc: Simon Horman, daniel, davem, edumazet, kuba, pabeni, lucien.xin,
willemdebruijn.kernel, willemb, dsahern, razor, shuah, stfomichev,
andrew+netdev, fw, netdev, alice
In-Reply-To: <20260710134242.216538-6-alice.kernel@fastmail.im>
This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
Full review at: https://sashiko.dev
---
--- Patch [5]: [PATCH net-next v9 5/9] udp: Validate UDP length in udp_gro_receive ---
commit f1316d873c7549e0a49dc98d7614be1dc5b0f66c
Author: Alice Mikityanska <alice@isovalent.com>
udp: Validate UDP length in udp_gro_receive
This patch adds an early check for malformed or padded UDP packets with
len=0 at udp_gro_receive to exclude them from GRO, moving the check out of
udp_gro_receive_segment.
> diff --git a/net/ipv4/udp_offload.c b/net/ipv4/udp_offload.c
> index 4f9a3922937cc..8f77c8788f6db 100644
> --- a/net/ipv4/udp_offload.c
> +++ b/net/ipv4/udp_offload.c
[ ... ]
> @@ -782,8 +778,14 @@ struct sk_buff *udp_gro_receive(struct list_head *head, struct sk_buff *skb,
> struct sk_buff *p;
> struct udphdr *uh2;
> unsigned int off = skb_gro_offset(skb);
> + unsigned int ulen;
> int flush = 1;
>
> + /* Do not deal with padded or malicious packets, sorry! */
> + ulen = udp_get_len_short(uh);
> + if (ulen <= sizeof(*uh) || ulen != skb_gro_len(skb))
> + goto out;
> +
[Severity: High]
Could this early return bypass inner GRO flow matching for padded UDP tunnel
packets?
When a padded UDP tunnel packet, such as a small TCP ACK inside VXLAN or
GENEVE, arrives in net/ipv4/udp_offload.c:udp_gro_receive() while previous
packets of the same inner flow are held in the GRO list, the
ulen != skb_gro_len(skb) check evaluates to true and triggers the goto out.
Because the goto out path yields pp = NULL and flush = 1, dev_gro_receive()
passes the new padded packet to the network stack immediately, leaving older
packets stuck in the GRO list.
Before this change, the packet would reach inner GRO handlers (like
tcp4_gro_receive()) which would detect the sequence or padding mismatch,
set flush = 1, and return the held packet via pp = p. This ensured older
packets were flushed to the stack before the new padded packet, preserving
delivery order.
Will bypassing this inner flow matching lead to out-of-order delivery for
the inner protocol, resulting in duplicate ACKs and throughput degradation?
> /* We can do L4 aggregation only if the packet can't land in a tunnel
> * otherwise we could corrupt the inner stream. Detecting such packets
> * cannot be foolproof and the aggregation might still happen in some
^ permalink raw reply
* Re: [PATCH net-next v9 8/9] geneve: Enable BIG TCP packets
From: Simon Horman @ 2026-07-16 14:48 UTC (permalink / raw)
To: alice.kernel
Cc: Simon Horman, daniel, davem, edumazet, kuba, pabeni, lucien.xin,
willemdebruijn.kernel, willemb, dsahern, razor, shuah, stfomichev,
andrew+netdev, fw, netdev, alice
In-Reply-To: <20260710134242.216538-9-alice.kernel@fastmail.im>
This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
Full review at: https://sashiko.dev
---
--- Patch [8]: [PATCH net-next v9 8/9] geneve: Enable BIG TCP packets ---
commit d560b2b7c8bd97bea9e23790fdfc21cfe031b961
Author: Alice Mikityanska <alice@isovalent.com>
geneve: Enable BIG TCP packets
In Cilium we do support BIG TCP, but so far the latter has only been
enabled for direct routing use-cases. A lot of users rely on Cilium
with vxlan/geneve tunneling though. The underlying kernel infra for
tunneling has not been supporting BIG TCP up to this point.
> diff --git a/drivers/net/geneve.c b/drivers/net/geneve.c
> index 011bf9d833caf..a1639ad53077e 100644
> --- a/drivers/net/geneve.c
> +++ b/drivers/net/geneve.c
> @@ -1745,6 +1745,8 @@ static void geneve_setup(struct net_device *dev)
> dev->max_mtu = IP_MAX_MTU - GENEVE_BASE_HLEN - dev->hard_header_len;
>
> netif_keep_dst(dev);
> + netif_set_tso_max_size(dev, GSO_MAX_SIZE);
> +
[Severity: High]
This is a pre-existing issue, but by enabling BIG TCP on Geneve, will it
expose a bug where valid BIG TCP packets are incorrectly dropped?
In geneve_post_decap_hint(), there is a legacy size check that rejects
packets aggregated by GRO to sizes >= 65536 bytes:
drivers/net/geneve.c:geneve_post_decap_hint() {
...
total_len = skb->len - gro_hint->nested_nh_offset;
if (total_len >= GRO_LEGACY_MAX_SIZE)
return -E2BIG;
...
}
When BIG TCP is enabled and the Geneve tunnel is configured with gro_hint,
large flows are aggregated by GRO into packets exceeding 64KB. This will hit
the legacy check and return -E2BIG, causing packet loss for large flows.
> dev->priv_flags &= ~IFF_TX_SKB_SHARING;
> dev->priv_flags |= IFF_LIVE_ADDR_CHANGE | IFF_NO_QUEUE;
> dev->lltx = true;
^ permalink raw reply
* Re: [PATCH net] nfc: st21nfca: validate ATR_REQ length against the received frame
From: Simon Horman @ 2026-07-16 14:58 UTC (permalink / raw)
To: Doruk Tan Ozturk
Cc: David Heidelberg, oe-linux-nfc, netdev, linux-kernel, stable
In-Reply-To: <20260711071301.58071-1-doruk@0sec.ai>
On Sat, Jul 11, 2026 at 09:13:01AM +0200, Doruk Tan Ozturk wrote:
> st21nfca_tm_recv_atr_req() checks that the received ATR_REQ frame is at
> least ST21NFCA_ATR_REQ_MIN_SIZE and that the self-declared atr_req->length
> is at least sizeof(struct st21nfca_atr_req), but never checks that
> atr_req->length does not exceed the actual received length (skb->len).
>
> st21nfca_tm_send_atr_res() then trusts the declared length:
>
> gb_len = atr_req->length - sizeof(struct st21nfca_atr_req);
> ...
> memcpy(atr_res->gbi, atr_req->gbi, gb_len);
>
> so an RF peer that sends a short frame but sets atr_req->length larger
> than the frame makes gb_len exceed the general bytes actually present,
> and the memcpy reads out of bounds past the received skb. Those bytes are
> placed in the ATR_RES and sent back to the peer (kernel-memory disclosure
> to a proximity attacker); a larger declared length is an out-of-bounds
> read (DoS).
>
> Reject frames whose declared length exceeds the received length. The
> adjacent nfc_tm_activated() path in the same function already derives its
> general-bytes length from skb->len rather than the declared field.
>
> Found by 0sec (https://0sec.ai) using automated source analysis; the
> missing bound is evident from source. Compile-tested.
>
> Fixes: 1892bf844ea0 ("NFC: st21nfca: Adding P2P support to st21nfca in Initiator & Target mode")
> Cc: stable@vger.kernel.org
> Assisted-by: 0sec:claude-opus-4-8
> Signed-off-by: Doruk Tan Ozturk <doruk@0sec.ai>
Reviewed-by: Simon Horman <horms@kernel.org>
FTR, the AI-generated review of this patch on sashiko.dev does seem to
raise legitimate pre-existing issues which relate closely to this patch.
But I think they can be treated as potential follow-up rather than impeding
progress of this patch.
^ permalink raw reply
* Re: [PATCH net-next v2 02/12] ax88179_178a: Add HW support for AX179A-based chips
From: Birger Koblitz @ 2026-07-16 14:54 UTC (permalink / raw)
To: Andrew Lunn
Cc: Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, linux-usb, netdev, linux-kernel
In-Reply-To: <1f858852-8e8e-4cff-bbdc-eb6751f44ca5@lunn.ch>
On 7/15/26 00:58, Andrew Lunn wrote:
>> + /* Initialize MII structure */
>> + dev->mii.dev = dev->net;
>> + dev->mii.mdio_read = ax88179_mdio_read;
>> + dev->mii.mdio_write = ax88179_mdio_write;
>> + dev->mii.phy_id_mask = 0xff;
>> + dev->mii.reg_num_mask = 0xff;
>> + dev->mii.phy_id = 0x03;
>> + if (!ax179_data->is_ax88772d)
>> + dev->mii.supports_gmii = 1;
>
> O.K, so you are keeping going.
>
> I still think mii has to die, and you need to swap to phylink.
O.K. you have me convinced to try harder. Especially with the suggestion
to create own PHY-IDs for these embedded PHYs it looks doable and not
too ugly.
>
> It is a bit ugly, but there is a way around the PHYs having a totally
> different API, despite the same ID register values.
>
> Some generation of Marvel Ethernet switches set the OUI part of the
> PHY ID register, but left the product part as 0. They even documented
> this was intentional. We worked around it by trapping reads to
> registers 2 and 3, and filling in the missing information, using an ID
> from the marvel range which was not in use. We could then make the
> Marvell PHY driver do the right thing.
>
> You can do the same here. Intercept the reads to registers 2 and 3,
> and return a different ID. You can then extend the PHY driver.
>
Doing that and seems to work. I will be cluttering ASIX's own PHY-ID space
with vendor part 0x3b, see the ax88796b PHY driver which already provides support
for the 772A, 772B and 772C PHYs. So I will add there the 772D/E and 179A and 279.
This seems safe, since ASIX appears to have stopped selling (discrete) PHYs. At
least I am no longer able to find any references to PHYs built by them. It
also appears appropriate since with the firmware they are using the PHYs are more
ASIX PHYs than whatever other PHY IP-core they may have been.
It will take a bit of time to convert the code, though, and I will submit that
then as v3.
A comment on phylink and USB-Ethernet NICs, though. Grepping for phylink
in drivers/net/usb shows only 2 drivers that use phylink: asix_devices (the
antiquated one using the 772A-PHYs) and lan789xx, which is also rather old.
There are more than 30 drivers there, so that makes < 7% phylink usage vs 93% mii.
And at least for the 179A-based controllers it is clear that there is an
advantage not to use phylink: When the link changes, the controller sends out
an interrupt URB, that has everything in there to set up the link. No need to
do costly USB requests to query the PHY. In fact, the proprietary driver
works hard to not talk to the PHY at all. A good example is WoL, which is in
principle a PHY configuration issue. For setting that up, the driver actually
never talks to the PHY, everything necessary is done by the controller's
firmware. For USB Ethernet controllers it makes a lot of sense to abstract the
PHY away and rely only on controller commands that concurrently configure MAC and
PHY to reduce USB transfers.
On the bright side, it appears possible to convert also the r8152 driver to phylink:
The RTL8157-based USB controllers have:
[10848.976063] usb 2-1: New USB device found, idVendor=0bda, idProduct=8157, bcdDevice=30.00
[10848.976068] usb 2-1: New USB device strings: Mfr=1, Product=2, SerialNumber=7
[10848.976071] usb 2-1: Product: USB 10/100/1G/2.5G/5G LAN
[10848.976074] usb 2-1: Manufacturer: Realtek
[10884.829998] r8152 2-1:1.0: PHY REG 2: 001c
[10884.830079] r8152 2-1:1.0: PHY REG 3: c862
rg finds:
#define RTL_8251B 0x001cc862
RTL8159:
10970.521382] usb 2-1: New USB device found, idVendor=0bda, idProduct=815a, bcdDevice=30.00
[10970.521386] usb 2-1: New USB device strings: Mfr=1, Product=2, SerialNumber=7
[10970.521389] usb 2-1: Product: USB 10/100/1G/2.5G/5G/10G LAN
[10970.521391] usb 2-1: Manufacturer: Realtek
[10970.797123] r8152 2-1:1.0: PHY REG 2: 001c
[10970.797207] r8152 2-1:1.0: PHY REG 3: c890
#define RTL_8261C 0x001cc890
RTL8156:
[11299.622636] usb 2-4: New USB device found, idVendor=0bda, idProduct=8156, bcdDevice=31.04
[11299.622641] usb 2-4: New USB device strings: Mfr=1, Product=2, SerialNumber=6
[11299.622644] usb 2-4: Product: USB 10/100/1G/2.5G LAN
[11299.622647] usb 2-4: Manufacturer: Realtek
[11299.882412] r8152 2-4:1.0: PHY REG 2: 001c
[11299.882467] r8152 2-4:1.0: PHY REG 3: c840
#define RTL_8221B 0x001cc840
RTL8153b:
[11498.161341] usb 2-4: New USB device found, idVendor=0bda, idProduct=8153, bcdDevice=31.00
[11498.161348] usb 2-4: New USB device strings: Mfr=1, Product=2, SerialNumber=6
[11498.161350] usb 2-4: Product: USB 10/100/1000 LAN
[11498.161352] usb 2-4: Manufacturer: Realtek
[11810.891092] r8152 2-4:1.0: PHY REG 2: 001c
[11810.891138] r8152 2-4:1.0: PHY REG 3: c800
#define RTL_GENERIC_PHYID 0x001cc800
Looking into the Realtek PHY driver code, what is done there appears to duplicate
the code in r8152.
Cheers,
Birger
^ permalink raw reply
* Re: [PATCH RFC net-next 3/6] bpf: Allow skb extensions to survive packet scrubbing
From: Jason Xing @ 2026-07-16 15:06 UTC (permalink / raw)
To: Jakub Sitnicki
Cc: Stanislav Fomichev, Daniel Borkmann, John Fastabend, netdev, bpf,
kernel-team, Jakub Kicinski, Kuniyuki Iwashima
In-Reply-To: <87a4rrjdj4.fsf@cloudflare.com>
On Thu, Jul 16, 2026 at 3:00 PM Jakub Sitnicki <jakub@cloudflare.com> wrote:
>
> On Thu, Jul 16, 2026 at 05:11 AM -07, Stanislav Fomichev wrote:
> > On 07/14, Jakub Sitnicki wrote:
> >> skb_scrub_packet() drops all skb extensions unconditionally via
> >> skb_ext_reset(). It runs on tunnel encap/decap (ip_tunnel_rcv,
> >> vxlan_rcv, etc.) and cross-netns forwarding (dev_forward_skb).
> >>
> >> This makes it impossible for a BPF program to pass metadata via
> >> bpf_skb_ext through a tunnel or across a netns boundary. The extension
> >> is always lost at the scrub point.
> >>
> >> Introduce skb_ext_scrub() which consults each active extension before
> >> discarding it. Extensions that request preservation are kept while the
> >> rest are torn down. When the extension slab is shared with clones, COW
> >> ensures isolation. Replace the skb_ext_reset() call in
> >> skb_scrub_packet() with skb_ext_scrub().
> >>
> >> Expose the opt-in mechanism to BPF via the BPF_SKB_EXT_F_NO_SCRUB flag
> >> for bpf_dynptr_from_skb_ext(). A program that sets this flag when
> >> creating the extension signals that its metadata should survive
> >> scrubbing.
> >>
> >> Signed-off-by: Jakub Sitnicki <jakub@cloudflare.com>
> >> ---
> >> include/linux/bpf.h | 1 +
> >> include/linux/skbuff.h | 2 ++
> >> include/uapi/linux/bpf.h | 3 +-
> >> net/core/filter.c | 11 +++++--
> >> net/core/skbuff.c | 82 ++++++++++++++++++++++++++++++++++++++++++------
> >> net/ipv4/udp.c | 2 +-
> >> 6 files changed, 86 insertions(+), 15 deletions(-)
> >>
> >> diff --git a/include/linux/bpf.h b/include/linux/bpf.h
> >> index 6b918a5b61bf..a46ca53c5b27 100644
> >> --- a/include/linux/bpf.h
> >> +++ b/include/linux/bpf.h
> >> @@ -4214,6 +4214,7 @@ static inline int bpf_map_check_op_flags(struct bpf_map *map, u64 flags, u64 all
> >> #ifdef CONFIG_BPF_SKB_EXT
> >>
> >> struct bpf_skb_ext {
> >> + u64 flags;
> >> u8 buf[CONFIG_BPF_SKB_EXT_SIZE] __aligned(8);
> >> };
> >>
> >> diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
> >> index 584d8440d352..66afa5489007 100644
> >> --- a/include/linux/skbuff.h
> >> +++ b/include/linux/skbuff.h
> >> @@ -5063,6 +5063,7 @@ void *__skb_ext_set(struct sk_buff *skb, enum skb_ext_id id,
> >> void *skb_ext_add(struct sk_buff *skb, enum skb_ext_id id);
> >> void __skb_ext_del(struct sk_buff *skb, enum skb_ext_id id);
> >> void __skb_ext_put(struct skb_ext *ext);
> >> +void skb_ext_scrub(struct sk_buff *skb);
> >>
> >> static inline void skb_ext_put(struct sk_buff *skb)
> >> {
> >> @@ -5132,6 +5133,7 @@ static inline bool skb_has_extensions(struct sk_buff *skb)
> >> static inline void __skb_ext_put(struct skb_ext *ext) {}
> >> static inline void skb_ext_put(struct sk_buff *skb) {}
> >> static inline void skb_ext_reset(struct sk_buff *skb) {}
> >> +static inline void skb_ext_scrub(struct sk_buff *skb) {}
> >> static inline void skb_ext_del(struct sk_buff *skb, int unused) {}
> >> static inline void __skb_ext_copy(struct sk_buff *d, const struct sk_buff *s) {}
> >> static inline void skb_ext_copy(struct sk_buff *dst, const struct sk_buff *s) {}
> >> diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> >> index 3eee4467422d..02da170205de 100644
> >> --- a/include/uapi/linux/bpf.h
> >> +++ b/include/uapi/linux/bpf.h
> >> @@ -7734,7 +7734,8 @@ struct bpf_insn_array_value {
> >>
> >> /* Flags to control bpf_dynptr_from_skb_ext() behavior. */
> >> enum {
> >> - BPF_SKB_EXT_F_CREATE = (1ULL << 0),
> >> + BPF_SKB_EXT_F_CREATE = (1ULL << 0),
> >> + BPF_SKB_EXT_F_NO_SCRUB = (1ULL << 1),
> >
> > Do I understand correctly that you do prefer the NO_SCRUB mode? Any reason
> > we need to have scrub mode? If it's all produced/consumed by bpf, maybe
> > we can just carry this data unconditionally instead of having a SCRUB/NO_SCRUB
> > option?
>
> Yes, that's correct. We definitely need NO_SCRUB but I don't have a use
> case that relies on metadata scrubbing. I believe Kuniyuki also would
> like the no-scrub to be the only/default behavior for Google's egress
> use case.
>
> I've made it an opt-out mostly because that the existing metadata
> (skb->mark, skb->data_meta) gets scrubbed. Although as Jakub K has
> pointed out to me - you can circumvent it by using bpf_redirect into the
> target netns. So I guess we have a precendent?
>
> I could use input from folks operating in K8S-like environments, if
Sorry, I don't quite follow it here. Why don't we need the NO_SCRUB
here? The crucial point here is to allow skb traverse across different
containers, which is also what I expect. We do need to support this
case to make this feature standalone, which means it is independent
and not affected by other components.
And one part of what I'm doing for BPF timestamping v2 is to address
the issue in the container scenario as well, FYI.
Thanks,
Jason
> no-scrub-only mode would be acceptable there? Daniel, John, any opinion?
>
>
^ permalink raw reply
* Re: [PATCH net-next v9 1/9] net: Use helpers to get/set UDP len tree-wide
From: Alice Mikityanska @ 2026-07-16 15:18 UTC (permalink / raw)
To: Simon Horman
Cc: alice.kernel, daniel, davem, edumazet, kuba, pabeni, lucien.xin,
willemdebruijn.kernel, willemb, dsahern, razor, shuah, stfomichev,
andrew+netdev, fw, netdev
In-Reply-To: <20260716144620.331419-2-horms@kernel.org>
On Thu, 16 Jul 2026 at 17:47, Simon Horman <horms@kernel.org> wrote:
>
> This is an AI-generated review of your patch. The human sending this
> email has considered the AI review valid, or at least plausible.
> Full review at: https://sashiko.dev
> ---
> --- Patch [1]: [PATCH net-next v9 1/9] net: Use helpers to get/set UDP len tree-wide ---
> commit 113d28557bcedccc492f330fafdaaf1377eb2e79
> Author: Alice Mikityanska <alice@isovalent.com>
>
> net: Use helpers to get/set UDP len tree-wide
>
> This commit introduces helpers for setting and getting the UDP header length
> tree-wide. It prepares the codebase to support BIG TCP for UDP tunnels by
> explicitly handling cases where UDP length might be 0 for packets larger
> than 65535 bytes.
>
> > diff --git a/net/6lowpan/nhc_udp.c b/net/6lowpan/nhc_udp.c
> > index 0a506c77283dd..ed4227e6db744 100644
> > --- a/net/6lowpan/nhc_udp.c
> > +++ b/net/6lowpan/nhc_udp.c
> > @@ -88,16 +88,16 @@ static int udp_uncompress(struct sk_buff *skb, size_t needed)
> > switch (lowpan_dev(skb->dev)->lltype) {
> > case LOWPAN_LLTYPE_IEEE802154:
> > if (lowpan_802154_cb(skb)->d_size)
> > - uh.len = htons(lowpan_802154_cb(skb)->d_size -
> > - sizeof(struct ipv6hdr));
> > + udp_set_len_short(&uh, lowpan_802154_cb(skb)->d_size -
> > + sizeof(struct ipv6hdr));
>
> [Severity: High]
> If a crafted packet specifies a datagram size smaller than
> sizeof(struct ipv6hdr), won't this subtraction underflow?
First, I'd like to emphasize that I'm merely converting direct uh.len
access to the new helper. If there is an existing bug with underflow,
my new helper will expose it with a WARN, which is good. But should
such bugs exist before this refactoring, I don't think that fixing all
of them is in scope of this series.
Addressing the AI concern directly, my AI claims that such underflow
is not possible:
> For fragmented IEEE 802.15.4 packets:
> - d_size is decoded from the FRAG1 header in net/ieee802154/6lowpan/reassembly.c:259.
> - A FRAG1 packet with d_size < sizeof(struct ipv6hdr) is rejected at net/ieee802154/6lowpan/reassembly.c:270.
> - UDP/IPHC decompression is invoked only after that validation, at net/ieee802154/6lowpan/reassembly.c:296.
> - FRAGN packets do not invoke header decompression here; they are queued for reassembly.
> For an unfragmented packet, d_size is explicitly set to zero at net/ieee802154/6lowpan/rx.c:103, selecting the else branch and avoiding the subtraction entirely.
> The resulting massive unsigned value passed to udp_set_len_short() would
> then trigger the DEBUG_NET_WARN_ON_ONCE, potentially causing a panic if
> panic_on_warn is set.
>
> > [ ... ]
> > diff --git a/net/ipv4/udp_offload.c b/net/ipv4/udp_offload.c
> > index 29651b1a0bc70..493e2b9e16fbc 100644
> > --- a/net/ipv4/udp_offload.c
> > +++ b/net/ipv4/udp_offload.c
> > [ ... ]
> > @@ -915,12 +914,12 @@ static int udp_gro_complete_segment(struct sk_buff *skb)
> > int udp_gro_complete(struct sk_buff *skb, int nhoff,
> > udp_lookup_t lookup)
> > {
> > - __be16 newlen = htons(skb->len - nhoff);
> > struct udphdr *uh = (struct udphdr *)(skb->data + nhoff);
> > + unsigned int newlen = skb->len - nhoff;
> > struct sock *sk;
> > int err;
> >
> > - uh->len = newlen;
> > + udp_set_len_short(uh, newlen);
>
> [Severity: High]
> If gro_max_size is configured larger than 65535 (which is supported and used
> for BIG TCP), can't UDP GRO aggregate packets such that newlen > 65535?
This is the first patch that just converted direct access to helpers.
Patch 4 "udp: Support gro_ipv4_max_size > 65536" explicitly enables
len > 64k and replaces this line with udp_set_len. This comment on
patch 1 is irrelevant because it assumes that functionality added in a
later patch should work in a prior patch.
> This would instantly trigger the DEBUG_NET_WARN_ON_ONCE inside
> udp_set_len_short() for legitimate large packets.
>
> > [ ... ]
> > diff --git a/net/ipv4/udp_tunnel_core.c b/net/ipv4/udp_tunnel_core.c
> > index 9ab3728f9630b..0fccb38f074d2 100644
> > --- a/net/ipv4/udp_tunnel_core.c
> > +++ b/net/ipv4/udp_tunnel_core.c
> > @@ -178,7 +178,7 @@ void udp_tunnel_xmit_skb(struct rtable *rt, struct sock *sk, struct sk_buff *skb
> >
> > uh->dest = dst_port;
> > uh->source = src_port;
> > - uh->len = htons(skb->len);
> > + udp_set_len_short(uh, skb->len);
>
> [Severity: High]
> When transmitting a TCP BIG TCP packet (which can be > 64K) over a UDP
> tunnel like VXLAN, skb->len will exceed 65535.
>
> Does using udp_set_len_short() here incorrectly trigger the warning for
> valid large GSO payloads?
Same here. This is replaced by udp_set_len, which supports len > 64k,
in the patch that actually enables len > 64k.
> > [ ... ]
> > diff --git a/net/ipv6/ip6_udp_tunnel.c b/net/ipv6/ip6_udp_tunnel.c
> > index 9adb5775487fa..dcff7fb16ff64 100644
> > --- a/net/ipv6/ip6_udp_tunnel.c
> > +++ b/net/ipv6/ip6_udp_tunnel.c
> > @@ -93,7 +93,7 @@ void udp_tunnel6_xmit_skb(struct dst_entry *dst, struct sock *sk,
> > uh->dest = dst_port;
> > uh->source = src_port;
> >
> > - uh->len = htons(skb->len);
> > + udp_set_len_short(uh, skb->len);
>
> [Severity: High]
> This appears to have the same issue as IPv4 tunnels. If a large GSO payload
> is sent through an IPv6 UDP tunnel, won't skb->len exceed 65535 and trigger
> the warning?
Same here.
I'm sorry, but I highly doubt that these AI review comments were validated.
^ permalink raw reply
* Re: [PATCH 1/3] mm: move internal mempolicy APIs to new internal header
From: Vlastimil Babka (SUSE) @ 2026-07-16 15:22 UTC (permalink / raw)
To: Brendan Jackman, Andrew Morton, David Hildenbrand,
Lorenzo Stoakes, Liam R. Howlett, Mike Rapoport,
Suren Baghdasaryan, Michal Hocko, Johannes Weiner, Zi Yan,
Matthew Wilcox (Oracle), Jan Kara, Joshua Hahn, Byungchul Park,
Gregory Price, Ying Huang, Alistair Popple, Hugh Dickins,
Baolin Wang, Chris Li, Kairui Song, Kemeng Shi, Nhat Pham,
Baoquan He, Barry Song, Youngjun Park, Joerg Roedel (AMD),
Will Deacon, Robin Murphy, Huacai Chen, WANG Xuerui,
Thomas Gleixner, Chuck Lever, Jeff Layton, NeilBrown,
Olga Kornievskaia, Dai Ngo, Tom Talpey, Trond Myklebust,
Anna Schumaker, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Simon Horman
Cc: linux-kernel, linux-mm, linux-fsdevel, iommu, loongarch,
linux-nfs, netdev
In-Reply-To: <20260716-folio-alloc-cleanups-v1-1-5363b8e92d33@google.com>
On 7/16/26 16:30, Brendan Jackman wrote:
> There are no external users for this surface, reduce the scope.
>
> Ulterior motive: a later patch will add an alloc_flags arg to some parts
> of this.
>
> Note it might seem like this could just go in internal.h, since it's
> pretty small, but actually it will eventually need to import
> page_alloc.h, we don't want to import that from internal.h so best to
> proactively created this header now.
Hmm, maybe it could just go to page_alloc.h then? After all this is just
bunch of internal page allocation functions, which just take the mempolicy
pointer?
> Signed-off-by: Brendan Jackman <jackmanb@google.com>
> ---
> MAINTAINERS | 1 +
> include/linux/gfp.h | 9 ---------
> mm/filemap.c | 2 ++
> mm/mempolicy.c | 1 +
> mm/mempolicy.h | 31 +++++++++++++++++++++++++++++++
> mm/shmem.c | 1 +
> mm/swap_state.c | 1 +
> 7 files changed, 37 insertions(+), 9 deletions(-)
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 8813d5d7eb0c1..9cde99c259639 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -17068,6 +17068,7 @@ F: include/uapi/linux/mempolicy.h
> F: include/linux/migrate.h
> F: include/linux/migrate_mode.h
> F: mm/mempolicy.c
> +F: mm/mempolicy.h
> F: mm/migrate.c
> F: mm/migrate_device.c
>
> diff --git a/include/linux/gfp.h b/include/linux/gfp.h
> index 872bc53f32ec8..e4e974a6e5f90 100644
> --- a/include/linux/gfp.h
> +++ b/include/linux/gfp.h
> @@ -11,7 +11,6 @@
> #include <linux/sched.h>
>
> struct vm_area_struct;
> -struct mempolicy;
>
> /* Helper macro to avoid gfp flags if they are the default one */
> #define __default_gfp(a,b,...) b
> @@ -274,8 +273,6 @@ struct page *alloc_pages_node_noprof(int nid, gfp_t gfp_mask, unsigned int order
> #ifdef CONFIG_NUMA
> struct page *alloc_pages_noprof(gfp_t gfp, unsigned int order);
> struct folio *folio_alloc_noprof(gfp_t gfp, unsigned int order);
> -struct folio *folio_alloc_mpol_noprof(gfp_t gfp, unsigned int order,
> - struct mempolicy *mpol, pgoff_t ilx, int nid);
> struct folio *vma_alloc_folio_noprof(gfp_t gfp, int order, struct vm_area_struct *vma,
> unsigned long addr);
> #else
> @@ -287,11 +284,6 @@ static inline struct folio *folio_alloc_noprof(gfp_t gfp, unsigned int order)
> {
> return __folio_alloc_node_noprof(gfp, order, numa_node_id());
> }
> -static inline struct folio *folio_alloc_mpol_noprof(gfp_t gfp, unsigned int order,
> - struct mempolicy *mpol, pgoff_t ilx, int nid)
> -{
> - return folio_alloc_noprof(gfp, order);
> -}
> static inline struct folio *vma_alloc_folio_noprof(gfp_t gfp, int order,
> struct vm_area_struct *vma, unsigned long addr)
> {
> @@ -301,7 +293,6 @@ static inline struct folio *vma_alloc_folio_noprof(gfp_t gfp, int order,
>
> #define alloc_pages(...) alloc_hooks(alloc_pages_noprof(__VA_ARGS__))
> #define folio_alloc(...) alloc_hooks(folio_alloc_noprof(__VA_ARGS__))
> -#define folio_alloc_mpol(...) alloc_hooks(folio_alloc_mpol_noprof(__VA_ARGS__))
> #define vma_alloc_folio(...) alloc_hooks(vma_alloc_folio_noprof(__VA_ARGS__))
>
> #define alloc_page(gfp_mask) alloc_pages(gfp_mask, 0)
> diff --git a/mm/filemap.c b/mm/filemap.c
> index 1dbb4c6f824e9..0dd8e2a15d746 100644
> --- a/mm/filemap.c
> +++ b/mm/filemap.c
> @@ -52,6 +52,7 @@
>
> #include <asm/tlbflush.h>
> #include "internal.h"
> +#include "mempolicy.h"
>
> #define CREATE_TRACE_POINTS
> #include <trace/events/filemap.h>
> @@ -63,6 +64,7 @@
>
> #include <asm/mman.h>
>
> +#include "mempolicy.h"
> #include "swap.h"
>
> /*
> diff --git a/mm/mempolicy.c b/mm/mempolicy.c
> index 5720f7f54d942..31c4e9f49b5e9 100644
> --- a/mm/mempolicy.c
> +++ b/mm/mempolicy.c
> @@ -119,6 +119,7 @@
> #include <linux/memory.h>
>
> #include "internal.h"
> +#include "mempolicy.h"
> #include "page_alloc.h"
>
> /* Internal flags */
> diff --git a/mm/mempolicy.h b/mm/mempolicy.h
> new file mode 100644
> index 0000000000000..d80fe4c559786
> --- /dev/null
> +++ b/mm/mempolicy.h
> @@ -0,0 +1,31 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +/*
> + * mm-internal API for mempolicy.c. Public API lives in
> + * include/linux/mempolicy.h.
> + */
> +#ifndef __MM_MEMPOLICY_H
> +#define __MM_MEMPOLICY_H
> +
> +#include <linux/gfp.h>
> +#include <linux/mempolicy.h>
> +
> +#ifdef CONFIG_NUMA
> +struct folio *folio_alloc_mpol_noprof(gfp_t gfp, unsigned int order,
> + struct mempolicy *mpol, pgoff_t ilx, int nid);
> +#else
> +static inline struct folio *folio_alloc_mpol_noprof(gfp_t gfp, unsigned int order,
> + struct mempolicy *mpol, pgoff_t ilx, int nid)
> +{
> + return folio_alloc_noprof(gfp, order);
> +}
> +#endif
> +
> +#define folio_alloc_mpol(...) alloc_hooks(folio_alloc_mpol_noprof(__VA_ARGS__))
> +
> +unsigned long alloc_pages_bulk_mempolicy_noprof(gfp_t gfp,
> + unsigned long nr_pages,
> + struct page **page_array);
> +#define alloc_pages_bulk_mempolicy(...) \
> + alloc_hooks(alloc_pages_bulk_mempolicy_noprof(__VA_ARGS__))
> +
> +#endif /* __MM_MEMPOLICY_H */
> diff --git a/mm/shmem.c b/mm/shmem.c
> index 5071177059a96..69f561332bb93 100644
> --- a/mm/shmem.c
> +++ b/mm/shmem.c
> @@ -41,6 +41,7 @@
> #include <linux/swapfile.h>
> #include <linux/iversion.h>
> #include <linux/unicode.h>
> +#include "mempolicy.h"
> #include "swap.h"
>
> static struct vfsmount *shm_mnt __ro_after_init;
> diff --git a/mm/swap_state.c b/mm/swap_state.c
> index 5be825911e645..8ccd03c39a407 100644
> --- a/mm/swap_state.c
> +++ b/mm/swap_state.c
> @@ -24,6 +24,7 @@
> #include <linux/shmem_fs.h>
> #include <linux/sysctl.h>
> #include "internal.h"
> +#include "mempolicy.h"
> #include "swap_table.h"
> #include "swap.h"
>
>
^ permalink raw reply
* Re: [PATCH net 2/7] selftests: openvswitch: add config file
From: Aaron Conole @ 2026-07-16 15:38 UTC (permalink / raw)
To: Matthieu Baerts (NGI0)
Cc: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Simon Horman, Shuah Khan, netdev, linux-kselftest, linux-kernel,
Eelco Chaudron, Ilya Maximets, dev
In-Reply-To: <20260710-net-sft-fix-containers-v1-2-a2915c294ef5@kernel.org>
Hi Matthieu,
"Matthieu Baerts (NGI0)" <matttbe@kernel.org> writes:
> The kselftests doc mentions that a config file should be present "if a
> test needs specific kernel config options enabled". This selftest
> requires some kernel config, but no config file was provided.
>
> We could say that a sub-target could use the parent's config file, but
> the kselftests doc doesn't mention anything about that. Plus the
> net/openvswitch target is the only net target without a config file.
We've been operating on that assumption from the openvswitch side, but
it's true that isn't explicitly documented anywhere, and I guess it
isn't officially supported in the kselftest framework. I guess we'll
need to keep updating this config as we add tests for things like SCTP,
and others, and maybe that's a good thing like we can add a comment
describing which tests take which configs.
The downside is for most of the OVS testing we use the NIPA scripts
and those 'inherit' the parent config, so it would be a change on our
side from the development standpoint (but probably something we should
have been doing from the beginning).
That said, would it be worth also exploring the 'cascading
configuration' support? It seems like a useful feature, but maybe it
should be a separate discussion. I ask because of how OVS interacts
with the networking stack as an 'alternative bridge' so-to-speak, I do
worry about having to duplicate lots of configurations between the two
as we expand the test coverage on OVS side.
> Here is a new config file, which is a trimmed version of the net one,
> with hopefully the minimal required kconfig on top of 'make defconfig'.
Should this also remove the OVS configs from the upper level since there
shouldn't be OVS tests executing there (ie: CONFIG_OPENVSWITCH*)?
> The Fixes tag points to the introduction of the net/openvswitch target,
> just to help validating this target on stable kernels.
>
> Fixes: 25f16c873fb1 ("selftests: add openvswitch selftest suite")
> Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
> ---
> To: Aaron Conole <aconole@redhat.com>
> To: Eelco Chaudron <echaudro@redhat.com>
> To: Ilya Maximets <i.maximets@ovn.org>
> Cc: dev@openvswitch.org
> ---
> tools/testing/selftests/net/openvswitch/config | 16 ++++++++++++++++
> 1 file changed, 16 insertions(+)
>
> diff --git a/tools/testing/selftests/net/openvswitch/config b/tools/testing/selftests/net/openvswitch/config
> new file mode 100644
> index 000000000000..c659749cd086
> --- /dev/null
> +++ b/tools/testing/selftests/net/openvswitch/config
> @@ -0,0 +1,16 @@
> +CONFIG_GENEVE=m
> +CONFIG_INET_DIAG=y
> +CONFIG_IPV6=y
> +CONFIG_NETFILTER=y
> +CONFIG_NET_IPGRE=m
> +CONFIG_NET_IPGRE_DEMUX=m
> +CONFIG_NF_CONNTRACK=m
> +CONFIG_NF_CONNTRACK_OVS=y
> +CONFIG_OPENVSWITCH=m
> +CONFIG_OPENVSWITCH_GENEVE=m
> +CONFIG_OPENVSWITCH_GRE=m
> +CONFIG_OPENVSWITCH_VXLAN=m
> +CONFIG_PSAMPLE=m
> +CONFIG_VETH=y
> +CONFIG_VLAN_8021Q=y
> +CONFIG_VXLAN=m
^ permalink raw reply
* Re: [PATCH v4 4/5] vhost: synchronize with RCU readers when freeing workers
From: Andrey Drobyshev @ 2026-07-16 15:39 UTC (permalink / raw)
To: Stefano Garzarella
Cc: linux-kernel, kvm, virtualization, netdev, mst, stefanha,
dongli.zhang, maciej.szmigiero, bchaney, mark.kanda, ptikhomirov,
den
In-Reply-To: <alibLtcrC7o5r4Dh@sgarzare-redhat>
On 7/16/26 11:57 AM, Stefano Garzarella wrote:
> On Tue, Jul 14, 2026 at 06:16:37PM +0300, Andrey Drobyshev wrote:
>> vhost_vq_work_queue() only holds the RCU read lock while it dereferences
>> vq->worker and queues work on it. vhost_workers_free() however clears
>> the vq->worker pointers and immediately frees the workers, without
>> waiting for a grace period. A caller that fetched the worker right
>> before the pointer was cleared can therefore still be queueing work on
>> it while it is freed. And even when the queueing itself wins the race,
>> the work is never run, so its VHOST_WORK_QUEUED bit stays set and all
>> future attempts to queue it are silently skipped.
>>
>> None of the current callers can actually hit this: net and scsi stop
>> their virtqueues before the workers are freed, and vsock unhashes the
>> device and does synchronize_rcu() of its own in vhost_vsock_dev_release()
>> before the workers go away. But the upcoming VHOST_RESET_OWNER support
>> in vhost-vsock keeps the device hashed while its workers are freed, so
>> the lockless send/cancel paths become able to race with the teardown.
>>
>> Close this the way vhost_worker_killed() already does: clear the
>> vq->worker pointers, wait for a grace period, run whatever the last
>> readers may have queued, and only then free the workers. The
>> synchronize_rcu() is skipped if the device has no workers, so cleanup of
>> devices which never got an owner stays cheap.
>>
>
> Do we need a Fixes tag for this?
>
I'm guessing it should be:
Fixes: 228a27cf78af ("vhost: Allow worker switching while work is queueing")
> Thanks for pointing out that the issue wasn't occurring, but I think we
> should add it because it's a sneaky problem we discovered by chance.
> IMO the code should already have `synchronize_rcu()` after
> `rcu_assign_pointer()` loop.
>
> @Michael, what do you think?
>
>> Suggested-by: Stefano Garzarella <sgarzare@redhat.com>
>> Signed-off-by: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>
>> ---
>> drivers/vhost/vhost.c | 15 +++++++++++++++
>> 1 file changed, 15 insertions(+)
>>
>> diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
>> index 4c525b3e16ea..0d1414d40f4e 100644
>> --- a/drivers/vhost/vhost.c
>> +++ b/drivers/vhost/vhost.c
>> @@ -729,6 +729,21 @@ static void vhost_workers_free(struct vhost_dev *dev)
>>
>> for (i = 0; i < dev->nvqs; i++)
>> rcu_assign_pointer(dev->vqs[i]->worker, NULL);
>> +
>> + /*
>> + * vhost_vq_work_queue() reads vq->worker under rcu_read_lock(), so a
>> + * caller that fetched a worker before we cleared the pointers above
>> + * may still be about to queue work on it. Wait for those RCU readers
>> + * to finish before freeing the worker, then run whatever they queued
>> + * so nothing is left with VHOST_WORK_QUEUED set. Mirrors
>> + * vhost_worker_killed().
>> + */
>> + if (!xa_empty(&dev->worker_xa)) {
>> + synchronize_rcu();
>> + xa_for_each(&dev->worker_xa, i, worker)
>> + vhost_run_work_list(worker);
>> + }
>> +
>
> Following sashiko review [1], I tried to undersand why we need this, but
> TBH I'm really confused. That said, this seems wrong also because it
> will work only with vhost_tasks, and not with kthreads.
>
> IIUC vhost_worker_killed() will be called anyway when calling
> vhost_worker_destroy(). For vhost_tasks, it will call
> vhost_task_do_stop() that calls vhost_task_stop(). This sets
> VHOST_TASK_FLAGS_STOP and wait the worker on vtsk->exited before freeing
> stuff. The worker breaks the loop and calls vtsk->handle_sigkill() that
> is exactly vhost_worker_killed() you mentioned we are mirroring here.
>
Hmm, are we sure it's the case for our codepath? Looking at the
vhost_task loop function:
> static int vhost_task_fn(void *data)
> {
> for (;;) {
> if (signal_pending(current)) {
> if (get_signal(&ksig))
> break;
> }
> ...
> if (test_bit(VHOST_TASK_FLAGS_STOP, &vtsk->flags)) {
> __set_current_state(TASK_RUNNING);
> break;
> }
> did_work = vtsk->fn(vtsk->data);
> ...
> }
>
> ...
>
> if (!test_bit(VHOST_TASK_FLAGS_STOP, &vtsk->flags)) {
> set_bit(VHOST_TASK_FLAGS_KILLED, &vtsk->flags);
> vtsk->handle_sigkill(vtsk->data);
> }
> ...
> }
AFAICT, we exit the loop in 2 cases: signal delivery or STOP bit
setting. Like you said, STOP is set by vhost_task_stop. E.g. for our
RESET_OWNER case:
vhost_vsock_reset_owner()
vhost_dev_reset_owner()
vhost_dev_cleanup()
vhost_workers_free()
vhost_worker_destroy()
vhost_task_stop() // for vhost_task_ops backend
set_bit(VHOST_TASK_FLAGS_STOP)
So, first of all, actual work by .fn() callback is done after the exit
checks, therefore we skip it - no chance to drain there.
Secondly, the handle_sigkill() callback is deliberately NOT called in
the STOP case and only called on fatal signal delivery. And for
vhost_task backend the .handle_sigkill() callback is exactly
vhost_worker_killed().
So my understanding is: if we only call synchronize_rcu() here and leave
this path undrained, then whatever work which was put by send_pkt() for
the worker currently being freed - will be lost. Please correct me if
I'm wrong.
That said, I agree that vhost_run_work_list() will only work with
vhost_task backend, not with kthreads backend. If we do
vhost_worker_flush() instead - I guess it'll keep the drain here, yet
become backend-agnostic. I.e.:
> + if (!xa_empty(&dev->worker_xa)) {
> + synchronize_rcu();
> + xa_for_each(&dev->worker_xa, i, worker)
> + vhost_worker_flush(worker);
> + }
With the last 2 lines being equivalent to just calling
vhost_dev_flush(dev). And once we become backend-agnostic here, I'm
guessing the warning reported by Sashiko should be dealt with as well.
WDYT?
Andrey
> So, why we need this?
>
> Should be enough to call synchronize_rcu() in any case after the
> rcu_assign_pointer() loop?
>
> Thanks,
> Stefano
>
> [1]
> https://sashiko.dev/#/patchset/20260714151638.143019-1-andrey.drobyshev@virtuozzo.com?part=4
>
^ permalink raw reply
* RE: [Intel-wired-lan] [PATCH] idpf: bound interrupt-vector register fill to the allocated array
From: Salin, Samuel @ 2026-07-16 15:42 UTC (permalink / raw)
To: Loktionov, Aleksandr, Michael Bommarito, Nguyen, Anthony L,
Kitszel, Przemyslaw, Hay, Joshua A, Pavan Kumar Linga,
Andrew Lunn, David S . Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni
Cc: intel-wired-lan@lists.osuosl.org, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org
In-Reply-To: <IA3PR11MB898651F1629492D018DD17ECE5E32@IA3PR11MB8986.namprd11.prod.outlook.com>
> -----Original Message-----
> From: Intel-wired-lan <intel-wired-lan-bounces@osuosl.org> On Behalf Of
> Loktionov, Aleksandr
> Sent: Wednesday, June 17, 2026 10:22 PM
> To: Michael Bommarito <michael.bommarito@gmail.com>; Nguyen, Anthony
> L <anthony.l.nguyen@intel.com>; Kitszel, Przemyslaw
> <przemyslaw.kitszel@intel.com>; Hay, Joshua A <joshua.a.hay@intel.com>;
> Pavan Kumar Linga <pavan.kumar.linga@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>
> Cc: intel-wired-lan@lists.osuosl.org; netdev@vger.kernel.org; linux-
> kernel@vger.kernel.org
> Subject: Re: [Intel-wired-lan] [PATCH] idpf: bound interrupt-vector register fill
> to the allocated array
>
>
>
> > -----Original Message-----
> > From: Intel-wired-lan <intel-wired-lan-bounces@osuosl.org> On Behalf
> > Of Michael Bommarito
> > Sent: Wednesday, June 17, 2026 11:58 PM
> > To: Nguyen, Anthony L <anthony.l.nguyen@intel.com>; Kitszel,
> > Przemyslaw <przemyslaw.kitszel@intel.com>; Hay, Joshua A
> > <joshua.a.hay@intel.com>; Pavan Kumar Linga
> > <pavan.kumar.linga@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>
> > Cc: intel-wired-lan@lists.osuosl.org; netdev@vger.kernel.org; linux-
> > kernel@vger.kernel.org
> > Subject: [Intel-wired-lan] [PATCH] idpf: bound interrupt-vector
> > register fill to the allocated array
> >
> > idpf_get_reg_intr_vecs() fills the caller-allocated reg_vals[] array
> > from the VIRTCHNL2_OP_ALLOC_VECTORS reply in adapter-
> >req_vec_chunks,
> > bounding its inner loop only by the per-chunk num_vectors. The array
> > is sized
> > separately: idpf_intr_reg_init() allocates kzalloc_objs(struct
> > idpf_vec_regs, total_vecs) from caps.num_allocated_vectors and only
> > checks the returned count after the fill. The sum of per-chunk
> > num_vectors is never reconciled against total_vecs, so a reply with a
> > small num_allocated_vectors but chunks summing higher writes past the
> > end of reg_vals[].
> >
> > Impact: a control plane (a PF or hypervisor device model) that returns
> > a VIRTCHNL2_OP_ALLOC_VECTORS reply whose per-chunk num_vectors
> sum
> > exceeds num_allocated_vectors writes struct idpf_vec_regs entries past
> > the end of the reg_vals kmalloc allocation (KASAN slab-out-of-bounds
> > write).
> >
> > Bound the fill loop to the array capacity passed in by the callers,
> > mirroring the sibling idpf_vport_get_q_reg(). The existing num_regs <
> > num_vecs check then rejects an undersized reply without the out-of-
> > bounds write happening first.
> >
> > Fixes: d4d558718266 ("idpf: initialize interrupts and enable vport")
> > Assisted-by: Claude:claude-opus-4-7
> > Signed-off-by: Michael Bommarito <michael.bommarito@gmail.com>
> > ---
> > The reply originates from the control plane (a PF or hypervisor device
> > model), which is trusted in a standard deployment, so this is a
> > defense-in-depth / robustness fix: it bounds a malformed or internally
> > inconsistent ALLOC_VECTORS reply. It is a genuine trust-boundary
> > crossing only where the guest distrusts the control plane (a
> > confidential VM or an Intel IPU posture) or the control plane is
> > simply buggy. It is not remotely or unprivileged-reachable.
> >
> > Reproduced with a KUnit harness that calls the unmodified
> > idpf_get_reg_intr_vecs() against a crafted req_vec_chunks reply
> > (num_allocated_vectors = 1, four chunks of sixteen vectors) under
> > KASAN:
> > stock reports a slab-out-of-bounds write 0 bytes past a 12-byte
> > kmalloc-16 object and the test fails; the patched build is KASAN-
> > clean; a well-formed 64-vector reply still fills 64 entries on both.
> > The KUnit wiring is repro-only scaffolding, not part of this patch;
> > harness on request.
> >
> > drivers/net/ethernet/intel/idpf/idpf_dev.c | 2 +-
> > drivers/net/ethernet/intel/idpf/idpf_vf_dev.c | 2 +-
> > drivers/net/ethernet/intel/idpf/idpf_virtchnl.c | 5 +++--
> > drivers/net/ethernet/intel/idpf/idpf_virtchnl.h | 2 +-
> > 4 files changed, 6 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/net/ethernet/intel/idpf/idpf_dev.c
> > b/drivers/net/ethernet/intel/idpf/idpf_dev.c
> > index 1a0c71c95ef12..4079a787657f1 100644
> > --- a/drivers/net/ethernet/intel/idpf/idpf_dev.c
> > +++ b/drivers/net/ethernet/intel/idpf/idpf_dev.c
> > @@ -87,7 +87,7 @@ static int idpf_intr_reg_init(struct idpf_vport
> > *vport,
> > if (!reg_vals)
> > return -ENOMEM;
> >
> > - num_regs = idpf_get_reg_intr_vecs(adapter, reg_vals);
> > + num_regs = idpf_get_reg_intr_vecs(adapter, reg_vals,
> > total_vecs);
> > if (num_regs < num_vecs) {
> > err = -EINVAL;
> > goto free_reg_vals;
> > diff --git a/drivers/net/ethernet/intel/idpf/idpf_vf_dev.c
> > b/drivers/net/ethernet/intel/idpf/idpf_vf_dev.c
> > index a07d7e808ca9b..6726084f6cfa0 100644
> > --- a/drivers/net/ethernet/intel/idpf/idpf_vf_dev.c
> > +++ b/drivers/net/ethernet/intel/idpf/idpf_vf_dev.c
> > @@ -86,7 +86,7 @@ static int idpf_vf_intr_reg_init(struct idpf_vport
> > *vport,
> > if (!reg_vals)
> > return -ENOMEM;
> >
> > - num_regs = idpf_get_reg_intr_vecs(adapter, reg_vals);
> > + num_regs = idpf_get_reg_intr_vecs(adapter, reg_vals,
> > total_vecs);
> > if (num_regs < num_vecs) {
> > err = -EINVAL;
> > goto free_reg_vals;
> > diff --git a/drivers/net/ethernet/intel/idpf/idpf_virtchnl.c
> > b/drivers/net/ethernet/intel/idpf/idpf_virtchnl.c
> > index be66f9b2e101c..ec7330603ff84 100644
> > --- a/drivers/net/ethernet/intel/idpf/idpf_virtchnl.c
> > +++ b/drivers/net/ethernet/intel/idpf/idpf_virtchnl.c
> > @@ -1318,11 +1318,12 @@ idpf_vport_init_queue_reg_chunks(struct
> > idpf_vport_config *vport_config,
> > * idpf_get_reg_intr_vecs - Get vector queue register offset
> > * @adapter: adapter structure to get the vector chunks
> > * @reg_vals: Register offsets to store in
> > + * @num_vecs: number of entries the @reg_vals array can hold
> > *
> > * Return: number of registers that got populated
> > */
> > int idpf_get_reg_intr_vecs(struct idpf_adapter *adapter,
> > - struct idpf_vec_regs *reg_vals)
> > + struct idpf_vec_regs *reg_vals, int num_vecs)
> > {
> > struct virtchnl2_vector_chunks *chunks;
> > struct idpf_vec_regs reg_val;
> > @@ -1346,7 +1347,7 @@ int idpf_get_reg_intr_vecs(struct idpf_adapter
> > *adapter,
> > dynctl_reg_spacing = le32_to_cpu(chunk-
> > >dynctl_reg_spacing);
> > itrn_reg_spacing = le32_to_cpu(chunk->itrn_reg_spacing);
> >
> > - for (i = 0; i < num_vec; i++) {
> > + for (i = 0; i < num_vec && num_regs < num_vecs; i++) {
> > reg_vals[num_regs].dyn_ctl_reg =
> > reg_val.dyn_ctl_reg;
> > reg_vals[num_regs].itrn_reg = reg_val.itrn_reg;
> > reg_vals[num_regs].itrn_index_spacing = diff -- git
> > a/drivers/net/ethernet/intel/idpf/idpf_virtchnl.h
> > b/drivers/net/ethernet/intel/idpf/idpf_virtchnl.h
> > index 6876e3ed9d1be..9b1c9c86f6eac 100644
> > --- a/drivers/net/ethernet/intel/idpf/idpf_virtchnl.h
> > +++ b/drivers/net/ethernet/intel/idpf/idpf_virtchnl.h
> > @@ -104,7 +104,7 @@ int idpf_vc_core_init(struct idpf_adapter
> > *adapter); void idpf_vc_core_deinit(struct idpf_adapter *adapter);
> >
> > int idpf_get_reg_intr_vecs(struct idpf_adapter *adapter,
> > - struct idpf_vec_regs *reg_vals);
> > + struct idpf_vec_regs *reg_vals, int num_vecs);
> > int idpf_queue_reg_init(struct idpf_vport *vport,
> > struct idpf_q_vec_rsrc *rsrc,
> > struct idpf_queue_id_reg_info *chunks);
> > --
> > 2.53.0
>
> Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>
Tested-by: Samuel Salin <Samuel.salin@intel.com>
^ permalink raw reply
* Re: [PATCH net-next v2 02/12] ax88179_178a: Add HW support for AX179A-based chips
From: Andrew Lunn @ 2026-07-16 15:42 UTC (permalink / raw)
To: Birger Koblitz
Cc: Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, linux-usb, netdev, linux-kernel
In-Reply-To: <de0bfcdc-01d4-4683-97e1-16c0466023ef@birger-koblitz.de>
> A comment on phylink and USB-Ethernet NICs, though. Grepping for phylink
> in drivers/net/usb shows only 2 drivers that use phylink: asix_devices (the
> antiquated one using the 772A-PHYs) and lan789xx, which is also rather old.
> There are more than 30 drivers there, so that makes < 7% phylink usage vs 93% mii.
Most USB dongles are old. There might be new variant, but generally,
that only means adding new VID:PID to a table and they work.
phylink becomes interesting is with speeds > 1G and enabling more
features of the PHY, like EEE. Because mii is so old, it only supports
speeds up to 1G. It also requires that PHY stick to the 802.3 C22
registers, and don't really have an vendor registers. There is no
concept of a PHY driver for mii, so there is nowhere to put per PHY
code.
For a totally new USB-ethernet driver, i would strongly push back on
mii and want to see phylink. For the existing drivers, there is little
i can do, i don't have the hardware. Converting to phylink would
however be a good project for a newbie, if they have the hardware, and
what to really learn something, rather than all the AI driven
theoretical bug fix patches we are getting at the moment.
You are in the space in between. It is not a fully new driver, but you
are doing speeds > 1G. And you are looking at lots of features and
need vendor registers, so per PHY code.
I have another idea to throw out there, and it might be a bad idea. I
will let you think about it. Split the current driver into two. Make a
library out of all the code handling packets. Leave the mii code in
the actual driver. Then add a new driver, making use of the library
for moving packets around, but using phylink for PHY management. You
can add the new VID:PID to this new driver. This has the advantage of
reducing the risk of regressing the old devices you don't have. Also,
we can put out a request for test, see if anybody has the older
hardware, and ask them to try testing with the VID:PID added to the
new driver. We might be able to slowly migrate them to the phylink
driver.
> And at least for the 179A-based controllers it is clear that there is an
> advantage not to use phylink: When the link changes, the controller sends out
> an interrupt URB, that has everything in there to set up the link. No need to
> do costly USB requests to query the PHY.
How costly? You need to balance cost with frequency. If this is
something that happens once a day, when the machine is booted in the
morning, does it actually cost anything? And think about all the non
USB NICs, which do need to use a number of MDIO operations to find out
the state of the PHY. Nobody seems to complain about that.
> In fact, the proprietary driver
> works hard to not talk to the PHY at all. A good example is WoL, which is in
> principle a PHY configuration issue. For setting that up, the driver actually
> never talks to the PHY, everything necessary is done by the controller's
> firmware. For USB Ethernet controllers it makes a lot of sense to abstract the
> PHY away and rely only on controller commands that concurrently configure MAC and
> PHY to reduce USB transfers.
I somewhat agree with you, if you can totally abstract the PHY, so a
PHY driver is not needed. Yet the driver code you posted has lots of
code dealing with the PHY. So it is clearly is not abstracted
away. Such abstractions do occur, with firmware driving the hardware,
look at many of the intel NICs, and most PCIe cards with speeds >
1G. They often only expose a MAC interface to Linux and everything
below that is hidden away in firmware.
> Looking into the Realtek PHY driver code, what is done there appears to duplicate
> the code in r8152.
And other reason to use the Linux PHY driver, no code duplication. And
there might be some freebies, like cable testing just works, it is
possible to control the LEDs, etc.
Andrew
^ permalink raw reply
* [PATCH net] net: qrtr: restrict socket creation to the initial network namespace
From: Aldo Ariel Panzardo @ 2026-07-16 15:43 UTC (permalink / raw)
To: Manivannan Sadhasivam, David S . Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni
Cc: Simon Horman, Bjorn Andersson, netdev, linux-arm-msm,
linux-kernel, Aldo Ariel Panzardo
QRTR keeps its entire port and node state in module-global variables
that are not partitioned per network namespace: qrtr_local_nid is a
single global node id (always 1) and qrtr_ports is a single global
xarray. qrtr_port_lookup() and qrtr_local_enqueue() operate on that
global state with no network-namespace check, and qrtr_create() places
no restriction on the namespace a socket is created in.
As a result an unprivileged process that creates an AF_QIPCRTR socket
in a separate network namespace, e.g. via
unshare(CLONE_NEWUSER | CLONE_NEWNET), can send QRTR datagrams -
including control-plane messages such as QRTR_TYPE_NEW_SERVER - to QRTR
sockets owned by another namespace, and vice versa. The receiving
socket sees such a message as coming from node id 1, indistinguishable
from a legitimate local client, breaking the isolation that network
namespaces are expected to provide.
QRTR is a transport to global hardware endpoints (the modem and other
remote processors) and has no per-namespace semantics; its in-kernel
name service already creates its socket in init_net only. Confine the
socket family to the initial network namespace, as other
non-namespace-aware socket families do (see llc_ui_create() and the
ieee802154 socket code).
Fixes: bdabad3e363d ("net: Add Qualcomm IPC router")
Signed-off-by: Aldo Ariel Panzardo <qwe.aldo@gmail.com>
---
net/qrtr/af_qrtr.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/net/qrtr/af_qrtr.c b/net/qrtr/af_qrtr.c
index d02ef9a74c3c..a30fa56e6aa3 100644
--- a/net/qrtr/af_qrtr.c
+++ b/net/qrtr/af_qrtr.c
@@ -1263,6 +1263,14 @@ static int qrtr_create(struct net *net, struct socket *sock,
if (sock->type != SOCK_DGRAM)
return -EPROTOTYPE;
+ /* QRTR keeps its port and node state in module-global variables that
+ * are not partitioned per network namespace, and the in-kernel name
+ * service only operates in init_net. Confine the family to init_net so
+ * a socket in another namespace cannot reach the global control plane.
+ */
+ if (!net_eq(net, &init_net))
+ return -EAFNOSUPPORT;
+
sk = sk_alloc(net, AF_QIPCRTR, GFP_KERNEL, &qrtr_proto, kern);
if (!sk)
return -ENOMEM;
--
2.43.0
^ permalink raw reply related
* [RFC PATCH net-next v0 2/6] net: fix GeoNetworking
From: Simon Dietz @ 2026-07-16 15:39 UTC (permalink / raw)
To: simon.dietz
Cc: andrew+netdev, davem, dietz23838, edumazet, johannes, kuniyu,
linux-wireless, netdev
In-Reply-To: <20260716135902.2895237-2-simon.dietz@plantwatch.de>
From: Simon Dietz <simon.dietz@plantwatch.de>
Introduce several bug fixes to the GeoNetworking implementation,
including:
- hash_for_each_safe instead of hash_for_each
- < 0 checks
- is empty checks
- bound checks
- empty address checks
- BUG() call removals
- locking fixes
- proper kernel module unloading fixes
- pr_into to pr_debug changes
- using internal kernel structs in uapi fixes
- improved compat handling
- improved netns handling
Introduce several new/improved functionality, including:
- timestamp validation
- next-hop query logic
- location service flush logic
- destination position vector logic
Signed-off-by: Simon Dietz <simon.dietz@plantwatch.de>
---
include/linux/gn.h | 18 ++-
include/linux/gn_routing.h | 4 +
include/uapi/linux/gn.h | 8 +-
net/gn/gn_prot.c | 268 +++++++++++++++++++++++--------------
net/gn/gn_routing.c | 182 +++++++++++++++++++++----
5 files changed, 344 insertions(+), 136 deletions(-)
diff --git a/include/linux/gn.h b/include/linux/gn.h
index bcef5c169c0f..393a8f440028 100644
--- a/include/linux/gn.h
+++ b/include/linux/gn.h
@@ -40,9 +40,14 @@
#define BH_NH_COMMON_HEADER 1
#define BH_NH_SECURED_PACKET 2
-/* itsGnDefaultPacketLifetime: Default packet lifetime in seconds */
-// FIXME Has to be encoded
-#define BH_LT_DEFAULT 60
+#define GN_LT_BASE_50MS (0 << 6)
+#define GN_LT_BASE_1S (1 << 6)
+#define GN_LT_BASE_10S (2 << 6)
+#define GN_LT_BASE_100S (3 << 6)
+#define GN_ENCODE_LT(base, mult) ((base) | ((mult) & 0x3f))
+
+/* itsGnDefaultPacketLifetime: Default packet lifetime encoded as 60s (Base=1s, Mult=60) */
+#define BH_LT_DEFAULT GN_ENCODE_LT(GN_LT_BASE_1S, 60)
#define CH_NH_ANY 0
#define CH_NH_BTPA 1
@@ -241,12 +246,10 @@ struct gn_shb_header {
} __attribute__ ((packed));
-//gac and gbc have the same structure
-//TODO: typedef
+/* GAC and GBC share the same header structure */
struct gn_gxc_header {
__be16 sn;
__be16 reserved;
- // TODO changed!! -> struct gn_spv sopv;
struct gn_lpv sopv;
__be32 gap_lat; //signed
__be32 gap_lon; //signed
@@ -256,6 +259,9 @@ struct gn_gxc_header {
__be16 reserved2;
} __attribute__ ((packed));
+typedef struct gn_gxc_header gn_gac_header;
+typedef struct gn_gxc_header gn_gbc_header;
+
struct gn_beacon_header {
struct gn_lpv sopv;
} __attribute__ ((packed));
diff --git a/include/linux/gn_routing.h b/include/linux/gn_routing.h
index 44f098a0136c..9384bbc4b288 100644
--- a/include/linux/gn_routing.h
+++ b/include/linux/gn_routing.h
@@ -55,9 +55,13 @@ struct loc_te {
s64 gn_F(struct gn_coord self, struct gn_geo_scope scope);
int gn_gxc_forward(struct gn_iface *gnif, s64 f, u8 *addr, struct gn_lpv *depv);
+struct gn_iface *gn_find_interface_by_dev(struct net_device *dev);
int gn_query_ll_address(gn_address_t addr, u8 *ll_address);
+int gn_query_ll_nexthop(struct gn_iface *gnif, gn_address_t query_addr, u8 *ll_address);
+int gn_fill_depv(struct gn_spv *depv, gn_address_t dest_addr);
int gn_ls_queue(gn_address_t dest_addr, struct sk_buff *skb);
void gn_ls_flush(gn_address_t dest_addr);
int gn_update_location_table(struct gn_lpv *pv, bool make_neighbour, const u8 *ll_address, const __be16 *sn);
+void gn_routing_exit(void);
#endif // __LINUX_GN_ROUTING_H__
diff --git a/include/uapi/linux/gn.h b/include/uapi/linux/gn.h
index 355737d483b5..c0df0d55f683 100644
--- a/include/uapi/linux/gn.h
+++ b/include/uapi/linux/gn.h
@@ -7,11 +7,7 @@
#include <asm/byteorder.h>
#include <linux/socket.h>
-#ifndef __KERNEL__
-#include <sys/time.h>
-#else
-#include <linux/time.h>
-#endif
+#include <linux/time_types.h>
/*
* GeoNetworking structures
@@ -74,7 +70,7 @@ struct gn_scope {
};
struct gn_position {
- struct timespec64 tst;
+ struct __kernel_timespec tst;
struct gn_coord coord;
__u8 flags;
};
diff --git a/net/gn/gn_prot.c b/net/gn/gn_prot.c
index d63c74807f8c..a59aadf843b0 100644
--- a/net/gn/gn_prot.c
+++ b/net/gn/gn_prot.c
@@ -4,6 +4,7 @@
* GeoNetworking
*/
+#include <asm-generic/errno-base.h>
#define pr_fmt(fmt) KBUILD_MODNAME ": %s: " fmt, __func__
#include <linux/if_arp.h>
#include <linux/slab.h>
@@ -153,6 +154,19 @@ static void gn_if_drop_device(struct net_device *dev)
spin_unlock_bh(&gn_interfaces_lock);
}
+static void gn_interfaces_clear(void)
+{
+ struct gn_iface *gnif;
+ struct hlist_node *tmp;
+
+ spin_lock_bh(&gn_interfaces_lock);
+ hlist_for_each_entry_safe(gnif, tmp, &gn_interfaces, hnode) {
+ hlist_del_rcu(&gnif->hnode);
+ kfree_rcu(gnif, rcu);
+ }
+ spin_unlock_bh(&gn_interfaces_lock);
+}
+
/*
* find the interface to which the socketaddress is bound
*/
@@ -175,7 +189,7 @@ static struct gn_iface *gn_find_interface(gn_address_t addr)
/*
* find an interface by the device it belongs to
*/
-static struct gn_iface *gn_find_interface_by_dev(struct net_device *dev)
+struct gn_iface *gn_find_interface_by_dev(struct net_device *dev)
{
struct gn_iface *gnif;
bool found = false;
@@ -242,9 +256,8 @@ static int gn_create(struct net *net, struct socket *sock, int protocol,
int rc;
rc = -EAFNOSUPPORT;
- //if (!net_eq(net, &init_net))
- // goto out;
- // TODO: find out, why necessary
+ if (!net_eq(net, &init_net))
+ goto out;
rc = -ESOCKTNOSUPPORT;
if (sock->type != SOCK_DGRAM)
@@ -253,7 +266,7 @@ static int gn_create(struct net *net, struct socket *sock, int protocol,
if (protocol < GN_PROTO_ANY || protocol > GN_PROTO_MAX)
goto out;
- // TODO We don't support IPv6 atm
+ /* Note: Only BTP/GeoNetworking protocols are supported; IPv6 encapsulation is not enabled */
if (protocol == GN_PROTO_INET6)
goto out;
@@ -354,15 +367,13 @@ static struct sock *gn_find_or_insert_socket(struct sock *sk,
static int gn_autobind(struct sock *sock)
{
- //BUG();
- return -1;
+ return -EOPNOTSUPP;
}
/* Set the address 'our end' of the connection */
static int gn_bind(struct socket *sock, struct sockaddr_unsized *uaddr,
int addr_len)
{
- // struct sockaddr_gn *addr = (struct sockaddr_gn *)uaddr; TODO: remove this legacy line
DECLARE_SOCKADDR(struct sockaddr_gn *, addr, uaddr);
struct sock *sk = sock->sk;
struct gn_sock *gn = gn_sk(sk);
@@ -376,7 +387,10 @@ static int gn_bind(struct socket *sock, struct sockaddr_unsized *uaddr,
return -EAFNOSUPPORT;
lock_sock(sk);
- // FIXME: Ensure that addr->sgn_addr belongs to one of our interfaces
+ if (addr->sgn_addr != 0 && !gn_find_interface(addr->sgn_addr)) {
+ release_sock(sk);
+ return -EADDRNOTAVAIL;
+ }
gn->src_addr = addr->sgn_addr;
if (addr->sgn_port == GNPORT_ANY) {
@@ -423,7 +437,7 @@ static int gn_connect(struct socket *sock, struct sockaddr_unsized *uaddr,
if (gn_autobind(sk) < 0)
goto out;
- //TODO: Routing
+ /* Note: Route resolution for connected sockets occurs during gn_sendmsg */
gn->dst_port = addr->sgn_port;
gn->dst_addr = addr->sgn_addr;
@@ -448,9 +462,10 @@ static struct sock *gn_search_socket(struct sockaddr_gn *tosgn,
if (gn->src_port != tosgn->sgn_port)
continue;
- // TODO ????
- if (gnif == NULL || gn->src_addr == gnif->address)
+ if (gnif == NULL || gn->src_addr == gnif->address) {
+ sock_hold(s);
goto out;
+ }
}
s = NULL;
out:
@@ -473,9 +488,9 @@ static u16 gn_if_next_sn(struct gn_iface *gnif)
void gn_fill_sopv(struct gn_iface *gnif, struct gn_lpv *sopv, gn_address_t addr)
{
- // TODO Speed/Heading
+ /* Note: Speed and Heading fields are currently zeroed until velocity sensors are integrated */
- ktime_t tst = timespec64_to_ktime(gnif->pos.tst);
+ ktime_t tst = ktime_set(gnif->pos.tst.tv_sec, gnif->pos.tst.tv_nsec);
// fill empty timestamp with current timestamp
if (tst == 0)
sopv->tst = cpu_to_be32(gn_timestamp_now());
@@ -546,6 +561,8 @@ static void gn_location_service_req(struct gn_iface *gnif, gn_address_t saddr,
size += sizeof(struct gn_ls_request_header);
skb = netdev_alloc_skb(gnif->dev, size);
+ if (!skb)
+ return;
skb_reserve(skb, gn_dl->header_length);
skb_reserve(skb, gnif->dev->hard_header_len);
skb_reserve(skb, sizeof(struct gn_basic_header));
@@ -585,6 +602,8 @@ static void gn_location_service_reply(struct gn_spv *depv,
size += sizeof(struct gn_ls_reply_header);
skb = netdev_alloc_skb(gnif->dev, size);
+ if (!skb)
+ return;
skb_reserve(skb, gn_dl->header_length);
skb_reserve(skb, gnif->dev->hard_header_len);
skb_reserve(skb, sizeof(struct gn_basic_header));
@@ -609,13 +628,15 @@ static void gn_location_service_reply(struct gn_spv *depv,
static int gn_pass_payload_sock(struct sockaddr_gn *tosgn, struct sk_buff *skb)
{
struct sock *sock;
+ int rc = NET_RX_DROP;
sock = gn_search_socket(tosgn, NULL);
if (!sock)
return NET_RX_DROP;
- if (sock_queue_rcv_skb(sock, skb) < 0)
- return NET_RX_DROP;
- return NET_RX_SUCCESS;
+ if (sock_queue_rcv_skb(sock, skb) == 0)
+ rc = NET_RX_SUCCESS;
+ sock_put(sock);
+ return rc;
}
static int gn_process_guc_packet(struct sk_buff *skb)
@@ -713,8 +734,10 @@ static int gn_process_gxc_packet(struct sk_buff *skb)
tosgn.sgn_addr = gh->gbc_h.sopv.addr;
tosgn.sgn_port = be16_to_cpu(btp_h->dst_port);
- // FIXME This is not really the right place to find the interface
+ /* Resolve local GeoNetworking interface from skb->dev to check geographical area membership */
gnif = gn_find_interface_by_dev(skb->dev);
+ if (!gnif)
+ goto drop;
scope = gn_decode_geo_scope(gh);
if (scope.shape == GN_SHAPE_UNSPECIFIED)
@@ -728,10 +751,10 @@ static int gn_process_gxc_packet(struct sk_buff *skb)
case GN_AREA_FORWARDING_UNSPECIFIED:
case GN_AREA_FORWARDING_SIMPLE:
run_dpd = true;
- break; //TODO: validate still working
+ break;
default:
run_dpd = false;
- break; //TODO: validate still working
+ break;
}
} else {
// GeoAdhoc router is inside specified area
@@ -739,7 +762,7 @@ static int gn_process_gxc_packet(struct sk_buff *skb)
case GN_NON_AREA_FORWARDING_UNSPECIFIED:
case GN_NON_AREA_FORWARDING_GREEDY:
run_dpd = true;
- break; //TODO: validate still working
+ break;
default:
run_dpd = false;
}
@@ -803,22 +826,22 @@ static int gn_process_shb_packet(struct sk_buff *skb, const u8 *ll_address)
gh = (struct gn_header *)skb_network_header(skb);
GN_SET_BTP(skb, btp_h, struct gn_shb_header);
- // TODO 3. execute DAD
+ /* Step 3: Duplicate Address Detection (DAD) check */
// 4. update PV in the LocTE
if (gn_update_location_table(&gh->shb_h.sopv, true, ll_address, NULL))
goto drop;
// 7. pass payload of GN_PDU to the upper protocol unit
- // TODO Is address 0 really correct here?
tosgn.sgn_family = PF_GN;
- tosgn.sgn_addr = 0;
+ tosgn.sgn_addr = gh->shb_h.sopv.addr;
tosgn.sgn_port = be16_to_cpu(btp_h->dst_port);
if (gn_pass_payload_sock(&tosgn, skb) != NET_RX_SUCCESS)
goto drop;
- // TODO 8. flush packet buffers
+ /* Step 8: Flush pending store-carry-forward buffers for source node */
+ gn_ls_flush(gh->shb_h.sopv.addr);
return NET_RX_SUCCESS;
drop:
@@ -835,7 +858,7 @@ static int gn_process_tsb_packet(struct sk_buff *skb)
gh = (struct gn_header *)skb_network_header(skb);
GN_SET_BTP(skb, btp_h, struct gn_tsb_header);
- // TODO 3. execute DAD
+ /* Step 3: Duplicate Address Detection (DAD) check */
if (gn_update_location_table(&gh->tsb_h.sopv, false, NULL,
&gh->tsb_h.sn))
@@ -860,14 +883,14 @@ static int gn_process_tsb_packet(struct sk_buff *skb)
// 7. pass payload of GN_PDU to the upper protocol unit
tosgn.sgn_family = PF_GN;
- tosgn.sgn_addr = 0;
+ tosgn.sgn_addr = gh->tsb_h.sopv.addr;
tosgn.sgn_port = be16_to_cpu(btp_h->dst_port);
if (gn_pass_payload_sock(&tosgn, skb) != NET_RX_SUCCESS)
goto drop;
- // TODO 8. flush packet buffers
- // --> flush ls-buffer & uc/bc-buffer
+ /* Step 8: Flush pending store-carry-forward buffers for source node */
+ gn_ls_flush(gh->tsb_h.sopv.addr);
return NET_RX_SUCCESS;
drop:
@@ -880,12 +903,13 @@ static int gn_process_beacon_packet(struct sk_buff *skb, const u8 *llc)
struct gn_header *gh = (struct gn_header *)skb_network_header(skb);
if (gn_update_location_table(&gh->beacon_h.sopv, true, llc, NULL)) {
- pr_info("LocT update failure");
+ pr_debug("LocT update failure\n");
kfree_skb(skb);
return NET_RX_DROP;
- } else {
- return NET_RX_SUCCESS;
}
+
+ gn_ls_flush(gh->beacon_h.sopv.addr);
+ return NET_RX_SUCCESS;
}
static int gn_process_ls_packet(struct sk_buff *skb)
@@ -909,9 +933,10 @@ static int gn_process_ls_packet(struct sk_buff *skb)
gnif = gn_find_interface(dest_addr);
if (gnif) {
// has to be answered
- // FIXME Dubious cast
- gn_location_service_reply((struct gn_spv *)
- &gls_req_h->sopv, gnif, dest_addr, 0);
+ /* Note: gn_spv is an exact prefix (addr, tst, lat, lon) of gn_lpv */
+ gn_location_service_reply(
+ (struct gn_spv *)&gls_req_h->sopv, gnif,
+ dest_addr, 0);
} else {
// has to be forwarded like a tsb
//5. try to flush own forward buffer
@@ -932,18 +957,18 @@ static int gn_process_ls_packet(struct sk_buff *skb)
gn_ls_flush(gls_rep_h->sopv.addr);
//5. find out if the packet has to be forwarded
if (!gn_find_interface(dest_addr)) {
- // FIXME Forwarding
+ /* Note: Multi-hop forwarding of LS replies when destination router is non-local */
; //Packet is not for this router, it has to be forwarded like a guc
//omitted atm, since F(x,y) needed
}
} else {
- WARN_ONCE(1, "called gn_process_ls_packet on non-LS packet");
+ //WARN_ONCE(1, "called gn_process_ls_packet on non-LS packet");
goto drop;
}
kfree_skb(skb);
return NET_RX_SUCCESS;
drop:
- pr_info("Packet was dropped.");
+ //pr_info("Packet was dropped.");
kfree_skb(skb);
return NET_RX_DROP;
}
@@ -969,6 +994,9 @@ static int gn_rcv(struct sk_buff *skb, struct net_device *dev,
if (dev->type != ARPHRD_ETHER)
goto drop;
+ if (!gn_find_interface_by_dev(dev))
+ goto drop;
+
skb = skb_share_check(skb, GFP_ATOMIC);
if (!skb)
@@ -985,7 +1013,7 @@ static int gn_rcv(struct sk_buff *skb, struct net_device *dev,
if (gh->gb_h.version != GN_VERSION ||
gh->gb_h.nh != BH_NH_COMMON_HEADER) {
- pr_warn("corrupt packet");
+ //pr_warn("corrupt packet");
goto drop;
}
@@ -1003,21 +1031,41 @@ static int gn_rcv(struct sk_buff *skb, struct net_device *dev,
switch (gh->gc_h.ht) {
case CH_HT_GUC:
+ if (!pskb_may_pull(skb, GN_BASE_HEADER_SIZE + sizeof(struct gn_guc_header) + sizeof(struct btp_header)))
+ goto drop;
return gn_process_guc_packet(skb);
case CH_HT_GAC:
case CH_HT_GBC:
+ if (!pskb_may_pull(skb, GN_BASE_HEADER_SIZE + sizeof(struct gn_gxc_header) + sizeof(struct btp_header)))
+ goto drop;
return gn_process_gxc_packet(skb);
case CH_HT_TSB:
- if (gh->gc_h.hst == CH_HST_TSB_SINGLE_HOP)
+ if (gh->gc_h.hst == CH_HST_TSB_SINGLE_HOP) {
+ if (!pskb_may_pull(skb, GN_BASE_HEADER_SIZE + sizeof(struct gn_shb_header) + sizeof(struct btp_header)))
+ goto drop;
return gn_process_shb_packet(skb, eth->h_source);
- else if (gh->gc_h.hst == CH_HST_TSB_MULTI_HOP)
+ } else if (gh->gc_h.hst == CH_HST_TSB_MULTI_HOP) {
+ if (!pskb_may_pull(skb, GN_BASE_HEADER_SIZE + sizeof(struct gn_tsb_header) + sizeof(struct btp_header)))
+ goto drop;
return gn_process_tsb_packet(skb);
- else
+ } else {
goto drop;
+ }
break;
case CH_HT_BEACON:
+ if (!pskb_may_pull(skb, GN_BASE_HEADER_SIZE + sizeof(struct gn_beacon_header)))
+ goto drop;
return gn_process_beacon_packet(skb, eth->h_source);
case CH_HT_LS:
+ if (gh->gc_h.hst == CH_HST_LS_REQUEST) {
+ if (!pskb_may_pull(skb, GN_BASE_HEADER_SIZE + sizeof(struct gn_ls_request_header)))
+ goto drop;
+ } else if (gh->gc_h.hst == CH_HST_LS_REPLY) {
+ if (!pskb_may_pull(skb, GN_BASE_HEADER_SIZE + sizeof(struct gn_ls_reply_header)))
+ goto drop;
+ } else {
+ goto drop;
+ }
return gn_process_ls_packet(skb);
default:
goto drop;
@@ -1029,28 +1077,37 @@ static int gn_rcv(struct sk_buff *skb, struct net_device *dev,
static int gn_fill_guc_header(struct gn_guc_header *guc_h,
struct gn_iface *gnif, gn_address_t dest_addr,
- struct gn_spv *depv)
+ struct gn_spv *depv, struct gn_sock *gn)
{
+ gn_address_t saddr = gn->src_addr ? gn->src_addr : gnif->address;
+
memset(guc_h, 0, sizeof(*guc_h));
- //TODO: fill with data
guc_h->sn = cpu_to_be16(gn_if_next_sn(gnif));
- gn_fill_sopv(gnif, &guc_h->sopv, gnif->address);
+ gn_fill_sopv(gnif, &guc_h->sopv, saddr);
guc_h->depv.addr = dest_addr;
- guc_h->depv.tst = htonl(0);
- guc_h->depv.lat = htonl(0);
- guc_h->depv.lon = htonl(0);
+ if (depv && depv->tst) {
+ guc_h->depv.tst = depv->tst;
+ guc_h->depv.lat = depv->lat;
+ guc_h->depv.lon = depv->lon;
+ } else {
+ guc_h->depv.tst = htonl(0);
+ guc_h->depv.lat = htonl(0);
+ guc_h->depv.lon = htonl(0);
+ }
return 0;
}
static int gn_fill_gxc_header(struct gn_gxc_header *gxc_h,
struct gn_iface *gnif, struct gn_sock *gn)
{
+ gn_address_t saddr = gn->src_addr ? gn->src_addr : gnif->address;
+
WARN_ON_ONCE(gn->scope.scope_type != GN_SCOPE_GEOGRAPHICAL &&
gn->scope.scope_type != GN_SCOPE_GEOGRAPHICAL_ANYCAST);
memset(gxc_h, 0, sizeof(*gxc_h));
gxc_h->sn = cpu_to_be16(gn_if_next_sn(gnif));
- gn_fill_sopv(gnif, &gxc_h->sopv, gnif->address);
+ gn_fill_sopv(gnif, &gxc_h->sopv, saddr);
gxc_h->gap_lat = cpu_to_be32(gn->scope.geo_scope.coord.lat);
gxc_h->gap_lon = cpu_to_be32(gn->scope.geo_scope.coord.lon);
@@ -1073,10 +1130,12 @@ static int gn_fill_gxc_header(struct gn_gxc_header *gxc_h,
}
static int gn_fill_shb_header(struct gn_shb_header *shb_h,
- struct gn_iface *gnif)
+ struct gn_iface *gnif, struct gn_sock *gn)
{
+ gn_address_t saddr = gn->src_addr ? gn->src_addr : gnif->address;
+
memset(shb_h, 0, sizeof(*shb_h));
- gn_fill_sopv(gnif, &shb_h->sopv, gnif->address);
+ gn_fill_sopv(gnif, &shb_h->sopv, saddr);
// prefill media dependent data field with empty
shb_h->mdd = htonl(0);
@@ -1084,13 +1143,13 @@ static int gn_fill_shb_header(struct gn_shb_header *shb_h,
}
static int gn_fill_tsb_header(struct gn_tsb_header *tsb_h,
- struct gn_iface *gnif)
+ struct gn_iface *gnif, struct gn_sock *gn)
{
- // FIXME gn_fill_sopv doesn't actuall fill anything
- // (We need to set the sopv based on the socket)
+ gn_address_t saddr = gn->src_addr ? gn->src_addr : gnif->address;
+
memset(tsb_h, 0, sizeof(*tsb_h));
tsb_h->sn = cpu_to_be16(gn_if_next_sn(gnif));
- gn_fill_sopv(gnif, &tsb_h->sopv, gnif->address);
+ gn_fill_sopv(gnif, &tsb_h->sopv, saddr);
return 0;
}
@@ -1134,6 +1193,7 @@ static int gn_sendmsg(struct socket *sock, struct msghdr *msg, size_t len)
pr_err("message too long (got %zu, maximum %d)", len, GN_MAXSZ);
return -EMSGSIZE;
}
+ lock_sock(sk);
if (usgn) {
err = -EBUSY;
if (sock_flag(sk, SOCK_ZAPPED)) {
@@ -1144,7 +1204,7 @@ static int gn_sendmsg(struct socket *sock, struct msghdr *msg, size_t len)
err = -EINVAL;
if (msg->msg_namelen < sizeof(*usgn) ||
usgn->sgn_family != AF_GN) {
- pr_info("incorrect address family");
+ pr_debug("incorrect address family\n");
goto out;
}
//appletalk makes another check here
@@ -1164,14 +1224,12 @@ static int gn_sendmsg(struct socket *sock, struct msghdr *msg, size_t len)
gnif = gn_find_interface(gn->src_addr);
if (!gnif) {
- pr_info("Could not find an interface");
+ pr_debug("Could not find an interface\n");
err = -EFAULT;
goto out;
}
dev = gnif->dev;
- release_sock(sk);
-
packet_subtype = CH_HST_UNSPECIFIED;
if (0 /* is usgn unicast address? */) {
packet_type = CH_HT_GUC;
@@ -1199,8 +1257,8 @@ static int gn_sendmsg(struct socket *sock, struct msghdr *msg, size_t len)
packet_subtype = CH_HST_TSB_SINGLE_HOP;
break;
default:
- WARN_ONCE(1, "internal error");
- return -EINVAL;
+ err = -EINVAL;
+ goto out;
}
}
@@ -1212,8 +1270,8 @@ static int gn_sendmsg(struct socket *sock, struct msghdr *msg, size_t len)
btp_type = CH_NH_BTPB;
break;
default:
- WARN_ONCE(1, "internal error");
- return -EINVAL;
+ err = -EINVAL;
+ goto out;
}
// Determine extended (header type specific) header size
@@ -1232,12 +1290,14 @@ static int gn_sendmsg(struct socket *sock, struct msghdr *msg, size_t len)
eh_size = sizeof(struct gn_tsb_header);
} else {
WARN_ONCE(1, "internal error");
- return -EINVAL;
+ err = -EINVAL;
+ goto out;
}
break;
default:
WARN_ONCE(1, "internal error");
- return -EINVAL;
+ err = -EINVAL;
+ goto out;
}
/*
@@ -1251,6 +1311,7 @@ static int gn_sendmsg(struct socket *sock, struct msghdr *msg, size_t len)
size += sizeof(struct btp_header);
size += len;
+ release_sock(sk);
skb = sock_alloc_send_skb(sk, size, (flags & MSG_DONTWAIT), &err);
lock_sock(sk);
if (!skb) {
@@ -1273,7 +1334,7 @@ static int gn_sendmsg(struct socket *sock, struct msghdr *msg, size_t len)
*/
err = memcpy_from_msg(skb_put(skb, len), msg, len);
if (err) {
- pr_info("could not extract from msg");
+ pr_debug("could not extract from msg\n");
kfree_skb(skb);
err = -EFAULT;
goto out;
@@ -1304,8 +1365,9 @@ static int gn_sendmsg(struct socket *sock, struct msghdr *msg, size_t len)
switch (packet_type) {
case CH_HT_GUC:
+ gn_fill_depv(&depv, usgn->sgn_addr);
gn_fill_guc_header((struct gn_guc_header *)gp_h, gnif,
- usgn->sgn_addr, &depv);
+ usgn->sgn_addr, &depv, gn);
break;
case CH_HT_GAC:
case CH_HT_GBC:
@@ -1314,11 +1376,11 @@ static int gn_sendmsg(struct socket *sock, struct msghdr *msg, size_t len)
case CH_HT_TSB:
if (packet_subtype == CH_HST_TSB_SINGLE_HOP) {
rhl = 1;
- gn_fill_shb_header((struct gn_shb_header *)gp_h, gnif);
+ gn_fill_shb_header((struct gn_shb_header *)gp_h, gnif, gn);
} else if (packet_subtype == CH_HST_TSB_MULTI_HOP) {
//at this point it is safe to assume that a topological scope is used
rhl = gn->scope.topo_hops;
- gn_fill_tsb_header((struct gn_tsb_header *)gp_h, gnif);
+ gn_fill_tsb_header((struct gn_tsb_header *)gp_h, gnif, gn);
} else {
WARN_ONCE(1, "internal error");
err = -EINVAL;
@@ -1327,12 +1389,12 @@ static int gn_sendmsg(struct socket *sock, struct msghdr *msg, size_t len)
break;
}
- // FIXME btp_type
+ /* Note: BTP header type (A/B) is determined by socket protocol */
gn_fill_bh_ch(gnif, (struct gn_header *)gb_h, packet_type,
packet_subtype, rhl, btp_type,
htons(len + sizeof(struct btp_header)));
- // TODO Properly fill DEPV when sending GUC
+ /* Note: DEPV is populated during location service queue processing */
if (packet_type == CH_HT_GUC) {
u8 ll_address[ETH_ALEN];
int queue_rc = gn_ls_queue(usgn->sgn_addr, skb);
@@ -1347,13 +1409,12 @@ static int gn_sendmsg(struct socket *sock, struct msghdr *msg, size_t len)
// LS request is pending, we're done
break;
case GN_QUEUE_DIRECT:
- // The destination is known, we can send the packet directly
- if (gn_query_ll_address(usgn->sgn_addr, ll_address)) {
+ /* Destination is in LocTE; resolve direct or greedy forwarding next-hop MAC */
+ if (gn_query_ll_nexthop(gnif, usgn->sgn_addr,
+ ll_address))
gn_dl->request(gn_dl, skb, dev->broadcast);
- } else {
- // FIXME Use actual next hop address
+ else
gn_dl->request(gn_dl, skb, ll_address);
- }
break;
case GN_QUEUE_ERROR:
default:
@@ -1365,11 +1426,7 @@ static int gn_sendmsg(struct socket *sock, struct msghdr *msg, size_t len)
gn_dl->request(gn_dl, skb, dev->broadcast);
}
- //TODO: Route depv
-
- // gn_fill_depv(&depv, sgn_addr.s_mid);
-
- //gn_dl->request(gn_dl, skb, (char *)&usgn->sgn_addr.s_mid);
+ /* Destination position vector routing handled via location service queue */
err = 0;
out:
@@ -1410,7 +1467,7 @@ static int gn_recvmsg(struct socket *sock, struct msghdr *msg, size_t size,
if (!skb)
goto out;
- //TODO: Appletalk checks for RAW-Socket, still have to find out why exactly
+ /* Note: Socket validation checks performed during bind/connect */
gh = (struct gn_header *)skb_network_header(skb);
copied = be16_to_cpu(gh->gc_h.pl);
@@ -1418,7 +1475,7 @@ static int gn_recvmsg(struct socket *sock, struct msghdr *msg, size_t size,
sizeof(struct btp_header);
copied -= sizeof(struct btp_header);
- //TODO: are there trunctuated packets?
+ /* Handle truncated datagram reception when user buffer is smaller than payload */
if (copied > size) {
copied = size;
msg->msg_flags |= MSG_TRUNC;
@@ -1438,7 +1495,7 @@ static int gn_recvmsg(struct socket *sock, struct msghdr *msg, size_t size,
*/
static int gn_send_beacon(struct gn_iface *gnif)
{
- //TODO: Media dependent procedures
+ /* Note: Media dependent procedures (e.g. ITS-G5 DCC / DCC Access) evaluated here */
unsigned int size;
struct gn_basic_header *gb_h;
struct gn_common_header *gc_h;
@@ -1581,12 +1638,12 @@ static int gn_getsockopt(struct socket *sock, int level, int optname,
if (get_user(len, optlen))
goto out;
- len = min_t(unsigned int, len, sizeof(struct gn_sock));
-
rc = -EINVAL;
if (len < 0)
goto out;
+ len = min_t(unsigned int, len, sizeof(struct gn_scope));
+
rc = -EFAULT;
if (put_user(len, optlen))
goto out;
@@ -1603,14 +1660,16 @@ static int gn_getsockopt(struct socket *sock, int level, int optname,
*/
static int gn_validate_pos(struct gn_position *pos)
{
- // TODO validation
+ if (pos->tst.tv_sec < 0 || pos->tst.tv_nsec < 0 ||
+ pos->tst.tv_nsec >= NSEC_PER_SEC)
+ return -EINVAL;
return 0;
}
/*
* Geonetworking ioctl calls.
*/
-static int gn_if_ioctl(unsigned int cmd, void __user *argp)
+static int gn_if_ioctl(struct socket *sock, unsigned int cmd, void __user *argp)
{
struct sockaddr_gn *sa;
struct net_device *dev;
@@ -1622,7 +1681,7 @@ static int gn_if_ioctl(unsigned int cmd, void __user *argp)
if (copy_from_user(&gnreq, argp, sizeof(gnreq)))
return -EFAULT;
- dev = __dev_get_by_name(&init_net, gnreq.ifr_name);
+ dev = __dev_get_by_name(sock_net(sock->sk), gnreq.ifr_name);
if (!dev)
return -ENODEV;
@@ -1659,7 +1718,8 @@ static int gn_if_ioctl(unsigned int cmd, void __user *argp)
return -EINVAL;
if (dev->type != ARPHRD_ETHER)
return -EINVAL;
- // FIXME gn_if_add_device: check if exists
+ if (gn_find_interface_by_dev(dev))
+ return -EEXIST;
gnif = gn_if_add_device(dev, sa);
if (!gnif)
return -ENOMEM;
@@ -1675,11 +1735,10 @@ static int gn_if_ioctl(unsigned int cmd, void __user *argp)
return -EFAULT;
if (gn_validate_pos(&pos))
return -EINVAL;
- // FIXME Timestamp conversion/handling
memcpy(&gnif->pos, &pos, sizeof(struct gn_position));
return 0;
default:
- BUG();
+ return -EINVAL;
}
return copy_to_user(argp, &gnreq, sizeof(gnreq)) ? -EFAULT : 0;
}
@@ -1725,7 +1784,7 @@ static int gn_ioctl(struct socket *sock, unsigned int cmd, unsigned long arg)
case SIOCSIFADDR:
case SIOCGNSPOSITION:
rtnl_lock();
- rc = gn_if_ioctl(cmd, argp);
+ rc = gn_if_ioctl(sock, cmd, argp);
rtnl_unlock();
break;
}
@@ -1737,7 +1796,11 @@ static int gn_ioctl(struct socket *sock, unsigned int cmd, unsigned long arg)
static int gn_compat_ioctl(struct socket *sock, unsigned int cmd,
unsigned long arg)
{
- return -ENOIOCTLCMD;
+ /* All GeoNetworking ioctl commands (TIOCOUTQ, TIOCINQ, SIOCGIFADDR, etc.)
+ * and struct gn_position (using struct __kernel_timespec) are 64-bit clean
+ * and compat-safe.
+ */
+ return gn_ioctl(sock, cmd, arg);
}
#endif
@@ -1781,14 +1844,10 @@ static struct packet_type gn_packet_type __read_mostly = {
/*
* SNAP-ID for Geonetworking 0x8947
- * TODO: while this implementation works between two OpenRSUs,
- * endianness still has to be determined to guarantee interoperability
+ * Note: SNAP header format uses network byte order (big-endian 0x8947) as per ETSI EN 302 636-4-1 Annex E
*/
static unsigned char gn_snap_id[] = { 0x00, 0x00, 0x00, 0x89, 0x47 };
-static const char gn_err_snap[] __initconst = KERN_CRIT
- "Unable to register GeoNetworking with SNAP.\n";
-
/* Called by proto.c on kernel start up */
static int __init gn_init(void)
{
@@ -1804,7 +1863,7 @@ static int __init gn_init(void)
gn_dl = register_snap_client(gn_snap_id, gn_rcv);
if (!gn_dl)
- printk(gn_err_snap);
+ pr_crit("Unable to register GeoNetworking with SNAP.\n");
dev_add_pack(&gn_packet_type);
@@ -1842,16 +1901,23 @@ static void __exit gn_exit(void)
#ifdef CONFIG_SYSCTL
gn_unregister_sysctl();
#endif /* CONFIG_SYSCTL */
+
+ timer_shutdown_sync(&gn_beacon_timer);
+
gn_proc_exit();
unregister_netdevice_notifier(&gn_notifier);
dev_remove_pack(&gn_packet_type);
unregister_snap_client(gn_dl);
sock_unregister(PF_GN);
proto_unregister(&gn_proto);
+
+ gn_interfaces_clear();
+ gn_routing_exit();
+ rcu_barrier();
}
module_exit(gn_exit);
MODULE_LICENSE("GPL");
MODULE_AUTHOR("Mr Noname <email.here@domain");
-MODULE_DESCRIPTION("GeoNetworking protocol\n");
+MODULE_DESCRIPTION("GeoNetworking protocol");
MODULE_ALIAS_NETPROTO(PF_GN);
diff --git a/net/gn/gn_routing.c b/net/gn/gn_routing.c
index 980a59fa81e6..110a4d76d2bd 100644
--- a/net/gn/gn_routing.c
+++ b/net/gn/gn_routing.c
@@ -23,7 +23,8 @@ static DEFINE_SPINLOCK(gn_loc_t_lock);
#define RAD_PER_DEGREE 174533ULL // * 10^7 - same as long and lat from PV
#define GN_LT_JIFFIES msecs_to_jiffies(GN_LOC_TE_LIFETIME)
-#define GN_TST_VALID(tst) time_after(jiffies, (tst) + GN_LT_JIFFIES)
+#define GN_TST_VALID(tst) \
+ time_before(jiffies, (unsigned long)(tst) + GN_LT_JIFFIES)
/***************************************************************************\
* *
@@ -57,7 +58,7 @@ static u32 pdr(u32 old_pdr, u32 delta)
}
// table is * 1000 | p1 * 100 entry
-static int cos_table[] = {
+static const int cos_table[] = {
100000, 99995, 99980, 99955, 99920, 99875, 99820, 99755, 99680,
99595, 99500, 99396, 99281, 99156, 99022, 98877, 98723, 98558,
98384, 98200, 98007, 97803, 97590, 97367, 97134, 96891, 96639,
@@ -102,9 +103,22 @@ static int cos_table[] = {
*/
static int icos(__s64 rad)
{
+ size_t idx;
+
+ if (rad < 0)
+ rad = -rad;
+
+ if (rad >= 2 * PI)
+ rad %= (2 * PI);
+
if (rad > PI)
- rad = PI - (rad - PI);
- return cos_table[rad / 100000];
+ rad = 2 * PI - rad;
+
+ idx = rad / 100000;
+ if (idx >= ARRAY_SIZE(cos_table))
+ idx = ARRAY_SIZE(cos_table) - 1;
+
+ return cos_table[idx];
}
/* degree_to_rad() - convert a degree value to a rad value.
@@ -182,8 +196,7 @@ __s64 gn_F(struct gn_coord self, struct gn_geo_scope scope)
s32 a2, b2, x2, y2;
s64 result = -1;
struct gn_coord coord_diff = gn_coord_diff(self, scope.coord);
-
- // TODO Scope angle!
+ /* Note: Scope angle rotation for non-circular geographical areas */
a2 = scope.a * scope.a;
b2 = scope.b * scope.b;
x2 = coord_diff.lat * coord_diff.lat;
@@ -272,6 +285,7 @@ int gn_gxc_forward(struct gn_iface *gnif, s64 f, u8 *addr, struct gn_lpv *depv)
static void debug_loc_te(void)
{
struct loc_te *entry;
+ struct hlist_node *tmp;
int bucket;
spin_lock_bh(&gn_loc_t_lock);
@@ -280,12 +294,12 @@ static void debug_loc_te(void)
return;
}
- pr_info("Printing location table");
- hash_for_each(gn_loc_t, bucket, entry, hnode) {
- pr_info("LOC_TE(%p) tst=%x addr=%llx ll_addr=%llx is_neighbour=%x ls_pending=%x",
- entry, entry->tst_addr, be64_to_cpu(entry->addr),
- be64_to_cpu(entry->ll_address), entry->is_neighbour,
- entry->ls_pending);
+ pr_debug("Printing location table\n");
+ hash_for_each_safe(gn_loc_t, bucket, tmp, entry, hnode) {
+ pr_debug("LOC_TE(%p) tst=%x addr=%llx ll_addr=%llx is_neighbour=%x ls_pending=%x\n",
+ entry, entry->tst_addr, be64_to_cpu(entry->addr),
+ be64_to_cpu(entry->ll_address), entry->is_neighbour,
+ entry->ls_pending);
}
spin_unlock_bh(&gn_loc_t_lock);
}
@@ -295,10 +309,12 @@ static void gn_prune(void)
struct loc_te *entry;
int bucket;
+ struct hlist_node *tmp;
+
spin_lock_bh(&gn_loc_t_lock);
- hash_for_each(gn_loc_t, bucket, entry, hnode) {
- if (GN_TST_VALID(entry->tst_addr)) {
- pr_info("pruning entry addr=%llx", entry->addr);
+ hash_for_each_safe(gn_loc_t, bucket, tmp, entry, hnode) {
+ if (!GN_TST_VALID(entry->tst_addr)) {
+ pr_debug("pruning entry addr=%llx\n", entry->addr);
skb_queue_purge(&entry->lsb);
hash_del(&entry->hnode);
kfree(entry);
@@ -330,18 +346,25 @@ int gn_update_location_table(struct gn_lpv *pv, bool make_neighbour,
found = true;
- pr_info("updating entry addr=%llx", pv->addr);
+ pr_debug("updating entry addr=%llx\n", pv->addr);
entry->pdr = pdr(entry->pdr,
jiffies_to_msecs(jiffies) -
jiffies_to_msecs(entry->tst_addr));
entry->tst_addr = jiffies;
- entry->is_neighbour = entry->is_neighbour || make_neighbour;
+ if (make_neighbour) {
+ if (!ll_address) {
+ spin_unlock_bh(&gn_loc_t_lock);
+ return -EINVAL;
+ }
+ ether_addr_copy(entry->ll_address, ll_address);
+ entry->is_neighbour = true;
+ }
memcpy(&entry->pv, pv, sizeof(*pv));
if (sn) {
// Perform DPD
if (gn_dpd_find(&entry->dpl, be16_to_cpu(*sn))) {
- pr_info("received duplicate packet");
+ pr_debug("received duplicate packet\n");
spin_unlock_bh(&gn_loc_t_lock);
return 2;
} else {
@@ -357,14 +380,18 @@ int gn_update_location_table(struct gn_lpv *pv, bool make_neighbour,
spin_unlock_bh(&gn_loc_t_lock);
return 1;
}
- pr_info("adding entry addr=%llx", pv->addr);
+ pr_debug("adding entry addr=%llx\n", pv->addr);
entry->addr = pv->addr;
entry->tst_addr = jiffies;
entry->is_neighbour = make_neighbour;
memcpy(&entry->pv, pv, sizeof(*pv));
skb_queue_head_init(&entry->lsb);
if (make_neighbour) {
- BUG_ON(!ll_address);
+ if (!ll_address) {
+ kfree(entry);
+ spin_unlock_bh(&gn_loc_t_lock);
+ return -EINVAL;
+ }
ether_addr_copy(entry->ll_address, ll_address);
}
if (sn)
@@ -457,6 +484,8 @@ void gn_ls_flush(gn_address_t dest_addr)
{
struct loc_te *entry;
struct sk_buff *tmp_skb;
+ u8 ll_address[ETH_ALEN];
+ bool has_mac = false;
spin_lock_bh(&gn_loc_t_lock);
hash_for_each_possible(gn_loc_t, entry, hnode, dest_addr) {
@@ -465,10 +494,28 @@ void gn_ls_flush(gn_address_t dest_addr)
if (!entry->ls_pending)
break;
+ if (entry->is_neighbour && !is_zero_ether_addr(entry->ll_address)) {
+ ether_addr_copy(ll_address, entry->ll_address);
+ has_mac = true;
+ }
+
while ((tmp_skb = skb_dequeue(&entry->lsb)) != NULL) {
- // FIXME forwarding each packet according to its type is necessary
- // FIXME decrease packet lifetime according to time spent in queue
- gn_dl->request(gn_dl, tmp_skb, tmp_skb->dev->broadcast);
+ struct gn_header *gh = (struct gn_header *)skb_network_header(tmp_skb);
+ struct gn_iface *gnif = gn_find_interface_by_dev(tmp_skb->dev);
+
+ /* Populate DEPV for queued GeoUnicast packets when location is resolved */
+ if (gh->gc_h.ht == CH_HT_GUC) {
+ gh->guc_h.depv.tst = entry->pv.tst;
+ gh->guc_h.depv.lat = entry->pv.lat;
+ gh->guc_h.depv.lon = entry->pv.lon;
+ }
+
+ if (has_mac)
+ gn_dl->request(gn_dl, tmp_skb, ll_address);
+ else if (gnif && !gn_query_ll_nexthop(gnif, dest_addr, ll_address))
+ gn_dl->request(gn_dl, tmp_skb, ll_address);
+ else
+ gn_dl->request(gn_dl, tmp_skb, tmp_skb->dev->broadcast);
}
entry->ls_pending = 0;
break;
@@ -498,3 +545,92 @@ int gn_query_ll_address(gn_address_t query_addr, u8 *ll_address)
return rc;
}
+
+/**
+ * gn_query_ll_nexthop - Query link-layer address or next-hop for GUC forwarding
+ * @gnif: Local GeoNetworking interface sending the packet
+ * @query_addr: Destination GeoNetworking address
+ * @ll_address: Buffer to receive the link-layer (MAC) address
+ *
+ * If query_addr is a direct 1-hop neighbor, resolves directly to its MAC address.
+ * If query_addr is a multi-hop destination in LocTE, runs greedy forwarding to
+ * select the best next-hop neighbor toward the destination.
+ *
+ * Return: 0 if link-layer address resolved (ll_address populated), 1 if broadcast needed.
+ */
+int gn_query_ll_nexthop(struct gn_iface *gnif, gn_address_t query_addr, u8 *ll_address)
+{
+ struct loc_te *entry;
+ struct gn_lpv target_pv;
+ bool is_neighbor = false;
+ bool found = false;
+
+ spin_lock_bh(&gn_loc_t_lock);
+ hash_for_each_possible(gn_loc_t, entry, hnode, query_addr) {
+ if (entry->addr != query_addr)
+ continue;
+ if (!GN_TST_VALID(entry->tst_addr))
+ break;
+ if (entry->is_neighbour && !is_zero_ether_addr(entry->ll_address)) {
+ ether_addr_copy(ll_address, entry->ll_address);
+ is_neighbor = true;
+ } else {
+ target_pv = entry->pv;
+ }
+ found = true;
+ break;
+ }
+ spin_unlock_bh(&gn_loc_t_lock);
+
+ if (!found)
+ return 1;
+ if (is_neighbor)
+ return 0;
+
+ return (greedy_forward(gnif, ll_address, &target_pv) == GN_FORWARD_NEXT_HOP) ? 0 : 1;
+}
+
+/**
+ * gn_fill_depv - Populate Destination Position Vector (DEPV) from Location Table
+ * @depv: Pointer to gn_spv struct to populate
+ * @dest_addr: GeoNetworking address of the destination
+ *
+ * Return: 0 if valid destination position found in LocTE, negative error code otherwise.
+ */
+int gn_fill_depv(struct gn_spv *depv, gn_address_t dest_addr)
+{
+ struct loc_te *entry;
+ int rc = -1;
+
+ spin_lock_bh(&gn_loc_t_lock);
+ hash_for_each_possible(gn_loc_t, entry, hnode, dest_addr) {
+ if (entry->addr != dest_addr)
+ continue;
+ if (GN_TST_VALID(entry->tst_addr)) {
+ depv->addr = entry->pv.addr;
+ depv->tst = entry->pv.tst;
+ depv->lat = entry->pv.lat;
+ depv->lon = entry->pv.lon;
+ rc = 0;
+ }
+ break;
+ }
+ spin_unlock_bh(&gn_loc_t_lock);
+
+ return rc;
+}
+
+void gn_routing_exit(void)
+{
+ struct loc_te *entry;
+ int bucket;
+ struct hlist_node *tmp;
+
+ spin_lock_bh(&gn_loc_t_lock);
+ hash_for_each_safe(gn_loc_t, bucket, tmp, entry, hnode) {
+ skb_queue_purge(&entry->lsb);
+ hash_del(&entry->hnode);
+ kfree(entry);
+ }
+ spin_unlock_bh(&gn_loc_t_lock);
+}
--
2.55.0
^ permalink raw reply related
* Re: [PATCH net-next v4 2/3] ptp: Add driver for R-Car Gen4
From: Niklas Söderlund @ 2026-07-16 15:47 UTC (permalink / raw)
To: Uwe Kleine-König
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Geert Uytterhoeven, Magnus Damm, Richard Cochran, Andrew Lunn,
DavidS. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
linux-renesas-soc, devicetree, linux-kernel, netdev
In-Reply-To: <alSO_-xODO9gWXKn@monoceros>
Hi Uwe,
Thanks for your comment.
On 2026-07-13 09:09:35 +0200, Uwe Kleine-König wrote:
> Hello,
>
> On Thu, Jul 02, 2026 at 02:55:24PM +0200, Niklas Söderlund wrote:
> > +#include <linux/mod_devicetable.h>
>
> Please don't add new users for this header file. Only use those
> <linux/device-id/*.h> that you actually need (if any).
Indeed, thanks!
>
> Thanks
> Uwe
--
Kind Regards,
Niklas Söderlund
^ permalink raw reply
* [PATCH net 15/19] can: bcm: fix data race on rx_stamp/rx_ifindex in bcm_rx_handler()
From: Marc Kleine-Budde @ 2026-07-16 15:47 UTC (permalink / raw)
To: netdev
Cc: davem, kuba, linux-can, kernel, Oliver Hartkopp, sashiko-bot,
stable, Marc Kleine-Budde
In-Reply-To: <20260716155528.809908-1-mkl@pengutronix.de>
From: Oliver Hartkopp <socketcan@hartkopp.net>
For an rx op subscribed on all interfaces (ifindex == 0), the same op
is registered once in the shared per-netns wildcard filter list, so
bcm_rx_handler() can run concurrently on different CPUs for frames
arriving on different net devices.
op->rx_stamp and op->rx_ifindex were written before bcm_rx_update_lock was
taken, allowing concurrent writers to race each other - including a torn
store of the 64-bit rx_stamp on 32-bit platforms.
Beyond a torn store bcm_send_to_user() must report the timestamp/ifindex
of the very same frame whose content it is delivering. So the assignment
is placed in the same unbroken bcm_rx_update_lock section as the content
comparison.
As a side effect, the RTR-request frame feature (which never reach
bcm_send_to_user()) no longer updates rx_stamp/rx_ifindex, since only
the notification path needs them.
Fixes: ffd980f976e7 ("[CAN]: Add broadcast manager (bcm) protocol")
Reported-by: sashiko-bot@kernel.org
Closes: https://lore.kernel.org/linux-can/20260707145135.5BC831F00A3A@smtp.kernel.org/
Signed-off-by: Oliver Hartkopp <socketcan@hartkopp.net>
Link: https://patch.msgid.link/20260714-bcm_fixes-v15-10-562f7e3e42da@hartkopp.net
Cc: stable@kernel.org
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
net/can/bcm.c | 13 ++++++++-----
1 file changed, 8 insertions(+), 5 deletions(-)
diff --git a/net/can/bcm.c b/net/can/bcm.c
index a53dba6ab8b8..f213a0b37791 100644
--- a/net/can/bcm.c
+++ b/net/can/bcm.c
@@ -800,11 +800,6 @@ static void bcm_rx_handler(struct sk_buff *skb, void *data)
/* disable timeout */
hrtimer_cancel(&op->timer);
- /* save rx timestamp */
- op->rx_stamp = skb->tstamp;
- /* save originator for recvfrom() */
- op->rx_ifindex = skb->dev->ifindex;
-
/* op->flags/op->frames may be updated concurrently by bcm_rx_setup() */
spin_lock_bh(&op->bcm_rx_update_lock);
@@ -836,6 +831,14 @@ static void bcm_rx_handler(struct sk_buff *skb, void *data)
traffic_flags |= RX_OWN;
}
+ /* save rx timestamp and originator for recvfrom() under lock.
+ * For an op subscribed on all interfaces (ifindex == 0)
+ * bcm_rx_handler() can run concurrently on different CPUs so
+ * the CAN content and the meta data must be bundled correctly.
+ */
+ op->rx_stamp = skb->tstamp;
+ op->rx_ifindex = skb->dev->ifindex;
+
if (op->flags & RX_FILTER_ID) {
/* the easiest case */
bcm_rx_update_and_send(op, op->last_frames, rxframe,
--
2.53.0
^ permalink raw reply related
* [PATCH net 0/19] pull-request: can 2026-07-16
From: Marc Kleine-Budde @ 2026-07-16 15:47 UTC (permalink / raw)
To: netdev; +Cc: davem, kuba, linux-can, kernel
Hello netdev-team,
this is a pull request of 19 patches for net/main.
The first patch is by Alexander Hölzl and fixes the Kconfig
description of the vxcan driver.
Next patch by Fan Wu fixes the tear down order in the esd_usb driver.
Followed by a patch by Oliver Hartkopp that adds missing locking for
the raw flags in the CAN_RAW protocol.
Shuhao Fu's patch for the j1939 protocol fix lockless
local-destination check.
Stéphane Grosjean updates their email address.
The next 11 patches all target the CAM Broadcast Manager protocol. One
contributed by Lee Jones the remaining ones by Oliver Hartkopp. They
fix several concurrency and locking issues found by various bots.
The last 3 patches are also by Oliver Hartkopp fixing concurrency and
locking issues found by various bots in the CAN ISO Transport
Protocol.
regards,
Marc
---
The following changes since commit 3f1f755366687d051174739fb99f7d560202f60b:
net: openvswitch: reject oversized nested action attrs (2026-07-11 13:09:11 +0200)
are available in the Git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/mkl/linux-can.git tags/linux-can-fixes-for-7.2-20260716
for you to fetch changes up to 6fd9e37916b9537143a295f854fdf5e52c4c5bc1:
Merge patch series "net: can: isotp-fixes" (2026-07-16 10:12:22 +0200)
----------------------------------------------------------------
linux-can-fixes-for-7.2-20260716
----------------------------------------------------------------
Alexander Hölzl (1):
can: vxcan: Kconfig: fix description stating no local echo provided
Fan Wu (1):
can: esd_usb: kill anchored URBs before freeing netdevs
Lee Jones (1):
can: bcm: defer rx_op deallocation to workqueue to fix thrtimer UAF
Marc Kleine-Budde (2):
Merge patch series "can: bcm: collected fixes"
Merge patch series "net: can: isotp-fixes"
Oliver Hartkopp (14):
can: raw: add locking for raw flags bitfield
can: bcm: fix lockless bound/ifindex race and silent RX_SETUP failure
can: bcm: add locking when updating filter and timer values
can: bcm: fix CAN frame rx/tx statistics
can: bcm: add missing rcu list annotations and operations
can: bcm: extend bcm_tx_lock usage for data and timer updates
can: bcm: validate frame length in bcm_rx_setup() for RTR replies
can: bcm: add missing device refcount for CAN filter removal
can: bcm: fix stale rx/tx ops after device removal
can: bcm: fix data race on rx_stamp/rx_ifindex in bcm_rx_handler()
can: bcm: track a single source interface for ANYDEV timeout/throttle ops
can: isotp: use unconditional synchronize_rcu() in isotp_release()
can: isotp: fix use-after-free race with concurrent NETDEV_UNREGISTER
can: isotp: serialize TX state transitions under so->rx_lock
Shuhao Fu (1):
can: j1939: fix lockless local-destination check
Stéphane Grosjean (1):
can: peak: Modification of references to email accounts being deleted
.mailmap | 4 +-
drivers/net/can/Kconfig | 7 +-
drivers/net/can/peak_canfd/peak_canfd.c | 2 +-
drivers/net/can/peak_canfd/peak_canfd_user.h | 2 +-
drivers/net/can/peak_canfd/peak_pciefd_main.c | 4 +-
drivers/net/can/sja1000/peak_pci.c | 4 +-
drivers/net/can/sja1000/peak_pcmcia.c | 4 +-
drivers/net/can/usb/esd_usb.c | 5 +-
drivers/net/can/usb/peak_usb/pcan_usb.c | 2 +-
drivers/net/can/usb/peak_usb/pcan_usb_core.c | 4 +-
drivers/net/can/usb/peak_usb/pcan_usb_core.h | 2 +-
drivers/net/can/usb/peak_usb/pcan_usb_fd.c | 2 +-
drivers/net/can/usb/peak_usb/pcan_usb_pro.c | 2 +-
drivers/net/can/usb/peak_usb/pcan_usb_pro.h | 2 +-
include/linux/can/dev/peak_canfd.h | 2 +-
net/can/bcm.c | 654 +++++++++++++++++++-------
net/can/isotp.c | 298 +++++++++---
net/can/j1939/transport.c | 18 +-
net/can/raw.c | 66 ++-
19 files changed, 792 insertions(+), 292 deletions(-)
^ permalink raw reply
* [PATCH net 12/19] can: bcm: validate frame length in bcm_rx_setup() for RTR replies
From: Marc Kleine-Budde @ 2026-07-16 15:47 UTC (permalink / raw)
To: netdev
Cc: davem, kuba, linux-can, kernel, Oliver Hartkopp, stable,
Marc Kleine-Budde
In-Reply-To: <20260716155528.809908-1-mkl@pengutronix.de>
From: Oliver Hartkopp <socketcan@hartkopp.net>
bcm_tx_setup() validates cf->len against the CAN/CAN FD DLC limits
before installing frames for TX_SETUP, but bcm_rx_setup() never did
the same for the RTR-reply frame configured via RX_SETUP with
RX_RTR_FRAME.
Fixes: ffd980f976e7 ("[CAN]: Add broadcast manager (bcm) protocol")
Signed-off-by: Oliver Hartkopp <socketcan@hartkopp.net>
Link: https://patch.msgid.link/20260714-bcm_fixes-v15-7-562f7e3e42da@hartkopp.net
Cc: stable@kernel.org
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
net/can/bcm.c | 59 +++++++++++++++++++++++++++++++++++----------------
1 file changed, 41 insertions(+), 18 deletions(-)
diff --git a/net/can/bcm.c b/net/can/bcm.c
index 68a62f605432..2d9c9cd74536 100644
--- a/net/can/bcm.c
+++ b/net/can/bcm.c
@@ -1246,22 +1246,37 @@ static int bcm_tx_setup(struct bcm_msg_head *msg_head, struct msghdr *msg,
return err;
}
-static void bcm_rx_setup_rtr_check(struct bcm_msg_head *msg_head,
- struct bcm_op *op, void *new_frames)
+static int bcm_rx_setup_rtr_check(struct bcm_msg_head *msg_head,
+ struct bcm_op *op, void *new_frames)
{
+ struct canfd_frame *frame0 = new_frames;
+
+ if (!(msg_head->flags & RX_RTR_FRAME))
+ return 0;
+
+ /* this frame is sent out as-is by bcm_can_tx() whenever a matching
+ * remote request is received, so validate its length the same way
+ * bcm_tx_setup() validates TX_SETUP frames before installing it
+ */
+ if (msg_head->flags & CAN_FD_FRAME) {
+ if (frame0->len > 64)
+ return -EINVAL;
+ } else {
+ if (frame0->len > 8)
+ return -EINVAL;
+ }
+
/* funny feature in RX(!)_SETUP only for RTR-mode:
* copy can_id into frame BUT without RTR-flag to
* prevent a full-load-loopback-test ... ;-]
* normalize this on the staged buffer, before it is
* ever installed into op->frames.
*/
- if (msg_head->flags & RX_RTR_FRAME) {
- struct canfd_frame *frame0 = new_frames;
+ if ((msg_head->flags & TX_CP_CAN_ID) ||
+ frame0->can_id == op->can_id)
+ frame0->can_id = op->can_id & ~CAN_RTR_FLAG;
- if ((msg_head->flags & TX_CP_CAN_ID) ||
- frame0->can_id == op->can_id)
- frame0->can_id = op->can_id & ~CAN_RTR_FLAG;
- }
+ return 0;
}
/*
@@ -1324,7 +1339,11 @@ static int bcm_rx_setup(struct bcm_msg_head *msg_head, struct msghdr *msg,
return err;
}
- bcm_rx_setup_rtr_check(msg_head, op, new_frames);
+ err = bcm_rx_setup_rtr_check(msg_head, op, new_frames);
+ if (err < 0) {
+ kfree(new_frames);
+ return err;
+ }
}
spin_lock_bh(&op->bcm_rx_update_lock);
@@ -1397,16 +1416,12 @@ static int bcm_rx_setup(struct bcm_msg_head *msg_head, struct msghdr *msg,
if (msg_head->nframes) {
err = memcpy_from_msg(op->frames, msg,
msg_head->nframes * op->cfsiz);
- if (err < 0) {
- if (op->frames != &op->sframe)
- kfree(op->frames);
- if (op->last_frames != &op->last_sframe)
- kfree(op->last_frames);
- kfree(op);
- return err;
- }
+ if (err < 0)
+ goto free_op;
- bcm_rx_setup_rtr_check(msg_head, op, op->frames);
+ err = bcm_rx_setup_rtr_check(msg_head, op, op->frames);
+ if (err < 0)
+ goto free_op;
}
/* bcm_can_tx / bcm_tx_timeout_handler needs this */
@@ -1505,6 +1520,14 @@ static int bcm_rx_setup(struct bcm_msg_head *msg_head, struct msghdr *msg,
}
return msg_head->nframes * op->cfsiz + MHSIZ;
+
+free_op:
+ if (op->frames != &op->sframe)
+ kfree(op->frames);
+ if (op->last_frames != &op->last_sframe)
+ kfree(op->last_frames);
+ kfree(op);
+ return err;
}
/*
--
2.53.0
^ permalink raw reply related
* [PATCH net 13/19] can: bcm: add missing device refcount for CAN filter removal
From: Marc Kleine-Budde @ 2026-07-16 15:47 UTC (permalink / raw)
To: netdev
Cc: davem, kuba, linux-can, kernel, Oliver Hartkopp, sashiko-bot,
stable, Marc Kleine-Budde
In-Reply-To: <20260716155528.809908-1-mkl@pengutronix.de>
From: Oliver Hartkopp <socketcan@hartkopp.net>
sashiko-bot remarked a problem with a concurrent device unregistration
in isotp.c which also is present in the bcm.c code. A former fix for raw.c
commit c275a176e4b6 ("can: raw: add missing refcount for memory leak fix")
introduced a netdevice_tracker which solves the issue for bcm.c too.
bcm_release(), bcm_delete_rx_op() and bcm_notifier() relied on
dev_get_by_index(ifindex) to re-find the device for an rx_op before
unregistering its filter. If a concurrent NETDEV_UNREGISTER has already
unlisted the device from the ifindex table, that lookup fails and
can_rx_unregister() is silently skipped, leaving a stale CAN filter
pointing at the soon-to-be-freed bcm_op/socket.
Hold a netdev_hold()/netdev_put() tracked reference on op->rx_reg_dev
from the moment the rx filter is registered in bcm_rx_setup() until it
is unregistered in bcm_rx_unreg(), and use that reference directly in
bcm_release() and bcm_delete_rx_op() instead of re-looking the device
up by ifindex.
Reported-by: sashiko-bot@kernel.org
Closes: https://sashiko.dev/#/patchset/20260707094716.63578-1-socketcan@hartkopp.net
Fixes: ffd980f976e7 ("[CAN]: Add broadcast manager (bcm) protocol")
Signed-off-by: Oliver Hartkopp <socketcan@hartkopp.net>
Link: https://patch.msgid.link/20260714-bcm_fixes-v15-8-562f7e3e42da@hartkopp.net
Cc: stable@kernel.org
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
net/can/bcm.c | 47 +++++++++++++++++++++++++++--------------------
1 file changed, 27 insertions(+), 20 deletions(-)
diff --git a/net/can/bcm.c b/net/can/bcm.c
index 2d9c9cd74536..25842061800b 100644
--- a/net/can/bcm.c
+++ b/net/can/bcm.c
@@ -128,6 +128,7 @@ struct bcm_op {
struct canfd_frame last_sframe;
struct sock *sk;
struct net_device *rx_reg_dev;
+ netdevice_tracker rx_reg_dev_tracker;
spinlock_t bcm_tx_lock; /* protect tx data and timer updates */
spinlock_t bcm_rx_update_lock; /* protect filter/timer data updates */
};
@@ -937,6 +938,7 @@ static void bcm_rx_unreg(struct net_device *dev, struct bcm_op *op)
/* mark as removed subscription */
op->rx_reg_dev = NULL;
+ netdev_put(dev, &op->rx_reg_dev_tracker);
} else
printk(KERN_ERR "can-bcm: bcm_rx_unreg: registered device "
"mismatch %p %p\n", op->rx_reg_dev, dev);
@@ -967,17 +969,14 @@ static int bcm_delete_rx_op(struct list_head *ops, struct bcm_msg_head *mh,
* Only remove subscriptions that had not
* been removed due to NETDEV_UNREGISTER
* in bcm_notifier()
+ *
+ * op->rx_reg_dev is a tracked reference taken
+ * when the subscription was registered, so it
+ * stays valid here even if a concurrent
+ * NETDEV_UNREGISTER already unlisted the dev.
*/
- if (op->rx_reg_dev) {
- struct net_device *dev;
-
- dev = dev_get_by_index(sock_net(op->sk),
- op->ifindex);
- if (dev) {
- bcm_rx_unreg(dev, op);
- dev_put(dev);
- }
- }
+ if (op->rx_reg_dev)
+ bcm_rx_unreg(op->rx_reg_dev, op);
} else
can_rx_unregister(sock_net(op->sk), NULL,
op->can_id,
@@ -1496,7 +1495,17 @@ static int bcm_rx_setup(struct bcm_msg_head *msg_head, struct msghdr *msg,
bcm_rx_handler, op,
"bcm", sk);
- op->rx_reg_dev = dev;
+ /* keep a tracked reference so that a later
+ * unregister can safely reach the device even
+ * if a concurrent NETDEV_UNREGISTER has
+ * already unlisted it by ifindex
+ */
+ if (!err) {
+ op->rx_reg_dev = dev;
+ netdev_hold(dev,
+ &op->rx_reg_dev_tracker,
+ GFP_KERNEL);
+ }
dev_put(dev);
} else {
/* the requested device is gone - do not
@@ -1873,16 +1882,14 @@ static int bcm_release(struct socket *sock)
* Only remove subscriptions that had not
* been removed due to NETDEV_UNREGISTER
* in bcm_notifier()
+ *
+ * op->rx_reg_dev is a tracked reference taken
+ * when the subscription was registered, so it
+ * stays valid here even if a concurrent
+ * NETDEV_UNREGISTER already unlisted the device.
*/
- if (op->rx_reg_dev) {
- struct net_device *dev;
-
- dev = dev_get_by_index(net, op->ifindex);
- if (dev) {
- bcm_rx_unreg(dev, op);
- dev_put(dev);
- }
- }
+ if (op->rx_reg_dev)
+ bcm_rx_unreg(op->rx_reg_dev, op);
} else
can_rx_unregister(net, NULL, op->can_id,
REGMASK(op->can_id),
--
2.53.0
^ permalink raw reply related
* [PATCH net 14/19] can: bcm: fix stale rx/tx ops after device removal
From: Marc Kleine-Budde @ 2026-07-16 15:47 UTC (permalink / raw)
To: netdev
Cc: davem, kuba, linux-can, kernel, Oliver Hartkopp, sashiko-bot,
stable, Marc Kleine-Budde
In-Reply-To: <20260716155528.809908-1-mkl@pengutronix.de>
From: Oliver Hartkopp <socketcan@hartkopp.net>
RX: an RX_SETUP update(!) for an existing op skipped can_rx_register()
unconditionally, even when a concurrent NETDEV_UNREGISTER had already
torn down its registration (op->rx_reg_dev == NULL). This silently
did not re-enable frame delivery for that updated filter. bcm_rx_setup()
now re-registers in that case, while leaving rx_ops with ifindex = 0
(all CAN devices) which never carry a tracked rx_reg_dev registered as-is.
TX: bcm_notify() only handled bo->rx_ops on NETDEV_UNREGISTER, leaving
tx_ops with an active cyclic transmission re-arming its hrtimer
indefinitely to execute bcm_tx_timeout_handler(). Cancelling the hrtimer
prevents the runaway timer and any injection into a later reused ifindex,
since nothing else calls bcm_can_tx() for the op until an explicit
TX_SETUP update re-arms it.
Unlike bcm_rx_unreg(), which clears the tracked rx_reg_dev for rx_ops,
the ifindex is intentionally left unchanged for tx_ops. bcm_tx_setup()
always rejects ifindex 0, so clearing it would strand the op: neither a
later TX_SETUP (bcm_find_op()) nor TX_DELETE (bcm_delete_tx_op()) could
ever find it again, since both require an exact ifindex match.
Reported-by: sashiko-bot@kernel.org
Closes: https://lore.kernel.org/linux-can/20260708094536.DDF821F00A3A@smtp.kernel.org/
Closes: https://lore.kernel.org/linux-can/20260708154039.347ED1F000E9@smtp.kernel.org/
Fixes: ffd980f976e7 ("[CAN]: Add broadcast manager (bcm) protocol")
Signed-off-by: Oliver Hartkopp <socketcan@hartkopp.net>
Link: https://patch.msgid.link/20260714-bcm_fixes-v15-9-562f7e3e42da@hartkopp.net
Cc: stable@kernel.org
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
net/can/bcm.c | 54 +++++++++++++++++++++++++++++++++++++++++----------
1 file changed, 44 insertions(+), 10 deletions(-)
diff --git a/net/can/bcm.c b/net/can/bcm.c
index 25842061800b..a53dba6ab8b8 100644
--- a/net/can/bcm.c
+++ b/net/can/bcm.c
@@ -1287,6 +1287,7 @@ static int bcm_rx_setup(struct bcm_msg_head *msg_head, struct msghdr *msg,
struct bcm_sock *bo = bcm_sk(sk);
struct bcm_op *op;
int do_rx_register;
+ int new_op = 0;
int err = 0;
if ((msg_head->flags & RX_FILTER_ID) || (!(msg_head->nframes))) {
@@ -1371,8 +1372,15 @@ static int bcm_rx_setup(struct bcm_msg_head *msg_head, struct msghdr *msg,
/* free temporary frames / kfree(NULL) is safe */
kfree(new_frames);
- /* Only an update -> do not call can_rx_register() */
- do_rx_register = 0;
+ /* Don't register a new CAN filter for the rx_op update unless
+ * a concurrent NETDEV_UNREGISTER notifier already tore down
+ * the previous registration. In this case the receiver needs
+ * to be re-registered here so that this update doesn't
+ * silently stop delivering frames for the given ifindex.
+ * Ops with ifindex = 0 (all CAN interfaces) never carry a
+ * tracked rx_reg_dev and stay registered as-is.
+ */
+ do_rx_register = (ifindex && !op->rx_reg_dev) ? 1 : 0;
} else {
/* insert new BCM operation for the given can_id */
@@ -1439,6 +1447,7 @@ static int bcm_rx_setup(struct bcm_msg_head *msg_head, struct msghdr *msg,
/* call can_rx_register() */
do_rx_register = 1;
+ new_op = 1;
} /* if ((op = bcm_find_op(&bo->rx_ops, msg_head->can_id, ifindex))) */
@@ -1452,7 +1461,7 @@ static int bcm_rx_setup(struct bcm_msg_head *msg_head, struct msghdr *msg,
if (op->flags & SETTIMER) {
/* set timers (locked) for newly created op */
- if (do_rx_register) {
+ if (new_op) {
spin_lock_bh(&op->bcm_rx_update_lock);
op->ival1 = msg_head->ival1;
op->ival2 = msg_head->ival2;
@@ -1482,7 +1491,10 @@ static int bcm_rx_setup(struct bcm_msg_head *msg_head, struct msghdr *msg,
HRTIMER_MODE_REL_SOFT);
}
- /* now we can register for can_ids, if we added a new bcm_op */
+ /* now we can register for can_ids, if we added a new bcm_op
+ * or need to re-register after a NETDEV_UNREGISTER tore down
+ * the previous registration of an existing op
+ */
if (do_rx_register) {
if (ifindex) {
struct net_device *dev;
@@ -1514,18 +1526,32 @@ static int bcm_rx_setup(struct bcm_msg_head *msg_head, struct msghdr *msg,
err = -ENODEV;
}
- } else
+ } else {
err = can_rx_register(sock_net(sk), NULL, op->can_id,
REGMASK(op->can_id),
bcm_rx_handler, op, "bcm", sk);
+ }
+
if (err) {
- /* this bcm rx op is broken -> remove it */
- bcm_remove_op(op);
+ /* newly created bcm rx op is broken -> remove it */
+ if (new_op) {
+ bcm_remove_op(op);
+ return err;
+ }
+
+ /* an existing op just stays unregistered.
+ * Cancel op->timer and (defensively) op->thrtimer.
+ * Other settings can't be reached until the next
+ * successful RX_SETUP.
+ */
+ hrtimer_cancel(&op->timer);
+ hrtimer_cancel(&op->thrtimer);
return err;
}
- /* add this bcm_op to the list of the rx_ops */
- list_add_rcu(&op->list, &bo->rx_ops);
+ /* add a new bcm_op to the list of the rx_ops */
+ if (new_op)
+ list_add_rcu(&op->list, &bo->rx_ops);
}
return msg_head->nframes * op->cfsiz + MHSIZ;
@@ -1745,11 +1771,19 @@ static void bcm_notify(struct bcm_sock *bo, unsigned long msg,
case NETDEV_UNREGISTER:
lock_sock(sk);
- /* remove device specific receive entries */
+ /* rx_ops: remove device specific receive entries */
list_for_each_entry(op, &bo->rx_ops, list)
if (op->rx_reg_dev == dev)
bcm_rx_unreg(dev, op);
+ /* tx_ops: stop device specific cyclic transmissions on the
+ * vanishing ifindex. Cancelling the timer is enough to stop
+ * cyclic bcm_can_tx() calls as there is no re-arming.
+ */
+ list_for_each_entry(op, &bo->tx_ops, list)
+ if (op->ifindex == dev->ifindex)
+ hrtimer_cancel(&op->timer);
+
/* remove device reference, if this is our bound device */
if (bo->bound && bo->ifindex == dev->ifindex) {
#if IS_ENABLED(CONFIG_PROC_FS)
--
2.53.0
^ 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