Re: [PATCH 4/7] block: Introduce a new ioctl for simple copy

On Tue, Aug 17, 2021 at 6:40 PM Greg KH [off-list ref] wrote:

On Tue, Aug 17, 2021 at 03:44:20PM +0530, SelvaKumar S wrote:

quoted

From: Nitesh Shetty <redacted>

Add new BLKCOPY ioctl that offloads copying of one or more sources ranges
to a destination in the device. COPY ioctl accepts a 'copy_range'
structure that contains destination (in sectors), no of sources and
pointer to the array of source ranges. Each source range is represented by
'range_entry' that contains start and length of source ranges (in sectors)

MAX_COPY_NR_RANGE, limits the number of entries for the IOCTL and
MAX_COPY_TOTAL_LENGTH limits the total copy length, IOCTL can handle.

Example code, to issue BLKCOPY:
/* Sample example to copy three source-ranges [0, 8] [16, 8] [32,8] to
 * [64,24], on the same device */

int main(void)
{
      int ret, fd;
      struct range_entry source_range[] = {{.src = 0, .len = 8},
              {.src = 16, .len = 8}, {.src = 32, .len = 8},};
      struct copy_range cr;

      cr.dest = 64;
      cr.nr_range = 3;
      cr.range_list = (__u64)&source_range;

      fd = open("/dev/nvme0n1", O_RDWR);
      if (fd < 0) return 1;

      ret = ioctl(fd, BLKCOPY, &cr);
      if (ret < 0) printf("copy failure\n");

      close(fd);

      return ret;
}

Signed-off-by: Nitesh Shetty <redacted>
Signed-off-by: SelvaKumar S <redacted>
Signed-off-by: Kanchan Joshi <redacted>
---
 block/ioctl.c           | 33 +++++++++++++++++++++++++++++++++
 include/uapi/linux/fs.h |  8 ++++++++
 2 files changed, 41 insertions(+)

diff --git a/block/ioctl.c b/block/ioctl.c
index eb0491e90b9a..2af56d01e9fe 100644
--- a/block/ioctl.c
+++ b/block/ioctl.c

@@ -143,6 +143,37 @@ static int blk_ioctl_discard(struct block_device *bdev, fmode_t mode,
                                  GFP_KERNEL, flags);
 }

+static int blk_ioctl_copy(struct block_device *bdev, fmode_t mode,
+             unsigned long arg)
+{
+     struct copy_range crange;
+     struct range_entry *rlist;
+     int ret;
+
+     if (!(mode & FMODE_WRITE))
+             return -EBADF;
+
+     if (copy_from_user(&crange, (void __user *)arg, sizeof(crange)))
+             return -EFAULT;
+
+     rlist = kmalloc_array(crange.nr_range, sizeof(*rlist),
+                     GFP_KERNEL);

No error checking for huge values of nr_range?  Is that wise?  You
really want userspace to be able to allocate "all" of the kernel memory
in the system?

thanks,

greg k-h

We added a kernel imposed limit MAX_COPY_NR_RANGE for that purpose,
but missed adding the check here.
Will have that fixed. Thanks for pointing this out.

Nitesh Shetty