[PATCH] iomap: enforce DIO alignment check in iomap]

[PATCH] iomap: enforce DIO alignment check in iomap]

[Carlos Maiolino]

Sorry, I screwed up two addresses, bouncing them to the right addresses
to avoid spamming the lists again.

----- Forwarded message from cem@kernel.org -----

Date: Wed, 10 Jun 2026 16:52:11 +0200
From: cem@kernel.org
To: brauner@vger.kernel.org
Cc: linux-block@vger.kernel.devel, linux-fsdevel@vger.kernel.org,
  linux-ext4@vger.kernel.org, linux-xfs@vger.kernel.org, Carlos Maiolino
 <cem@kernel.org>,  Keith Busch <kbusch@kernel.org>, Hannes Reinecke
 <hare@suse.de>,  "Martin K. Petersen" <martin.petersen@oracle.com>,
 Christoph Hellwig <hch@lst.de>, Jens Axboe <axboe@kernel.dk>
Subject: [PATCH] iomap: enforce DIO alignment check in iomap
Message-ID: <20260610145218.141369-1-cem@kernel.org>
X-Mailer: git-send-email 2.54.0
Tags: sent

From: Carlos Maiolino <cem@kernel.org>

The DIO alignment check has been lifted from iomap layer to rely on the
block layer to enforce proper alignment when issuing direct IO
operations. This though, depending on the IO size and buffer address
passed to the IO operation may lead to user-visible behavior change.

This has been caught initially by LTP test diotest4 running on
PPC architecture, where the test fails because a read() operation
with a supposedly misaligned buffer succeeds instead of an expected
-EINVAL.
This has no direct relationship with PPC, but seems to do with the
IO size crossing page borders or not.

The test allocates a 4k buffer, and then increments the buffer pointer
by a single byte to enforce a misaligned address. It then issues a 4k
read() using such buffer. The read is supposed to return an -EINVAL but
it ends up succeeding.

The allocated buffer is at least a single page, so the read() size being
smaller will end up most of the time within the very same page initially
allocated which seems to suffice the block layer to accept the IO.

On x86 though, the same 4k read will end up crossing page boundaries
causing a bio_split which ends up properly checking the address and
rejecting it due to misalignment.
The test itself is buggy (which seems by design) because it ends up
attempting to read 4096 bytes into a 4095, but I believe the test
expected the address to be rejected prior to any write attempt.

The problematic behavior is reproducible on x86 by reducing the IO size
to something < PAGE_SIZE, so the misaligned read()s will also be accepted
by the block layer.

Fixing this is just a matter of enforcing daddr and memory
alignment back into iomap.

This behavior is reproducible in ext4 and xfs due to both relying on
iomap layer, btrfs does not present this behavior change as it does its
own DIO alignment checking.

Fixes: 7eac33186957 ("iomap: simplify direct io validity check")
Cc: Keith Busch <kbusch@kernel.org>
Cc: Hannes Reinecke <hare@suse.de>
Cc: Martin K. Petersen <martin.petersen@oracle.com>
Cc: Christoph Hellwig <hch@lst.de>
Cc: Jens Axboe <axboe@kernel.dk>
Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>
Signed-off-by: Carlos Maiolino <cem@kernel.org>
---

While I didn't spot any memory/disk corruption looking into this, it
changes the user behavior that dictates buffer addresses must be
properly aligned when issuing direct IO operations so I thought making
iomap check again for the buffer address alignment is reasonable.

 fs/iomap/direct-io.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/fs/iomap/direct-io.c b/fs/iomap/direct-io.c
index 95254aa1b654..0064984e64e5 100644
--- a/fs/iomap/direct-io.c
+++ b/fs/iomap/direct-io.c
@@ -400,6 +400,9 @@ static int iomap_dio_bio_iter(struct iomap_iter *iter, struct iomap_dio *dio)
 	if ((pos | length) & (alignment - 1))
 		return -EINVAL;
 
+	if (iov_iter_alignment(dio->submit.iter) & (alignment - 1))
+		return -EINVAL;
+
 	if (dio->flags & IOMAP_DIO_WRITE) {
 		bool need_completion_work = true;
 
-- 
2.54.0



----- End forwarded message -----

Re: [PATCH] iomap: enforce DIO alignment check in iomap]

[Keith Busch]

On Wed, Jun 10, 2026 at 05:27:42PM +0200, Carlos Maiolino wrote:
> The DIO alignment check has been lifted from iomap layer to rely on the
> block layer to enforce proper alignment when issuing direct IO
> operations. This though, depending on the IO size and buffer address
> passed to the IO operation may lead to user-visible behavior change.
> 
> This has been caught initially by LTP test diotest4 running on
> PPC architecture, where the test fails because a read() operation
> with a supposedly misaligned buffer succeeds instead of an expected
> -EINVAL.

It's not supposed to matter where in the stack we determined this be an
invalid request: it should still fail if it's misaligned. Could you
clarify how this is succeeding?

Re: [PATCH] iomap: enforce DIO alignment check in iomap]

[Carlos Maiolino]

