Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Critical] block: divide-by-zero in bio_discard_limit() when discard_granularity is 0 --
commit c2a5682afd9058576d36c11b303c1f6073d6bfa9 Author: Hungyu Lin <[email protected]> block: fix discard alignment for non-power-of-two granularities This commit replaces round_up() and round_down() in bio_discard_limit() with sector_div() and rounddown() to properly align discard bios when discard_granularity is not a power of two. Link: https://lore.kernel.org/r/CALjSBEvjbiGDVewuapWKhGDtaxZwcn4=22rauibeoyj22ry...@mail.gmail.com > diff --git a/block/blk-lib.c b/block/blk-lib.c > index 688bc67cbf733..e0de5bb4d969e 100644 > --- a/block/blk-lib.c > +++ b/block/blk-lib.c > @@ -13,26 +13,25 @@ > static sector_t bio_discard_limit(struct block_device *bdev, sector_t sector) > { > unsigned int discard_granularity = bdev_discard_granularity(bdev); > - sector_t granularity_aligned_sector; > + unsigned int remainder; > > if (bdev_is_partition(bdev)) > sector += bdev->bd_start_sect; > > - granularity_aligned_sector = > - round_up(sector, discard_granularity >> SECTOR_SHIFT); > + remainder = sector_div(sector, discard_granularity >> SECTOR_SHIFT); > > /* > * Make sure subsequent bios start aligned to the discard granularity if > * it needs to be split. > */ > - if (granularity_aligned_sector != sector) > - return granularity_aligned_sector - sector; > + if (remainder) > + return (discard_granularity >> SECTOR_SHIFT) - remainder; > > /* > * Align the bio size to the discard granularity to make splitting the > bio > * at discard granularity boundaries easier in the driver if needed. > */ > - return round_down(BIO_MAX_SIZE, discard_granularity) >> SECTOR_SHIFT; > + return rounddown(BIO_MAX_SIZE, discard_granularity) >> SECTOR_SHIFT; > } [Severity: Critical] Can this trigger a divide-by-zero when discard_granularity is 0? When a block device does not support discard or has discard disabled via sysfs, blk_validate_limits() in block/blk-settings.c explicitly sets discard_granularity to 0: if (lim->max_discard_sectors) lim->discard_granularity = max(lim->discard_granularity, lim->physical_block_size); else lim->discard_granularity = 0; Since commit 094259204578 ("block: remove the discard_granularity check in __blkdev_issue_discard") and commit 30f1e7241422 ("block: move discard checks into the ioctl handler"), neither blkdev_issue_discard(), __blkdev_issue_discard(), nor blk_alloc_discard_bio() checks bdev_max_discard_sectors(bdev) or bdev_discard_granularity(bdev) before calling bio_discard_limit(bdev, *sector): block/blk-lib.c:blk_alloc_discard_bio() { sector_t bio_sects = min(*nr_sects, bio_discard_limit(bdev, *sector)); ... } Instead, they rely on submit_bio_noacct() -> blk_check_bio() in block/blk-core.c to reject unsupported REQ_OP_DISCARD bios with BLK_STS_NOTSUPP. Several callers can reach blk_alloc_discard_bio() on a device with discard_granularity == 0, such as: - Callers that do not pre-check bdev_max_discard_sectors(), for example hpfs_ioctl() -> hpfs_trim_fs() -> do_trim() -> sb_issue_discard(), nvmet_bdev_execute_discard() -> __blkdev_issue_discard(), fat_free_clusters() / fat_trim_fs() when mounted with -o discard, nilfs_discard_segments(), jfs_issue_discard(), __ext4_remount() with -o remount,discard, f2fs_secure_erase(), Xen dispatch_discard_io(), r5l_write_super_and_discard_space(), and fd_execute_unmap(). - Any discard caller (such as blk_ioctl_discard(), blkdev_cmd_discard(), or active filesystem/swap discard) racing with runtime discard disablement when 0 is written to discard_max_bytes via queue_max_discard_sectors_store(). Previously, round_up(sector, 0) and round_down(BIO_MAX_SIZE, 0) used bitwise masks and did not trap. With sector_div() and rounddown(), a zero discard_granularity divides by zero. Should discard_granularity be guarded here (similar to max(lim->discard_granularity >> 9, 1U) in __bio_split_discard()), or should bio_discard_limit() / blk_alloc_discard_bio() handle discard_granularity == 0 before dividing? -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=1
