Re: [PATCH -next] block: Fix the partition start may overflow in add_partition()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi,

在 2023/05/25 10:14, Yu Kuai 写道:
Hi,

在 2023/05/22 15:06, Zhong Jinghua 写道:
From: Zhong Jinghua <zhongjinghua@xxxxxxxxxx>

In the block_ioctl, we can pass in the unsigned number 0x8000000000000000
as an input parameter, like below:

block_ioctl
   blkdev_ioctl
     blkpg_ioctl
       blkpg_do_ioctl
         copy_from_user
         bdev_add_partition
           add_partition
             p->start_sect = start; // start = 0x8000000000000000

start_sect is 0x8..... >> SECTOR_SHIFT.


Then, there was an warning when submit bio:

WARNING: CPU: 0 PID: 382 at fs/iomap/apply.c:54
Call trace:
  iomap_apply+0x644/0x6e0
  __iomap_dio_rw+0x5cc/0xa24
  iomap_dio_rw+0x4c/0xcc
  ext4_dio_read_iter
  ext4_file_read_iter
  ext4_file_read_iter+0x318/0x39c
  call_read_iter
  lo_rw_aio.isra.0+0x748/0x75c
  do_req_filebacked+0x2d4/0x370
  loop_handle_cmd
  loop_queue_work+0x94/0x23c
  kthread_worker_fn+0x160/0x6bc
  loop_kthread_worker_fn+0x3c/0x50
  kthread+0x20c/0x25c
  ret_from_fork+0x10/0x18

Stack:

submit_bio_noacct
   submit_bio_checks
     blk_partition_remap
       bio->bi_iter.bi_sector += p->start_sect
       // bio->bi_iter.bi_sector = 0xffc0000000000000 + 65408
..
loop_queue_work
  loop_handle_cmd
   do_req_filebacked
    pos = ((loff_t) blk_rq_pos(rq) << 9) + lo->lo_offset // pos < 0
    lo_rw_aio
      call_read_iter
       ext4_dio_read_iter
        __iomap_dio_rw
         iomap_apply
          ext4_iomap_begin
            map.m_lblk = offset >> blkbits
              ext4_set_iomap
              iomap->offset = (u64) map->m_lblk << blkbits
              // iomap->offset = 64512
          WARN_ON(iomap.offset > pos) // iomap.offset = 64512 and pos < 0

This is wrong, and please update above stack, it seems they're not based
on latest kernel.

Thanks,
Kuai

This is unreasonable for start + length > disk->part0.nr_sects. There is
already a similar check in blk_add_partition().
Fix it by adding a check in bdev_add_partition().

The checking that you add is blkpg_do_ioctl().


Reported-by: Zhihao Cheng <chengzhihao1@xxxxxxxxxx>
Signed-off-by: Zhong Jinghua <zhongjinghua@xxxxxxxxxx>
---
  block/ioctl.c | 7 +++++++
  1 file changed, 7 insertions(+)

diff --git a/block/ioctl.c b/block/ioctl.c
index 9c5f637ff153..3223ea862523 100644
--- a/block/ioctl.c
+++ b/block/ioctl.c
@@ -33,9 +33,16 @@ static int blkpg_do_ioctl(struct block_device *bdev,
      if (op == BLKPG_DEL_PARTITION)
          return bdev_del_partition(disk, p.pno);
+    if (p.start < 0 || p.length <= 0 || p.start + p.length < 0)
+        return -EINVAL;
+
      start = p.start >> SECTOR_SHIFT;
      length = p.length >> SECTOR_SHIFT;
+    /* length may be equal to 0 after right shift */
+    if (!length || start + length > get_capacity(bdev->bd_disk))
+        return -EINVAL;
+
      switch (op) {
      case BLKPG_ADD_PARTITION:
          /* check if partition is aligned to blocksize */


The change itself looks good to me, feel free to add:

Reviewed-by: Yu Kuai <yukuai3@xxxxxxxxxx>

.





[Index of Archives]     [Linux RAID]     [Linux SCSI]     [Linux ATA RAID]     [IDE]     [Linux Wireless]     [Linux Kernel]     [ATH6KL]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Device Mapper]

  Powered by Linux