* Re: [PATCH] md: protect read mddev->recovery in md_sync_action()
From: Chen Cheng @ 2026-06-27 10:52 UTC (permalink / raw)
To: Abd-Alrhman Masalkhi, Chen Cheng, linux-raid, yukuai; +Cc: linux-kernel
In-Reply-To: <m25x341cro.fsf@gmail.com>
在 2026/6/27 18:46, Abd-Alrhman Masalkhi 写道:
> On Sat, Jun 27, 2026 at 18:29 +0800, Chen Cheng wrote:
>> From: Chen Cheng <chencheng@fnnas.com>
>>
>> md_sync_action() read mddev->recovery in lockless path, use READ_ONCE()
>> instead of u64 plain read.
>>
> unisgned long is not always u64...
> I can not see what it fixes, is this just to silence KCSAN? If so,
> please say so in the commit message.
I need to find KCSAN report , I lost this one, but I think it's easy to
find locklessly path to call md_sync_action(), so. READ_ONCE is need.
>
>> Fixes: e792a4c2156a3 ("md: add new helpers for sync_action")
>>
>> Signed-off-by: Chen Cheng <chencheng@fnnas.com>
>> ---
>> drivers/md/md.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/md/md.c b/drivers/md/md.c
>> index c5c50640b684..f4415c1a79d9 100644
>> --- a/drivers/md/md.c
>> +++ b/drivers/md/md.c
>> @@ -5074,11 +5074,11 @@ static enum sync_action md_get_active_sync_action(struct mddev *mddev)
>> return is_recover ? ACTION_RECOVER : ACTION_IDLE;
>> }
>>
>> enum sync_action md_sync_action(struct mddev *mddev)
>> {
>> - unsigned long recovery = mddev->recovery;
>> + unsigned long recovery = READ_ONCE(mddev->recovery);
>> enum sync_action active_action;
>>
>> /*
>> * frozen has the highest priority, means running sync_thread will be
>> * stopped immediately, and no new sync_thread can start.
>> --
>> 2.54.0
>>
>
^ permalink raw reply
* Re: [PATCH] md: protect read mddev->recovery in md_sync_action()
From: Abd-Alrhman Masalkhi @ 2026-06-27 10:46 UTC (permalink / raw)
To: Chen Cheng, linux-raid, yukuai; +Cc: chencheng, linux-kernel
In-Reply-To: <20260627102909.141036-1-chencheng@fnnas.com>
On Sat, Jun 27, 2026 at 18:29 +0800, Chen Cheng wrote:
> From: Chen Cheng <chencheng@fnnas.com>
>
> md_sync_action() read mddev->recovery in lockless path, use READ_ONCE()
> instead of u64 plain read.
>
unisgned long is not always u64...
I can not see what it fixes, is this just to silence KCSAN? If so,
please say so in the commit message.
> Fixes: e792a4c2156a3 ("md: add new helpers for sync_action")
>
> Signed-off-by: Chen Cheng <chencheng@fnnas.com>
> ---
> drivers/md/md.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/md/md.c b/drivers/md/md.c
> index c5c50640b684..f4415c1a79d9 100644
> --- a/drivers/md/md.c
> +++ b/drivers/md/md.c
> @@ -5074,11 +5074,11 @@ static enum sync_action md_get_active_sync_action(struct mddev *mddev)
> return is_recover ? ACTION_RECOVER : ACTION_IDLE;
> }
>
> enum sync_action md_sync_action(struct mddev *mddev)
> {
> - unsigned long recovery = mddev->recovery;
> + unsigned long recovery = READ_ONCE(mddev->recovery);
> enum sync_action active_action;
>
> /*
> * frozen has the highest priority, means running sync_thread will be
> * stopped immediately, and no new sync_thread can start.
> --
> 2.54.0
>
--
Best Regards,
Abd-Alrhman
^ permalink raw reply
* [PATCH] md: protect read mddev->recovery in md_sync_action()
From: Chen Cheng @ 2026-06-27 10:29 UTC (permalink / raw)
To: linux-raid, yukuai; +Cc: chencheng, linux-kernel
From: Chen Cheng <chencheng@fnnas.com>
md_sync_action() read mddev->recovery in lockless path, use READ_ONCE()
instead of u64 plain read.
Fixes: e792a4c2156a3 ("md: add new helpers for sync_action")
Signed-off-by: Chen Cheng <chencheng@fnnas.com>
---
drivers/md/md.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/md/md.c b/drivers/md/md.c
index c5c50640b684..f4415c1a79d9 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -5074,11 +5074,11 @@ static enum sync_action md_get_active_sync_action(struct mddev *mddev)
return is_recover ? ACTION_RECOVER : ACTION_IDLE;
}
enum sync_action md_sync_action(struct mddev *mddev)
{
- unsigned long recovery = mddev->recovery;
+ unsigned long recovery = READ_ONCE(mddev->recovery);
enum sync_action active_action;
/*
* frozen has the highest priority, means running sync_thread will be
* stopped immediately, and no new sync_thread can start.
--
2.54.0
^ permalink raw reply related
* [PATCH] md/raid5: protect lockless recovery_offset accesses during reshape
From: Chen Cheng @ 2026-06-27 10:25 UTC (permalink / raw)
To: linux-raid, yukuai; +Cc: chencheng, linux-kernel
From: Chen Cheng <chencheng@fnnas.com>
During reshape:
- reshape_request() advances rdev->recovery_offset for non-In_sync
devices locklessly.
- analyse_stripe() reads rdev->recovery_offset locklessly to decide:
a. use a replacement device to read ?
b. a device can already be treated as in-sync for the current
stripe ?
one possible scenario is:
CPU1 CPU2
reshape_request()
-> mddev->curr_resync_completed = sector_nr
-> if (!mddev->reshape_backwards)
-> rdev->recovery_offset = sector_nr
analyse_stripe(sh)
-> rdev = conf->disks[i].replacement
-> if (rdev->recovery_offset >=
sh->sector + stripe_sectors)
set_bit(R5_ReadRepl)
-> or
-> if (sh->sector + stripe_sectors <=
rdev->recovery_offset)
set_bit(R5_Insync)
And it could be:
- reading from a replacement before it is recovered far enough; or
- treating a not-yet-recovered device as in-sync for the current stripe.
Fixes: db0505d32066 ("md: be cautious about using ->curr_resync_completed for ->recovery_offset")
The race report:
==================================================================
BUG: KCSAN: data-race in ops_run_io / reshape_request
write to 0xffff8bdee168b270 of 8 bytes by task 1704 on cpu 10:
reshape_request+0x1292/0x17b0
raid5_sync_request+0x815/0xa00
md_do_sync.cold+0xf8d/0x1516
[......]
read to 0xffff8bdee168b270 of 8 bytes by task 1696 on cpu 9:
ops_run_io+0xc25/0x1960
handle_stripe+0x2273/0x4570
handle_active_stripes.isra.0+0x6e0/0xa50
raid5d+0x7d5/0xb90
[......]
value changed: 0x0000000000091a00 -> 0x0000000000091b00
==================================================================
Signed-off-by: Chen Cheng <chencheng@fnnas.com>
---
drivers/md/raid5.c | 34 +++++++++++++++++-----------------
1 file changed, 17 insertions(+), 17 deletions(-)
diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
index cacdf4211628..eaee7f206ab8 100644
--- a/drivers/md/raid5.c
+++ b/drivers/md/raid5.c
@@ -3808,11 +3808,11 @@ static int want_replace(struct stripe_head *sh, int disk_idx)
rdev = sh->raid_conf->disks[disk_idx].replacement;
if (rdev
&& !test_bit(Faulty, &rdev->flags)
&& !test_bit(In_sync, &rdev->flags)
- && (rdev->recovery_offset <= sh->sector
+ && (READ_ONCE(rdev->recovery_offset) <= sh->sector
|| rdev->mddev->resync_offset <= sh->sector))
rv = 1;
return rv;
}
@@ -4726,11 +4726,11 @@ static void analyse_stripe(struct stripe_head *sh, struct stripe_head_state *s)
/* Prefer to use the replacement for reads, but only
* if it is recovered enough and has no bad blocks.
*/
rdev = conf->disks[i].replacement;
if (rdev && !test_bit(Faulty, &rdev->flags) &&
- rdev->recovery_offset >= sh->sector + RAID5_STRIPE_SECTORS(conf) &&
+ READ_ONCE(rdev->recovery_offset) >= sh->sector + RAID5_STRIPE_SECTORS(conf) &&
!rdev_has_badblock(rdev, sh->sector,
RAID5_STRIPE_SECTORS(conf)))
set_bit(R5_ReadRepl, &dev->flags);
else {
if (rdev && !test_bit(Faulty, &rdev->flags))
@@ -4768,11 +4768,11 @@ static void analyse_stripe(struct stripe_head *sh, struct stripe_head_state *s)
set_bit(R5_ReadError, &dev->flags);
}
} else if (test_bit(In_sync, &rdev->flags))
set_bit(R5_Insync, &dev->flags);
else if (sh->sector + RAID5_STRIPE_SECTORS(conf) <=
- rdev->recovery_offset) {
+ READ_ONCE(rdev->recovery_offset)) {
/*
* in sync if:
* - normal IO, or
* - resync IO that is not lazy recovery
*
@@ -5514,17 +5514,17 @@ static int raid5_read_one_chunk(struct mddev *mddev, struct bio *raid_bio)
if (r5c_big_stripe_cached(conf, sector))
return 0;
rdev = conf->disks[dd_idx].replacement;
if (!rdev || test_bit(Faulty, &rdev->flags) ||
- rdev->recovery_offset < end_sector) {
+ READ_ONCE(rdev->recovery_offset) < end_sector) {
rdev = conf->disks[dd_idx].rdev;
if (!rdev)
return 0;
if (test_bit(Faulty, &rdev->flags) ||
!(test_bit(In_sync, &rdev->flags) ||
- rdev->recovery_offset >= end_sector))
+ READ_ONCE(rdev->recovery_offset) >= end_sector))
return 0;
}
atomic_inc(&rdev->nr_pending);
@@ -6463,12 +6463,12 @@ static sector_t reshape_request(struct mddev *mddev, sector_t sector_nr, int *sk
/* Can update recovery_offset */
rdev_for_each(rdev, mddev)
if (rdev->raid_disk >= 0 &&
!test_bit(Journal, &rdev->flags) &&
!test_bit(In_sync, &rdev->flags) &&
- rdev->recovery_offset < sector_nr)
- rdev->recovery_offset = sector_nr;
+ READ_ONCE(rdev->recovery_offset) < sector_nr)
+ WRITE_ONCE(rdev->recovery_offset, sector_nr);
conf->reshape_checkpoint = jiffies;
set_bit(MD_SB_CHANGE_DEVS, &mddev->sb_flags);
md_wakeup_thread(mddev->thread);
wait_event(mddev->sb_wait, READ_ONCE(mddev->sb_flags) == 0 ||
@@ -6572,12 +6572,12 @@ static sector_t reshape_request(struct mddev *mddev, sector_t sector_nr, int *sk
/* Can update recovery_offset */
rdev_for_each(rdev, mddev)
if (rdev->raid_disk >= 0 &&
!test_bit(Journal, &rdev->flags) &&
!test_bit(In_sync, &rdev->flags) &&
- rdev->recovery_offset < sector_nr)
- rdev->recovery_offset = sector_nr;
+ READ_ONCE(rdev->recovery_offset) < sector_nr)
+ WRITE_ONCE(rdev->recovery_offset, sector_nr);
conf->reshape_checkpoint = jiffies;
set_bit(MD_SB_CHANGE_DEVS, &mddev->sb_flags);
md_wakeup_thread(mddev->thread);
wait_event(mddev->sb_wait,
!test_bit(MD_SB_CHANGE_DEVS, &mddev->sb_flags)
@@ -8106,13 +8106,13 @@ static int raid5_run(struct mddev *mddev)
* to worry about reshape going forwards.
*/
/* Hack because v0.91 doesn't store recovery_offset properly. */
if (mddev->major_version == 0 &&
mddev->minor_version > 90)
- rdev->recovery_offset = reshape_offset;
+ WRITE_ONCE(rdev->recovery_offset, reshape_offset);
- if (rdev->recovery_offset < reshape_offset) {
+ if (READ_ONCE(rdev->recovery_offset) < reshape_offset) {
/* We need to check old and new layout */
if (!only_parity(rdev->raid_disk,
conf->algorithm,
conf->raid_disks,
conf->max_degraded))
@@ -8264,11 +8264,11 @@ static int raid5_spare_active(struct mddev *mddev)
for (i = 0; i < conf->raid_disks; i++) {
rdev = conf->disks[i].rdev;
replacement = conf->disks[i].replacement;
if (replacement
- && replacement->recovery_offset == MaxSector
+ && READ_ONCE(replacement->recovery_offset) == MaxSector
&& !test_bit(Faulty, &replacement->flags)
&& !test_and_set_bit(In_sync, &replacement->flags)) {
/* Replacement has just become active. */
if (!rdev
|| !test_and_clear_bit(In_sync, &rdev->flags))
@@ -8282,13 +8282,13 @@ static int raid5_spare_active(struct mddev *mddev)
sysfs_notify_dirent_safe(
rdev->sysfs_state);
}
sysfs_notify_dirent_safe(replacement->sysfs_state);
} else if (rdev
- && rdev->recovery_offset == MaxSector
- && !test_bit(Faulty, &rdev->flags)
- && !test_and_set_bit(In_sync, &rdev->flags)) {
+ && READ_ONCE(rdev->recovery_offset) == MaxSector
+ && !test_bit(Faulty, &rdev->flags)
+ && !test_and_set_bit(In_sync, &rdev->flags)) {
count++;
sysfs_notify_dirent_safe(rdev->sysfs_state);
}
}
spin_lock_irqsave(&conf->device_lock, flags);
@@ -8653,11 +8653,11 @@ static int raid5_start_reshape(struct mddev *mddev)
if (raid5_add_disk(mddev, rdev) == 0) {
if (rdev->raid_disk
>= conf->previous_raid_disks)
set_bit(In_sync, &rdev->flags);
else
- rdev->recovery_offset = 0;
+ WRITE_ONCE(rdev->recovery_offset, 0);
/* Failure here is OK */
sysfs_link_rdev(mddev, rdev);
}
} else if (rdev->raid_disk >= conf->previous_raid_disks
@@ -8705,11 +8705,11 @@ static void end_reshape(struct r5conf *conf)
conf->mddev->reshape_position = MaxSector;
rdev_for_each(rdev, conf->mddev)
if (rdev->raid_disk >= 0 &&
!test_bit(Journal, &rdev->flags) &&
!test_bit(In_sync, &rdev->flags))
- rdev->recovery_offset = MaxSector;
+ WRITE_ONCE(rdev->recovery_offset, MaxSector);
spin_unlock_irq(&conf->device_lock);
wake_up(&conf->wait_for_reshape);
mddev_update_io_opt(conf->mddev,
conf->raid_disks - conf->max_degraded);
--
2.54.0
^ permalink raw reply related
* Re: [PATCH 08/18] raid6: warn when using less than four devices
From: H. Peter Anvin @ 2026-06-27 1:14 UTC (permalink / raw)
To: Christoph Hellwig, Andrew Morton
Cc: Catalin Marinas, Will Deacon, Ard Biesheuvel, Huacai Chen,
WANG Xuerui, Madhavan Srinivasan, Michael Ellerman,
Nicholas Piggin, Christophe Leroy (CS GROUP), Paul Walmsley,
Palmer Dabbelt, Albert Ou, Alexandre Ghiti, Heiko Carstens,
Vasily Gorbik, Alexander Gordeev, Christian Borntraeger,
Sven Schnelle, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
Dave Hansen, x86, Herbert Xu, Dan Williams, Chris Mason,
David Sterba, Arnd Bergmann, Song Liu, Yu Kuai, Li Nan,
linux-kernel, linux-arm-kernel, loongarch, linuxppc-dev,
linux-riscv, linux-s390, linux-crypto, linux-btrfs, linux-arch,
linux-raid
In-Reply-To: <20260518051804.462141-9-hch@lst.de>
On 2026-05-17 22:17, Christoph Hellwig wrote:
> Quoting H. Peter Anvin who came up with the RAID6 P/Q algorithm, and
> who wrote the initial implementation, then still part of the md driver:
>
> The RAID-6 code has *never* supported only 3 units, and if it ever
> worked for *any* of the implementations it was purely by accident.
> Speaking as the original author I should know; this was deliberate as
> in some cases the degenerate case (3) would have required extra trays
Stupid autocorrect. That was of course supposed to be "tests" (as in extra
code paths) not "trays" :)
> in the code to no user benefit.
>
> While md never allowed less than 4 devices, btrfs does. This new
> warning will trigger for such file systems, but given how it already
> causes havoc that is a good thing. If btrfs wants to fix third, it
> should switch to transparently use three-way mirroring underneath,
> which will work as P and Q are copies of the single data device by
> the definition of the Linux RAID 6 P/Q algorithm.
For what it's worth, this is also true in the degenerate two-drive RAID-4|5
case (D = P).
-hpa
^ permalink raw reply
* Re: cleanup the RAID6 P/Q library v3
From: H. Peter Anvin @ 2026-06-27 0:52 UTC (permalink / raw)
To: Christoph Hellwig, Andrew Morton
Cc: Catalin Marinas, Will Deacon, Ard Biesheuvel, Huacai Chen,
WANG Xuerui, Madhavan Srinivasan, Michael Ellerman,
Nicholas Piggin, Christophe Leroy (CS GROUP), Paul Walmsley,
Palmer Dabbelt, Albert Ou, Alexandre Ghiti, Heiko Carstens,
Vasily Gorbik, Alexander Gordeev, Christian Borntraeger,
Sven Schnelle, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
Dave Hansen, x86, Herbert Xu, Dan Williams, Chris Mason,
David Sterba, Arnd Bergmann, Song Liu, Yu Kuai, Li Nan,
linux-kernel, linux-arm-kernel, loongarch, linuxppc-dev,
linux-riscv, linux-s390, linux-crypto, linux-btrfs, linux-arch,
linux-raid
In-Reply-To: <20260519082432.GA14956@lst.de>
On 2026-05-19 01:24, Christoph Hellwig wrote:
>
> raid6: rework registration of optimized algorithms
>
> - avx2 instead of avx512 is probably the right thing for no
> benchmarking, but if it was intentional (it wasn't), that should
> be document. So I'll just switch back to the previous version to
> keep the state of the art
It is unlikely to be the right thing *going forward*, though.
The very unfortunate performance inversion is likely model-specific. It is one
of those things where you largely would have to have a list of quirks :(
-hpa
^ permalink raw reply
* Re: [PATCH 2/8] um: Check for missing AVX and AVX-512 xstate bits
From: David Laight @ 2026-06-26 21:33 UTC (permalink / raw)
To: Eric Biggers
Cc: Anton Ivanov, x86, linux-um, linux-raid, linux-crypto,
linux-kernel, Christoph Hellwig, Andrew Morton
In-Reply-To: <20260626205544.GB2368695@google.com>
On Fri, 26 Jun 2026 20:55:44 +0000
Eric Biggers <ebiggers@kernel.org> wrote:
> On Fri, Jun 26, 2026 at 11:49:57AM +0100, David Laight wrote:
> > > UML is just another userland application from this perspective, so
> > > there is no reason for it to behave any different from the rest of
> > > the userland.
>
> Which is why it should do the XCR0 check that the vast majority of
> userspace programs do, right? It has always been part of the documented
> way to detect AVX and AVX-512 support.
That and looking at the other 'cpuid' registers rather than reading
/proc/cpuinfo.
If you want to run 'um' on (say) NetBSD you'd need to do it differently.
> (I think this helps explain why LLMs notice this too. They've been
> trained on lots of code that does it correctly.)
>
> That being said, it does seem likely that it's basically obsolete now.
> So maybe we could take a shortcut and omit it.
It would need the linux kernel to report a cpu feature that needed
kernel support, but that the kernel didn't support.
But for things like popcnt um should probably check the relevant cpuid bit
rather than scanning /proc/cpuinfo.
>
> The important thing is really that we make a definitive decision *once*
> for each of UML and native x86. The status quo is that the decision is
> instead punted to every individual AVX optimized function in the kernel,
> which isn't working well.
Indeed.
David
>
> - Eric
^ permalink raw reply
* Re: [PATCH 2/8] um: Check for missing AVX and AVX-512 xstate bits
From: Eric Biggers @ 2026-06-26 20:55 UTC (permalink / raw)
To: David Laight
Cc: Anton Ivanov, x86, linux-um, linux-raid, linux-crypto,
linux-kernel, Christoph Hellwig, Andrew Morton
In-Reply-To: <20260626114957.1a2b7e5b@pumpkin>
On Fri, Jun 26, 2026 at 11:49:57AM +0100, David Laight wrote:
> > UML is just another userland application from this perspective, so
> > there is no reason for it to behave any different from the rest of
> > the userland.
Which is why it should do the XCR0 check that the vast majority of
userspace programs do, right? It has always been part of the documented
way to detect AVX and AVX-512 support.
(I think this helps explain why LLMs notice this too. They've been
trained on lots of code that does it correctly.)
That being said, it does seem likely that it's basically obsolete now.
So maybe we could take a shortcut and omit it.
The important thing is really that we make a definitive decision *once*
for each of UML and native x86. The status quo is that the decision is
instead punted to every individual AVX optimized function in the kernel,
which isn't working well.
- Eric
^ permalink raw reply
* Re: [PATCH v2 next] drivers/md/md: Use bounded strings ops when setting disk->disk_name
From: David Laight @ 2026-06-26 10:52 UTC (permalink / raw)
To: sashiko-bot; +Cc: sashiko-reviews, yukuai, linux-raid
In-Reply-To: <20260626085141.3AEBC1F000E9@smtp.kernel.org>
On Fri, 26 Jun 2026 08:51:40 +0000
sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Low] Inconsistent coding style for the sizeof operator in newly added code.
> --
>
> commit 663613700e83a5bb9c1749d80b4fae68752645fb
> Author: David Laight <david.laight.linux@gmail.com>
>
> drivers/md/md: Use bounded strings ops when setting disk->disk_name
>
> This commit replaces unbounded strcpy() and sprintf() with bounded strscpy()
> and snprintf() operations when setting the disk_name in md_alloc() to improve
> safety.
>
> > diff --git a/drivers/md/md.c b/drivers/md/md.c
> > index d1465bcd86c81..afbdc52a11270 100644
> > --- a/drivers/md/md.c
> > +++ b/drivers/md/md.c
> > @@ -6354,11 +6354,11 @@ struct mddev *md_alloc(dev_t dev, char *name)
> [ ... ]
> > if (name)
> > - strcpy(disk->disk_name, name);
> > + strscpy(disk->disk_name, name);
> > else if (partitioned)
> > - sprintf(disk->disk_name, "md_d%d", unit);
> > + snprintf(disk->disk_name, sizeof(disk->disk_name), "md_d%d", unit);
> > else
> > - sprintf(disk->disk_name, "md%d", unit);
> > + snprintf(disk->disk_name, sizeof (disk->disk_name), "md%d", unit);
>
> [Severity: Low]
> This isn't a bug, but there is an inconsistent coding style for the sizeof
> operator here in md_alloc(). The adjacent branch uses sizeof(disk->disk_name)
> without a space, while this newly added branch includes a space. The kernel
> coding style prefers no space after sizeof.
>
B****r I only fixed one of them :-(
David
^ permalink raw reply
* Re: [PATCH 2/8] um: Check for missing AVX and AVX-512 xstate bits
From: David Laight @ 2026-06-26 10:49 UTC (permalink / raw)
To: Anton Ivanov
Cc: Eric Biggers, x86, linux-um, linux-raid, linux-crypto,
linux-kernel, Christoph Hellwig, Andrew Morton
In-Reply-To: <6a20b442-b97f-4cae-9168-30201d5ef82c@cambridgegreys.com>
On Fri, 26 Jun 2026 09:21:49 +0100
Anton Ivanov <anton.ivanov@cambridgegreys.com> wrote:
> On 26/06/2026 08:41, David Laight wrote:
> > On Thu, 25 Jun 2026 21:37:25 -0700
> > Eric Biggers <ebiggers@kernel.org> wrote:
> >
> >> If the CPU declares AVX or AVX-512 support, verify that all the
> >> corresponding xstate bits are also set. If any are missing, warn and
> >> don't set the corresponding X86_FEATURE_* flags.
> >>
> >> This eliminates the perceived need for UML-supporting AVX and AVX-512
> >> optimized code in the kernel (that is, lib/raid/ currently) to start
> >> checking the xstate bits in addition to X86_FEATURE_AVX*.
> >>
> > ...
> >> static void __init parse_host_cpu_flags(char *line)
> >> {
> >> + u64 xcr0 = read_xcr0();
> >> int i;
> >> +
> >> for (i = 0; i < 32*NCAPINTS; i++) {
> >> if ((x86_cap_flags[i] != NULL) && strstr(line, x86_cap_flags[i]))
> >
> > 'line' comes from /proc/cpuinfo
> > Surely something would be terribly wrong if that included something the kernel
> > had disabled (or didn't support).
> >
> > David
> >
> >
> >> - set_cpu_cap(&boot_cpu_data, i);
> >> + validate_and_set_cpu_cap(i, xcr0);
> >> }
> >> }
> >>
> >> static void __init parse_cache_line(char *line)
> >> {
> >
> >
> >
> >
> Lots of other stuff will go wrong before that. Glibc, things compiled with LLVM, python, perl, etc.
>
> Half of the userland will go belly up, because AVX is used in string operations and hashing if it is available.
And glibc will check xcr0.
>
> UML is just another userland application from this perspective, so there is no reason for it to behave any different from the rest of the userland.
^ permalink raw reply
* Re: [PATCH 4/7] md/raid10: raid10_write_request() drops the barrier before calling
From: Abd-Alrhman Masalkhi @ 2026-06-26 9:35 UTC (permalink / raw)
To: yu kuai, song, magiclinan, xiao, axboe, john.g.garry,
martin.petersen, yukuai
Cc: linux-raid, linux-kernel
In-Reply-To: <draft-m2h5ms1i5p.fsf@gmail.com>
Hi Kuai,
On Wed, Jun 24, 2026 at 10:01 +0200, Abd-Alrhman Masalkhi wrote:
> On Wed, Jun 24, 2026 at 15:23 +0800, yu kuai wrote:
>> Hi,
>>
>> 在 2026/6/23 15:24, Abd-Alrhman Masalkhi 写道:
>>> bio_submit_split_bioset() and reacquires it afterwards. This is
>>> unnecessary because bio_submit_split_bioset() does not require
>>> releasing the barrier protection.
>>
>> Why is this safe?
>>
>> It's possible plug is not enabled in this context, and the allocated new
>> aplit bio will be submitted and enter wait_barrier() again. wait_barrier()
>> is not supposed to be called recursively. Otherwise deadlock can happend:
>>
>> t1: sync_thread grab barrier and waiting for nr_pending to be 0 from raise_barrier()
>> t2: wait_barrier() first inc nr_pending, then recursive wait_barrier() waiting
>> for barrier to be released.
>>
>
> raid10_write_request() is only called from the bio submission path and
> not from raid10d thread, so we do not re-enter it while already holding
> barrier. Therefore, the split bio submitted by bio_submit_split_bioset()
> cannot recurse back into wait_barrier() from the same execution path.
I looked at commit e820d55cb99d. At the time, it addressed this issue
because submit_flushes() called md_handle_request() directly. Since
v5.2, submit_flushes() has gone through submit_bio() instead before
submit_flushes() was eventually removed.
Today, md_handle_request() is called from md_submit_bio() and dm-raid.
I don't have much experience with dm, but at first glance it seems safe
to remove these calls. However, if md_handle_request() were ever invoked
from a worker thread or kthread, we could run into the same kind of
issues, not only for RAID10, but also for RAID1.
What do you think? Should I drop this patch and guard RAID1 as well, or
is it safe to assume that dm-raid cannot hit this path from a kthread
or a workqueue worker?
>
> The situation is different for reads, where raid10_read_request() will
> be called both from the submission path and from raid10d. There,
> dropping and reacquiring the barrier protection remains necessary.
>
>>>
>>> Remove the redundant allow_barrier()/wait_barrier() pair around
>>> bio_submit_split_bioset().
>>>
>>> Signed-off-by: Abd-Alrhman Masalkhi <abd.masalkhi@gmail.com>
>>> ---
>>> drivers/md/raid10.c | 2 --
>>> 1 file changed, 2 deletions(-)
>>>
>>> diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
>>> index 840f0446c231..4bc1d5553ec7 100644
>>> --- a/drivers/md/raid10.c
>>> +++ b/drivers/md/raid10.c
>>> @@ -1493,10 +1493,8 @@ static bool raid10_write_request(struct mddev *mddev, struct bio *bio,
>>> if (atomic)
>>> goto err_handle;
>>>
>>> - allow_barrier(conf);
>>> bio = bio_submit_split_bioset(bio, r10_bio->sectors,
>>> &conf->bio_split);
>>> - wait_barrier(conf, false);
>>> if (!bio) {
>>> set_bit(R10BIO_Returned, &r10_bio->state);
>>> goto err_handle;
>>
>> --
>> Thanks,
>> Kuai
>
> --
> Best Regards,
> Abd-Alrhman
--
Best Regards,
Abd-Alrhman
^ permalink raw reply
* Re: [PATCH 2/8] um: Check for missing AVX and AVX-512 xstate bits
From: Anton Ivanov @ 2026-06-26 8:21 UTC (permalink / raw)
To: David Laight, Eric Biggers
Cc: x86, linux-um, linux-raid, linux-crypto, linux-kernel,
Christoph Hellwig, Andrew Morton
In-Reply-To: <20260626084113.42eae31c@pumpkin>
On 26/06/2026 08:41, David Laight wrote:
> On Thu, 25 Jun 2026 21:37:25 -0700
> Eric Biggers <ebiggers@kernel.org> wrote:
>
>> If the CPU declares AVX or AVX-512 support, verify that all the
>> corresponding xstate bits are also set. If any are missing, warn and
>> don't set the corresponding X86_FEATURE_* flags.
>>
>> This eliminates the perceived need for UML-supporting AVX and AVX-512
>> optimized code in the kernel (that is, lib/raid/ currently) to start
>> checking the xstate bits in addition to X86_FEATURE_AVX*.
>>
> ...
>> static void __init parse_host_cpu_flags(char *line)
>> {
>> + u64 xcr0 = read_xcr0();
>> int i;
>> +
>> for (i = 0; i < 32*NCAPINTS; i++) {
>> if ((x86_cap_flags[i] != NULL) && strstr(line, x86_cap_flags[i]))
>
> 'line' comes from /proc/cpuinfo
> Surely something would be terribly wrong if that included something the kernel
> had disabled (or didn't support).
>
> David
>
>
>> - set_cpu_cap(&boot_cpu_data, i);
>> + validate_and_set_cpu_cap(i, xcr0);
>> }
>> }
>>
>> static void __init parse_cache_line(char *line)
>> {
>
>
>
>
Lots of other stuff will go wrong before that. Glibc, things compiled with LLVM, python, perl, etc.
Half of the userland will go belly up, because AVX is used in string operations and hashing if it is available.
UML is just another userland application from this perspective, so there is no reason for it to behave any different from the rest of the userland.
--
Anton R. Ivanov
Cambridgegreys Limited. Registered in England. Company Number 10273661
^ permalink raw reply
* Re: [PATCH v2 next] drivers/md/md: Use bounded strings ops when setting disk->disk_name
From: sashiko-bot @ 2026-06-26 8:51 UTC (permalink / raw)
To: David Laight; +Cc: yukuai, linux-raid
In-Reply-To: <20260626084334.3012-1-david.laight.linux@gmail.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] Inconsistent coding style for the sizeof operator in newly added code.
--
commit 663613700e83a5bb9c1749d80b4fae68752645fb
Author: David Laight <david.laight.linux@gmail.com>
drivers/md/md: Use bounded strings ops when setting disk->disk_name
This commit replaces unbounded strcpy() and sprintf() with bounded strscpy()
and snprintf() operations when setting the disk_name in md_alloc() to improve
safety.
> diff --git a/drivers/md/md.c b/drivers/md/md.c
> index d1465bcd86c81..afbdc52a11270 100644
> --- a/drivers/md/md.c
> +++ b/drivers/md/md.c
> @@ -6354,11 +6354,11 @@ struct mddev *md_alloc(dev_t dev, char *name)
[ ... ]
> if (name)
> - strcpy(disk->disk_name, name);
> + strscpy(disk->disk_name, name);
> else if (partitioned)
> - sprintf(disk->disk_name, "md_d%d", unit);
> + snprintf(disk->disk_name, sizeof(disk->disk_name), "md_d%d", unit);
> else
> - sprintf(disk->disk_name, "md%d", unit);
> + snprintf(disk->disk_name, sizeof (disk->disk_name), "md%d", unit);
[Severity: Low]
This isn't a bug, but there is an inconsistent coding style for the sizeof
operator here in md_alloc(). The adjacent branch uses sizeof(disk->disk_name)
without a space, while this newly added branch includes a space. The kernel
coding style prefers no space after sizeof.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260626084334.3012-1-david.laight.linux@gmail.com?part=1
^ permalink raw reply
* [PATCH v2 next] drivers/md/md: Use bounded strings ops when setting disk->disk_name
From: David Laight @ 2026-06-26 8:43 UTC (permalink / raw)
To: Kees Cook, linux-hardening, linux-kernel, linux-raid
Cc: Arnd Bergmann, Song Liu, Yu Kuai, David Laight
Replace strcpy() with strscpy() and sprintf() with snprintf() for safety.
Signed-off-by: David Laight <david.laight.linux@gmail.com>
---
v2: Make sizeof look like a function call for consistency.
This is one of a group of patches that remove potentially unbounded
strcpy() calls.
They are mostly replaced by strscpy() or, when strlen() has just been
called, with memcpy() (usually including the '\0').
Calls with copy string literals into arrays are left unchanged.
They are safe and easily detected as such.
The changes were made by getting the compiler to detect the calls and
then fixing the code by hand.
Note that all the changes are only compile tested.
Some Makefiles were changed to allow files to contain strcpy().
As well as 'difficult to fix' files, this included 'show' functions
as they really need to use sysfs_emit() or seq_printf().
All the patches are being sent individually to avoid very long cc lists.
Apologies for the terse commit messages and likely unexpected tags.
(There are about 100 patches in total.)
drivers/md/md.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/md/md.c b/drivers/md/md.c
index 8b568eee8743..4b659d10fa4d 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -6346,11 +6346,11 @@ struct mddev *md_alloc(dev_t dev, char *name)
disk->first_minor = unit << shift;
disk->minors = 1 << shift;
if (name)
- strcpy(disk->disk_name, name);
+ strscpy(disk->disk_name, name);
else if (partitioned)
- sprintf(disk->disk_name, "md_d%d", unit);
+ snprintf(disk->disk_name, sizeof(disk->disk_name), "md_d%d", unit);
else
- sprintf(disk->disk_name, "md%d", unit);
+ snprintf(disk->disk_name, sizeof (disk->disk_name), "md%d", unit);
disk->fops = &md_fops;
disk->private_data = mddev;
--
2.39.5
^ permalink raw reply related
* Re: [PATCH 01/19] btrfs: require at least 4 devices for RAID 6
From: patchwork-bot+linux-riscv @ 2026-06-26 8:21 UTC (permalink / raw)
To: Christoph Hellwig
Cc: linux-riscv, akpm, catalin.marinas, will, ardb, chenhuacai,
kernel, maddy, mpe, npiggin, chleroy, pjw, palmer, aou, alex, hca,
gor, agordeev, borntraeger, svens, tglx, mingo, bp, dave.hansen,
x86, hpa, herbert, dan.j.williams, clm, dsterba, arnd, song,
yukuai, linan122, linux-kernel, linux-arm-kernel, loongarch,
linuxppc-dev, linux-s390, linux-crypto, linux-btrfs, linux-arch,
linux-raid
In-Reply-To: <20260512052230.2947683-2-hch@lst.de>
Hello:
This series was applied to riscv/linux.git (fixes)
by Andrew Morton <akpm@linux-foundation.org>:
On Tue, 12 May 2026 07:20:41 +0200 you wrote:
> While the RAID6 algorithm could in theory support 3 devices by just
> copying the data disk to the two parity disks, this version is not only
> useless because it is a suboptimal version of 3-way mirroring, but also
> broken with various crashes and incorrect parity generation in various
> architecture-optimized implementations. Disallow it similar to mdraid
> which requires at least 4 devices for RAID 6.
>
> [...]
Here is the summary with links:
- [01/19] btrfs: require at least 4 devices for RAID 6
(no matching commit)
- [02/19] raid6: turn the userspace test harness into a kunit test
(no matching commit)
- [03/19] raid6: remove __KERNEL__ ifdefs
https://git.kernel.org/riscv/c/3d6beb659ddf
- [04/19] raid6: move to lib/raid/
(no matching commit)
- [05/19] raid6: remove unused defines in pq.h
https://git.kernel.org/riscv/c/06d2a66fb7c0
- [06/19] raid6: remove raid6_get_zero_page
https://git.kernel.org/riscv/c/885d31423183
- [07/19] raid6: use named initializers for struct raid6_calls
https://git.kernel.org/riscv/c/7e91f76a9668
- [08/19] raid6: improve the public interface
(no matching commit)
- [09/19] raid6: hide internals
(no matching commit)
- [10/19] raid6: rework the init helpers
(no matching commit)
- [11/19] raid6: use static_call for gen_syndrom and xor_syndrom
https://git.kernel.org/riscv/c/10f4b8e2a164
- [12/19] raid6: use static_call for raid6_recov_2data and raid6_recov_datap
(no matching commit)
- [13/19] raid6: update top of file comments
https://git.kernel.org/riscv/c/30bf04bd13a5
- [14/19] raid6_kunit: use KUNIT_CASE_PARAM
https://git.kernel.org/riscv/c/2175395f76c3
- [15/19] raid6_kunit: dynamically allocate data buffers using vmalloc
https://git.kernel.org/riscv/c/d67c25712fe3
- [16/19] raid6_kunit: cleanup dataptr handling
https://git.kernel.org/riscv/c/562bcbfcb99b
- [17/19] raid6_kunit: randomize parameters and increase limits
(no matching commit)
- [18/19] raid6_kunit: randomize parameters and increase limits
(no matching commit)
- [19/19] raid6_kunit: randomize buffer alignment
https://git.kernel.org/riscv/c/8cf0a6c4bb9e
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply
* Re: [PATCH 01/18] raid6: turn the userspace test harness into a kunit test
From: patchwork-bot+linux-riscv @ 2026-06-26 8:20 UTC (permalink / raw)
To: Christoph Hellwig
Cc: linux-riscv, akpm, catalin.marinas, will, ardb, chenhuacai,
kernel, maddy, mpe, npiggin, chleroy, pjw, palmer, aou, alex, hca,
gor, agordeev, borntraeger, svens, tglx, mingo, bp, dave.hansen,
x86, hpa, herbert, dan.j.williams, clm, dsterba, arnd, song,
yukuai, linan122, linux-kernel, linux-arm-kernel, loongarch,
linuxppc-dev, linux-s390, linux-crypto, linux-btrfs, linux-arch,
linux-raid
In-Reply-To: <20260518051804.462141-2-hch@lst.de>
Hello:
This series was applied to riscv/linux.git (fixes)
by Andrew Morton <akpm@linux-foundation.org>:
On Mon, 18 May 2026 07:17:44 +0200 you wrote:
> Currently the raid6 code can be compiled as userspace code to run the
> test suite. Convert that to be a kunit case with minimal changes to
> avoid mutating global state so that we can drop this requirement.
>
> Note that this is not a good kunit test case yet and will need a lot more
> work, but that is deferred until the raid6 code is moved to it's new
> place, which is easier if the userspace makefile doesn't need adjustments
> for the new location first.
>
> [...]
Here is the summary with links:
- [01/18] raid6: turn the userspace test harness into a kunit test
https://git.kernel.org/riscv/c/c4697486fc23
- [02/18] raid6: remove __KERNEL__ ifdefs
https://git.kernel.org/riscv/c/3d6beb659ddf
- [03/18] raid6: move to lib/raid/
https://git.kernel.org/riscv/c/3626738bc714
- [04/18] raid6: remove unused defines in pq.h
https://git.kernel.org/riscv/c/06d2a66fb7c0
- [05/18] raid6: remove raid6_get_zero_page
https://git.kernel.org/riscv/c/885d31423183
- [06/18] raid6: use named initializers for struct raid6_calls
https://git.kernel.org/riscv/c/7e91f76a9668
- [07/18] raid6: improve the public interface
https://git.kernel.org/riscv/c/35472bc6f31b
- [08/18] raid6: warn when using less than four devices
https://git.kernel.org/riscv/c/2790045a62eb
- [09/18] raid6: hide internals
(no matching commit)
- [10/18] raid6: rework registration of optimized algorithms
(no matching commit)
- [11/18] raid6: use static_call for gen_syndrom and xor_syndrom
https://git.kernel.org/riscv/c/10f4b8e2a164
- [12/18] raid6: use static_call for raid6_recov_2data and raid6_recov_datap
https://git.kernel.org/riscv/c/dd83de0341da
- [13/18] raid6: update top of file comments
https://git.kernel.org/riscv/c/30bf04bd13a5
- [14/18] raid6_kunit: use KUNIT_CASE_PARAM
https://git.kernel.org/riscv/c/2175395f76c3
- [15/18] raid6_kunit: dynamically allocate data buffers using vmalloc
https://git.kernel.org/riscv/c/d67c25712fe3
- [16/18] raid6_kunit: cleanup dataptr handling
https://git.kernel.org/riscv/c/562bcbfcb99b
- [17/18] raid6_kunit: randomize parameters and increase limits
https://git.kernel.org/riscv/c/fa0c812c0aa5
- [18/18] raid6_kunit: randomize buffer alignment
https://git.kernel.org/riscv/c/8cf0a6c4bb9e
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply
* Re: [PATCH 2/8] um: Check for missing AVX and AVX-512 xstate bits
From: David Laight @ 2026-06-26 7:41 UTC (permalink / raw)
To: Eric Biggers
Cc: x86, linux-um, linux-raid, linux-crypto, linux-kernel,
Christoph Hellwig, Andrew Morton
In-Reply-To: <20260626043731.319287-3-ebiggers@kernel.org>
On Thu, 25 Jun 2026 21:37:25 -0700
Eric Biggers <ebiggers@kernel.org> wrote:
> If the CPU declares AVX or AVX-512 support, verify that all the
> corresponding xstate bits are also set. If any are missing, warn and
> don't set the corresponding X86_FEATURE_* flags.
>
> This eliminates the perceived need for UML-supporting AVX and AVX-512
> optimized code in the kernel (that is, lib/raid/ currently) to start
> checking the xstate bits in addition to X86_FEATURE_AVX*.
>
...
> static void __init parse_host_cpu_flags(char *line)
> {
> + u64 xcr0 = read_xcr0();
> int i;
> +
> for (i = 0; i < 32*NCAPINTS; i++) {
> if ((x86_cap_flags[i] != NULL) && strstr(line, x86_cap_flags[i]))
'line' comes from /proc/cpuinfo
Surely something would be terribly wrong if that included something the kernel
had disabled (or didn't support).
David
> - set_cpu_cap(&boot_cpu_data, i);
> + validate_and_set_cpu_cap(i, xcr0);
> }
> }
>
> static void __init parse_cache_line(char *line)
> {
^ permalink raw reply
* Re: [PATCH 8/8] lib/raid/xor: x86: Add AVX-512 optimized xor_gen()
From: Christoph Hellwig @ 2026-06-26 5:47 UTC (permalink / raw)
To: Eric Biggers
Cc: x86, linux-um, linux-raid, linux-crypto, linux-kernel,
Andrew Morton, David Laight
In-Reply-To: <20260626054731.GC9629@lst.de>
On Fri, Jun 26, 2026 at 07:47:31AM +0200, Christoph Hellwig wrote:
> On Thu, Jun 25, 2026 at 09:37:31PM -0700, Eric Biggers wrote:
> > + if (IS_ENABLED(CONFIG_X86_64) && boot_cpu_has(X86_FEATURE_AVX512F) &&
> > + !boot_cpu_has(X86_FEATURE_PREFER_YMM)) {
> > + /* AVX-512 will be the best; no need to try others. */
> > + /* !PREFER_YMM excludes CPUs with overly-eager downclocking. */
>
> Can you turn this into a single block comment using full sentences?
> Right now the two separate comments almost feel contradictory even
> if I get what you mean. While you're at it also through in a blurb
> why we dont bother with AVX-512 (number of register, no one in the right
> mind would bother running high performance code on modern cpus in 32-bit
> mode).
Otherwise looks good, btw:
Reviewed-by: Christoph Hellwig <hch@lst.de>
^ permalink raw reply
* Re: [PATCH 8/8] lib/raid/xor: x86: Add AVX-512 optimized xor_gen()
From: Christoph Hellwig @ 2026-06-26 5:47 UTC (permalink / raw)
To: Eric Biggers
Cc: x86, linux-um, linux-raid, linux-crypto, linux-kernel,
Christoph Hellwig, Andrew Morton, David Laight
In-Reply-To: <20260626043731.319287-9-ebiggers@kernel.org>
On Thu, Jun 25, 2026 at 09:37:31PM -0700, Eric Biggers wrote:
> + if (IS_ENABLED(CONFIG_X86_64) && boot_cpu_has(X86_FEATURE_AVX512F) &&
> + !boot_cpu_has(X86_FEATURE_PREFER_YMM)) {
> + /* AVX-512 will be the best; no need to try others. */
> + /* !PREFER_YMM excludes CPUs with overly-eager downclocking. */
Can you turn this into a single block comment using full sentences?
Right now the two separate comments almost feel contradictory even
if I get what you mean. While you're at it also through in a blurb
why we dont bother with AVX-512 (number of register, no one in the right
mind would bother running high performance code on modern cpus in 32-bit
mode).
^ permalink raw reply
* Re: [PATCH 7/8] lib/raid/xor: x86: Remove redundant X86_FEATURE_OSXSAVE check
From: Christoph Hellwig @ 2026-06-26 5:40 UTC (permalink / raw)
To: Eric Biggers
Cc: x86, linux-um, linux-raid, linux-crypto, linux-kernel,
Christoph Hellwig, Andrew Morton
In-Reply-To: <20260626043731.319287-8-ebiggers@kernel.org>
Looks good:
Reviewed-by: Christoph Hellwig <hch@lst.de>
^ permalink raw reply
* Re: [PATCH 1/8] x86/fpu: Check for missing AVX and AVX-512 xstate bits
From: Christoph Hellwig @ 2026-06-26 5:39 UTC (permalink / raw)
To: Eric Biggers
Cc: x86, linux-um, linux-raid, linux-crypto, linux-kernel,
Christoph Hellwig, Andrew Morton
In-Reply-To: <20260626043731.319287-2-ebiggers@kernel.org>
On Thu, Jun 25, 2026 at 09:37:24PM -0700, Eric Biggers wrote:
> If the CPU declares AVX or AVX-512 support, verify that the
> corresponding xstate bits are also set. If not, warn and clear them.
>
> This eliminates the perceived need for AVX and AVX-512 optimized code in
> the kernel to call cpu_has_xfeatures(). That has never been universally
> done, which strongly suggests that it has never really been needed in
> practice, but this should remove any remaining doubt.
I'll leave it to the x86-experts if the low-level details are right,
but the model behind this makes life so much easier, thanks a lot!
^ permalink raw reply
* Re: [PATCH 1/8] x86/fpu: Check for missing AVX and AVX-512 xstate bits
From: sashiko-bot @ 2026-06-26 5:00 UTC (permalink / raw)
To: Eric Biggers; +Cc: linux-raid, yukuai
In-Reply-To: <20260626043731.319287-2-ebiggers@kernel.org>
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
New issues:
- [High] The patch fails to clear AVX and AVX-512 CPU capabilities when the CPU natively enumerates AVX but omits XSAVE or FPU support, bypassing the new sanitization logic and leading to #UD panics.
Pre-existing issues:
- [Critical] Memory Protection Keys (PKU) isolation can be bypassed because the kernel enables X86_FEATURE_PKU without verifying the presence of XFEATURE_PKRU in xstate.
--
commit 0cd2494a6ed30cd9eb21bc480a6d7d789ab47e00
Author: Eric Biggers <ebiggers@kernel.org>
x86/fpu: Check for missing AVX and AVX-512 xstate bits
If the CPU declares AVX or AVX-512 support, verify that the
corresponding xstate bits are also set. If not, warn and clear them.
This eliminates the perceived need for AVX and AVX-512 optimized code in
the kernel to call cpu_has_xfeatures().
> diff --git a/arch/x86/kernel/fpu/xstate.c b/arch/x86/kernel/fpu/xstate.c
> index a7b6524a9dea2..7f7e62e4ebc5b 100644
> --- a/arch/x86/kernel/fpu/xstate.c
> +++ b/arch/x86/kernel/fpu/xstate.c
> @@ -799,6 +799,23 @@ static u64 __init guest_default_mask(void)
> return ~(u64)XFEATURE_MASK_USER_DYNAMIC;
> }
>
> +/* Clear any X86_FEATURE_* used by the kernel whose xfeatures are missing. */
> +static void __init clear_cpu_caps_with_missing_xfeatures(u64 xfeatures)
> +{
> + u64 mask;
> +
> + mask = XFEATURE_MASK_FPSSE | XFEATURE_MASK_YMM;
> + if (boot_cpu_has(X86_FEATURE_AVX) && (xfeatures & mask) != mask) {
> + pr_err("x86/fpu: Disabling AVX support due to missing xstate features\n");
> + setup_clear_cpu_cap(X86_FEATURE_AVX);
> + }
> + mask = XFEATURE_MASK_FPSSE | XFEATURE_MASK_YMM | XFEATURE_MASK_AVX512;
> + if (boot_cpu_has(X86_FEATURE_AVX512F) && (xfeatures & mask) != mask) {
> + pr_err("x86/fpu: Disabling AVX-512 support due to missing xstate features\n");
> + setup_clear_cpu_cap(X86_FEATURE_AVX512F);
> + }
> +}
> +
[Severity: Critical]
This is a pre-existing issue, but Memory Protection Keys (PKU) isolation
can be bypassed because we don't verify X86_FEATURE_PKU against the
presence of XFEATURE_PKRU in xstate here.
If a hypervisor reports X86_FEATURE_PKU in CPUID but masks out the
XFEATURE_PKRU xstate component, the kernel's setup_pku() enables CR4.PKE
and forces X86_FEATURE_OSPKE. Because X86_FEATURE_OSPKE is artificially
forced, the feature is not cleared.
CR4.PKE=1 permits userspace to use WRPKRU, but because XCR0.PKRU=0,
xsaves/xrstors silently ignore the PKRU register during task context
switches, leading to state leakage across tasks.
Should we also validate PKU against missing xstate components in this
function to prevent isolation bypass?
> /*
> * Enable and initialize the xsave feature.
> * Called once per system bootup.
[ ... ]
> @@ -855,6 +872,8 @@ void __init fpu__init_system_xstate(unsigned int legacy_size)
> goto out_disable;
> }
>
> + clear_cpu_caps_with_missing_xfeatures(fpu_kernel_cfg.max_features);
> +
[Severity: High]
Does this logic get bypassed if XSAVE or FPU support is missing?
Earlier in fpu__init_system_xstate(), there are early returns if
X86_FEATURE_FPU or X86_FEATURE_XSAVE are missing:
if (!boot_cpu_has(X86_FEATURE_XSAVE)) {
pr_info("x86/fpu: x87 FPU will use %s\n",
boot_cpu_has(X86_FEATURE_FXSR) ? "FXSAVE" : "FSAVE");
return;
}
If a hypervisor natively enumerates AVX=1 but XSAVE=0 (such as QEMU
with -cpu host,-xsave), check_cpufeature_deps() only warns about unmet
dependencies without clearing them. The lack of X86_FEATURE_XSAVE triggers
the early return, completely bypassing this new capability cleanup logic.
Since the kernel will then attempt to execute VEX-encoded AVX instructions
while CR4.OSXSAVE=0, will this result in an invalid opcode (#UD) panic?
> fpu_kernel_cfg.independent_features = fpu_kernel_cfg.max_features &
> XFEATURE_MASK_INDEPENDENT;
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260626043731.319287-1-ebiggers@kernel.org?part=1
^ permalink raw reply
* Re: [PATCH 4/8] lib/crypto: x86: Stop using cpu_has_xfeatures()
From: sashiko-bot @ 2026-06-26 4:53 UTC (permalink / raw)
To: Eric Biggers; +Cc: linux-raid, yukuai
In-Reply-To: <20260626043731.319287-5-ebiggers@kernel.org>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Removing `cpu_has_xfeatures()` causes a kernel #UD panic on systems advertising AVX without XSAVE.
--
commit c4725d088a9c170c27e417f42ccdfa48f567b845
Author: Eric Biggers <ebiggers@kernel.org>
lib/crypto: x86: Stop using cpu_has_xfeatures()
This commit removes calls to cpu_has_xfeatures() across the x86 crypto
library, relying entirely on boot_cpu_has() to check for features. The
assumption is that x86 explicitly clears AVX and AVX-512 flags if their
xfeatures are missing.
> diff --git a/lib/crypto/x86/poly1305.h b/lib/crypto/x86/poly1305.h
> index ee92e3740a787..b061b9926fa5d 100644
> --- a/lib/crypto/x86/poly1305.h
> +++ b/lib/crypto/x86/poly1305.h
> @@ -143,15 +143,12 @@ static void poly1305_emit(const struct poly1305_state *ctx,
> #define poly1305_mod_init_arch poly1305_mod_init_arch
> static void poly1305_mod_init_arch(void)
> {
> - if (boot_cpu_has(X86_FEATURE_AVX) &&
> - cpu_has_xfeatures(XFEATURE_MASK_SSE | XFEATURE_MASK_YMM, NULL))
> + if (boot_cpu_has(X86_FEATURE_AVX))
> static_branch_enable(&poly1305_use_avx);
[Severity: High]
Will this cause a invalid opcode panic on systems advertising AVX without XSAVE
such as virtualized environments using a missing xsave cpu flag?
If XSAVE is missing natively, fpu__init_system_xstate() in
arch/x86/kernel/fpu/xstate.c returns early without clearing features:
arch/x86/kernel/fpu/xstate.c:fpu__init_system_xstate() {
...
if (!boot_cpu_has(X86_FEATURE_XSAVE)) {
pr_info("x86/fpu: x87 FPU will use %s\n",
boot_cpu_has(X86_FEATURE_FXSR) ? "FXSAVE" : "FSAVE");
return;
}
...
}
This early return bypasses clear_cpu_caps_with_missing_xfeatures(). Since
the CPUID dependency enforcement is only triggered when a feature is explicitly
cleared, X86_FEATURE_AVX is never cleared in this specific situation.
Without the cpu_has_xfeatures() check here, the crypto subsystems will see
boot_cpu_has(X86_FEATURE_AVX) as true and enable AVX instructions. Because
XSAVE is disabled, the OS never sets CR4.OSXSAVE=1, which can lead to a panic
when AVX instructions subsequently execute.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260626043731.319287-1-ebiggers@kernel.org?part=4
^ permalink raw reply
* Re: [PATCH 7/8] lib/raid/xor: x86: Remove redundant X86_FEATURE_OSXSAVE check
From: sashiko-bot @ 2026-06-26 4:51 UTC (permalink / raw)
To: Eric Biggers; +Cc: linux-raid, yukuai
In-Reply-To: <20260626043731.319287-8-ebiggers@kernel.org>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Removing the X86_FEATURE_OSXSAVE check can cause an Invalid Opcode (#UD) kernel panic during XOR RAID operations if the CPU natively advertises AVX but not XSAVE.
--
commit 6fce8e80966a4c49b0b494779bd6206c6d6bed1f
Author: Eric Biggers <ebiggers@kernel.org>
lib/raid/xor: x86: Remove redundant X86_FEATURE_OSXSAVE check
This commit removes the X86_FEATURE_OSXSAVE check when selecting the AVX
XOR implementation, assuming X86_FEATURE_AVX implies X86_FEATURE_OSXSAVE.
> diff --git a/lib/raid/xor/x86/xor_arch.h b/lib/raid/xor/x86/xor_arch.h
> index 99fe85a213c66..991abe3f4bbda 100644
> --- a/lib/raid/xor/x86/xor_arch.h
> +++ b/lib/raid/xor/x86/xor_arch.h
> @@ -18,8 +18,7 @@ extern struct xor_block_template xor_block_avx;
> */
> static __always_inline void __init arch_xor_init(void)
> {
> - if (boot_cpu_has(X86_FEATURE_AVX) &&
> - boot_cpu_has(X86_FEATURE_OSXSAVE)) {
> + if (boot_cpu_has(X86_FEATURE_AVX)) {
[Severity: High]
Can this lead to an Invalid Opcode (#UD) kernel panic during XOR RAID
operations if a CPU advertises AVX but not XSAVE?
If a hypervisor exposes AVX but hides XSAVE (for example, using QEMU's
-cpu host,-xsave flag), get_cpu_cap() still sets X86_FEATURE_AVX in
boot_cpu_data.
Although check_cpufeature_deps() detects the missing dependency, it only
issues a pr_warn_once warning without actually clearing X86_FEATURE_AVX.
During boot, fpu__init_system_xstate() evaluates
!boot_cpu_has(X86_FEATURE_XSAVE) as true and executes an early return.
This early return bypasses clear_cpu_caps_with_missing_xfeatures(), meaning
X86_FEATURE_AVX is never cleared by the kernel.
Since the X86_FEATURE_OSXSAVE check was removed here, arch_xor_init()
will forcibly select xor_block_avx based solely on X86_FEATURE_AVX.
When VEX-encoded AVX instructions are later executed, the CPU will throw an
Invalid Opcode (#UD) exception because CR4.OSXSAVE was never set to 1 by
the OS.
Is there another mechanism that guarantees X86_FEATURE_AVX is cleared in
this scenario, or should the X86_FEATURE_OSXSAVE check be retained to safely
ensure the OS has fully initialized and enabled XSAVE?
> xor_force(&xor_block_avx);
> } else if (IS_ENABLED(CONFIG_X86_64) || boot_cpu_has(X86_FEATURE_XMM)) {
> xor_register(&xor_block_sse);
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260626043731.319287-1-ebiggers@kernel.org?part=7
^ permalink raw reply
* [PATCH 8/8] lib/raid/xor: x86: Add AVX-512 optimized xor_gen()
From: Eric Biggers @ 2026-06-26 4:37 UTC (permalink / raw)
To: x86
Cc: linux-um, linux-raid, linux-crypto, linux-kernel,
Christoph Hellwig, Andrew Morton, Eric Biggers, David Laight
In-Reply-To: <20260626043731.319287-1-ebiggers@kernel.org>
Add an implementation of xor_gen() using AVX-512.
It uses 512-bit vectors, i.e. ZMM registers. It also uses the
vpternlogq instruction to do three-input XORs when applicable.
It's enabled on x86_64 CPUs that have AVX512F && !PREFER_YMM. In
practice that means:
- AMD Zen 4 and later (client and server)
- Intel Sapphire Rapids and later (server)
- Intel Rocket Lake (client)
- Intel Nova Lake and later (client)
The !PREFER_YMM condition excludes the older AVX-512 implementations in
Intel Skylake Server and Intel Ice Lake. They could run this code, but
they're known to have overly-eager downclocking when ZMM registers are
used. This is the same policy that the crypto and CRC code uses.
Benchmark on AMD Ryzen 9 9950X (Zen 5):
src_cnt avx avx512 Improvement
======= ========== ========== ===========
1 56353 MB/s 75388 MB/s 33%
2 54274 MB/s 68409 MB/s 26%
3 44649 MB/s 64042 MB/s 43%
4 41315 MB/s 55002 MB/s 33%
Reviewed-by: David Laight <david.laight.linux@gmail.com>
Signed-off-by: Eric Biggers <ebiggers@kernel.org>
---
lib/raid/xor/Makefile | 2 +-
lib/raid/xor/x86/xor-avx512.c | 121 ++++++++++++++++++++++++++++++++++
lib/raid/xor/x86/xor_arch.h | 23 ++++---
3 files changed, 135 insertions(+), 11 deletions(-)
create mode 100644 lib/raid/xor/x86/xor-avx512.c
diff --git a/lib/raid/xor/Makefile b/lib/raid/xor/Makefile
index e8ecec3c09f9..4a0e5c6d8298 100644
--- a/lib/raid/xor/Makefile
+++ b/lib/raid/xor/Makefile
@@ -27,11 +27,11 @@ xor-$(CONFIG_ALTIVEC) += powerpc/xor_vmx.o powerpc/xor_vmx_glue.o
xor-$(CONFIG_RISCV_ISA_V) += riscv/xor.o riscv/xor-glue.o
xor-$(CONFIG_SPARC32) += sparc/xor-sparc32.o
xor-$(CONFIG_SPARC64) += sparc/xor-sparc64.o sparc/xor-sparc64-glue.o
xor-$(CONFIG_S390) += s390/xor.o
xor-$(CONFIG_X86_32) += x86/xor-avx.o x86/xor-sse.o x86/xor-mmx.o
-xor-$(CONFIG_X86_64) += x86/xor-avx.o x86/xor-sse.o
+xor-$(CONFIG_X86_64) += x86/xor-avx.o x86/xor-sse.o x86/xor-avx512.o
obj-y += tests/
CFLAGS_xor-neon.o += $(CC_FLAGS_FPU) -I$(src)/$(SRCARCH)
CFLAGS_REMOVE_xor-neon.o += $(CC_FLAGS_NO_FPU)
diff --git a/lib/raid/xor/x86/xor-avx512.c b/lib/raid/xor/x86/xor-avx512.c
new file mode 100644
index 000000000000..17f57900d827
--- /dev/null
+++ b/lib/raid/xor/x86/xor-avx512.c
@@ -0,0 +1,121 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * AVX-512 optimized implementation of xor_gen()
+ *
+ * Copyright 2026 Google LLC
+ */
+
+#include <linux/types.h>
+#include <asm/fpu/api.h>
+#include "xor_impl.h"
+#include "xor_arch.h"
+
+/*
+ * Implementation notes:
+ *
+ * Unrolling by the number of buffers (2-5) is very important.
+ *
+ * Unrolling by length is less important, especially when using register-indexed
+ * addressing with negative indices from the end of the buffers. That approach
+ * results in just two loop control instructions being needed per iteration,
+ * regardless of the number of buffers.
+ *
+ * In fact, benchmarks showed that the 2 and 3 buffer cases require only 2x
+ * unrolling by length, while the 4 and 5 buffer cases don't require any
+ * unrolling by length. Benchmarks also showed that the register-indexed
+ * addressing isn't a bottleneck either; i.e., we can't do any better by
+ * incrementing the pointers as we go along, even with more unrolling.
+ */
+
+static void xor_avx512_2(long bytes, u8 *p1, const u8 *p2)
+{
+ long i = -bytes;
+
+ asm volatile("1: vmovdqa64 (%1,%0), %%zmm0\n"
+ "vmovdqa64 64(%1,%0), %%zmm1\n"
+ "vpxorq (%2,%0), %%zmm0, %%zmm0\n"
+ "vpxorq 64(%2,%0), %%zmm1, %%zmm1\n"
+ "vmovdqa64 %%zmm0, (%1,%0)\n"
+ "vmovdqa64 %%zmm1, 64(%1,%0)\n"
+ "add $128, %0\n"
+ "jnz 1b\n"
+ : "+&r"(i)
+ : "r"(p1 + bytes), "r"(p2 + bytes)
+ : "memory", "cc");
+}
+
+static void xor_avx512_3(long bytes, u8 *p1, const u8 *p2, const u8 *p3)
+{
+ long i = -bytes;
+
+ asm volatile("1: vmovdqa64 (%1,%0), %%zmm0\n"
+ "vmovdqa64 64(%1,%0), %%zmm1\n"
+ "vmovdqa64 (%2,%0), %%zmm2\n"
+ "vmovdqa64 64(%2,%0), %%zmm3\n"
+ "vpternlogq $0x96, (%3,%0), %%zmm2, %%zmm0\n"
+ "vpternlogq $0x96, 64(%3,%0), %%zmm3, %%zmm1\n"
+ "vmovdqa64 %%zmm0, (%1,%0)\n"
+ "vmovdqa64 %%zmm1, 64(%1,%0)\n"
+ "add $128, %0\n"
+ "jnz 1b\n"
+ : "+&r"(i)
+ : "r"(p1 + bytes), "r"(p2 + bytes), "r"(p3 + bytes)
+ : "memory", "cc");
+}
+
+static void xor_avx512_4(long bytes, u8 *p1, const u8 *p2, const u8 *p3,
+ const u8 *p4)
+{
+ long i = -bytes;
+
+ asm volatile("1: vmovdqa64 (%1,%0), %%zmm0\n"
+ "vmovdqa64 (%2,%0), %%zmm1\n"
+ "vpxorq (%3,%0), %%zmm0, %%zmm0\n"
+ "vpternlogq $0x96, (%4,%0), %%zmm1, %%zmm0\n"
+ "vmovdqa64 %%zmm0, (%1,%0)\n"
+ "add $64, %0\n"
+ "jnz 1b\n"
+ : "+&r"(i)
+ : "r"(p1 + bytes), "r"(p2 + bytes), "r"(p3 + bytes),
+ "r"(p4 + bytes)
+ : "memory", "cc");
+}
+
+static void xor_avx512_5(long bytes, u8 *p1, const u8 *p2, const u8 *p3,
+ const u8 *p4, const u8 *p5)
+{
+ long i = -bytes;
+
+ asm volatile("1: vmovdqa64 (%1,%0), %%zmm0\n"
+ "vmovdqa64 (%2,%0), %%zmm1\n"
+ "vpternlogq $0x96, (%3,%0), %%zmm1, %%zmm0\n"
+ "vmovdqa64 (%4,%0), %%zmm1\n"
+ "vpternlogq $0x96, (%5,%0), %%zmm1, %%zmm0\n"
+ "vmovdqa64 %%zmm0, (%1,%0)\n"
+ "add $64, %0\n"
+ "jnz 1b\n"
+ : "+&r"(i)
+ : "r"(p1 + bytes), "r"(p2 + bytes), "r"(p3 + bytes),
+ "r"(p4 + bytes), "r"(p5 + bytes)
+ : "memory", "cc");
+}
+
+DO_XOR_BLOCKS(avx512_inner, xor_avx512_2, xor_avx512_3, xor_avx512_4,
+ xor_avx512_5);
+
+/*
+ * Preconditions: bytes is a nonzero multiple of 512, and all buffers are
+ * 64-byte aligned.
+ */
+static void xor_gen_avx512(void *dest, void **srcs, unsigned int src_cnt,
+ unsigned int bytes)
+{
+ kernel_fpu_begin();
+ xor_gen_avx512_inner(dest, srcs, src_cnt, bytes);
+ kernel_fpu_end();
+}
+
+struct xor_block_template xor_block_avx512 = {
+ .name = "avx512",
+ .xor_gen = xor_gen_avx512,
+};
diff --git a/lib/raid/xor/x86/xor_arch.h b/lib/raid/xor/x86/xor_arch.h
index 991abe3f4bbd..d5e192b8793f 100644
--- a/lib/raid/xor/x86/xor_arch.h
+++ b/lib/raid/xor/x86/xor_arch.h
@@ -4,25 +4,28 @@
extern struct xor_block_template xor_block_pII_mmx;
extern struct xor_block_template xor_block_p5_mmx;
extern struct xor_block_template xor_block_sse;
extern struct xor_block_template xor_block_sse_pf64;
extern struct xor_block_template xor_block_avx;
+extern struct xor_block_template xor_block_avx512;
-/*
- * When SSE is available, use it as it can write around L2. We may also be able
- * to load into the L1 only depending on how the cpu deals with a load to a line
- * that is being prefetched.
- *
- * When AVX2 is available, force using it as it is better by all measures.
- *
- * 32-bit without MMX can fall back to the generic routines.
- */
static __always_inline void __init arch_xor_init(void)
{
- if (boot_cpu_has(X86_FEATURE_AVX)) {
+ if (IS_ENABLED(CONFIG_X86_64) && boot_cpu_has(X86_FEATURE_AVX512F) &&
+ !boot_cpu_has(X86_FEATURE_PREFER_YMM)) {
+ /* AVX-512 will be the best; no need to try others. */
+ /* !PREFER_YMM excludes CPUs with overly-eager downclocking. */
+ xor_force(&xor_block_avx512);
+ } else if (boot_cpu_has(X86_FEATURE_AVX)) {
+ /* AVX will be the best; no need to try others. */
xor_force(&xor_block_avx);
} else if (IS_ENABLED(CONFIG_X86_64) || boot_cpu_has(X86_FEATURE_XMM)) {
+ /*
+ * When SSE is available, use it as it can write around L2. We
+ * may also be able to load into the L1 only depending on how
+ * the cpu deals with a load to a line that is being prefetched.
+ */
xor_register(&xor_block_sse);
xor_register(&xor_block_sse_pf64);
} else if (boot_cpu_has(X86_FEATURE_MMX)) {
xor_register(&xor_block_pII_mmx);
xor_register(&xor_block_p5_mmx);
--
2.54.0
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox