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
| ||
|
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.
Powered by blists - more mailing lists