Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
From:   Zhong Jinghua <zhongjinghua@huaweicloud.com>
To:     axboe@kernel.dk, kay.sievers@vrfy.org
Cc:     linux-block@vger.kernel.org, linux-kernel@vger.kernel.org,
        zhongjinghua@huawei.com, yi.zhang@huawei.com, yukuai3@huawei.com,
        yangerkun@huawei.com
Subject: [PATCH -next] loop: Add parm check in loop_control_ioctl
Date:   Mon,  5 Jun 2023 20:28:38 +0800
Message-Id: <20230605122838.2148878-1-zhongjinghua@huaweicloud.com>
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
Precedence: bulk

From: Zhong Jinghua <zhongjinghua@huawei.com>

We found that in loop_control_ioctl, the kernel panic can be easily caused:

1. syscall(__NR_ioctl, r[1], 0x4c80, 0x80000200000ul);
Create a loop device 0x80000200000ul.
In fact, in the code, it is used as the first_minor number, and the
first_minor number is 0.
So the created loop device number is 7:0.

2. syscall(__NR_ioctl, r[2], 0x4c80, 0ul);
Create a loop device 0x0ul.
Since the 7:0 device has been created in 1, add_disk will fail because
the major and first_minor numbers are consistent.

3. syscall(__NR_ioctl, r[5], 0x4c81, 0ul);
Delete the device that failed to create, the kernel panics.

Panic like below:
BUG: KASAN: null-ptr-deref in device_del+0xb3/0x840 drivers/base/core.c:3107
Call Trace:
 kill_device drivers/base/core.c:3079 [inline]
 device_del+0xb3/0x840 drivers/base/core.c:3107
 del_gendisk+0x463/0x5f0 block/genhd.c:971
 loop_remove drivers/block/loop.c:2190 [inline]
 loop_control_ioctl drivers/block/loop.c:2289 [inline]

The stack like below:
Create loop device:
loop_control_ioctl
  loop_add
    add_disk
      device_add_disk
        bdi_register
          bdi_register_va
            device_create
              device_create_groups_vargs
                device_add
                  kfree(dev->p);
                    dev->p = NULL;

Remove loop device:
loop_control_ioctl
  loop_remove
    del_gendisk
      device_del
        kill_device
          if (dev->p->dead) // p is null

Fix it by adding a check for parm.

Fixes: 770fe30a46a1 ("loop: add management interface for on-demand device allocation")
Signed-off-by: Zhong Jinghua <zhongjinghua@huawei.com>
---
 drivers/block/loop.c | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/drivers/block/loop.c b/drivers/block/loop.c
index 76b96c42f417..60f2a31c4a24 100644
--- a/drivers/block/loop.c
+++ b/drivers/block/loop.c
@@ -2084,6 +2084,17 @@ static int loop_add(struct loop_device **l, int i)
 	struct gendisk *disk;
 	int err;
 
+	/*
+	 * i << part_shift is actually used as the first_minor.
+	 * So here should avoid i << part_shift overflow.
+	 * And, MKDEV() expect that the max bits of
+	 * first_minor is 20.
+	 */
+	if (i > 0 && i > MINORMASK >> part_shift) {
+		err = -EINVAL;
+		goto out;
+	}
+
 	err = -ENOMEM;
 	lo = kzalloc(sizeof(*lo), GFP_KERNEL);
 	if (!lo)
@@ -2097,7 +2108,8 @@ static int loop_add(struct loop_device **l, int i)
 		if (err == -ENOSPC)
 			err = -EEXIST;
 	} else {
-		err = idr_alloc(&loop_index_idr, lo, 0, 0, GFP_KERNEL);
+		err = idr_alloc(&loop_index_idr, lo, 0,
+				(MINORMASK >> part_shift) + 1, GFP_KERNEL);
 	}
 	if (err < 0)
 		goto out_free_dev;
-- 
2.31.1