linux-kernel - Re: [PATCH 5.10/5.15] md/raid5: fix atomicity violation in raid5_cache

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CAPhsuW4HV0tkuFA8AQP4hZ9OxuwT_+kiqZiWBfcsXrHoaViw6A@mail.gmail.com>
Date: Thu, 16 Jan 2025 10:43:23 -0800
From: Song Liu <song@...nel.org>
To: Vasiliy Kovalev <kovalev@...linux.org>
Cc: stable@...r.kernel.org, NeilBrown <neilb@...e.de>, linux-raid@...r.kernel.org, 
	linux-kernel@...r.kernel.org, Gui-Dong Han <2045gemini@...il.com>, 
	Yu Kuai <yukuai3@...wei.com>, lvc-project@...uxtesting.org
Subject: Re: [PATCH 5.10/5.15] md/raid5: fix atomicity violation in raid5_cache_count

On Fri, Jan 10, 2025 at 10:35 AM Vasiliy Kovalev <kovalev@...linux.org> wrote:
>
> From: Gui-Dong Han <2045gemini@...il.com>
>
> [ Upstream commit dfd2bf436709b2bccb78c2dda550dde93700efa7 ]
>
> In raid5_cache_count():
>     if (conf->max_nr_stripes < conf->min_nr_stripes)
>         return 0;
>     return conf->max_nr_stripes - conf->min_nr_stripes;
> The current check is ineffective, as the values could change immediately
> after being checked.
>
> In raid5_set_cache_size():
>     ...
>     conf->min_nr_stripes = size;
>     ...
>     while (size > conf->max_nr_stripes)
>         conf->min_nr_stripes = conf->max_nr_stripes;
>     ...
>
> Due to intermediate value updates in raid5_set_cache_size(), concurrent
> execution of raid5_cache_count() and raid5_set_cache_size() may lead to
> inconsistent reads of conf->max_nr_stripes and conf->min_nr_stripes.
> The current checks are ineffective as values could change immediately
> after being checked, raising the risk of conf->min_nr_stripes exceeding
> conf->max_nr_stripes and potentially causing an integer overflow.
>
> This possible bug is found by an experimental static analysis tool
> developed by our team. This tool analyzes the locking APIs to extract
> function pairs that can be concurrently executed, and then analyzes the
> instructions in the paired functions to identify possible concurrency bugs
> including data races and atomicity violations. The above possible bug is
> reported when our tool analyzes the source code of Linux 6.2.
>
> To resolve this issue, it is suggested to introduce local variables
> 'min_stripes' and 'max_stripes' in raid5_cache_count() to ensure the
> values remain stable throughout the check. Adding locks in
> raid5_cache_count() fails to resolve atomicity violations, as
> raid5_set_cache_size() may hold intermediate values of
> conf->min_nr_stripes while unlocked. With this patch applied, our tool no
> longer reports the bug, with the kernel configuration allyesconfig for
> x86_64. Due to the lack of associated hardware, we cannot test the patch
> in runtime testing, and just verify it according to the code logic.
>
> Fixes: edbe83ab4c27 ("md/raid5: allow the stripe_cache to grow and shrink.")
> Cc: stable@...r.kernel.org
> Signed-off-by: Gui-Dong Han <2045gemini@...il.com>
> Reviewed-by: Yu Kuai <yukuai3@...wei.com>
> Signed-off-by: Song Liu <song@...nel.org>
> Link: https://lore.kernel.org/r/20240112071017.16313-1-2045gemini@gmail.com
> Signed-off-by: Song Liu <song@...nel.org>
> Signed-off-by: Vasiliy Kovalev <kovalev@...linux.org>
> ---
> Backport to fix CVE-2024-23307
> Link: https://www.cve.org/CVERecord/?id=CVE-2024-23307

Looks good to me.

Thanks,
Song

[...]