Re: [BUG] general protection fault, probably for non-canonical address 0x280766500040001: 0000 [#1] PREEMPT SMP PTI

[Brian Foster <bfoster@redhat.com>]

On Wed, Jan 17, 2024 at 12:20:55PM +0800, Su Yue wrote:
> 
> On Tue 16 Jan 2024 at 12:33, Kent Overstreet <kent.overstreet@linux.dev>
> wrote:
> 
> > On Tue, Jan 16, 2024 at 12:24:48PM -0500, Brian Foster wrote:
> > > On Tue, Jan 16, 2024 at 12:03:09PM -0500, Kent Overstreet wrote:
> > > > On Tue, Jan 16, 2024 at 10:33:08AM -0500, Brian Foster wrote:
> > > > > Hi Kent,
> > > > >
> > > > > JFYI, I'm seeing the following splat pretty reliably via > >
> > > generic/361 on
> > > > > an 80xcpu test box. The CI doesn't seem to produce this > >
> > > failure for
> > > > > whatever reason. This bisects down to commit 023f9ac9f70f > >
> > > ("bcachefs:
> > > > > Delete dio read alignment check"), before which the test > >
> > > still fails but
> > > > > the kernel doesn't explode.
> > > > >
> > > > > Brian
> > > > >
> > > >
> > > > Can you test the following?
> > > >
> > > 
> > > Still blows up... repeated a couple times to be sure.
> > 
> > That sounds like a driver bug then - what driver?
> 
> 
> I think it's not a drive bug. It's related to bcachefs block_size.
> I can reproduce it by running generic/361 with block_size 4096.
> 
> The test devices are normal qemu disks backing by files in host.
> The bug disappears after hanging mkfs block_size to 512.
> 

Hi Su,

Yes, I think this is the issue as block_bytes(c) is 4k in my test.
Thanks for testing/confirming.

The immediate reason for the crash appears to be bio_copy_data_iter()
going off the rails trying to copy a larger source bio into a smaller
destination bio with seemingly inconsistent size/bvec.

The broader context is we start with a sub-block sized read, so e.g. I
see a request for 1024 bytes at offset 4096. This results in a bio with
bi_size == 4096, however, because right after the alignment check that
was removed we do this:

	ret = min_t(loff_t, iter->count,
		    max_t(loff_t, 0, i_size_read(&inode->v) - offset));
	...
	shorten = iov_iter_count(iter) - round_up(ret, block_bytes(c));
	iter->count -= shorten;

... which appears to underflow the iter count and is presumably fixed up
somewhere to cap at the block size. I'm assuming this ends up in a
situation where an internally inconsistent iov_iter leads to a similarly
broken bvec_iter on the bio, or otherwise this gets further confused
when creating the bounce bio, but I haven't traced to that level of
detail.

In any event, we end up in bio_copy_data_iter() with a destination bio
of bi_size == 4k that only appears to have a single 1k sized bvec. The
first 1k copies as expected, but this doesn't reduce bi_size to zero and
so the iteration just continues to advance the bio vec index until it
finds some garbage data it can infer as a non-zero bv_len bvec to copy
into.

Brian

> --
> Su
>