5.15.216-rc1 review
Greg Kroah-Hartman
maintainerPATCH 433/456 misc: fastrpc: fix memory leak in fastrpc_channel_ctx_free
In reply to Greg Kroah-Hartman5.15-stable review patch. If anyone has any objections, please let me know.
------------------
From: Eddie Lin <[email protected]>
commit 2fae94ee14f7fea11d3f95e10383a87c01d21518 upstream.
The 'ctx_idr' is initialized but never destroyed when the channel context is freed, leading to a memory leak. Add idr_destroy() to properly clean up the IDR resources.
--- drivers/misc/fastrpc.c | 1 + 1 file changed, 1 insertion(+)
drivers/misc/fastrpc.c +1 -0
@@ -342,6 +342,7 @@ static void fastrpc_channel_ctx_free(str cctx = container_of(ref, struct fastrpc_channel_ctx, refcount); +idr_destroy (&cctx->ctx_idr);kfree (cctx); }
Greg Kroah-Hartman
maintainerPATCH 434/456 ALSA: usx2y: bound the hwdep mmap fault offset
In reply to Greg Kroah-Hartman5.15-stable review patch. If anyone has any objections, please let me know.
------------------
From: Baul Lee <[email protected]>
commit 2ca1eea3cd17930daffe9e429a7c89232036ec24 upstream.
snd_us428ctls_vm_fault() turns the faulting page offset into a kernel address with no bound of any kind:
offset = vmf->pgoff << PAGE_SHIFT; vaddr = (char *)(...)->us428ctls_sharedmem + offset; page = virt_to_page(vaddr); get_page(page); vmf->page = page; return 0;
snd_us428ctls_mmap() checks only the length of the mapping, never the offset, and us428ctls_sharedmem is a single page from alloc_pages_exact(). For a character device file_mmap_size_max() returns ULONG_MAX, so the mm layer imposes no ceiling either. Every page offset above zero resolves to a struct page outside the object, and the handler installs it into the caller's address space read-write; the vma is not marked read-only.
The caller picks the page frame with a single mmap() argument and gets read-write access to a page of kernel memory it does not own; an offset that lands in an unpopulated vmemmap region oopses instead.
A process that can open the hwdep node of an attached US-X2Y reaches this after loading the FPGA image through the same node; no capability check is involved.
On 7.2.0-rc5 (arm64), mmap() with a large offset:
Unable to handle kernel paging request at virtual address fffffdffc45d5ac8 pc : snd_us428ctls_vm_fault+0x68/0x140 [snd_usb_usx2y] Call trace: snd_us428ctls_vm_fault+0x68/0x140 [snd_usb_usx2y] __do_fault __handle_mm_fault handle_mm_fault el0_da
Reject any offset outside the shared region. The pcm hwdep handler in usx2yhwdeppcm.c computes its address the same way and needs the same bound.
Discovered by XBOW, triaged by Baul Lee <[email protected]>
--- sound/usb/usx2y/usX2Yhwdep.c | 2 ++ sound/usb/usx2y/usx2yhwdeppcm.c | 2 ++ 2 files changed, 4 insertions(+)
sound/usb/usx2y/usX2Yhwdep.c +2 -0
@@ -29,6 +29,8 @@ static vm_fault_t snd_us428ctls_vm_fault vmf->pgoff); offset = vmf->pgoff <<PAGE_SHIFT ; +if (offset >=US428_SHAREDMEM_PAGES ) +return VM_FAULT_SIGBUS ; vaddr = (char *)((struct usx2ydev *)vmf->vma->vm_private_data)->us428ctls_sharedmem + offset; page =virt_to_page (vaddr);get_page (page);
sound/usb/usx2y/usx2yhwdeppcm.c +2 -0
@@ -677,6 +677,8 @@ static vm_fault_t snd_usx2y_hwdep_pcm_vmvoid *vaddr; offset = vmf->pgoff <<PAGE_SHIFT ; +if (offset >=USX2Y_HWDEP_PCM_PAGES ) +return VM_FAULT_SIGBUS ; vaddr = (char *)((struct usx2ydev *)vmf->vma->vm_private_data)->hwdep_pcm_shm + offset; vmf->page =virt_to_page (vaddr);get_page (vmf->page);
Greg Kroah-Hartman
maintainerPATCH 435/456 tracing: Fix race between update_event_fields and, event_define_fields
In reply to Greg Kroah-Hartman5.15-stable review patch. If anyone has any objections, please let me know.
------------------
From: Michael Wu <[email protected]>
commit c3730b8373bb5059d735509b9e6a00d7eb337d7c upstream.
The following sequence may leads race between event_define_fields() and update_event_fields():
CPU0 (loads module A) CPU1 (loads module B)
=============================== ===============================
load_module(A) load_module(B)
notifier_call_chain notifier_call_chain
trace_module_notify trace_module_notify
mutex_lock(&event_mutex) trace_event_update_all()
trace_module_add_events(A) down_write(&trace_event_sem)
__register_event(call_A)
__add_event_to_tracers(call_A)
event_define_fields(call_A)
for each f: list_for_each_entry(field,
list_add(&f->link, &class->fields, link)
&class->fields) field = class->fields->next;Where access to the class->fields is not protected by the event_mutex in trace_event_update_all().
This produces the following panic:
Unable to handle kernel access ... at virtual address 0000000000000018
pc : update_event_fields+0xf8/0x368
Call trace:
update_event_fields+0xf8/0x368
trace_event_update_all+0x7c/0x2b4
trace_module_notify+0x4c/0x1dc
notifier_call_chain+0x84/0x168
blocking_notifier_call_chain_robust+0x64/0xd4
load_module+0x10c8/0x123c
__arm64_sys_finit_module+0x230/0x31cFix by taking event_mutex in trace_event_update_all() before trace_event_sem.
--- kernel/trace/trace_events.c | 2 ++ 1 file changed, 2 insertions(+)
kernel/trace/trace_events.c +2 -0
@@ -2875,6 +2875,7 @@ void trace_event_eval_update(struct tracint last_i;int i; +mutex_lock (&event_mutex);down_write (&trace_event_sem);list_for_each_entry_safe (call, p, &ftrace_events, list) {/* events are usually grouped together with systems */
@@ -2908,6 +2909,7 @@ void trace_event_eval_update(struct traccond_resched (); }up_write (&trace_event_sem); +mutex_unlock (&event_mutex); } static struct trace_event_file *
Greg Kroah-Hartman
maintainerPATCH 436/456 fbdev: bitblit: bound-check glyph index in bit_cursor()
In reply to Greg Kroah-Hartman5.15-stable review patch. If anyone has any objections, please let me know.
------------------
From: Rik van Riel <[email protected]>
commit e033cbf3975a8465f879ebd5989dc35b04423a4d upstream.
bit_cursor() fetches the glyph under the cursor with
c = scr_readw(vc_pos); src = vc_font.data + ((c & charmask) * w * height);
where charmask is 0x1ff when vc_hi_font_mask is set. The screen buffer value comes directly from scr_readw() and may be larger than the current font's glyph count.
Syzkaller triggers this via vcs_write(). The Call Trace shows vcs_write() in vc_screen.c writing an arbitrary 16-bit value with writev() to /dev/vcsa, which vcs_write_buf() in vc_screen.c stores via vcs_scr_writew() without checking charcount. The stored value is later read in bit_cursor() in bitblit.c.
When the font is changed from a font with 512 glyphs to a font with 256 glyphs, the screen buffer can retain characters with the high bit set from the previous mode, which could also produce the same out-of-bounds access.
BUG: KASAN: global-out-of-bounds in soft_cursor+0x378/0x6bc drivers/video/fbdev/core/softcursor.c:70 Read of size 16 at addr ffff800086c57970 Call Trace: soft_cursor+0x378/0x6bc drivers/video/fbdev/core/softcursor.c:70 bit_cursor+0xa90/0x1108 drivers/video/fbdev/core/bitblit.c:365 fbcon_cursor+0x344/0x498 drivers/video/fbdev/core/fbcon.c:1427 hide_cursor+0xdc/0x2d0 drivers/tty/vt/vt.c:883 update_region+0x100/0x18c drivers/tty/vt/vt.c:669 vcs_write+0x8ec/0xaf0 drivers/tty/vt/vc_screen.c:685
bit_putcs_aligned() and bit_putcs_unaligned() already clamp the glyph index to vc_font.charcount. Apply the same clamp in bit_cursor() after extracting the attribute and masking, before indexing fontdata.
The fix completes the bounds checking started in commit 18c4ef4e765a ("fbdev: bitblit: bound-check glyph index in bit_putcs*"), which missed the cursor path.
This change should be safe because the clamp reuses the existing contract from fbcon: charcount is maintained under console_lock in con_font_set() and fbcon_font_set(), and hi_font_mask is cleared when switching from 512 to 256 glyphs. When stale screen data with high bits remains after a font switch, or when vcs_write() stores an arbitrary value, clamping the index to 0 prevents the out-of-bounds read without changing cursor semantics — the same fallback bit_putcs uses.
--- drivers/video/fbdev/core/bitblit.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-)
drivers/video/fbdev/core/bitblit.c +7 -2
@@ -274,9 +274,14 @@ static void bit_cursor(struct vc_data *vif (!vc->vc_font.data)return ; - c =scr_readw ((u16 *) vc->vc_pos); + c =scr_readw ((u16 *) vc->vc_pos); attribute =get_attribute (info, c); - src = vc->vc_font.data + ((c & charmask) * (w * vc->vc_font.height)); + c &= charmask; + +/* Clamp to font size, same as bit_putcs_aligned() */ +if (c >= vc->vc_font.charcount) + c =0 ; + src = vc->vc_font.data + (c * (w * vc->vc_font.height));if (ops->cursor_state.image.data != src || ops->cursor_reset) {
Greg Kroah-Hartman
maintainerPATCH 437/456 ipv6: prevent in6_dev_get() from resurrecting inet6_dev
In reply to Greg Kroah-Hartman5.15-stable review patch. If anyone has any objections, please let me know.
------------------
From: Kyle Zeng <[email protected]>
commit 0e243671bc7b8eaf00f83dd2f4367436dc0cff98 upstream.
in6_dev_get() reads dev->ip6_ptr under RCU and then unconditionally increments its refcount. Device teardown can clear the pointer and drop the last reference between these operations. The increment then resurrects an object whose RCU free has already been queued, so callers can use it after it is freed.
Use refcount_inc_not_zero() and return NULL when the object has already reached zero. RCU keeps the memory accessible through the attempted reference acquisition, and a successful increment pins the object for the caller.
An independent run on the exact unpatched 6f5156d7a31a (v7.2-rc3) kernel reproduced the invalid reference acquisition as UID 1000:
refcount_t: addition on 0; use-after-free.
ip6_mc_source+0xef4/0x17e0
It was followed by the corresponding reference underflow in ip6_mc_source(). The supplied trace from the same unpatched revision additionally shows the access after the RCU read-side section ends:
BUG: KASAN: slab-use-after-free in mutex_lock+0x76/0xe0 Write of size 8 at addr ffff888015b50240 by task poc/1219
Bug found and triaged by OpenAI Security Research and validated by Trail of Bits.
--- include/net/addrconf.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
include/net/addrconf.h +2 -2
@@ -368,8 +368,8 @@ static inline struct inet6_dev *in6_dev_rcu_read_lock (); idev =rcu_dereference (dev->ip6_ptr); -if (idev) -refcount_inc (&idev->refcnt); +if (idev && !refcount_inc_not_zero (&idev->refcnt)) + idev =NULL ;rcu_read_unlock ();return idev; }
Greg Kroah-Hartman
maintainerPATCH 438/456 netfilter: bridge: release template ct on non-IP path
In reply to Greg Kroah-Hartman5.15-stable review patch. If anyone has any objections, please let me know.
------------------
From: Zhiling Zou <[email protected]>
commit d45cc8020d7c0a9f01dee42ff5c40bc14c9af72f upstream.
A bridge nftables ct zone set rule can attach a conntrack template to an skb before nf_ct_bridge_pre() sees it. For non-IPv4 and non-IPv6 EtherTypes, nf_ct_bridge_pre() currently overwrites skb->_nfct with IP_CT_UNTRACKED without releasing the existing template reference.
That makes the per-cpu template, and any temporary templates allocated for concurrent use, unreachable and leaks memory until the host runs out of slab.
Reset the skb conntrack state before marking the frame untracked so the existing template reference is dropped on the non-IP path.
--- net/bridge/netfilter/nf_conntrack_bridge.c | 1 + 1 file changed, 1 insertion(+)
net/bridge/netfilter/nf_conntrack_bridge.c +1 -0
@@ -280,6 +280,7 @@ static unsigned int nf_ct_bridge_pre(voi ret =nf_ct_br_defrag6 (skb, &bridge_state);break ;default : +nf_reset_ct (skb);nf_ct_set (skb,NULL ,IP_CT_UNTRACKED );return NF_ACCEPT ; }
Greg Kroah-Hartman
maintainerPATCH 439/456 net: atlantic: free RX pages of consumed but not refilled buffers
In reply to Greg Kroah-Hartman5.15-stable review patch. If anyone has any objections, please let me know.
------------------
From: Yangyu Chen <[email protected]>
commit e8e7471ef686b6c002218fee9671cc61992ae01a upstream.
aq_ring_rx_deinit() only walks [sw_head, sw_tail), the region posted to hardware. Since the page reuse strategy was added, a cleaned RX buffer keeps its page (and its DMA mapping) in the ring for reuse, and refill is batched: aq_ring_rx_fill() returns early until AQ_CFG_RX_REFILL_THRES slots are free. Slots that were consumed but not yet reposted therefore sit in the complementary [sw_tail, sw_head) gap with a live page, and the deinit walk never visits them: up to a refill batch worth of pages and DMA mappings leak on every interface down.
Walk the whole ring instead and release whatever is still there. Also bail out if the buffer ring is already gone: a partial aq_ptp_ring_alloc() failure frees the ring but leaves aq_nic set, so aq_ptp_ring_deinit() still gets here on the unwind path.
--- drivers/net/ethernet/aquantia/atlantic/aq_ring.c | 22 ++++++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-)
drivers/net/ethernet/aquantia/atlantic/aq_ring.c +18 -4
@@ -565,15 +565,29 @@ err_exit:void aq_ring_rx_deinit (struct aq_ring_s *self) { -if (!self) +unsigned int i; + +if (!self || !self->buff_ring)return ; -for (; self->sw_head != self->sw_tail; - self->sw_head =aq_ring_next_dx (self, self->sw_head)) { -struct aq_ring_buff_s *buff = &self->buff_ring[self->sw_head]; +/* Release every page still owned by the ring. +* +* Walking [sw_head, sw_tail) is not enough: refill is batched +* (aq_ring_rx_fill() waits for AQ_CFG_RX_REFILL_THRES free slots), +* so slots that were cleaned but not yet reposted accumulate in the +* [sw_tail, sw_head) gap, and they keep their page for reuse. Walk +* the whole ring and release whatever is left. +*/ +for (i =0 ; i < self->size; i++) { +struct aq_ring_buff_s *buff = &self->buff_ring[i]; + +if (!buff->rxdata.page) +continue ;aq_free_rxpage (&buff->rxdata,aq_nic_get_dev (self->aq_nic)); } + + self->sw_head = self->sw_tail; }void aq_ring_free (struct aq_ring_s *self)
Greg Kroah-Hartman
maintainerPATCH 440/456 net/sched: act_gact, act_police: range check the fallback control action
In reply to Greg Kroah-Hartman5.15-stable review patch. If anyone has any objections, please let me know.
------------------
From: Hyunjung Ko <[email protected]>
commit 883b56ae58fe657d8497806c7059646e9ba6dbd0 upstream.
tcf_action_check_ctrlact() range checks the primary control action:
if (!opcode) ret = action > TC_ACT_VALUE_MAX ? -EINVAL : 0;
TC_ACT_VALUE_MAX is TC_ACT_TRAP, so kernel-internal verdicts above it cannot be set that way. But act_gact and act_police each carry a second, independent control action supplied by user space that never reaches that helper - TCA_GACT_PROB.paction and TCA_POLICE_RESULT. Both only reject TC_ACT_GOTO_CHAIN, so any other value is stored verbatim and returned verbatim from the action.
In particular user space can store TC_ACT_CONSUMED, which is TC_ACT_VALUE_MAX + 1 and is deliberately not part of the UAPI value range. That verdict tells every caller the action took ownership of the skb, so nobody frees it: sch_handle_ingress(), sch_handle_egress() and tcf_qevent_handle() all deliberately skip the free for it. The result is one leaked sk_buff plus its data buffer per packet traversing the filter, unbounded, for all traffic on the chain including kernel-generated packets.
Both are trivially deterministic. act_gact clamps tcfg_pval to >= 1, so with pval = 1 gact_determ() returns the fallback for every packet. act_police has no mandatory rate, so rate = 0 leaves tcfp_mtu = ~0 and tcf_police_mtu_check() always passes.
TC_ACT_CONSUMED was added by commit 720f22fed81b ("net: sched: refactor reinsert action"), after both goto-chain guards were written:
commit 9469f375ab09 ("net/sched: act_gact: disallow 'goto chain' on fallback control action") and
commit c08f5ed5d625 ("net/sched: act_police: disallow 'goto chain' on fallback control action"). Neither guard was widened when the new verdict appeared.
Factor the existing range test out of tcf_action_check_ctrlact() as tcf_action_valid() and apply it to both fallbacks. The helper cannot call tcf_action_check_ctrlact() directly because that also allocates a goto_chain, which is exactly what these two sites must not do.
Reproduced on v7.2-rc6: kmemleak reports one leaked 232-byte skbuff_head_cache object plus its 704-byte data buffer per packet. With this patch both configurations are rejected with -EINVAL and kmemleak reports none.
--- include/net/act_api.h | 19 +++++++++++++++++++ net/sched/act_gact.c | 5 +++++ net/sched/act_police.c | 6 ++++++ 3 files changed, 30 insertions(+)
include/net/act_api.h +19 -0
@@ -249,6 +249,25 @@ int tcf_action_check_ctrlact(int action,struct tcf_chain *tcf_action_set_ctrlact (struct tc_action *a,int action,struct tcf_chain *newchain); +/* Range check for a control action supplied by user space. +* +* This is the same test tcf_action_check_ctrlact() applies to the primary +* control action, factored out for the *fallback* control actions +* (act_gact's TCA_GACT_PROB.paction and act_police's TCA_POLICE_RESULT), +* which must not reach tcf_action_check_ctrlact() because they have no +* goto_chain to allocate. Without it, user space can store kernel-internal +* verdicts such as TC_ACT_CONSUMED, which is TC_ACT_VALUE_MAX + 1 and is +* deliberately not part of the UAPI value range. +*/ +static inline bool tcf_action_valid (int action) +{ +int opcode =TC_ACT_EXT_OPCODE (action); + +if (!opcode) +return action <=TC_ACT_VALUE_MAX ; +return opcode <=TC_ACT_EXT_OPCODE_MAX || action ==TC_ACT_UNSPEC ; +} +#ifdef CONFIG_INET DECLARE_STATIC_KEY_FALSE (tcf_frag_xmit_count);#endif
net/sched/act_gact.c +5 -0
@@ -88,6 +88,11 @@ static int tcf_gact_init(struct net *net p_parm =nla_data (tb[TCA_GACT_PROB ]);if (p_parm->ptype >=MAX_RAND )return -EINVAL ; +if (!tcf_action_valid (p_parm->paction)) { +NL_SET_ERR_MSG (extack, +"invalid fallback control action" ); +return -EINVAL ; + }if (TC_ACT_EXT_CMP (p_parm->paction,TC_ACT_GOTO_CHAIN )) {NL_SET_ERR_MSG (extack,"goto chain not allowed on fallback" );
net/sched/act_police.c +6 -0
@@ -136,6 +136,12 @@ static int tcf_police_init(struct net *nif (tb[TCA_POLICE_RESULT ]) { tcfp_result =nla_get_u32 (tb[TCA_POLICE_RESULT ]); +if (!tcf_action_valid (tcfp_result)) { +NL_SET_ERR_MSG (extack, +"invalid fallback control action" ); + err = -EINVAL ; + goto failure; + }if (TC_ACT_EXT_CMP (tcfp_result,TC_ACT_GOTO_CHAIN )) {NL_SET_ERR_MSG (extack,"goto chain not allowed on fallback" );
Greg Kroah-Hartman
maintainerPATCH 441/456 xdp: reject clones that overrun skb_shared_info tailroom
In reply to Greg Kroah-Hartman5.15-stable review patch. If anyone has any objections, please let me know.
------------------
From: Zhiling Zou <[email protected]>
commit e48e8edbef2eb824201495daa5234560f632b23c upstream.
xdpf_clone() clones broadcast copies into a single page and sets frame_sz to PAGE_SIZE. __xdp_build_skb_from_frame() later treats that page like a normal XDP frame and expects the usual skb_shared_info tailroom at the end of the buffer.
The current check only rejects frames whose linear xdp_frame header, headroom, and packet data exceed PAGE_SIZE. A source frame backed by a larger allocation can still satisfy that check while extending into the clone's required shared-info area. When such a clone is converted back into an skb, build_skb_around() places skb_shared_info over live packet bytes and later writes can corrupt XDP return metadata.
Reject clones unless their linear area fits inside SKB_WITH_OVERHEAD(PAGE_SIZE), matching the tailroom requirement already enforced by the XDP-to-skb conversion path.
--- net/core/xdp.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
net/core/xdp.c +1 -1
@@ -625,7 +625,7 @@ struct xdp_frame *xdpf_clone(struct xdp_ headroom = xdpf->headroom +sizeof (*xdpf); totalsize = headroom + xdpf->len; -if (unlikely (totalsize >PAGE_SIZE )) +if (unlikely (totalsize >SKB_WITH_OVERHEAD (PAGE_SIZE )))return NULL ; page =dev_alloc_page ();if (!page)
Greg Kroah-Hartman
maintainerPATCH 442/456 vxlan: do not arm the ageing timer on a device that is down
In reply to Greg Kroah-Hartman5.15-stable review patch. If anyone has any objections, please let me know.
------------------
From: Baul Lee <[email protected]>
commit b37971686ec59fb027fa4910ba16805e68fddb97 upstream.
vxlan_changelink() arms vxlan->age_timer whenever the requested ageing interval differs from the configured one:
if (conf.age_interval != vxlan->cfg.age_interval) mod_timer(&vxlan->age_timer, jiffies);
There is no netif_running() test, so the timer is armed even on a device that was never brought up. The only synchronous cancel in the driver is the timer_delete_sync() in vxlan_stop(), which is .ndo_stop. netif_close_many() drops devices without IFF_UP before __dev_close_many() runs, so that cancel is skipped for such a device.
vxlan_setup() sets dev->needs_free_netdev = true and age_timer is a member of struct vxlan_dev, so free_netdev() releases the allocation the timer lives in while it is still queued on a timer_base. expire_timers() unlinks the entry before it loads timer->function, so the timer core writes through the freed object's list pointers:
BUG: KASAN: slab-use-after-free in __run_timers+0x208/0x654 Write of size 8 at addr ffff00001adace68 by task true/192 __asan_store8+0x84/0xac __run_timers+0x208/0x654 run_timer_softirq+0x154/0x18c Allocated by task 189: alloc_netdev_mqs+0x64/0x720 rtnl_create_link+0x4ac/0x520 rtnl_newlink+0x758/0xd00 Freed by task 191: netdev_release+0x40/0x58 netdev_run_todo+0x4a4/0x8c0 rtnl_dellink+0x200/0x4e8
The rtnl operations involved are netns-scoped, so an unprivileged user can perform them in a new user and network namespace.
Arming the timer on a down device never had an effect: vxlan_cleanup() returns early on !netif_running(), and vxlan_open() arms the timer for any non-zero interval once the device is brought up. Add the missing test.
Discovered by XBOW, triaged by Baul Lee <[email protected]>
--- drivers/net/vxlan/vxlan_core.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/net/vxlan/vxlan_core.c +1 -1
@@ -4319,7 +4319,7 @@ static int vxlan_changelink(struct net_dspin_unlock_bh (&vxlan->hash_lock[hash_index]); } -if (conf.age_interval != vxlan->cfg.age_interval) +if (netif_running (dev) && conf.age_interval != vxlan->cfg.age_interval)mod_timer (&vxlan->age_timer, jiffies);netdev_adjacent_change_commit (dst->remote_dev, lowerdev, dev);
Greg Kroah-Hartman
maintainerPATCH 443/456 vsock/virtio: read virtqueues under worker locks
In reply to Greg Kroah-Hartman5.15-stable review patch. If anyone has any objections, please let me know.
------------------
From: Weiming Shi <[email protected]>
commit ebac8f6b1ef0e9278afe204b8692a7479988dace upstream.
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.
--- net/vmw_vsock/virtio_transport.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-)
net/vmw_vsock/virtio_transport.c +6 -5
@@ -292,12 +292,13 @@ static void virtio_transport_tx_work(strstruct virtqueue *vq;bool added = false; - vq = vsock->vqs[VSOCK_VQ_TX ];mutex_lock (&vsock->tx_lock);if (!vsock->tx_run) goto out; + vq = vsock->vqs[VSOCK_VQ_TX ]; + do { struct virtio_vsock_pkt *pkt; unsigned int len;
@@ -397,13 +398,13 @@ static void virtio_transport_event_work( container_of(work, struct virtio_vsock, event_work);struct virtqueue *vq; - vq = vsock->vqs[VSOCK_VQ_EVENT ]; -mutex_lock (&vsock->event_lock);if (!vsock->event_run) goto out; + vq = vsock->vqs[VSOCK_VQ_EVENT ]; + do { struct virtio_vsock_event *event; unsigned int len;
@@ -519,13 +520,13 @@ static void virtio_transport_rx_work(str container_of(work, struct virtio_vsock, rx_work);struct virtqueue *vq; - vq = vsock->vqs[VSOCK_VQ_RX ]; -mutex_lock (&vsock->rx_lock);if (!vsock->rx_run) goto out; + vq = vsock->vqs[VSOCK_VQ_RX ]; + do { virtqueue_disable_cb(vq); for (;;) {
Greg Kroah-Hartman
maintainerPATCH 444/456 vsock/virtio: avoid refilling the RX queue after teardown
In reply to Greg Kroah-Hartman5.15-stable review patch. If anyone has any objections, please let me know.
------------------
From: Weiming Shi <[email protected]>
commit a31e0ad444698d8aa7534a0f89fda543730f97a5 upstream.
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.
--- net/vmw_vsock/virtio_transport.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
net/vmw_vsock/virtio_transport.c +2 -1
@@ -523,7 +523,7 @@ static void virtio_transport_rx_work(strmutex_lock (&vsock->rx_lock);if (!vsock->rx_run) - goto out; + goto out_nofill; vq = vsock->vqs[VSOCK_VQ_RX ];
@@ -564,6 +564,7 @@ static void virtio_transport_rx_work(str out:if (vsock->rx_buf_nr < vsock->rx_buf_max_nr /2 )virtio_vsock_rx_fill (vsock); +out_nofill:mutex_unlock (&vsock->rx_lock); }
Greg Kroah-Hartman
maintainerPATCH 445/456 vhost: reset the vring metadata cache on vring reconfiguration
In reply to Greg Kroah-Hartman5.15-stable review patch. If anyone has any objections, please let me know.
------------------
From: Jun Yang <[email protected]>
commit de845981da67a6b049080c87e605130b0c30adc5 upstream.
vq->meta_iotlb[] caches the vhost_iotlb_map that backs each vring metadata region, and iotlb_access_ok() returns early on a cache hit, taking the hit as proof that the region has already been validated:
if (vhost_vq_meta_fetch(vq, addr, len, type)) return true;
The cache is reset on VHOST_IOTLB_UPDATE and VHOST_IOTLB_INVALIDATE, on device IOTLB (re)initialisation and on vq reset, but not when VHOST_SET_VRING_ADDR replaces vq->desc, vq->avail and vq->used, nor when VHOST_SET_VRING_NUM changes the region sizes.
With a device IOTLB attached both ioctls are accepted while the vq is live, and neither validates the addresses at ioctl time: vq_access_ok() and vq_log_used_access_ok() return true early because the addresses are GIOVAs, deferring validation to prefetch time. Once the cache has been populated that deferred validation no longer runs -- vq_meta_prefetch() hits the stale entry and returns true -- and vhost_vq_meta_fetch() keeps translating through the old mapping as
map->addr + addr - map->start
for an address the mapping no longer covers. vhost_copy_to_user() and vhost_copy_from_user() consume the result with __copy_to_user() and __copy_from_user(), which do not check it either, so a subsequent used ring update or descriptor fetch accesses memory outside the region the IOTLB actually maps.
Reset the metadata cache whenever the vring is reconfigured, so the new addresses are pushed back through iotlb_access_ok()'s slow path.
Message-ID: <[email protected]>
--- drivers/vhost/vhost.c | 8 ++++++++ 1 file changed, 8 insertions(+)
drivers/vhost/vhost.c +8 -0
@@ -1582,6 +1582,14 @@ static long vhost_vring_set_num_addr(strBUG (); } +/* +* The metadata cache holds the IOTLB mapping that backed the previous +* desc/avail/used addresses and vring size, both of which are being +* replaced here. iotlb_access_ok() takes a cache hit as proof that the +* region was validated, so the stale entries have to go. +*/ +__vhost_vq_meta_reset (vq); +mutex_unlock (&vq->mutex);return r;
Greg Kroah-Hartman
maintainerPATCH 446/456 tipc: read le->link under the node lock in tipc_node_link_down()
In reply to Greg Kroah-Hartman5.15-stable review patch. If anyone has any objections, please let me know.
------------------
From: Jun Yang <[email protected]>
commit cba9ccb47e9fa4cc77692fb896cc5ab57a667882 upstream.
tipc_node_link_down() caches the link pointer before taking n->lock:
struct tipc_link *l = le->link; /* unlocked */
if (!l)
return;
tipc_node_write_lock(n);
if (!tipc_link_is_establishing(l)) { /* deref l */
...
tipc_link_reset(l); /* write into l */
if (delete) {
kfree(l);
le->link = NULL;The delete=true caller frees that very object under n->lock, so the lock does not protect the cached pointer against it:
- CPU A, delete=false: tipc_rcv() on TIPC_LINK_DOWN_EVT, or the link supervision timer via tipc_node_timeout(), reads l unlocked and then dereferences it under n->lock;
- CPU B, delete=true: netlink TIPC_NL_BEARER_DISABLE -> bearer_disable() -> tipc_node_delete_links() -> tipc_node_link_down(n, bearer_id, true) -> kfree(l).
The link is freed with plain kfree(), not kfree_rcu(), and for UDP bearers disable_media() only schedules the asynchronous cleanup_bearer() work, so its synchronize_net() runs after the links are already gone. An in-flight CPU A that has read l therefore dereferences freed memory once B frees it:
a use-after-free read in tipc_link_is_establishing(), and a use-after-free write via tipc_link_reset() on the establishing branch.
The following trace was captured on 7.2.0-rc5-00284-gaf39eb111ce6:
BUG: KASAN: slab-use-after-free in tipc_link_is_establishing (net/tipc/link.c:285) Read of size 4 at addr ffff88802e2aa068 by task swapper/2/0 tipc_link_is_establishing (net/tipc/link.c:285) tipc_node_link_down (net/tipc/node.c:1076) tipc_node_timeout (net/tipc/node.c:843) Allocated by task 9549: tipc_link_create (net/tipc/link.c:490) tipc_node_check_dest (net/tipc/node.c:1279) tipc_disc_rcv (net/tipc/discover.c:252) tipc_udp_recv (net/tipc/udp_media.c:389) Freed by task 9549: tipc_node_link_down (net/tipc/node.c:1084) tipc_node_delete_links (net/tipc/node.c:1320) bearer_disable (net/tipc/bearer.c:414) __tipc_nl_bearer_disable (net/tipc/bearer.c:992)
Move the le->link read inside tipc_node_write_lock(), so it is serialised against the kfree() in the delete path. A racing teardown now either has not run yet, and we see a valid link, or has already run, and we see NULL.
--- net/tipc/node.c | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-)
net/tipc/node.c +10 -5
@@ -1063,18 +1063,23 @@ static void __tipc_node_link_down(structstatic void tipc_node_link_down (struct tipc_node *n,int bearer_id,bool delete) { -struct tipc_link_entry *le = &n->links[bearer_id];struct tipc_media_addr *maddr =NULL ; -struct tipc_link *l = le->link;int old_bearer_id = bearer_id; +struct tipc_link_entry *le;struct sk_buff_head xmitq; - -if (!l) -return ; +struct tipc_link *l;__skb_queue_head_init (&xmitq); +/* Synchronize the link lookup with bearer teardown. */ tipc_node_write_lock (n); + le = &n->links[bearer_id]; + l = le->link; +if (!l) { +tipc_node_write_unlock_fast (n); +return ; + } +if (!tipc_link_is_establishing (l)) {__tipc_node_link_down (n, &bearer_id, &xmitq, &maddr); }else {
Greg Kroah-Hartman
maintainerPATCH 447/456 Revert "thermal/drivers/hwmon: Cleanup coding style a bit"
In reply to Greg Kroah-Hartman5.15-stable review patch. If anyone has any objections, please let me know.
------------------
From: Rafael J. Wysocki <[email protected]>
commit ff8da20b6f47c48d46e47f93f7a59e2d56ee9107 upstream.
Revert commit 030a48b0f6ce ("thermal/drivers/hwmon: Cleanup coding style a bit") that introduced a use-after-free into the error path of thermal_add_hwmon_sysfs() by removing a valid check from it.
--- drivers/thermal/thermal_hwmon.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
drivers/thermal/thermal_hwmon.c +2 -1
@@ -209,7 +209,8 @@ int thermal_add_hwmon_sysfs(struct thermif (new_hwmon_device)hwmon_device_unregister (hwmon->device); free_mem: -kfree (hwmon); +if (new_hwmon_device) +kfree (hwmon);return result; }
Greg Kroah-Hartman
maintainerPATCH 448/456 ring-buffer: Use current_context for safe per-CPU buffer swap
In reply to Greg Kroah-Hartman5.15-stable review patch. If anyone has any objections, please let me know.
------------------
From: Tengda Wu <[email protected]>
commit f27bdc43077e4fcb5557dfc315ee8d91e741f483 upstream.
The ring_buffer_swap_cpu() function currently checks the per-CPU committing counter to determine if a buffer is actively being written to before performing the swap. However, there exists a race window where this check can be bypassed:
ring_buffer_lock_reserve
cpu_buffer = buffer->buffers[cpu]; // cpu_buffer_a
rb_reserve_next_event
rb_start_commit // inc committing
if (unlikely(READ_ONCE(cpu_buffer->buffer) != buffer)) {...}
__rb_reserve_next
rb_move_tail
rb_end_commit(cpu_buffer); // dec committing => 0
/* interrupt hits here, successfully swaps! */
local_inc(&cpu_buffer->committing);
ring_buffer_unlock_commit
cpu_buffer = buffer->buffers[cpu]; // cpu_buffer_b
rb_commit
rb_end_commit
RB_WARN_ON(cpu_buffer, !local_read(&cpu_buffer->committing))
// triggers warningThe committing counter can temporarily drop to 0 during a single write operation (within rb_move_tail), creating a window where swap can succeed even though the write is still in progress. This leads to inconsistent buffer state and triggers the RB_WARN_ON in rb_commit().
Replace the committing counter check with current_context checks, which are set at the entry of ring_buffer_lock_reserve() and remain valid throughout the entire write operation, providing a reliable indicator of buffer busy state during swap.
--- kernel/trace/ring_buffer.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
kernel/trace/ring_buffer.c +4 -4
@@ -5415,7 +5415,7 @@ int ring_buffer_swap_cpu(struct trace_bu {struct ring_buffer_per_cpu *cpu_buffer_a;struct ring_buffer_per_cpu *cpu_buffer_b; -int ret = -EINVAL ; +int ret = -EBUSY ;if (!cpumask_test_cpu (cpu, buffer_a->cpumask) || !cpumask_test_cpu (cpu, buffer_b->cpumask))
@@ -5451,10 +5451,10 @@ int ring_buffer_swap_cpu(struct trace_buatomic_inc (&cpu_buffer_a->record_disabled);atomic_inc (&cpu_buffer_b->record_disabled); - ret = -EBUSY ; -if (local_read (&cpu_buffer_a->committing)) +/* Do not swap if either buffer is in the process of writing */ +if (cpu_buffer_a->current_context) goto out_dec; -if (local_read (&cpu_buffer_b->committing)) +if (cpu_buffer_b->current_context) goto out_dec;/*
Greg Kroah-Hartman
maintainerPATCH 449/456 ipv6: fix Route Information option length validation
In reply to Greg Kroah-Hartman5.15-stable review patch. If anyone has any objections, please let me know.
------------------
From: Yuejie Shi <[email protected]>
commit d1ad8fb2ac6a1afb71dc22d9ae8efb4dda96c824 upstream.
rt6_route_rcv() validates the Route Information option (RFC 4191) length against the prefix length, but both checks are off by one.
rinfo->length is the ND option length in units of 8 octets and it *includes* the 8-byte option header, so an option carrying N bytes of prefix has length == 1 + N/8. RFC 4191 section 2.3 requires length 3 when Prefix Length is greater than 64, and 2 or 3 when it is greater than 0. The code accepts length >= 2 and length >= 1 respectively.
ipv6_addr_prefix() then copies prefix_len/8 bytes out of rinfo->prefix, so a Router Advertisement with (prefix_len=128, length=2) or (prefix_len=64, length=1) makes the kernel read up to 8 bytes past the end of the option. Those bytes end up in the prefix of the route that gets installed, so they are visible to userspace:
# RA with a Route Information option (prefix_len=128, length=2)
# followed by a source link-layer address option, 01 01 de ad be ef ca fe
$ ip -6 route show
2001:db8:dead:beef:101:dead:beef:cafe via fe80::1234 dev veth0 proto ra
^^^^^^^^^^^^^^^^^^ the next option, read out of boundsWhen the Route Information option is the last one in the packet, those eight bytes come from the skb tail room instead.
Reject the option lengths RFC 4191 does not allow.
--- net/ipv6/route.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
net/ipv6/route.c +4 -4
@@ -947,13 +947,13 @@ int rt6_route_rcv(struct net_device *dev }else if (rinfo->prefix_len >128 ) {return -EINVAL ; }else if (rinfo->prefix_len >64 ) { -if (rinfo->length <2 ) { +/* RFC 4191: Length MUST be 3 when Prefix Length > 64 */ +if (rinfo->length <3 )return -EINVAL ; - } }else if (rinfo->prefix_len >0 ) { -if (rinfo->length <1 ) { +/* RFC 4191: Length MUST be 2 or 3 when Prefix Length > 0 */ +if (rinfo->length <2 )return -EINVAL ; - } } pref = rinfo->route_pref;
Greg Kroah-Hartman
maintainerPATCH 450/456 ip6_tunnel: clear skb2->cb[] in ip6ip6_err()
In reply to Greg Kroah-Hartman5.15-stable review patch. If anyone has any objections, please let me know.
------------------
From: Zhiling Zou <[email protected]>
commit f803c086399da277b5d0ff36a107d0f162751800 upstream.
ip6ip6_err() clones an outer IPv6 ICMP error skb, pulls it to the quoted inner IPv6 packet, and then passes the clone to icmpv6_send(). The clone still carries the outer packet's inet6_skb_parm in skb->cb.
If the outer packet had a Home Address Option, IP6CB(skb2)->dsthao remains non-zero after skb_pull(). icmpv6_send() later calls mip6_addr_swap(), which uses that stale dsthao offset against the quoted inner packet. A malformed inner destination-options header can then make the HAO lookup and address swap run past the end of the quoted packet and corrupt skb_shared_info.
Clear skb2->cb[] before pulling the quoted inner IPv6 packet so the reply path does not reuse metadata left by the outer IPv6 stack.
--- net/ipv6/ip6_tunnel.c | 3 +++ 1 file changed, 3 insertions(+)
net/ipv6/ip6_tunnel.c +3 -0
@@ -677,6 +677,9 @@ ip6ip6_err(struct sk_buff *skb, struct iif (!skb2)return 0 ; +/* Remove debris left by outer IPv6 stack. */ +memset (IP6CB (skb2),0 ,sizeof (*IP6CB (skb2))); +skb_dst_drop (skb2);skb_pull (skb2, offset);skb_reset_network_header (skb2);
Greg Kroah-Hartman
maintainerPATCH 451/456 bpf, sockmap: Fix sk_redir use-after-free in send verdict
In reply to Greg Kroah-Hartman5.15-stable review patch. If anyone has any objections, please let me know.
------------------
From: Chengfeng Ye <[email protected]>
commit a76624733730e541e4955fdecf506af2f6b20558 upstream.
sk_psock_msg_verdict() takes a socket reference for psock->sk_redir. tcp_bpf_send_verdict() copies that pointer while holding the source socket lock, but does not take a reference for the local copy before dropping the lock around tcp_bpf_sendmsg_redir().
When apply_bytes keeps the cached verdict active, another sendmsg() on the same source socket can consume the remaining bytes and release the cached reference while the first thread still holds only the raw local pointer:
CPU 0 CPU 1
sk_redir = psock->sk_redir
apply_bytes remains nonzero
release_sock(sk)
lock_sock(sk) apply_bytes reaches zero psock->sk_redir = NULL release_sock(sk) tcp_bpf_sendmsg_redir(sk_redir) sock_put(sk_redir)
tcp_bpf_sendmsg_redir(sk_redir)
The final sock_put() can free sk_redir before CPU 0 dereferences it.
KASAN reported:
BUG: KASAN: slab-use-after-free in tcp_bpf_sendmsg_redir+0xf39/0x1020 Read of size 8 at addr ffff888108537090 by task poc/87 Call Trace: tcp_bpf_sendmsg_redir+0xf39/0x1020 tcp_bpf_sendmsg+0x977/0x1a50 __sys_sendto+0x32c/0x3a0 __x64_sys_sendto+0xdb/0x1b0 Allocated by task 85: sk_prot_alloc+0x56/0x210 sk_clone+0x6f/0x14b0 inet_csk_clone_lock+0x24/0x740 tcp_create_openreq_child+0x25/0x2710 tcp_v4_syn_recv_sock+0x10a/0xe00 Freed by task 0: __kasan_slab_free+0x43/0x70 slab_free_after_rcu_debug+0xa6/0x1e0 rcu_core+0x50a/0x1850 Last potentially related work creation: __sk_destruct+0x3da/0x540 sk_psock_destroy+0x81e/0xab0 process_one_work+0x63a/0x1070
Take a temporary socket reference while the source socket lock still protects psock->sk_redir, and drop it after tcp_bpf_sendmsg_redir() returns. This keeps each unlocked use independent of cached-verdict ownership.
--- net/ipv4/tcp_bpf.c | 2 ++ 1 file changed, 2 insertions(+)
net/ipv4/tcp_bpf.c +2 -0
@@ -390,6 +390,7 @@ more_data:case __SK_REDIRECT: redir_ingress = psock->redir_ingress; sk_redir = psock->sk_redir; +sock_hold (sk_redir);sk_msg_apply_bytes (psock, tosend);if (!psock->apply_bytes) {/* Clean up before releasing the sock lock. */
@@ -410,6 +411,7 @@ more_data:if (eval == __SK_REDIRECT)sock_put (sk_redir); +sock_put (sk_redir);lock_sock (sk);sk_mem_uncharge (sk, sent);
Greg Kroah-Hartman
maintainerPATCH 452/456 scsi: scsi_debug: Negate wrapped memcmp() result
In reply to Greg Kroah-Hartman5.15-stable review patch. If anyone has any objections, please let me know.
------------------
From: Xu Rao <[email protected]>
commit c4f6916a99cf105c3ff340b6210fcbba3fa66b35 upstream.
comp_write_worker() returns true when the compared data matches. memcmp() returns zero for equal data and non-zero for different data, so its result must be negated before it is stored in a bool.
The first segment already uses !memcmp(), but the wrapped segment uses memcmp() directly, reversing the match result. Use !memcmp() there as well.
--- drivers/scsi/scsi_debug.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/scsi/scsi_debug.c +2 -2
@@ -3005,8 +3005,8 @@ static bool comp_write_worker(struct sdeif (!res)return res;if (rest) - res =memcmp (fsp, arr + ((num - rest) * lb_size), - rest * lb_size); + res = !memcmp (fsp, arr + ((num - rest) * lb_size), + rest * lb_size);if (!res)return res;if (compare_only)
Greg Kroah-Hartman
maintainerPATCH 453/456 sctp: keep chunk->transport in step with the list it is queued on
In reply to Greg Kroah-Hartman5.15-stable review patch. If anyone has any objections, please let me know.
------------------
From: Baul Lee <[email protected]>
commit 9f2cf069a9a72a2d6b97ca8b4c70e714aac99749 upstream.
__sctp_outq_flush_rtx() moves a gap-acked chunk onto another transport's transmitted list without updating chunk->transport:
if (chunk->tsn_gap_acked) {
list_move_tail(&chunk->transmitted_list,
&transport->transmitted);
continue;
}The chunk then sits on a live transport's list while chunk->transport still names a different one. If that transport is removed - sctp_assoc_rm_peer() from an ASCONF Delete-IP - sctp_transport_free() RCU-frees it and the chunk is left with a dangling pointer. sctp_assoc_rm_peer() scrubs peer->transmitted and asoc->outqueue.out_chunk_list, but the chunk is on neither.
The pointer is not followed while tsn_gap_acked is set. A SACK that reneges on the TSN clears the flag, and the next SACK reaches
tchunk->transport->flight_size -= sctp_data_size(tchunk);
inside the freed transport. KASAN reports a slab-use-after-free read in sctp_check_transmitted(), freed from sctp_assoc_rm_peer(). Both the removal and the SACKs come from the association peer.
Set chunk->transport at the move. The ordinary resend path needs nothing:
it reaches its list_move_tail() only after sctp_packet_append_chunk() returned SCTP_XMIT_OK, and __sctp_packet_append_chunk() has rebound the chunk by then.
Discovered by XBOW, triaged by Baul Lee <[email protected]>
--- net/sctp/outqueue.c | 1 + 1 file changed, 1 insertion(+)
net/sctp/outqueue.c +1 -0
@@ -647,6 +647,7 @@ static int __sctp_outq_flush_rtx(structif (chunk->tsn_gap_acked) {list_move_tail (&chunk->transmitted_list, &transport->transmitted); + chunk->transport = transport;continue ; }
Greg Kroah-Hartman
maintainerPATCH 454/456 sctp: fix use-after-free of cached ASCONF chunk
In reply to Greg Kroah-Hartman5.15-stable review patch. If anyone has any objections, please let me know.
------------------
From: Yuxiang Yang <[email protected]>
commit 8c283e7b56adce00193837f3311b06662466fb21 upstream.
addip_last_asconf caches the outstanding outbound ASCONF chunk. The normal ASCONF-ACK completion path releases the chunk and clears the pointer.
However, sctp_asconf_queue_teardown() releases the cached chunk without clearing addip_last_asconf. During peer restart handling, sctp_sf_do_dupcook_a() queues SCTP_CMD_PURGE_ASCONF_QUEUE, which invokes sctp_asconf_queue_teardown() while the association remains alive and leaves the pointer dangling.
A delayed authenticated ASCONF-ACK can then reach sctp_sf_do_asconf_ack(), which accesses the stale chunk and passes it to sctp_process_asconf_ack(), causing a use-after-free and a second release.
Clearing the pointer exposes a race with T4 expiry. Peer restart handling queues the timer stop before the purge, but SCTP_CMD_TIMER_STOP uses timer_delete(), which does not wait for a callback already running on another CPU. Such a callback can reach sctp_sf_t4_timer_expire() after the purge and dereference NULL.
Clear addip_last_asconf after releasing the cached chunk, and make sctp_sf_t4_timer_expire() consume a stale T4 expiry if no outstanding ASCONF remains.
--- net/sctp/associola.c | 4 +++- net/sctp/sm_statefuns.c | 6 +++++- 2 files changed, 8 insertions(+), 2 deletions(-)
net/sctp/associola.c +3 -1
@@ -1732,6 +1732,8 @@ void sctp_asconf_queue_teardown(struct ssctp_assoc_free_asconf_queue (asoc);/* Free any cached ASCONF chunk. */ -if (asoc->addip_last_asconf) +if (asoc->addip_last_asconf) {sctp_chunk_free (asoc->addip_last_asconf); + asoc->addip_last_asconf =NULL ; + } }
net/sctp/sm_statefuns.c +5 -1
@@ -6098,8 +6098,12 @@ enum sctp_disposition sctp_sf_t4_timer_e struct sctp_cmd_seq *commands) {struct sctp_chunk *chunk = asoc->addip_last_asconf; -struct sctp_transport *transport = chunk->transport; +struct sctp_transport *transport; +if (!chunk) +return SCTP_DISPOSITION_CONSUME ; + + transport = chunk->transport;SCTP_INC_STATS (net,SCTP_MIB_T4_RTO_EXPIREDS );/* ADDIP 4.1 B1) Increment the error counters and perform path failure
Greg Kroah-Hartman
maintainerPATCH 455/456 sctp: clear new_transport when removing a peer
In reply to Greg Kroah-Hartman5.15-stable review patch. If anyone has any objections, please let me know.
------------------
From: Qing Ming <[email protected]>
commit beb33f8ee1ca83acddb2a5ae80f3d22ec550b4c3 upstream.
sctp_process_asconf_param() stores a newly added peer transport in asoc->new_transport. After all parameters in the ASCONF chunk have been processed, sctp_sf_do_asconf() uses this pointer to send a HEARTBEAT to the new transport.
An authenticated ASCONF from a remote SCTP peer can add a transport and remove it again with a wildcard DEL-IP parameter in the same chunk. The wildcard deletion preserves the transport on which the ASCONF arrived, but removes the newly added transport through sctp_assoc_del_nonprimary_peers(). The removal does not clear asoc->new_transport, leaving it pointing to the removed transport.
sctp_sf_do_asconf() then creates a HEARTBEAT whose chunk->transport points to the removed transport without holding a transport reference. During local address replacement, src_out_of_asoc_ok keeps this HEARTBEAT on control_chunk_list. After the transport is freed by RCU, a successful ASCONF_ACK for the replacement address releases the queued HEARTBEAT and sctp_outq_select_transport() reads the freed transport's state.
The issue was found during a static audit of SCTP objects. With an authenticated peer, the reproducer triggered the same KASAN report in 2 of 2 unpatched runs on a KASAN-enabled netdev/main kernel:
BUG: KASAN: slab-use-after-free in sctp_outq_select_transport Read of size 4 at addr ffff88800b9bd95c by task python3/197 Call Trace: sctp_outq_select_transport+0x549/0x8b0 [sctp] sctp_outq_flush+0x306/0x2c60 [sctp] sctp_transport_immediate_rtx+0xaf/0x260 [sctp] sctp_process_asconf_ack+0xa48/0xf70 [sctp] Allocated by task 197: sctp_transport_new+0x68/0x650 [sctp] sctp_assoc_add_peer+0x258/0x12a0 [sctp] sctp_process_asconf+0x5e9/0x1090 [sctp] Last potentially related work creation: __call_rcu_common.constprop.0+0x77/0xb70 sctp_assoc_del_nonprimary_peers+0x7c/0xd0 [sctp] sctp_process_asconf+0xd9c/0x1090 [sctp]
The first invalid access was a four-byte read of transport->state at net/sctp/outqueue.c:833. The same reproducer completed the full authenticated ASCONF and local-address replacement sequence with this change without a KASAN report or oops.
Clear new_transport when its peer is removed, before it can be used to create the HEARTBEAT.
--- net/sctp/associola.c | 3 +++ 1 file changed, 3 insertions(+)
net/sctp/associola.c +3 -0
@@ -545,6 +545,9 @@ void sctp_assoc_rm_peer(struct sctp_asso asoc->addip_last_asconf->transport == peer) asoc->addip_last_asconf->transport = NULL; +if (asoc->new_transport == peer) + asoc->new_transport =NULL ; +/* If we have something on the transmitted list, we have to * save it off. The best place is the active path. */
Greg Kroah-Hartman
maintainerPATCH 456/456 thunderbolt: Bound the DROM dual link port number before indexing sw->ports
In reply to Greg Kroah-Hartman5.15-stable review patch. If anyone has any objections, please let me know.
------------------
From: Bryam Vargas <[email protected]>
commit d6764992f17b23d91ff93ce905ab53c2aa7191f0 upstream.
tb_drom_parse_entry_port() validates the device-supplied header->index against sw->config.max_port_number before indexing sw->ports[], but the sibling field entry->dual_link_port_nr -- a 6-bit value also read from the DROM -- indexes the same array with no such check. A malicious or malformed Thunderbolt device can set dual_link_port_nr beyond the allocated sw->ports[] (max_port_number + 1 entries), producing an out-of-bounds tb_port pointer that is stored and later dereferenced.
Reject a port entry whose dual_link_port_nr exceeds max_port_number, the same bound already applied to header->index.
--- drivers/thunderbolt/eeprom.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-)
drivers/thunderbolt/eeprom.c +8 -1
@@ -392,9 +392,16 @@ static int tb_drom_parse_entry_port(strureturn -EIO ; } port->link_nr = entry->link_nr; -if (entry->has_dual_link_port) +if (entry->has_dual_link_port) { +if (entry->dual_link_port_nr > sw->config.max_port_number) { +tb_sw_warn (sw, +"port entry has invalid dual link port number %u\n" , + entry->dual_link_port_nr); +return -EIO ; + } port->dual_link_port = &port->sw->ports[entry->dual_link_port_nr]; + } }return 0 ; }
Hi!
Quoted 4 lines
This is the start of the stable review cycle for the 5.15.216 release. There are 456 patches in this series, all will be posted as a response to this one. If anyone has any issues with these being applied, please let me know.
CIP testing did not find any problems here:
https://gitlab.com/cip-project/cip-testing/linux-stable-rc-ci/-/tree/linux-5.15.y
Best regards,
Pavel
Quoted 9 lines · Greg Kroah-Hartman
This is the start of the stable review cycle for the 5.15.216 release. There are 456 patches in this series, all will be posted as a response to this one. If anyone has any issues with these being applied, please let me know.
Responses should be made by Wed, 19 Aug 2026 13:23:44 +0000. Anything received after that time might be too late.
The whole patch series can be found in one patch at:
Quoted 8 lines
https://www.kernel.org/pub/linux/kernel/v5.x/stable-review/patch-5.15.216-rc1.gzor in the git tree and branch at:
git://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable-rc.gitlinux-5.15.y
and the diffstat can be found below.thanks,
greg k-h
On ARCH_BRCMSTB using 32-bit and 64-bit ARM kernels, build tested on BMIPS_GENERIC:
Signature · 1 line
Florian
Quoted 9 lines · Greg Kroah-Hartman
This is the start of the stable review cycle for the 5.15.216 release. There are 456 patches in this series, all will be posted as a response to this one. If anyone has any issues with these being applied, please let me know.
Responses should be made by Wed, 19 Aug 2026 13:23:44 +0000. Anything received after that time might be too late.
The whole patch series can be found in one patch at:
Quoted 8 lines
https://www.kernel.org/pub/linux/kernel/v5.x/stable-review/patch-5.15.216-rc1.gzor in the git tree and branch at:
git://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable-rc.gitlinux-5.15.y
and the diffstat can be found below.thanks,
greg k-h
Tested on an amd64 laptop (Lenovo ThinkPad T14 Gen 1). Working well, no regressions observed.
Signature · 1 line
-Barry K. Nathan <[email protected]>
Quoted 9 lines · Greg Kroah-Hartman
This is the start of the stable review cycle for the 5.15.216 release. There are 456 patches in this series, all will be posted as a response to this one. If anyone has any issues with these being applied, please let me know.
Responses should be made by Wed, 19 Aug 2026 13:23:44 +0000. Anything received after that time might be too late.
The whole patch series can be found in one patch at:
Quoted 8 lines
https://www.kernel.org/pub/linux/kernel/v5.x/stable-review/patch-5.15.216-rc1.gzor in the git tree and branch at:
git://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable-rc.gitlinux-5.15.y
and the diffstat can be found below.thanks,
greg k-h
Built and booted successfully on RISC-V RV64 (HiFive Unmatched).
# Librecast Test Results
020/020 [ OK ] liblcrq
010/010 [ OK ] libmld
120/120 [ OK ] liblibrecast
CPU/kernel: Linux auntie 5.15.216-rc1-01277-g968da80b58ab #1 SMP Tue Aug 18 13:45:17 -00 2026 x86_64 AMD Ryzen 9 9950X 16-Core Processor AuthenticAMD GNU/Linux
Quoted 4 lines · Greg Kroah-Hartman
This is the start of the stable review cycle for the 5.15.216 release. There are 456 patches in this series, all will be posted as a response to this one. If anyone has any issues with these being applied, please let me know.
Quoted 9 lines · Greg Kroah-Hartman
This is the start of the stable review cycle for the 5.15.216 release. There are 456 patches in this series, all will be posted as a response to this one. If anyone has any issues with these being applied, please let me know.
Responses should be made by Wed, 19 Aug 2026 13:23:44 +0000. Anything received after that time might be too late.
The whole patch series can be found in one patch at:
Quoted 9 lines
https://www.kernel.org/pub/linux/kernel/v5.x/stable-review/patch-5.15.216-rc1.gzor in the git tree and branch at:
git://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable-rc.gitlinux-5.15.y
and the diffstat can be found below.thanks,
greg k-h
Compiled and booted on my test system. No dmesg regressions.
thanks,
-- Shuah