linux-kernel - Re: [RFC V2] block: reject I/O for same fd if block size changed

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <BYAPR04MB49659FFF1A820C9E7BBC1D0386D20@BYAPR04MB4965.namprd04.prod.outlook.com>
Date:   Mon, 4 Jan 2021 00:57:07 +0000
From:   Chaitanya Kulkarni <Chaitanya.Kulkarni@....com>
To:     Minwoo Im <minwoo.im.dev@...il.com>,
        "linux-block@...r.kernel.org" <linux-block@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "linux-fsdevel@...r.kernel.org" <linux-fsdevel@...r.kernel.org>
CC:     Jens Axboe <axboe@...nel.dk>,
        Alexander Viro <viro@...iv.linux.org.uk>,
        Christoph Hellwig <hch@....de>
Subject: Re: [RFC V2] block: reject I/O for same fd if block size changed

On 12/30/20 08:03, Minwoo Im wrote:
> Let's say, for example of NVMe device, Format command to change out
> LBA format to another logical block size and BLKRRPART to re-read
> partition information with a same file descriptor like:
>
> 	fd = open("/dev/nvme0n1", O_RDONLY);
>
> 	nvme_format(fd, ...);
> 	if (ioctl(fd, BLKRRPART) < 0)
> 		..
>
> In this case, ioctl causes invalid Read operations which are triggered
> by buffer_head I/O path to re-read partition information.  This is
> because it's still playing around with i_blksize and i_blkbits.  So,
> 512 -> 4096 -> 512 logical block size changes will cause an under-flowed
> length of Read operations.
>
> Case for NVMe:
>   (LBAF 1 512B, LBAF 0 4K logical block size)
>
>   nvme format /dev/nvme0n1 --lbaf=1 --force  # to 512B LBA
>   nvme format /dev/nvme0n1 --lbaf=0 --force  # to 4096B LBA
>
> [dmesg-snip]
>   [   10.771740] blk_update_request: operation not supported error, dev nvme0n1, sector 0 op 0x0:(READ) flags 0x0 phys_seg 1 prio class 0
>   [   10.780262] Buffer I/O error on dev nvme0n1, logical block 0, async page read
>
> [event-snip]
>   kworker/0:1H-56      [000] ....   913.456922: nvme_setup_cmd: nvme0: disk=nvme0n1, qid=1, cmdid=216, nsid=1, flags=0x0, meta=0x0, cmd=(nvme_cmd_read slba=0, len=65535, ctrl=0x0, dsmgmt=0, reftag=0)
>    ksoftirqd/0-9       [000] .Ns.   916.566351: nvme_complete_rq: nvme0: disk=nvme0n1, qid=1, cmdid=216, res=0x0, retries=0, flags=0x0, status=0x4002
>
> As the previous discussion [1], this patch introduced a gendisk flag
> to indicate that block size has been changed in the runtime.  This flag
> is set when logical block size is changed in the runtime in the block
> layer.  It will be cleared when the file descriptor for the
> block devie is opened again through __blkdev_get() which updates the block
> size via set_init_blocksize().
>
> This patch rejects I/O from the path of add_partitions() to avoid
> issuing invalid Read operations to device.  It also sets a flag to
> gendisk in blk_queue_logical_block_size to minimize caller-side updates.
>
> [1] https://lore.kernel.org/linux-nvme/20201223183143.GB13354@localhost.localdomain/T/#t
>
> Signed-off-by: Minwoo Im <minwoo.im.dev@...il.com>
Rewrite the change-log similar to what we have in the repo and fix
the spelling mistakes. Add a cover-letter to explain the testcase
and the execution effect, also I'd move discussion link into
cover-letter.