All of lore.kernel.org
 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.