The logic for handling SVE traps manipulates saved FPSIMD/SVE state
incorrectly, and a race with preemption can result in a task having
TIF_SVE set and TIF_FOREIGN_FPSTATE clear even though the live CPU state
is stale (e.g. with SVE traps enabled). This has been observed to result
in warnings from do_sve_acc() where SVE traps are not expected while
TIF_SVE is set:
| if (test_and_set_thread_flag(TIF_SVE))
| WARN_ON(1); /* SVE access shouldn't have trapped */
Warnings of this form have been reported intermittently, e.g.
https://lore.kernel.org/linux-arm-kernel/CA+G9fYtEGe_DhY2Ms7+L7NKsLYUomGsgq…https://lore.kernel.org/linux-arm-kernel/000000000000511e9a060ce5a45c@googl…
The race can occur when the SVE trap handler is preempted before and
after manipulating the saved FPSIMD/SVE state, starting and ending on
the same CPU, e.g.
| void do_sve_acc(unsigned long esr, struct pt_regs *regs)
| {
| // Trap on CPU 0 with TIF_SVE clear, SVE traps enabled
| // task->fpsimd_cpu is 0.
| // per_cpu_ptr(&fpsimd_last_state, 0) is task.
|
| ...
|
| // Preempted; migrated from CPU 0 to CPU 1.
| // TIF_FOREIGN_FPSTATE is set.
|
| get_cpu_fpsimd_context();
|
| if (test_and_set_thread_flag(TIF_SVE))
| WARN_ON(1); /* SVE access shouldn't have trapped */
|
| sve_init_regs() {
| if (!test_thread_flag(TIF_FOREIGN_FPSTATE)) {
| ...
| } else {
| fpsimd_to_sve(current);
| current->thread.fp_type = FP_STATE_SVE;
| }
| }
|
| put_cpu_fpsimd_context();
|
| // Preempted; migrated from CPU 1 to CPU 0.
| // task->fpsimd_cpu is still 0
| // If per_cpu_ptr(&fpsimd_last_state, 0) is still task then:
| // - Stale HW state is reused (with SVE traps enabled)
| // - TIF_FOREIGN_FPSTATE is cleared
| // - A return to userspace skips HW state restore
| }
Fix the case where the state is not live and TIF_FOREIGN_FPSTATE is set
by calling fpsimd_flush_task_state() to detach from the saved CPU
state. This ensures that a subsequent context switch will not reuse the
stale CPU state, and will instead set TIF_FOREIGN_FPSTATE, forcing the
new state to be reloaded from memory prior to a return to userspace.
Fixes: cccb78ce89c4 ("arm64/sve: Rework SVE access trap to convert state in registers")
Reported-by: Mark Rutland <mark.rutland(a)arm.com>
Signed-off-by: Mark Brown <broonie(a)kernel.org>
Cc: stable(a)vger.kernel.org
Reviewed-by: Mark Rutland <mark.rutland(a)arm.com>
Link: https://lore.kernel.org/r/20241030-arm64-fpsimd-foreign-flush-v1-1-bd7bd669…
Signed-off-by: Will Deacon <will(a)kernel.org>
[Backported to 5.15 -- broonie]
(cherry picked from commit 751ecf6afd6568adc98f2a6052315552c0483d18)
---
arch/arm64/kernel/fpsimd.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/arm64/kernel/fpsimd.c b/arch/arm64/kernel/fpsimd.c
index 7a3fcf21b18a..e22571e57ae1 100644
--- a/arch/arm64/kernel/fpsimd.c
+++ b/arch/arm64/kernel/fpsimd.c
@@ -964,6 +964,7 @@ void do_sve_acc(unsigned long esr, struct pt_regs *regs)
fpsimd_bind_task_to_cpu();
} else {
fpsimd_to_sve(current);
+ fpsimd_flush_task_state(current);
}
put_cpu_fpsimd_context();
--
2.39.5
From: Takashi Iwai <tiwai(a)suse.de>
Upstream commit a3dd4d63eeb452cfb064a13862fb376ab108f6a6
The current USB-audio driver code doesn't check bLength of each
descriptor at traversing for clock descriptors. That is, when a
device provides a bogus descriptor with a shorter bLength, the driver
might hit out-of-bounds reads.
For addressing it, this patch adds sanity checks to the validator
functions for the clock descriptor traversal. When the descriptor
length is shorter than expected, it's skipped in the loop.
For the clock source and clock multiplier descriptors, we can just
check bLength against the sizeof() of each descriptor type.
OTOH, the clock selector descriptor of UAC2 and UAC3 has an array
of bNrInPins elements and two more fields at its tail, hence those
have to be checked in addition to the sizeof() check.
This patch ports the upstream commit a3dd4d63eeb4 ("ALSA: usb-audio: Fix
out of bounds reads when finding clock sources") to trees that do not
include the refactoring commit 9ec730052fa2 ("ALSA: usb-audio:
Refactoring UAC2/3 clock setup code"). That commit provides union
objects for pointing both UAC2 and UAC3 objects and unifies the clock
source, selector and multiplier helper functions. This means we need to
perform the check in each version specific helper function, but on the
other hand do not need to do version specific union dereferencing in the
macros and helper functions.
Reported-by: Benoît Sevens <bsevens(a)google.com>
Cc: <stable(a)vger.kernel.org>
Link: https://lore.kernel.org/20241121140613.3651-1-bsevens@google.com
Link: https://patch.msgid.link/20241125144629.20757-1-tiwai@suse.de
Signed-off-by: Takashi Iwai <tiwai(a)suse.de>
(cherry picked from commit a3dd4d63eeb452cfb064a13862fb376ab108f6a6)
Signed-off-by: Benoît Sevens <bsevens(a)google.com>
---
Changes in v3:
- add patch changelog
Changes in v2:
- provide better changelog description of how the upstream patch is
backported
sound/usb/clock.c | 32 ++++++++++++++++++++++++++++++--
1 file changed, 30 insertions(+), 2 deletions(-)
diff --git a/sound/usb/clock.c b/sound/usb/clock.c
index 3d1c0ec11753..58902275c815 100644
--- a/sound/usb/clock.c
+++ b/sound/usb/clock.c
@@ -21,6 +21,10 @@
#include "clock.h"
#include "quirks.h"
+/* check whether the descriptor bLength has the minimal length */
+#define DESC_LENGTH_CHECK(p) \
+ (p->bLength >= sizeof(*p))
+
static void *find_uac_clock_desc(struct usb_host_interface *iface, int id,
bool (*validator)(void *, int), u8 type)
{
@@ -38,36 +42,60 @@ static void *find_uac_clock_desc(struct usb_host_interface *iface, int id,
static bool validate_clock_source_v2(void *p, int id)
{
struct uac_clock_source_descriptor *cs = p;
+ if (!DESC_LENGTH_CHECK(cs))
+ return false;
return cs->bClockID == id;
}
static bool validate_clock_source_v3(void *p, int id)
{
struct uac3_clock_source_descriptor *cs = p;
+ if (!DESC_LENGTH_CHECK(cs))
+ return false;
return cs->bClockID == id;
}
static bool validate_clock_selector_v2(void *p, int id)
{
struct uac_clock_selector_descriptor *cs = p;
- return cs->bClockID == id;
+ if (!DESC_LENGTH_CHECK(cs))
+ return false;
+ if (cs->bClockID != id)
+ return false;
+ /* additional length check for baCSourceID array (in bNrInPins size)
+ * and two more fields (which sizes depend on the protocol)
+ */
+ return cs->bLength >= sizeof(*cs) + cs->bNrInPins +
+ 1 /* bmControls */ + 1 /* iClockSelector */;
}
static bool validate_clock_selector_v3(void *p, int id)
{
struct uac3_clock_selector_descriptor *cs = p;
- return cs->bClockID == id;
+ if (!DESC_LENGTH_CHECK(cs))
+ return false;
+ if (cs->bClockID != id)
+ return false;
+ /* additional length check for baCSourceID array (in bNrInPins size)
+ * and two more fields (which sizes depend on the protocol)
+ */
+ return cs->bLength >= sizeof(*cs) + cs->bNrInPins +
+ 4 /* bmControls */ + 2 /* wCSelectorDescrStr */;
}
static bool validate_clock_multiplier_v2(void *p, int id)
{
struct uac_clock_multiplier_descriptor *cs = p;
+ if (!DESC_LENGTH_CHECK(cs))
+ return false;
return cs->bClockID == id;
}
static bool validate_clock_multiplier_v3(void *p, int id)
{
struct uac3_clock_multiplier_descriptor *cs = p;
+ if (!DESC_LENGTH_CHECK(cs))
+ return false;
return cs->bClockID == id;
}
--
2.47.0.338.g60cca15819-goog
The patch below does not apply to the 6.12-stable tree.
If someone wants it applied there, or to any other stable or longterm
tree, then please email the backport, including the original git commit
id to <stable(a)vger.kernel.org>.
To reproduce the conflict and resubmit, you may use the following commands:
git fetch https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/ linux-6.12.y
git checkout FETCH_HEAD
git cherry-pick -x 63dffecfba3eddcf67a8f76d80e0c141f93d44a5
# <resolve conflicts, build, test, etc.>
git commit -s
git send-email --to '<stable(a)vger.kernel.org>' --in-reply-to '2024120656-jelly-gore-aa4c@gregkh' --subject-prefix 'PATCH 6.12.y' HEAD^..
Possible dependencies:
thanks,
greg k-h
------------------ original commit in Linus's tree ------------------
From 63dffecfba3eddcf67a8f76d80e0c141f93d44a5 Mon Sep 17 00:00:00 2001
From: Frederic Weisbecker <frederic(a)kernel.org>
Date: Sat, 23 Nov 2024 00:48:11 +0100
Subject: [PATCH] posix-timers: Target group sigqueue to current task only if
not exiting
A sigqueue belonging to a posix timer, which target is not a specific
thread but a whole thread group, is preferrably targeted to the current
task if it is part of that thread group.
However nothing prevents a posix timer event from queueing such a
sigqueue from a reaped yet running task. The interruptible code space
between exit_notify() and the final call to schedule() is enough for
posix_timer_fn() hrtimer to fire.
If that happens while the current task is part of the thread group
target, it is proposed to handle it but since its sighand pointer may
have been cleared already, the sigqueue is dropped even if there are
other tasks running within the group that could handle it.
As a result posix timers with thread group wide target may miss signals
when some of their threads are exiting.
Fix this with verifying that the current task hasn't been through
exit_notify() before proposing it as a preferred target so as to ensure
that its sighand is still here and stable.
complete_signal() might still reconsider the choice and find a better
target within the group if current has passed retarget_shared_pending()
already.
Fixes: bcb7ee79029d ("posix-timers: Prefer delivery of signals to the current thread")
Reported-by: Anthony Mallet <anthony.mallet(a)laas.fr>
Suggested-by: Oleg Nesterov <oleg(a)redhat.com>
Signed-off-by: Frederic Weisbecker <frederic(a)kernel.org>
Signed-off-by: Thomas Gleixner <tglx(a)linutronix.de>
Acked-by: Oleg Nesterov <oleg(a)redhat.com>
Cc: stable(a)vger.kernel.org
Link: https://lore.kernel.org/all/20241122234811.60455-1-frederic@kernel.org
Closes: https://lore.kernel.org/all/26411.57288.238690.681680@gargle.gargle.HOWL
diff --git a/kernel/signal.c b/kernel/signal.c
index 98b65cb35830..989b1cc9116a 100644
--- a/kernel/signal.c
+++ b/kernel/signal.c
@@ -1959,14 +1959,15 @@ static void posixtimer_queue_sigqueue(struct sigqueue *q, struct task_struct *t,
*
* Where type is not PIDTYPE_PID, signals must be delivered to the
* process. In this case, prefer to deliver to current if it is in
- * the same thread group as the target process, which avoids
- * unnecessarily waking up a potentially idle task.
+ * the same thread group as the target process and its sighand is
+ * stable, which avoids unnecessarily waking up a potentially idle task.
*/
static inline struct task_struct *posixtimer_get_target(struct k_itimer *tmr)
{
struct task_struct *t = pid_task(tmr->it_pid, tmr->it_pid_type);
- if (t && tmr->it_pid_type != PIDTYPE_PID && same_thread_group(t, current))
+ if (t && tmr->it_pid_type != PIDTYPE_PID &&
+ same_thread_group(t, current) && !current->exit_state)
t = current;
return t;
}
The logic for handling SVE traps manipulates saved FPSIMD/SVE state
incorrectly, and a race with preemption can result in a task having
TIF_SVE set and TIF_FOREIGN_FPSTATE clear even though the live CPU state
is stale (e.g. with SVE traps enabled). This has been observed to result
in warnings from do_sve_acc() where SVE traps are not expected while
TIF_SVE is set:
| if (test_and_set_thread_flag(TIF_SVE))
| WARN_ON(1); /* SVE access shouldn't have trapped */
Warnings of this form have been reported intermittently, e.g.
https://lore.kernel.org/linux-arm-kernel/CA+G9fYtEGe_DhY2Ms7+L7NKsLYUomGsgq…https://lore.kernel.org/linux-arm-kernel/000000000000511e9a060ce5a45c@googl…
The race can occur when the SVE trap handler is preempted before and
after manipulating the saved FPSIMD/SVE state, starting and ending on
the same CPU, e.g.
| void do_sve_acc(unsigned long esr, struct pt_regs *regs)
| {
| // Trap on CPU 0 with TIF_SVE clear, SVE traps enabled
| // task->fpsimd_cpu is 0.
| // per_cpu_ptr(&fpsimd_last_state, 0) is task.
|
| ...
|
| // Preempted; migrated from CPU 0 to CPU 1.
| // TIF_FOREIGN_FPSTATE is set.
|
| get_cpu_fpsimd_context();
|
| if (test_and_set_thread_flag(TIF_SVE))
| WARN_ON(1); /* SVE access shouldn't have trapped */
|
| sve_init_regs() {
| if (!test_thread_flag(TIF_FOREIGN_FPSTATE)) {
| ...
| } else {
| fpsimd_to_sve(current);
| current->thread.fp_type = FP_STATE_SVE;
| }
| }
|
| put_cpu_fpsimd_context();
|
| // Preempted; migrated from CPU 1 to CPU 0.
| // task->fpsimd_cpu is still 0
| // If per_cpu_ptr(&fpsimd_last_state, 0) is still task then:
| // - Stale HW state is reused (with SVE traps enabled)
| // - TIF_FOREIGN_FPSTATE is cleared
| // - A return to userspace skips HW state restore
| }
Fix the case where the state is not live and TIF_FOREIGN_FPSTATE is set
by calling fpsimd_flush_task_state() to detach from the saved CPU
state. This ensures that a subsequent context switch will not reuse the
stale CPU state, and will instead set TIF_FOREIGN_FPSTATE, forcing the
new state to be reloaded from memory prior to a return to userspace.
Fixes: cccb78ce89c4 ("arm64/sve: Rework SVE access trap to convert state in registers")
Reported-by: Mark Rutland <mark.rutland(a)arm.com>
Signed-off-by: Mark Brown <broonie(a)kernel.org>
Cc: stable(a)vger.kernel.org
Reviewed-by: Mark Rutland <mark.rutland(a)arm.com>
Link: https://lore.kernel.org/r/20241030-arm64-fpsimd-foreign-flush-v1-1-bd7bd669…
Signed-off-by: Will Deacon <will(a)kernel.org>
[Backported to 6.1 -- broonie]
(cherry picked from commit 751ecf6afd6568adc98f2a6052315552c0483d18)
---
arch/arm64/kernel/fpsimd.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/arm64/kernel/fpsimd.c b/arch/arm64/kernel/fpsimd.c
index 59b5a16bab5d..43afe07c74fd 100644
--- a/arch/arm64/kernel/fpsimd.c
+++ b/arch/arm64/kernel/fpsimd.c
@@ -1383,6 +1383,7 @@ static void sve_init_regs(void)
fpsimd_bind_task_to_cpu();
} else {
fpsimd_to_sve(current);
+ fpsimd_flush_task_state(current);
}
}
--
2.39.5