Linux block layer
 help / color / mirror / Atom feed
From: Niklas Cassel <cassel@kernel.org>
To: Jens Axboe <axboe@kernel.dk>, Shaohua Li <shli@fb.com>,
	Kyungchan Koh <kkc6196@fb.com>
Cc: Damien Le Moal <dlemoal@kernel.org>,
	syzbot+643a6dd130546afdf1fb@syzkaller.appspotmail.com,
	linux-block@vger.kernel.org
Subject: Re: [PATCH] null_blk: serialize configfs attribute updates with device setup
Date: Thu, 13 Aug 2026 17:22:25 +0200	[thread overview]
Message-ID: <an3hMSr9MiQzDC-9@ryzen> (raw)
In-Reply-To: <20260813141456.1625857-2-cassel@kernel.org>

On Thu, Aug 13, 2026 at 04:14:56PM +0200, Niklas Cassel wrote:
> The attribute store methods generated with NULLB_DEVICE_ATTR() refuse to
> change the configuration of a live device by testing
> NULLB_DEV_FL_CONFIGURED, but that flag is only set by
> nullb_device_power_store() after null_add_dev() has returned, and the
> store methods take no lock at all. configfs only serializes writes to
> the same open file (buffer->mutex), so a write to any attribute can run
> concurrently with null_add_dev() and change the device configuration
> while it is being used.
> 
> null_add_dev() reads the configuration several times, e.g. dev->zoned is
> read once to set up the queue limits and once to initialize the zone
> resources:
> 
>   CPU0: echo 1 > nullb0/power         CPU1: echo 1 > nullb0/zoned
>   nullb_device_power_store()
>     mutex_lock(&lock)
>     null_add_dev()
>       if (dev->zoned) -> false
>         /* no BLK_FEAT_ZONED */       nullb_device_zoned_store()
>                                         test_bit(FL_CONFIGURED) -> 0
>                                         dev->zoned = true
>       blk_mq_alloc_disk()
>         /* queue is not zoned */
>       if (nullb->dev->zoned) -> true
>         null_register_zoned_dev()
>           blk_revalidate_disk_zones()
> 
> blk_revalidate_disk_zones() is then called for a queue that does not
> have BLK_FEAT_ZONED set, which triggers its WARN_ON_ONCE() and fails the
> device setup with -EIO:
> 
>   WARNING: CPU: 2 PID: 322 at block/blk-zoned.c:2357 blk_revalidate_disk_zones+0x4c/0x560
> 
> Clearing dev->zoned in the same window is worse: the queue is created
> with BLK_FEAT_ZONED but the zone resources are never initialized, so
> add_disk() succeeds for a zoned disk that has no zones. And a store that
> lands after the last dev->zoned test leaves dev->zoned set while
> dev->zones is still NULL, which null_process_zoned_cmd() dereferences on
> the first write.
> 
> Fix this by taking the global lock, which nullb_device_power_store()
> already holds across null_add_dev() and null_del_dev(), around both the
> NULLB_DEV_FL_CONFIGURED test and the update of the device configuration.
> The submit_queues and poll_queues apply callbacks are now called with
> that lock held, so remove the locking they did themselves.
> 
> Since the store methods can run as soon as configfs_register_subsystem()
> returns, that is, before null_init() gets to mutex_init(&lock), also
> initialize the lock statically with DEFINE_MUTEX().
> 
> Fixes: 3bf2bd20734e ("nullb: add configfs interface")
> Reported-by: syzbot+643a6dd130546afdf1fb@syzkaller.appspotmail.com
> Closes: https://lore.kernel.org/linux-block/6a7d0b3f.ac361c09.22ff0a.004c.GAE@google.com/
> Signed-off-by: Niklas Cassel <cassel@kernel.org>

Patch was verified using reproducer:

$ while :; do echo 1 > /sys/kernel/config/nullb/nullb0/power; echo 0 > /sys/kernel/config/nullb/nullb0/power; done &
$ while :; do echo 1 > /sys/kernel/config/nullb/nullb0/zoned; echo 0 > /sys/kernel/config/nullb/nullb0/zoned; done &


With the fix patch in $subject:
No WARNING after 1 minute of running.

Without the fix patch in $subject:
[ 1120.067545] WARNING: block/blk-zoned.c:2357 at blk_revalidate_disk_zones+0x274/0x2b0 
within 1 second of running.


Kind regards,
Niklas

      reply	other threads:[~2026-08-13 15:22 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-13 14:14 [PATCH] null_blk: serialize configfs attribute updates with device setup Niklas Cassel
2026-08-13 15:22 ` Niklas Cassel [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=an3hMSr9MiQzDC-9@ryzen \
    --to=cassel@kernel.org \
    --cc=axboe@kernel.dk \
    --cc=dlemoal@kernel.org \
    --cc=kkc6196@fb.com \
    --cc=linux-block@vger.kernel.org \
    --cc=shli@fb.com \
    --cc=syzbot+643a6dd130546afdf1fb@syzkaller.appspotmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox