* [PATCH] md/raid5: set conf->mddev before the first setup_conf() error path
@ 2026-10-04 6:02 Yogesh Gaur
2026-10-05 15:53 ` Logan Gunthorpe
2026-10-05 17:02 ` yu kuai
0 siblings, 2 replies; 3+ messages in thread
From: Yogesh Gaur @ 2026-10-04 6:02 UTC (permalink / raw)
To: Song Liu, Yu Kuai
Cc: linux-raid, linux-kernel, Li Nan, Xiao Ni, Christoph Hellwig,
Hannes Reinecke, Logan Gunthorpe, Yogesh Gaur,
syzbot+270624bb31d478afe62e, stable
free_conf() starts with log_exit(), which falls through to
raid5_has_ppl() when conf->log is NULL:
static inline void log_exit(struct r5conf *conf)
{
if (conf->log)
r5l_exit_log(conf);
else if (raid5_has_ppl(conf))
ppl_exit_log(conf);
}
raid5_has_ppl() reads conf->mddev->flags. setup_conf() only assigns
conf->mddev after bioset_init(), but five of its error paths -- the
pending_data, alloc_thread_groups(), conf->disks, extra_page and
bioset_init() failures -- jump to "abort:" before that, and abort: calls
free_conf(). conf comes from kzalloc, so conf->mddev is still NULL and
free_conf() dereferences it:
BUG: KASAN: null-ptr-deref in raid5_has_ppl drivers/md/raid5-log.h:54 [inline]
BUG: KASAN: null-ptr-deref in log_exit drivers/md/raid5-log.h:128 [inline]
BUG: KASAN: null-ptr-deref in free_conf+0x81/0x5d0 drivers/md/raid5.c:7549
Read of size 8 at addr 0000000000000028 by task syz.3.20/5681
Call Trace:
<TASK>
free_conf+0x81/0x5d0 drivers/md/raid5.c:7549
setup_conf+0x1720/0x2ad0 drivers/md/raid5.c:7885
raid5_run+0x8cc/0x2560 drivers/md/raid5.c:8129
md_run+0xc3d/0x1cd0 drivers/md/md.c:6779
do_md_run+0x35/0x720 drivers/md/md.c:6880
array_state_store+0x958/0xe90 drivers/md/md.c:-1
</TASK>
The faulting address is offsetof(struct mddev, flags).
conf->mddev is a back pointer that is constant for the lifetime of the
conf and does not depend on anything computed in between, so assign it
as soon as the conf is allocated. Nothing between the allocation and the
old assignment reads conf->mddev -- alloc_thread_groups() takes the conf
but never looks at its mddev, and the rdev_for_each() loop walks the
mddev argument directly.
All five paths are allocation failures, so this needs memory pressure or
fault injection to hit, which is how syzbot found it.
Reported-by: syzbot+270624bb31d478afe62e@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=270624bb31d478afe62e
Fixes: ff875738edd4 ("raid5: separate header for log functions")
Cc: stable@vger.kernel.org
Assisted-by: LLM
Signed-off-by: Yogesh Gaur <yogeshgaur.83@gmail.com>
---
drivers/md/raid5.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
index b91545ce090d..f98d3bdd3484 100644
--- a/drivers/md/raid5.c
+++ b/drivers/md/raid5.c
@@ -7675,6 +7675,9 @@ static struct r5conf *setup_conf(struct mddev *mddev)
if (conf == NULL)
goto abort;
+ /* free_conf() dereferences this, so set it before the first goto */
+ conf->mddev = mddev;
+
#if PAGE_SIZE != DEFAULT_STRIPE_SIZE
conf->stripe_size = DEFAULT_STRIPE_SIZE;
conf->stripe_shift = ilog2(DEFAULT_STRIPE_SIZE) - 9;
@@ -7743,7 +7746,6 @@ static struct r5conf *setup_conf(struct mddev *mddev)
ret = bioset_init(&conf->bio_split, BIO_POOL_SIZE, 0, 0);
if (ret)
goto abort;
- conf->mddev = mddev;
ret = -ENOMEM;
conf->stripe_hashtbl = kzalloc(PAGE_SIZE, GFP_KERNEL);
--
2.55.0.windows.5
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH] md/raid5: set conf->mddev before the first setup_conf() error path
2026-10-04 6:02 [PATCH] md/raid5: set conf->mddev before the first setup_conf() error path Yogesh Gaur
@ 2026-10-05 15:53 ` Logan Gunthorpe
2026-10-05 17:02 ` yu kuai
1 sibling, 0 replies; 3+ messages in thread
From: Logan Gunthorpe @ 2026-10-05 15:53 UTC (permalink / raw)
To: Yogesh Gaur, Song Liu, Yu Kuai
Cc: linux-raid, linux-kernel, Li Nan, Xiao Ni, Christoph Hellwig,
Hannes Reinecke, syzbot+270624bb31d478afe62e, stable
On 2026-10-04 00:02, Yogesh Gaur wrote:
> free_conf() starts with log_exit(), which falls through to
> raid5_has_ppl() when conf->log is NULL:
>
> static inline void log_exit(struct r5conf *conf)
> {
> if (conf->log)
> r5l_exit_log(conf);
> else if (raid5_has_ppl(conf))
> ppl_exit_log(conf);
> }
>
> raid5_has_ppl() reads conf->mddev->flags. setup_conf() only assigns
> conf->mddev after bioset_init(), but five of its error paths -- the
> pending_data, alloc_thread_groups(), conf->disks, extra_page and
> bioset_init() failures -- jump to "abort:" before that, and abort: calls
> free_conf(). conf comes from kzalloc, so conf->mddev is still NULL and
> free_conf() dereferences it:
>
> BUG: KASAN: null-ptr-deref in raid5_has_ppl drivers/md/raid5-log.h:54 [inline]
> BUG: KASAN: null-ptr-deref in log_exit drivers/md/raid5-log.h:128 [inline]
> BUG: KASAN: null-ptr-deref in free_conf+0x81/0x5d0 drivers/md/raid5.c:7549
> Read of size 8 at addr 0000000000000028 by task syz.3.20/5681
> Call Trace:
> <TASK>
> free_conf+0x81/0x5d0 drivers/md/raid5.c:7549
> setup_conf+0x1720/0x2ad0 drivers/md/raid5.c:7885
> raid5_run+0x8cc/0x2560 drivers/md/raid5.c:8129
> md_run+0xc3d/0x1cd0 drivers/md/md.c:6779
> do_md_run+0x35/0x720 drivers/md/md.c:6880
> array_state_store+0x958/0xe90 drivers/md/md.c:-1
> </TASK>
>
> The faulting address is offsetof(struct mddev, flags).
>
> conf->mddev is a back pointer that is constant for the lifetime of the
> conf and does not depend on anything computed in between, so assign it
> as soon as the conf is allocated. Nothing between the allocation and the
> old assignment reads conf->mddev -- alloc_thread_groups() takes the conf
> but never looks at its mddev, and the rdev_for_each() loop walks the
> mddev argument directly.
>
> All five paths are allocation failures, so this needs memory pressure or
> fault injection to hit, which is how syzbot found it.
>
> Reported-by: syzbot+270624bb31d478afe62e@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=270624bb31d478afe62e
> Fixes: ff875738edd4 ("raid5: separate header for log functions")
> Cc: stable@vger.kernel.org
> Assisted-by: LLM
> Signed-off-by: Yogesh Gaur <yogeshgaur.83@gmail.com>
Looks good to me, thanks!
Reviewed-by: Logan Gunthorpe <logang@deltatee.com>
Logan
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH] md/raid5: set conf->mddev before the first setup_conf() error path
2026-10-04 6:02 [PATCH] md/raid5: set conf->mddev before the first setup_conf() error path Yogesh Gaur
2026-10-05 15:53 ` Logan Gunthorpe
@ 2026-10-05 17:02 ` yu kuai
1 sibling, 0 replies; 3+ messages in thread
From: yu kuai @ 2026-10-05 17:02 UTC (permalink / raw)
To: Yogesh Gaur, Song Liu, yu kuai
Cc: linux-raid, linux-kernel, Li Nan, Xiao Ni, Christoph Hellwig,
Hannes Reinecke, Logan Gunthorpe, syzbot+270624bb31d478afe62e,
stable
Hi,
在 2026/10/4 14:02, Yogesh Gaur 写道:
> free_conf() starts with log_exit(), which falls through to
> raid5_has_ppl() when conf->log is NULL:
>
> static inline void log_exit(struct r5conf *conf)
> {
> if (conf->log)
> r5l_exit_log(conf);
> else if (raid5_has_ppl(conf))
> ppl_exit_log(conf);
> }
>
> raid5_has_ppl() reads conf->mddev->flags. setup_conf() only assigns
> conf->mddev after bioset_init(), but five of its error paths -- the
> pending_data, alloc_thread_groups(), conf->disks, extra_page and
> bioset_init() failures -- jump to "abort:" before that, and abort: calls
> free_conf(). conf comes from kzalloc, so conf->mddev is still NULL and
> free_conf() dereferences it:
>
> BUG: KASAN: null-ptr-deref in raid5_has_ppl drivers/md/raid5-log.h:54 [inline]
> BUG: KASAN: null-ptr-deref in log_exit drivers/md/raid5-log.h:128 [inline]
> BUG: KASAN: null-ptr-deref in free_conf+0x81/0x5d0 drivers/md/raid5.c:7549
> Read of size 8 at addr 0000000000000028 by task syz.3.20/5681
> Call Trace:
> <TASK>
> free_conf+0x81/0x5d0 drivers/md/raid5.c:7549
> setup_conf+0x1720/0x2ad0 drivers/md/raid5.c:7885
> raid5_run+0x8cc/0x2560 drivers/md/raid5.c:8129
> md_run+0xc3d/0x1cd0 drivers/md/md.c:6779
> do_md_run+0x35/0x720 drivers/md/md.c:6880
> array_state_store+0x958/0xe90 drivers/md/md.c:-1
> </TASK>
>
> The faulting address is offsetof(struct mddev, flags).
>
> conf->mddev is a back pointer that is constant for the lifetime of the
> conf and does not depend on anything computed in between, so assign it
> as soon as the conf is allocated. Nothing between the allocation and the
> old assignment reads conf->mddev -- alloc_thread_groups() takes the conf
> but never looks at its mddev, and the rdev_for_each() loop walks the
> mddev argument directly.
>
> All five paths are allocation failures, so this needs memory pressure or
> fault injection to hit, which is how syzbot found it.
>
> Reported-by: syzbot+270624bb31d478afe62e@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=270624bb31d478afe62e
> Fixes: ff875738edd4 ("raid5: separate header for log functions")
> Cc: stable@vger.kernel.org
> Assisted-by: LLM
> Signed-off-by: Yogesh Gaur <yogeshgaur.83@gmail.com>
> ---
> drivers/md/raid5.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
> index b91545ce090d..f98d3bdd3484 100644
> --- a/drivers/md/raid5.c
> +++ b/drivers/md/raid5.c
> @@ -7675,6 +7675,9 @@ static struct r5conf *setup_conf(struct mddev *mddev)
> if (conf == NULL)
> goto abort;
>
> + /* free_conf() dereferences this, so set it before the first goto */
> + conf->mddev = mddev;
> +
> #if PAGE_SIZE != DEFAULT_STRIPE_SIZE
> conf->stripe_size = DEFAULT_STRIPE_SIZE;
> conf->stripe_shift = ilog2(DEFAULT_STRIPE_SIZE) - 9;
> @@ -7743,7 +7746,6 @@ static struct r5conf *setup_conf(struct mddev *mddev)
> ret = bioset_init(&conf->bio_split, BIO_POOL_SIZE, 0, 0);
> if (ret)
> goto abort;
> - conf->mddev = mddev;
>
> ret = -ENOMEM;
> conf->stripe_hashtbl = kzalloc(PAGE_SIZE, GFP_KERNEL);
The same patch is already applied.
https://patch.msgid.link/20260915081731.122933-1-chengzhihao1@huawei.com
--
Thanks,
Kuai
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2026-10-05 17:03 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-10-04 6:02 [PATCH] md/raid5: set conf->mddev before the first setup_conf() error path Yogesh Gaur
2026-10-05 15:53 ` Logan Gunthorpe
2026-10-05 17:02 ` yu kuai
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®