| Age | Commit message (Collapse) | Author | Files | Lines |
|---|
|
git://git.kernel.org/pub/scm/linux/kernel/git/netfilter/nf-next
Pablo Neira Ayuso says:
====================
Netfilter updates for net-next
The following patchset contains Netfilter updates for net-next:
1) Update conncount to use the original tuple after ct lookup to ensure
consistent counting, from Fernando F. Mancera.
2) Remove redundant net_device field in info structure that helps
parse the flowtable path discovery.
3) Move net_device to flowtable check to the flowtable discovery
path parser. This is preparation work to pass the tunnel dst_entry
via .fill_forward_path.
4) Update DSA .fill_forward_path to break at the user DSA, since
the conduit DSA is not used in the datapath. This slighly simplifies
the flowtable path discovery parser.
5) Do not advance index in the path stack prematurely, otherwise
it points to uninitialized slots on error. Not an issue currently
but it could be once tunnel dst_entry is passed via .fill_forward_path.
6) Pass the tunnel dst_entry via dev_fill_forward_path().
7) Update ipip and ip6ip6 tunnels to pass the dst_entry through
dev_fill_forward_path().
8) Call skb_valid_dst() before accessing skb_dst() to ensure dst_entry
is not a template.
9) Use UNACK timeout when RST packet does not match the expected
window while in ESTABLISHED state, the existing approach the CLOSE
state timeout which is only 10 seconds. Adopt a more conservative
timeout by default for this case.
* tag 'nf-next-26-07-31' of git://git.kernel.org/pub/scm/linux/kernel/git/netfilter/nf-next:
netfilter: conntrack: tcp: use UNACK timeout for non-closing RST packets
netfilter: nf_tables: call skb_valid_dst() before skb_dst()
netfilter: flowtable: release tunnel route on error when building forward path
net: pass dst via net_device_path in dev_fill_forward_path()
net: do not advance stack index from dev_fwd_path()
net: dsa: stop at the user device in .fill_forward_path
netfilter: flowtable: consolidate flowtable device check
netfilter: flowtable: consolidate net_device field in nft_forward_info struct
netfilter: conncount: normalize tuple and zone on successful ct lookup
====================
Link: https://patch.msgid.link/20260731153402.851224-1-pablo@netfilter.org
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
|
|
Partial pull of the nf-26-07-31 tag
Pablo says:
====================
The following patchset contains Netfilter/IPVS fixes net, this
includes fixes for ebtables nflog target, ipset hash type,
IPVS kthread estimator
1) Prevent IPVS kthread estimator from draining the est_temp_list
when netns is being dismantled. From Zhiling Zou.
2) Missing module nflog refcount bump from ebtables nflog target from
.checkentry path. Similar dependency exists already in xt_NFLOG and
nft_log. From Chengfeng Ye.
3) Use RCU to fix ipset bookkeeping of cidr values on weakly-ordered
architectures. From Jozsef Kadlecsik.
4) Use atomic64_t for set->ext_size in ipset to fix parallel inserts
and deletes racing on updating it. From Jozsef Kadlecsik.
5) Add small wrappers for hash and bucket size to prepare the update
of ipset hash set types to rhashtable, from Florian Westphal.
6) Add mtype_del_cidr_all() and use it to prepare the migration of
ipset hash types to rhashtable. From Florian Westphal.
7) Replace existing ipset call_rcu() based destruction with rcu_work
api also to ease the transition to rhashtable. Also from Florian.
8) Avoid reading the IPv4 ihl field multiple times to prevent local
attacker to cause out-of-bounds write in ip_vs_nat_icmp(), from
Julian Anastasov.
9) Restore the checksum validations that could be needed by the IPVS
FORWARD hook. Also from Julian.
====================
Link: https://patch.msgid.link/20260731151806.849724-1-pablo@netfilter.org
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
|
|
bpf_tcp_gen_syncookie and bpf_tcp_check_syncookie accept a socket pointer
'sk' with argument type ARG_PTR_TO_BTF_ID_SOCK_COMMON. However, they access
sk->sk_protocol without validating whether 'sk' represents a full socket.
Fix this issue by checking sk->sk_state != TCP_LISTEN before inspecting
sk->sk_protocol in both bpf_tcp_gen_syncookie and bpf_tcp_check_syncookie.
Since mini-sockets are never in the TCP_LISTEN state, the condition
short-circuits and prevents dereferencing fullsock-specific fields.
Fixes: 399040847084 ("bpf: add helper to check for a valid SYN cookie")
Fixes: 70d66244317e ("bpf: add bpf_tcp_gen_syncookie helper")
Reported-by: Vega <vega@nebusec.ai>
Signed-off-by: Luxiao Xu <rakukuip@gmail.com>
Signed-off-by: Ren Wei <enjou1224z@gmail.com>
Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
Reviewed-by: Eric Dumazet <edumazet@google.com>
Reviewed-by: Kuniyuki Iwashima <kuniyu@google.com>
Link: https://lore.kernel.org/bpf/6218aa3534d0d2d3f448fde70a8dc2769d7a8201.1785823138.git.rakukuip@gmail.com
|
|
CONFIG_SYSCTL used to make sense as a separate hidden bool before commit
61a47c1ad3a4 ("sysctl: Remove the sysctl system call"); it was selected
by both CONFIG_SYSCTL_SYSCALL and CONFIG_PROC_SYSCTL.
Today CONFIG_PROC_SYSCTL is the only selector, so the two are always
equal. Kill the hidden bool, rename the PROC_SYSCTL prompt to SYSCTL,
and s/CONFIG_PROC_SYSCTL/CONFIG_SYSCTL/ tree-wide.
Signed-off-by: Oleg Nesterov <oleg@redhat.com>
Signed-off-by: Joel Granados <joel.granados@kernel.org>
|
|
When the mesh interface is getting deleted, then
batadv_bla_del_backbone_claims() (via batadv_bla_purge_backbone_gw()) could
make sure that all claims gets removed. But this function is only executed
when bat_priv->bla.claim_hash is not NULL. And since batadv_bla_free() is
always setting it to NULL before it is (indirectly) called, it was never
actually executed.
But the batadv_bla_purge_claims() -> batadv_handle_unclaim() is at the
moment too fragile because the BLA code is not handling the rehashing in
batadv_bla_update_orig_address(). The stored backbone address doesn't have
to be the one actually used for the hash bucket selection during the
initial adding of the backbone. The batadv_handle_unclaim() can therefore
fail to find the respective backbone for the unclaim and then stop the
deletion.
But the actual backbone_gw object is not needed for the unclaim because all
relevant information is always provided by the caller. And the check for
the existence of the backbone_gw doesn't provide any additional security
check for the deletion of a claim.
Cc: stable@kernel.org
Fixes: 23721387c409 ("batman-adv: add basic bridge loop avoidance code")
Signed-off-by: Sven Eckelmann <sven@narfation.org>
|
|
batadv_mcast_forw_packet() and batadv_mcast_forw_scrape() is not only
called (indirectly) by the unsharing+linearizing batadv_recv_mcast_packet()
handler. When it is called (indirectly) by batadv_mcast_forw_mcsend() then
it will be unshared but not linearized. The SKB_LINEAR_ASSERT() can
therefore cause a fatal BUG().
The linearization should happen during the expansion of the head because
the scrape function can be hit already during the initial
batadv_mcast_forw_mode() selection code:
* batadv_interface_tx
* batadv_mcast_forw_mode
* batadv_mcast_forw_mode_by_count()
* batadv_mcast_forw_push()
-> calls batadv_mcast_forw_expand_head() before everything else
* batadv_mcast_forw_push_tvlvs()
* batadv_mcast_forw_push_dests()
* batadv_mcast_forw_push_adjust_padding()
* batadv_mcast_forw_scrape()
Cc: stable@vger.kernel.org
Reported-by: Sashiko <sashiko-bot@kernel.org>
Fixes: 90039133221e ("batman-adv: mcast: implement multicast packet generation")
Signed-off-by: Sven Eckelmann <sven@narfation.org>
|
|
When a packet is transmitted via a batman-adv interface and has already
enough room for the header then nothing will make sure that the skbuff is
unshared. But it is not allowed to modify a currently shared skbuff.
Always make sure that the pskb_expand_head() is not only called for a too
small header but also for shared skbuffs.
Cc: stable@vger.kernel.org
Fixes: 90039133221e ("batman-adv: mcast: implement multicast packet generation")
Signed-off-by: Sven Eckelmann <sven@narfation.org>
|
|
batadv_mcast_forw_mode_by_count() pushs the skb->data for BATADV_FORW_MCAST
forwarding via batadv_mcast_forw_mcsend(). But the
batadv_skb_set_priority() expects the ethernet header directly before
(skb->data + offset). With the moved skb->data, just some random data would
be accessed to get the priority data.
Move the batadv_skb_set_priority() before the decision about the handling
multicast packets and potential header modifications.
Cc: stable@vger.kernel.org
Fixes: 90039133221e ("batman-adv: mcast: implement multicast packet generation")
Signed-off-by: Sven Eckelmann <sven@narfation.org>
|
|
When a MAC address is updated in batadv_dat_entry_add(), it is done using a
simple copy function. A parallel reader might only see parts of this
update. In worst case, the reader is transporting the half updated MAC
address over the network or is creating an ARP response using it -
poisoning the ARP cache.
atomic64_t can be used to store the 48 bit of a mac address. A reader will
then either see the old mac address or the new one - never a mixture of
both.
Cc: stable@vger.kernel.org
Reported-by: Sashiko <sashiko-bot@kernel.org>
Fixes: 2f1dfbe18507 ("batman-adv: Distributed ARP Table - implement local storage")
Signed-off-by: Sven Eckelmann <sven@narfation.org>
|
|
Independent of the alignment of the ARP packet in the SKB, either the
batadv_arp_ip_src or the batadv_arp_ip_dst will have an unaligned access
(on HW without native unaligned read support).
Use get_unaligned() to handle this properly on all architectures.
Cc: stable@vger.kernel.org
Reported-by: Sashiko <sashiko-bot@kernel.org>
Fixes: 5c3a0e553593 ("batman-adv: Distributed ARP Table - add ARP parsing functions")
Signed-off-by: Sven Eckelmann <sven@narfation.org>
|
|
When batadv_bla_del_backbone_claims() tried to remove all claims of a
backbone, it sets the CRC to 0. It assumes that the it had the last
reference of the claims because batadv_claim_release() (which runs after
the last reference was released), is XORing the crc16 of the claim address
with the backbone CRC.
If there would be a parallel holder of any of these references, it could
happen that the backbone CRC is (0 ^ crc16(delayed_released_claim)). Which
is the wrong starting point for the new claims it may receive when the
remote answers the claim request from batadv_bla_send_request().
This reinitializations can be completely dropped to avoid this problem.
batadv_claim_release() will take care of fixing the backbone CRC.
Cc: stable@vger.kernel.org
Fixes: 23721387c409 ("batman-adv: add basic bridge loop avoidance code")
Signed-off-by: Sven Eckelmann <sven@narfation.org>
|
|
batadv_bla_add_claim() is used to add claims and modify the backbone of
claims for CLAIM frames from remote backbones and local packets. When it
handles a claim, it needs to either
* add the new claim's CRC to the backbone CRC
* remove the already existing claim's CRC from the old backbone and add it
to the new backbone
But when the "new" claim code was running in parallel to the "change
backbone" code, it can happen that the CRC was invalid because the
backbone_gw of the claim was changed twice in the "new" claim code path:
* CPU0 creates the claim for gateway A and publishes it in the claim
hash. The crc16 of the address has not yet been added to A's crc at
this point.
* CPU1 processes a claim frame of gateway B for the same client, finds
the just published claim, and performs the ownership change: it
switches the pointer to B, removes the crc16 from A's crc - which
never contained it - and adds it to B's crc.
* CPU0 continues behind the creation branch, unconditionally switches
the pointer back to A without compensating B's crc (its remove_crc
is false for the creation path), and finally adds the crc16 to A's
crc
The CRC is then wrong for both:
* claim belongs to A: but CRC is not part of backbone A's CRC
* claim doesn't belong to B: CRC is still part of backbone B's CRC
This wrong CRC is never recomputated from the stored claims. For local
backbone claims, this can also not recovered using syncs.
To avoid this, split the functionality in clear separate parts:
* new claim which always adds claim CRC to the backbone CRC (but never
changes the already set backbone_gw of the claim back)
* update of existing claim which automatically changes the backbone_gw
entry and only updates both backbone CRCs when there was an actual change
Cc: stable@vger.kernel.org
Reported-by: Sashiko <sashiko-bot@kernel.org>
Fixes: 23721387c409 ("batman-adv: add basic bridge loop avoidance code")
Signed-off-by: Sven Eckelmann <sven@narfation.org>
|
|
Fragment reassembly reuses the skb from the highest-numbered buffered
fragment as the merged packet. When that fragment was received on a hard
interface which is deleted before the chain completes, the merged skb can
re-enter the receive path with a stale skb->dev and skb_iif.
batadv_batman_skb_recv() passes such merged packets through the normal
receive handlers again. DAT and bridge loop avoidance both derive the ARP
header length from skb->dev, so they can dereference the freed net_device
before the packet reaches the local mesh interface.
Refresh the receive device metadata from the current receive device before
running the packet handlers. This keeps internally reinjected merged
fragments consistent with the normal receive path after hard interface
teardown.
Fixes: 610bfc6bc99b ("batman-adv: Receive fragmented packets and merge")
Cc: stable@vger.kernel.org
Reported-by: Vega <vega@nebusec.ai>
Signed-off-by: Zhiling Zou <zhilinz@nebusec.ai>
Signed-off-by: Sven Eckelmann <sven@narfation.org>
|
|
The kernel documentation for batadv_send_skb_unicast() states that only the
return values NET_XMIT_DROP and NET_XMIT_SUCCESS are valid. Functions like
batadv_dat_snoop_incoming_arp_request() are only checking if the return is
not NET_XMIT_DROP to check if send was successful or not. Negative values
were therefore also handled as success.
Similar functions are not returning the batadv_send_skb_to_orig() return
value directly but are checking if it is a direct success and only then
marking the return as such. This must also be adopted for
batadv_send_skb_unicast().
The callers of this function are mostly not affected. Only packet counting
in batadv_dat_snoop_incoming_arp_request() will now work as expected in
case of a negative return value from batadv_send_skb_to_orig().
Signed-off-by: Sven Eckelmann <sven@narfation.org>
|
|
batadv_recv_icmp_ttl_exceeded() is a receive function. It must therefore
return NET_RX_* and not NET_XMIT_*. And batadv_send_skb_to_orig() is an
xmit function and is returning NET_XMIT_*.
This doesn't change the behavior because both NET_RX_SUCCESS and
NET_RX_SUCCESS are using the same underlying value (0).
Signed-off-by: Sven Eckelmann <sven@narfation.org>
|
|
batadv_init() ignores errors from several initialization helpers, so the
module can load without those registrations in place.
Check the fallible init steps and unwind prior initialization in reverse
order of acquisition on failure.
Signed-off-by: Minhong He <heminhong@kylinos.cn>
Signed-off-by: Sven Eckelmann <sven@narfation.org>
|
|
In sctp_process_asconf(), when sctp_make_asconf_ack() fails to allocate
the ASCONF_ACK chunk due to memory pressure, the code jumps to the
done label where asoc->peer.addip_serial is unconditionally incremented.
This leaves the peer's ASCONF (serial N) unacknowledged while the local
endpoint now expects serial N+1. When the peer retransmits serial N, it
falls into the serial < addip_serial + 1 branch ,
which attempts to look up a cached ACK for serial N. No cached ACK
exists since the allocation failed, so the retransmission is silently
discarded. The peer eventually times out and ABORTs the association.
Move the addip_serial increment inside the if (asconf_ack) block so that
the serial number is only advanced when the ASCONF_ACK is successfully
created and cached. This way, on allocation failure, the serial number
is unchanged and the peer's retransmitted ASCONF will be correctly
re-processed.
Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Signed-off-by: Qing Luo <luoqing@kylinos.cn>
Acked-by: Xin Long <lucien.xin@gmail.com>
Link: https://patch.msgid.link/20260804025514.241767-1-l1138897701@163.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
|
|
failover_init() ignores register_netdevice_notifier() errors and always
returns success, which can leave the failover module loaded without its
netdev notifier registered.
Return the notifier registration result directly so module initialization
fails when registration fails.
This is a future looking check, register_netdevice_notifier()
only fails on double registration or if the registered notifier
itself returns an error.
Signed-off-by: Minhong He <heminhong@kylinos.cn>
Link: https://patch.msgid.link/20260731030338.82508-1-heminhong@kylinos.cn
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
|
|
nexthop_init() ignores errors from register_pernet_subsys() and
register_netdevice_notifier(), so a partial initialization can appear
successful.
Check those steps and unwind prior registrations on failure.
Do not check rtnl_register_many(): for built-in code it panics on
failure, so the call cannot return an error to nexthop_init().
Cc: stable+noautosel@kernel.org # untested fix to unlikely error path
Signed-off-by: Minhong He <heminhong@kylinos.cn>
Reviewed-by: Ido Schimmel <idosch@nvidia.com>
Link: https://patch.msgid.link/20260731025249.80026-1-heminhong@kylinos.cn
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
|
|
ovs_flow_cmd_new() preallocates the optional reply skb before it takes
ovs_mutex and before it knows which existing flow will be updated.
That is normally fine because the skb is sized from the request flow
identifier. That identifier also becomes the inserted flow's identifier.
For updates, however, a request with a UFID may miss the UFID lookup and
then fall back to the flow key lookup. That lookup can legitimately find
an existing key-identified flow. UFIDs are optional and the flow key is
the primary identifier.
For echoed replies, ovs_flow_cmd_fill_info() writes the matched flow's
identifier, not the request identifier used for the preallocation. A short
request UFID can therefore leave too little room for the key identifier.
The fill can then fail with -EMSGSIZE and hit the BUG_ON(error < 0) in the
update path.
Once the update target has been resolved, reallocate the reply skb if the
matched flow needs a larger reply than the request identifier allowed. Do
this before replacing the actions so the request can still fail cleanly if
the rare extra allocation fails.
Fixes: 74ed7ab9264c ("openvswitch: Add support for unique flow IDs.")
Cc: stable@vger.kernel.org
Reported-by: Vega <vega@nebusec.ai>
Signed-off-by: Zhiling Zou <zhilinz@nebusec.ai>
Reviewed-by: Ilya Maximets <i.maximets@ovn.org>
Link: https://patch.msgid.link/f7bbd3c30ce81a39156e226b3872d73abed21d2f.1785644623.git.zhilinz@nebusec.ai
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
|
|
The pages backing an RDS memory region are pinned in __rds_rdma_map()
with rds_pin_pages(), which uses pin_user_pages_fast(): each page's
refcount is biased by GUP_PIN_COUNTING_BIAS to account the pin. The
scatterlist is then handed to the IB transport, and the transport
releases the pages in __rds_ib_teardown_mr() with
set_page_dirty(page);
put_page(page);
put_page() drops a single reference instead of removing the pin bias,
so every MR teardown permanently strands the remaining references and
the pages are never freed - a userspace-triggerable memory leak of up
to RDS_MAX_MSG_SIZE per RDS_GET_MR/RDS_GET_MR_FOR_DEST call.
The conversion to the pin API updated the unpin sites in rdma.c but
missed this one on the transport side. Release the pages with
unpin_user_pages_dirty_lock(), which removes the pin bias and also
dirties the page under the folio lock, closing the truncation race
that a bare set_page_dirty() leaves open.
Dirtying under the folio lock can sleep, which is safe in every path
that reaches __rds_ib_teardown_mr(): the registration-reuse path
(rds_ib_map_frmr()) runs in syscall context, and the pool flush
(rds_ib_unreg_frmr()) runs under pool->flush_lock, a mutex, and
already sleeps in rds_ib_post_inv(). The WARN_ON that guarded the
old irq-context set_page_dirty() case is dropped along with it.
Signed-off-by: Allison Henderson <achender@kernel.org>
Link: https://patch.msgid.link/20260730041629.3512480-5-achender@kernel.org
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
|
|
When rds_send_xmit() picks the next message off cp_send_queue it takes
its own reference with rds_message_addref(). If the message then hits
the never-retransmit check (RDS_MSG_FLUSH, or an RDMA op that was
already retransmitted), it is moved to the local to_be_dropped list and
that reference is dropped after the batch.
However, if RDS_MSG_ON_CONN has already been cleared, the message is
not added to to_be_dropped and the reference taken above is never
dropped: cp_xmit_rm has not been set at this point, so the loop simply
abandons rm and the rds_message (and everything it pins: pages, MRs,
notifiers) leaks after an RDMA error.
The only other places that clear RDS_MSG_ON_CONN are
rds_send_path_drop_acked() and rds_send_drop_to(), and both can run
while rds_send_xmit() has dropped cp_lock between moving the message
to cp_retrans and re-taking the lock in the never-retransmit check:
rds_send_path_drop_acked() can ack away a message that already sat on
cp_retrans - the RDS_MSG_RETRANSMITTED case above - and
rds_send_drop_to() runs on socket close. Both unlink the message
under cp_lock and put their own reference, leaving the xmit-path
reference stranded.
Drop the reference directly in that case.
This mirrors Oracle UEK commit "net/rds: fix rds_message memleak in
rds_send_xmit".
Signed-off-by: Gerd Rausch <gerd.rausch@oracle.com>
Signed-off-by: Sharath Srinivasan <sharath.srinivasan@oracle.com>
[achender: port to net-next; update commit message, checkpatch nits]
Signed-off-by: Allison Henderson <achender@kernel.org>
Link: https://patch.msgid.link/20260730041629.3512480-4-achender@kernel.org
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
|
|
Each rds_mr stores a bare back pointer to the socket that created it
(mr->r_sock) but takes no reference on it. When the mr is destroyed it
references the rs. Hence, provisions must be made to avoid the rs
being destroyed before all mrs referencing it have been destroyed.
The MR itself is refcounted, and in-flight messages legitimately hold
MR krefs that can outlive the socket: rds_release() drops the rb-tree
references via rds_rdma_drop_keys(), but a send completion arriving
afterwards drops the final message reference from the CQ handler and
ends up in
rds_message_purge()
__rds_put_mr_final()
rds_destroy_mr() -> takes rs->rs_rdma_lock
dereferencing a socket that may already have been freed.
Oracle UEK fixed the same use-after-free ("rds: Add proper refcnt when
an RDS MR references an RDS Socket") after seeing crashes of the form:
PF: supervisor write access in kernel mode
_raw_spin_lock_irqsave+0x4a/0x6a
__rds_put_mr_final+0x2c/0xe0 [rds]
rds_message_purge+0x13c/0x150 [rds]
rds_message_put+0x39/0x54 [rds]
rds_ib_send_cqe_handler+0x147/0x3dd [rds_rdma]
To fix this, take a socket reference when an MR is created and drop it
when the final MR kref goes away. The reference cycle is broken by
rds_release(), which always runs rds_rdma_drop_keys() on close. So the
socket reference held by an MR never prevents release, it only delays
sk_free() until the last MR user is done.
The hold sits next to kref_init() at both allocation sites -
__rds_rdma_map() and the on-demand-paging path in
rds_cmsg_rdma_args() - so every MR owns exactly one socket reference
from the moment it becomes kref-managed. For that to work on the ODP
path, its get_mr() error handling is converted from a bare kfree() to
kref_put(..., __rds_put_mr_final), with r_trans_private cleared first
since it holds an ERR_PTR there; both sites then tear down through
the same path and a future error-path change cannot silently leak or
double-drop the reference.
Signed-off-by: Håkon Bugge <haakon.bugge@oracle.com>
[achender: port to net-next (sock_hold/sock_put in place of the UEK
rds_sock_addref/rds_sock_put helpers); also balance the reference on
the rds_cmsg_rdma_args() ODP path and unify its error path with
__rds_put_mr_final(); update commit message]
Signed-off-by: Allison Henderson <achender@kernel.org>
Link: https://patch.msgid.link/20260730041629.3512480-3-achender@kernel.org
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
|
|
rds_rdma_free_op() and rds_atomic_free_op() are reached from the IB
send completion path via
rds_ib_tasklet_fn_send()
rds_ib_send_cqe_handler()
rds_message_put()
rds_message_purge()
rds_rdma_free_op() / rds_atomic_free_op()
which runs in tasklet (softirq) context. Both functions unpin the
user pages of the op with unpin_user_pages_dirty_lock(), which uses
set_page_dirty_lock() and thus may take the folio lock and sleep.
Sleeping in softirq context is not allowed and can deadlock or crash.
Dirtying the pages with the non-sleeping set_page_dirty() instead
would just trade one bug for another, as pointed out during review:
the pinned range can be file-backed. rds_pin_pages() pins with
FOLL_LONGTERM, which refuses fs-dax but takes the page-cache pages
of a MAP_SHARED file mapping just fine, and RDS does not restrict
what memory the caller registers as an RDMA destination.
For a file-backed page, set_page_dirty() from a tasklet can take
non-irq-safe filesystem locks (e.g. mapping->i_private_lock and
inode->i_lock in block_dirty_folio()) and deadlock against the task
it interrupted. Without the folio lock, it races with truncation
clearing folio->mapping, which is the race set_page_dirty_lock()
exists to close. The pre-pin_user_pages() version of this code
dirtied pages that way from the tasklet, so that bug is older than
the sleeping unpin.
The page dirtying therefore has to move to process context, not
merely avoid the folio lock. When the final rds_message_put() runs
in atomic context, rds_rdma_free_op() and rds_atomic_free_op() now
leave the op's pages pinned and flag the op. Later, rds_message_put()
hands the message to a work item that unpins the flagged ops' pages
and frees the message from process context. Here,
unpin_user_pages_dirty_lock() is safe outside the atomic context.
Everything else keeps running in the caller's context exactly as
before: the rest of the purge - the zerocopy completion, the socket
put and the MR reference drops - as well as RDMA writes, whose pages
the remote side only reads and which unpin without dirtying,
everything on rds_tcp, and final puts that already happen in process
context (socket close, connection teardown).
Deferring only the unpin means the work item touches nothing but the
pinned pages and the rds module's own memory: it cannot call back
into a transport module, so it changes nothing about the transports'
shutdown and unload ordering. rds_exit() drains any pending unpin
work via destroy_workqueue(rds_wq) before the module goes away.
The Oracle UEK kernel avoids the sleeping unpin by calling
set_page_dirty() directly from the tasklet, which is subject to the
file-backed page problem above, so this deliberately does not follow
UEK here.
Signed-off-by: Allison Henderson <achender@kernel.org>
Link: https://patch.msgid.link/20260730041629.3512480-2-achender@kernel.org
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
|
|
The network related code should use for local variable declarations an
ordering scheme which orders lines longest to shortest. Initializations
should only be kept in the declarations when the dependencies between them
are not preventing the reverse x-mas tree order.
Many functions are already using this order. The remaining ones were
supposed to slowly convert to the x-mas tree order when working on them.
But this never happened because the patches tried to only modify the
relevant lines. Instead of getting better, the order often just became
worse.
Just fix the remaining offending functions to finally solve this coding
style (minor) problem. The anonymous dhcp structs were only extracted to
have a clean reverse x-mas tree in functions and are not yet meant as an
opportunity for further cleanups.
Signed-off-by: Sven Eckelmann <sven@narfation.org>
|
|
The Linux coding style suggests to use single variable declarations per
line. This suggestion turned out to make reviewing patches easier when
single variable declarations are modified. Instead of having to search for
the modified variable, it is directly visible as a line change in the diff.
Most functions are already using this style. The remaining ones are just
adjusted by splitting the lines without ensuring the reverse x-mas tree
order because this makes it easier to check the modification.
Signed-off-by: Sven Eckelmann <sven@narfation.org>
|
|
When a function is called which reallocated the skbuff, it is necessary to
reacquire the pointers into the skb data. Otherwise they might cause an
use-after-free.
But is hard to identify such case when it is not clear that helpers are
actually using skb-reallocating functions.
Signed-off-by: Sven Eckelmann <sven@narfation.org>
|
|
Most functions in batman-adv will take the ownership of an skb when they
receive it as argument. Their NET_RX_DROP return value is only indicating
whether there was direct visible problem while processing it. The caller
must not try to also free the skb when such a negative return code was
received.
Signed-off-by: Sven Eckelmann <sven@narfation.org>
|
|
batman-adv requires kernel-doc for all functions and data types visible
outside their own translation unit. Promoting a function from static to
module-wide visibility currently requires adding documentation from
scratch. However, this burden falls on whoever promotes a function from
static to module-wide visibility, rather than its original implementer.
Add the missing kernel-doc comments for the remaining undocumented
functions and data types to reduce the complexity for new contributions.
Signed-off-by: Sven Eckelmann <sven@narfation.org>
|
|
batadv_tvlv_containers_process() was implemented with only two return codes
from the handlers in mind:
* NET_RX_SUCCESS (0)
* NET_RX_DROP (1)
The multicast handlers broke this convention and are also returning
negative return codes. But the processing code was never updated to
correctly aggregate them.
To handle negative return codes for non-OGM(2) handlers, they are now
aggregated to:
* NET_RX_SUCCESS when no handlers returned a different return code
* the last negative return code when at least one handler returned a
negative return code
* NET_RX_DROP otherwise
With the current callers, the old implementation is not triggering any
unexpected behavior. The behavior is only adjusted for new code which might
need more reliable return values.
Signed-off-by: Sven Eckelmann <sven@narfation.org>
|
|
The 4addr unicast packet support is mandatory in compat version 15. No
older compat version is supported and the kernel doesn't need to keep code
to talk to nodes which cannot be in the same mesh.
Acked-by: Antonio Quartulli <antonio@mandelbit.com>
Signed-off-by: Sven Eckelmann <sven@narfation.org>
|
|
Another challenge with unlocked filters.
There is a short window in tc_new_tfilter where a tcf_proto can be found
and briefly referenced by a totally unrelated, unlocked classifier's request
and cause a race.
Feng created a poc which created this race with two threads, one creating a
u32 filter and other a flower filter in the same chain/prio:
1. Both threads enter tc_new_tfilter, both find the chain empty, both
drop filter_chain_lock
2. u32 finishes tcf_proto_create("u32") first, calls
tcf_chain_tp_insert_unique() -> inserts u32_tp into the chain
3. flower finishes tcf_proto_create("flower") later, calls
tcf_chain_tp_insert_unique() -> tcf_chain_tp_find() now sees u32_tp
already there, takes a reference on it, destroys flower's own tp_new
and returns u32_tp to the caller.
Flower then hits the kind mismatch check (because it requested for kind
"flower" but tp->ops->kind is "u32") and goes through the errout path
which calls tcf_proto_put() on u32_tp. If the u32 thread has already
gone through its own errout (its change() call failed on the PoC's empty
options) and dropped its create and insert refs, flower's put is the
last one and drops u32_tp's refcnt to zero.
At this point tp->ops->destroy() runs in a context that never took
rtnl_lock. When that happens, it might cause a UAF like the following
(illustrated by the PoC):
[ +0.000710] BUG: KASAN: slab-use-after-free in u32_init (net/sched/cls_u32.c:393)
[ +0.000281] Read of size 8 at addr ffff888120022f00 by task poc_feng_xue/524
Call Trace:
u32_init (net/sched/cls_u32.c:393)
tc_new_tfilter (net/sched/cls_api.c:2378)
Allocated by task 526:
u32_init (net/sched/cls_u32.c:378)
tc_new_tfilter (net/sched/cls_api.c:2378)
Freed by task 522:
kfree
u32_destroy (net/sched/cls_u32.c:662)
tcf_proto_destroy (net/sched/cls_api.c:446)
tcf_proto_put (net/sched/cls_api.c:459)
tc_new_tfilter (net/sched/cls_api.c:2459)
Fix this by having tcf_proto_destroy() take rtnl_lock around
tp->ops->destroy() for locked classifiers whenever rtnl is not held.
To explain why I used a temp variable "not_lockless" I'd like to point to a
semi-related note on rtnl_held vs TCF_PROTO_OPS_DOIT_UNLOCKED (adding here
for future cleanup if deemed necessary):
The rtnl_held parameter and the TCF_PROTO_OPS_DOIT_UNLOCKED flag are
redundant sources of truth for whether rtnl_lock is held. Among the nine
classifier destroy(..rtnl_held..) callbacks, only flower consults the
rtnl_held parameter which it propagates to tc_setup_cb_destroy()
and tc_setup_cb_call(). The other eight (u32, flow, bpf, cgroup, route, basic,
fw, mall) ignore it entirely;-> those that call tc_setup_cb_destroy()
(u32, bpf, mall) hardcode true always instead of forwarding the parameter.
A future cleanup should remove the rtnl_held parameter from the destroy callback
signature entirely and have callers rely solely on their knowledge whether
they are running in an unlocked context.
Fixes: 12db03b65c2b ("net: sched: extend proto ops to support unlocked classifiers")
Reported-by: Feng Xue <feng.xue@outlook.com>
Tested-by: Victor Nogueira <victor@mojatatu.com>
Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
Link: https://patch.msgid.link/20260801125632.360365-1-jhs@mojatatu.com
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
|
|
When a packet arrives on an ARPHRD_NONE device (e.g. TUN),
ovs_flow_key_extract() trusts the user-provided skb->protocol field: if
it is ETH_P_TEB, the packet is classified as MAC_PROTO_ETHERNET and
key_extract() is called without ensuring the skb has ETH_HLEN (14) bytes
of linear data. key_extract() unconditionally pulls 2 * ETH_ALEN bytes
for MAC addresses and parse_ethertype() pulls 2 more, either of which
triggers a kernel BUG in __skb_pull() when the linear area is too small.
kernel BUG at include/linux/skbuff.h:2848!
RIP: 0010:key_extract+0xa7e/0xd90 net/openvswitch/flow.c:933
ovs_flow_key_extract+0x419/0xa70
ovs_vport_receive+0x222/0x390
netdev_frame_hook+0x3e0/0x630
tun_get_user+0x2d0c/0x38e0
Fixed by calling check_header() in key_extract() before accessing the
Ethernet header.
Fixes: 217ac77a3c25 ("openvswitch: allow L3 netdev ports")
Reported-by: AutonomousCodeSecurity@microsoft.com
Reviewed-by: Eelco Chaudron <echaudro@redhat.com>
Signed-off-by: Cen Zhang (Microsoft) <blbllhy@gmail.com>
Reviewed-by: Ilya Maximets <i.maximets@ovn.org>
Link: https://patch.msgid.link/20260730222006.118652-1-blbllhy@gmail.com
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
|
|
packet_set_ring() updates the RX ring state under sk_receive_queue.lock,
but used to publish the tpacket receive mode through po->prot_hook.func
after releasing that lock. packet_poll() and packet_recvmsg() can then
run the pressure clearing path after the ring has been cleared while
still seeing tpacket_rcv, causing __packet_rcv_has_room() to dereference
stale or NULL ring storage.
Move the existing receive hook assignment into the same
sk_receive_queue.lock section as the ring state update. Keep the
assignment otherwise unchanged, including on TX ring reconfiguration, to
avoid adding behavior changes that are not required for the fix.
Serialize packet_recvmsg() pressure clearing with the same queue lock
only after PACKET_SOCK_PRESSURE has been observed. If the flag is clear
and the socket has moved away from tpacket_rcv, packet_set_ring() has
already detached the socket and waited for synchronize_net(), so no new
packet input can set the flag again.
packet_poll() already holds sk_receive_queue.lock, so it uses the new
unlocked helper directly.
Fixes: 2ccdbaa6d55b ("packet: rollover lock contention avoidance")
Cc: stable@vger.kernel.org
Reported-by: Vega <vega@nebusec.ai>
Assisted-by: Codex:gpt-5.4
Signed-off-by: Zihan Xi <zihanx@nebusec.ai>
Link: https://patch.msgid.link/f90b5688311fa278d1361ea8c6be0bf25967d591.1785247446.git.zihanx@nebusec.ai
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
|
|
The sch_cake ACK filter parses packets to find the TCP header and filter
duplicated ACKs if the flow is backlogged. The parsing code contains a
WARN_ON(1) which can be triggered by a malformed IP header in certain
cases. Depending on the system configuration, this leads either to
either spamming dmesg with warnings, or a panic if panic_on_warn is set.
The code already correctly skips the offending packet in the branch that
triggers the warning, so the WARN_ON itself doesn't really serve any
purpose. So just drop it altogether to avoid the inconvenient side
effects.
Fixes: 8b7138814f29 ("sch_cake: Add optional ACK filter")
Reported-by: Zhiling Zou <zhilinz@nebusec.ai>
Reported-by: Ren Wei <enjou1224z@gmail.com>
Signed-off-by: Toke Høiland-Jørgensen <toke@toke.dk>
Link: https://patch.msgid.link/20260729191417.45665-1-toke@toke.dk
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
|
|
__skb_udp_tunnel_segment() gets the UDP header before ensuring the
tunnel header is in the skb head. If the pull reallocates skb->head,
the saved UDP header pointer is no longer valid.
Get the UDP header after the pull to avoid a potential use-after-free.
Fixes: dbef491ebe7f ("udp: Use uh->len instead of skb->len to compute checksum in segmentation")
Signed-off-by: Xuanqiang Luo <luoxuanqiang@kylinos.cn>
Reviewed-by: Antoine Tenart <atenart@kernel.org>
Link: https://patch.msgid.link/20260730093554.68127-1-xuanqiang.luo@linux.dev
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
|
|
fib_nlmsg_size() still estimates nexthop space as if every gateway is
encoded as an IPv4 RTA_GATEWAY attribute. IPv4 routes can also carry an
IPv6 gateway, which fib_nexthop_info() dumps as RTA_VIA.
As a result, route notifications can allocate an skb that is too small.
fib_dump_info() then fails with -EMSGSIZE and rtmsg_fib() hits the
WARN_ON() that marks such failures as a fib_nlmsg_size() bug. With
panic_on_warn set, this becomes a kernel panic.
Mirror the actual nexthop dump layout in fib_nlmsg_size(): account for
IPv6 nexthop gateways dumped as RTA_VIA, for the no-header rtnexthop
layout used inside RTA_MULTIPATH, and for RTA_FLOW only when it is
actually present.
Fixes: d15662682db2 ("ipv4: Allow ipv6 gateway with ipv4 routes")
Cc: stable@vger.kernel.org
Reported-by: Vega <vega@nebusec.ai>
Signed-off-by: Zihan Xi <zihanx@nebusec.ai>
Reviewed-by: Ido Schimmel <idosch@nvidia.com>
Link: https://patch.msgid.link/6f53fa797fcaeb26966432ed7ae9bb87c4961f37.1785411220.git.zihanx@nebusec.ai
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
|
|
smc_rx_splice() passes pages to splice_to_pipe() before taking the
references that cover the lifetime of each splice entry. In the
VM-backed RMB path, splice_to_pipe() may drop unqueued entries through
smc_rx_spd_release(), while queued entries are released later via the
pipe buffer callback.
The old post-splice accounting also derives the number of queued VM pages
from an offset mutated while building the descriptor, and a multi-page
splice pairs one sock_hold() with multiple sock_put() calls.
Take the page and socket references for every candidate entry before
splice_to_pipe(), and drop the matching private state, page reference,
and socket reference from smc_rx_spd_release() for entries that never
get queued. This fixes a refcount imbalance that can underflow page
refcounts and trigger a use-after-free.
Fixes: 9014db202cb7 ("smc: add support for splice()")
Cc: stable@vger.kernel.org
Reported-by: Vega <vega@nebusec.ai>
Co-developed-by: Xiao Liu <lx24@stu.ynu.edu.cn>
Signed-off-by: Xiao Liu <lx24@stu.ynu.edu.cn>
Signed-off-by: Daming Li <d4n.for.sec@gmail.com>
Signed-off-by: Ren Wei <enjou1224z@gmail.com>
Reviewed-by: Dust Li <dust.li@linux.alibaba.com>
Reviewed-by: Sidraya Jayagond <sidraya@linux.ibm.com>
Link: https://patch.msgid.link/20260730145552.360287-2-enjou1224z@gmail.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
|
|
Continue converting the proto-layer getsockopt callbacks to the sockopt_t
interface, converting do_tls_getsockopt() and its per-option helpers to
take a sockopt_t.
The thin tls_getsockopt() wrapper keeps its __user signature for now: it
builds a user-backed sockopt_t with sockopt_init_user(), calls the helper,
and writes the returned length back to optlen. The helpers use
copy_to_iter() instead of copy_to_user(); the NULL optval check in the
TLS_TX/TLS_RX path is preserved by testing the iterator user buffer.
No functional change.
Reviewed-by: Sabrina Dubroca <sd@queasysnail.net>
Reviewed-by: Joe Damato <joe@dama.to>
Acked-by: Stanislav Fomichev <sdf@fomichev.me>
Signed-off-by: Breno Leitao <leitao@debian.org>
Link: https://patch.msgid.link/20260729-getsockopt_phase4-v4-6-c44576757c17@debian.org
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
|
|
Continue converting the proto-layer getsockopt callbacks to the
sockopt_t interface, splitting pep_getsockopt() into a
do_pep_getsockopt() helper that takes a sockopt_t.
The thin pep_getsockopt() wrapper keeps its __user signature for now:
it builds a user-backed sockopt_t with sockopt_init_user(), calls the
helper, and writes the returned length back to optlen. The helper uses
copy_to_iter() instead of copy_to_user(). No functional change.
Acked-by: Rémi Denis-Courmont <remi@remlab.net>
Reviewed-by: Joe Damato <joe@dama.to>
Acked-by: Stanislav Fomichev <sdf@fomichev.me>
Signed-off-by: Breno Leitao <leitao@debian.org>
Link: https://patch.msgid.link/20260729-getsockopt_phase4-v4-5-c44576757c17@debian.org
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
|
|
pep_getsockopt() clamps the reported length to the caller's buffer with
min_t(), but then stores the value with put_user(val, (int __user *)
optval), which always writes sizeof(int) bytes. A getsockopt() call with
an optlen smaller than sizeof(int) thus reports the clamped length yet
writes a full int, one to three bytes past the user buffer.
Write the value with copy_to_user() bounded by len, so at most optlen
bytes are copied, matching the length reported back to userspace.
Fixes: 02a47617cdce ("Phonet: implement GPRS virtual interface over PEP socket")
Acked-by: Rémi Denis-Courmont <remi@remlab.net>
Reviewed-by: Joe Damato <joe@dama.to>
Acked-by: Stanislav Fomichev <sdf@fomichev.me>
Signed-off-by: Breno Leitao <leitao@debian.org>
Link: https://patch.msgid.link/20260729-getsockopt_phase4-v4-4-c44576757c17@debian.org
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
|
|
Continue converting the proto-layer getsockopt callbacks to the sockopt_t
interface, splitting dgram_getsockopt() into a do_dgram_getsockopt() helper
that takes a sockopt_t.
No functional change.
Reviewed-by: Joe Damato <joe@dama.to>
Acked-by: Stanislav Fomichev <sdf@fomichev.me>
Signed-off-by: Breno Leitao <leitao@debian.org>
Link: https://patch.msgid.link/20260729-getsockopt_phase4-v4-3-c44576757c17@debian.org
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
|
|
Convert do_rawv6_getsockopt to the new sockopt_t model, mirroring what
we have in ipv4. The overall goal is to move these callbacks gradually
from __user points to use sockopt_t, and this part touches
do_rawv6_getsockopt.
No functional change.
Reviewed-by: Joe Damato <joe@dama.to>
Acked-by: Stanislav Fomichev <sdf@fomichev.me>
Signed-off-by: Breno Leitao <leitao@debian.org>
Link: https://patch.msgid.link/20260729-getsockopt_phase4-v4-2-c44576757c17@debian.org
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
|
|
do_rawv6_getsockopt() takes a level argument but never uses it; the
level dispatch is handled by the caller, rawv6_getsockopt(). Drop it,
matching ipv4's do_raw_getsockopt().
No functional change.
Reviewed-by: Joe Damato <joe@dama.to>
Acked-by: Stanislav Fomichev <sdf@fomichev.me>
Signed-off-by: Breno Leitao <leitao@debian.org>
Link: https://patch.msgid.link/20260729-getsockopt_phase4-v4-1-c44576757c17@debian.org
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
|
|
The zero-copy path validates TX metadata while obtaining the descriptor
context, then reads it again later when preparing the hardware request.
User space can change the metadata between those operations and bypass the
original validation.
Validate the metadata in xsk_tx_metadata_request() and use the resulting
flags snapshot for every feature check. Read request fields once so all
zero-copy drivers process only values observed after successful
validation.
Fixes: ca4419f15abd ("xsk: Add launch time hardware offload support to XDP Tx metadata")
Cc: Cen Zhang (Microsoft) <blbllhy@gmail.com>
Signed-off-by: Stanislav Fomichev <sdf@fomichev.me>
Link: https://patch.msgid.link/20260727161959.885642-7-sdf@fomichev.me
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
|
|
Launch-time metadata extends beyond the first 16 bytes of struct
xsk_tx_metadata. Reject the request when the registered metadata area does
not contain the complete field.
Snapshot the validated flags for the generic transmit path and use that
snapshot for request and completion processing, avoiding inconsistent
decisions if user space changes the flags concurrently.
Note that only xsk_skb_metadata is properly using the flags,
__xsk_buff_get_metadata ignores them. Next commits address that.
Fixes: ca4419f15abd ("xsk: Add launch time hardware offload support to XDP Tx metadata")
Cc: Cen Zhang (Microsoft) <blbllhy@gmail.com>
Signed-off-by: Stanislav Fomichev <sdf@fomichev.me>
Link: https://patch.msgid.link/20260727161959.885642-5-sdf@fomichev.me
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
|
|
User space can change metadata flags after request processing. Rereading
them during completion can therefore make the kernel write a timestamp
that was not requested when the packet was submitted.
Clear the metadata pointer during request processing unless timestamp
completion is requested. Completion handling can then use the pointer
itself instead of rereading the flags.
On the mlx5 multi-packet WQE path metadata is evaluated per batch:
xsk_tx_metadata_request() runs only for the descriptor that starts a
session, just like the checksum offload that is applied once through the
shared WQE. Only that descriptor's pointer is reset, so completion
handling can record a timestamp for the other descriptors of the session
regardless of their own XDP_TXMD_FLAGS_TIMESTAMP bit. The write stays
inside the metadata area; the single-WQE, other zero-copy, and generic
paths reset the pointer per descriptor and are unaffected.
Fixes: ca4419f15abd ("xsk: Add launch time hardware offload support to XDP Tx metadata")
Cc: Cen Zhang (Microsoft) <blbllhy@gmail.com>
Signed-off-by: Stanislav Fomichev <sdf@fomichev.me>
Link: https://patch.msgid.link/20260727161959.885642-4-sdf@fomichev.me
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
|
|
AF_XDP accepts a TX metadata length as small as eight bytes, but every
supported request needs the flags plus at least one eight-byte request
field. Such short metadata also lets the kernel read beyond the registered
area.
Require 16 bytes rather than sizeof(struct xsk_tx_metadata) to preserve
compatibility with applications that do not use launch-time metadata.
Fixes: 341ac980eab9 ("xsk: Support tx_metadata_len")
Reported-by: AutonomousCodeSecurity@microsoft.com
Reported-by: Cen Zhang (Microsoft) <blbllhy@gmail.com>
Link: https://lore.kernel.org/netdev/20260720155210.34229-1-blbllhy@gmail.com/
Signed-off-by: Stanislav Fomichev <sdf@fomichev.me>
Link: https://patch.msgid.link/20260727161959.885642-2-sdf@fomichev.me
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
|
|
Commit b917507e5ad9 ("vsock/virtio: stop workers during the .remove()")
made the RX worker jump to its common exit when rx_run is clear. That
exit still refills the RX queue when the buffer count is low, so work
queued across virtio_vsock_vqs_del() can add buffers after the virtqueues
have been deleted.
BUG: KASAN: slab-use-after-free in virtqueue_add_sgs
Read of size 4 by task kworker/0:1
Workqueue: virtio_vsock virtio_transport_rx_work
Call Trace:
virtqueue_add_sgs (drivers/virtio/virtio_ring.c:2796)
virtio_vsock_rx_fill (net/vmw_vsock/virtio_transport.c:332)
virtio_transport_rx_work (net/vmw_vsock/virtio_transport.c:701)
process_one_work (kernel/workqueue.c:3314)
worker_thread (kernel/workqueue.c:3478)
kthread (kernel/kthread.c:436)
ret_from_fork (arch/x86/kernel/process.c:158)
ret_from_fork_asm (arch/x86/entry/entry_64.S:245)
...
Freed by task 141:
kfree (mm/slub.c:6566)
vp_del_vq (drivers/virtio/virtio_pci_common.c:259)
vp_del_vqs (drivers/virtio/virtio_pci_common.c:285)
virtio_vsock_freeze (net/vmw_vsock/virtio_transport.c:912)
virtio_device_freeze (drivers/virtio/virtio.c:658)
virtio_pci_freeze (drivers/virtio/virtio_pci_common.c:601)
pci_pm_freeze (drivers/pci/pci-driver.c:1098)
device_suspend (drivers/base/power/main.c:1968)
Kernel panic - not syncing: KASAN: panic_on_warn set ...
Jump to a no-refill exit when rx_run is clear, leaving the normal exit
to replenish a running queue.
Fixes: b917507e5ad9 ("vsock/virtio: stop workers during the .remove()")
Cc: stable@vger.kernel.org
Reported-by: Xiang Mei <xmei5@asu.edu>
Link: https://lore.kernel.org/r/20260727035804.1860862-1-bestswngs@gmail.com
Suggested-by: Stefano Garzarella <sgarzare@redhat.com>
Signed-off-by: Weiming Shi <bestswngs@gmail.com>
Reviewed-by: Bobby Eshleman <bobbyeshleman@meta.com>
Link: https://patch.msgid.link/f9c8c1d64cad9d262f305d02ffe164c2f900fadf.1785352330.git.bestswngs@gmail.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
|
|
Commit bd50c5dc182b ("vsock/virtio: add support for device
suspend/resume") made the *_run flags transition from false to true when
restore installs replacement virtqueues. The RX, TX and event workers
read their virtqueue before locking and checking the corresponding flag,
so a worker delayed across freeze and restore can observe the replacement
queue's running state while retaining a pointer to the deleted queue.
Read each virtqueue under its mutex after checking the run flag, keeping
the pointer and state in the same queue generation.
Fixes: bd50c5dc182b ("vsock/virtio: add support for device suspend/resume")
Cc: stable@vger.kernel.org
Reported-by: Xiang Mei <xmei5@asu.edu>
Link: https://lore.kernel.org/r/20260727035804.1860862-1-bestswngs@gmail.com
Signed-off-by: Weiming Shi <bestswngs@gmail.com>
Reviewed-by: Bobby Eshleman <bobbyeshleman@meta.com>
Link: https://patch.msgid.link/e79f68ad9284c983364fc3ac46904b6d9ef50231.1785352330.git.bestswngs@gmail.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
|
