linux-kernel - Re: [PATCH] iov_iter: Use iov_offset for length calculation in iov_iter_aligned

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAOSviJ1d3=NzwGb6kKfD5zucdbrOQfyO0my_b0wr-NXuQzFGKg@mail.gmail.com>
Date: Sat, 26 Apr 2025 01:25:14 +0530
From: Nitesh Shetty <nitheshshetty@...il.com>
To: Andrew Morton <akpm@...ux-foundation.org>
Cc: Nitesh Shetty <nj.shetty@...sung.com>, Alexander Viro <viro@...iv.linux.org.uk>, axboe@...nel.dk, 
	gost.dev@...sung.com, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] iov_iter: Use iov_offset for length calculation in iov_iter_aligned_bvec

On Thu, Apr 17, 2025 at 5:25 AM Andrew Morton <akpm@...ux-foundation.org> wrote:
>
> On Tue, 15 Apr 2025 23:44:19 +0530 Nitesh Shetty <nj.shetty@...sung.com> wrote:
>
> > If iov_offset is non-zero, then we need to consider iov_offset in length
> > calculation, otherwise we might pass smaller IOs such as 512 bytes
> > with 256 bytes offset.
> >
>
> Please describe the userspace-visible effects of this flaw, if any?
>
a.
At present we see different behaviour based on whether the device is
512 byte LBA formatted or 4K byte LBA formatted.

Here is the scenario, where this case is possible, when using io-uring
fixed buffer IO with 512 LBA device we can get,
i->count = 512, i->iov_offset = 3584, bvec->bv_offset = 256,
bvec->bv_len = 3840, len_mask = 511

In this effectively, the first 256 bytes are in the current page,
next 256 bytes are in the second page.
This should fail the IO, but here we pass.
(For a similar setting with i->count=4096, this check fails).

To reproduce the test from user space, we can use a liburing application [1]
with patch[2], on a 512 LBA format block device.

b.
I went through file history and looks like this is a regression,
previously the fix was present, but commit
2263639f96f24a121ec9f037981b81daf5a8d60a introduced this issue it seems.

Thanks,
Nitesh Shetty

[1] https://github.com/axboe/liburing/blob/master/test/fixed-seg.c
[2]
diff --git a/test/fixed-seg.c b/test/fixed-seg.c
index 4154abb..e7c461a 100644
--- a/test/fixed-seg.c
+++ b/test/fixed-seg.c
@@ -64,7 +64,7 @@ static int test(struct io_uring *ring, int fd, int vec_off)
                return T_EXIT_FAIL;
        }

-       ret = read_it(ring, fd, 4096, vec_off);
+       ret = read_it(ring, fd, 512, vec_off + 512*7);
        if (ret) {
                fprintf(stderr, "4096 0 failed\n");
                return T_EXIT_FAIL;
@@ -158,7 +158,7 @@ int main(int argc, char *argv[])
                goto err;
        }

-       ret = test(&ring, fd, 0);
+       ret = test(&ring, fd, 256);
        if (ret) {
                fprintf(stderr, "test 0 failed\n");
                goto err;