On Wed, Jun 10, 2026 at 09:37:17AM -0600, Keith Busch wrote:
> On Wed, Jun 10, 2026 at 05:27:42PM +0200, Carlos Maiolino wrote:
> > The DIO alignment check has been lifted from iomap layer to rely on the
> > block layer to enforce proper alignment when issuing direct IO
> > operations. This though, depending on the IO size and buffer address
> > passed to the IO operation may lead to user-visible behavior change.
> > 
> > This has been caught initially by LTP test diotest4 running on
> > PPC architecture, where the test fails because a read() operation
> > with a supposedly misaligned buffer succeeds instead of an expected
> > -EINVAL.
> 
> It's not supposed to matter where in the stack we determined this be an
> invalid request: it should still fail if it's misaligned. Could you
> clarify how this is succeeding?

Fair enough, can you point me to where the alignment is supposed to be
checked? I've been seeing kind of different behaviors with different
machines so I kind of am not sure where the alignment is supposed to be
validated within the block layer (I should probably have tagged this
patch as RFC as my understanding of the block layer is superficial).

A few runs when the test failed (because the read() call succeded) I
could see this for example:

openat(dirfd=AT_FDCWD, pathname="testdata-4.2063", flags=O_RDWR|O_DIRECT) = 3
_llseek(fd=3, offset=4096, result=[4096], whence=SEEK_SET) = 0
read(arg1=0x3, arg2=0x25960001, arg3=0x1000) = 0x1000

FWIW, on my laptop, the test fails because read() returns an -EIO (while
the test expects -EINVAL), I'm pointing the fingers to a faulty hardware
now but I didn't discard the possibility of an EIO being returned EINVAL
should have been returned instead.
I'm trying to find another machine to test and see whatever differences
appear...

Cheers

Re: [PATCH] iomap: enforce DIO alignment check in iomap]

[Keith Busch]

On Wed, Jun 10, 2026 at 06:58:13PM +0200, Carlos Maiolino wrote:
> On Wed, Jun 10, 2026 at 09:37:17AM -0600, Keith Busch wrote:
> > On Wed, Jun 10, 2026 at 05:27:42PM +0200, Carlos Maiolino wrote:
> > > The DIO alignment check has been lifted from iomap layer to rely on the
> > > block layer to enforce proper alignment when issuing direct IO
> > > operations. This though, depending on the IO size and buffer address
> > > passed to the IO operation may lead to user-visible behavior change.
> > > 
> > > This has been caught initially by LTP test diotest4 running on
> > > PPC architecture, where the test fails because a read() operation
> > > with a supposedly misaligned buffer succeeds instead of an expected
> > > -EINVAL.
> > 
> > It's not supposed to matter where in the stack we determined this be an
> > invalid request: it should still fail if it's misaligned. Could you
> > clarify how this is succeeding?
> 
> Fair enough, can you point me to where the alignment is supposed to be
> checked? 

https://elixir.bootlin.com/linux/v7.1-rc7/source/block/blk-merge.c#L352

It does require that someone calls the bio split-to-limits routine,
which I had taken for granted as a given, but I realize that some
drivers don't do that. What block device are you using for your test?

Re: [PATCH] iomap: enforce DIO alignment check in iomap]

[Carlos Maiolino]

On Wed, Jun 10, 2026 at 11:14:30AM -0600, Keith Busch wrote:
> On Wed, Jun 10, 2026 at 06:58:13PM +0200, Carlos Maiolino wrote:
> > On Wed, Jun 10, 2026 at 09:37:17AM -0600, Keith Busch wrote:
> > > On Wed, Jun 10, 2026 at 05:27:42PM +0200, Carlos Maiolino wrote:
> > > > The DIO alignment check has been lifted from iomap layer to rely on the
> > > > block layer to enforce proper alignment when issuing direct IO
> > > > operations. This though, depending on the IO size and buffer address
> > > > passed to the IO operation may lead to user-visible behavior change.
> > > > 
> > > > This has been caught initially by LTP test diotest4 running on
> > > > PPC architecture, where the test fails because a read() operation
> > > > with a supposedly misaligned buffer succeeds instead of an expected
> > > > -EINVAL.
> > > 
> > > It's not supposed to matter where in the stack we determined this be an
> > > invalid request: it should still fail if it's misaligned. Could you
> > > clarify how this is succeeding?
> > 
> > Fair enough, can you point me to where the alignment is supposed to be
> > checked? 
> 
> https://elixir.bootlin.com/linux/v7.1-rc7/source/block/blk-merge.c#L352
> 
> It does require that someone calls the bio split-to-limits routine,
> which I had taken for granted as a given, but I realize that some
> drivers don't do that. What block device are you using for your test?

In the PPC machine, it's a virtual scsi vdasd device from one of the
virtual nodes

NAME HCTL       TYPE VENDOR   MODEL  REV SERIAL                             TRAN
sda  0:0:1:0    disk AIX      VDASD 0001 000a508a00007a0000000175dcba35ac.5

ibmvfc                262144  0
ibmvscsi              196608  2

For my x86 machine (remind I reduce the buffer size to 512 on x86), it's
a commodity sata samsung SSD:

sdb  6:0:0:0    disk ATA      Samsung SSD 860 EVO 1TB RVT02B6Q S3Z9NB0KB82416J   sata

Both with an ext4 straight on top of a partition (no device-mapper or
any other volume layer in between), although I also tried with a
linear device-mapper (lvm) device and the results seemed the same.

Those are two machines I have had more reliable results. My laptop with
a samsung NVME makes read() return an -EIO but I can't tell if it's a
device failure or just a wrong error being returned by now