linux-kernel - Re: [PATCH v3 05/11] readahead: allocate folios with mapping_min

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 for Android: free password hash cracker in your pocket

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

Message-ID: <i4c6xe6jdei2to6kah4kgjehpjlanaqfulju2jzsu5ny2gmegv@2b2oh44oilnj>
Date: Mon, 22 Apr 2024 13:03:45 +0200
From: "Pankaj Raghav (Samsung)" <kernel@...kajraghav.com>
To: Matthew Wilcox <willy@...radead.org>
Cc: linux-xfs@...r.kernel.org, linux-fsdevel@...r.kernel.org, 
	gost.dev@...sung.com, chandan.babu@...cle.com, hare@...e.de, mcgrof@...nel.org, 
	djwong@...nel.org, linux-mm@...ck.org, linux-kernel@...r.kernel.org, 
	david@...morbit.com, akpm@...ux-foundation.org, Pankaj Raghav <p.raghav@...sung.com>
Subject: Re: [PATCH v3 05/11] readahead: allocate folios with
 mapping_min_order in readahead

> > @@ -515,7 +562,7 @@ void page_cache_ra_order(struct readahead_control *ractl,
> >  		if (index & ((1UL << order) - 1))
> >  			order = __ffs(index);
> >  		/* Don't allocate pages past EOF */
> > -		while (index + (1UL << order) - 1 > limit)
> > +		while (order > min_order && index + (1UL << order) - 1 > limit)
> >  			order--;
> 
> This raises an interesting question that I don't know if we have a test
> for.  POSIX says that if we mmap, let's say, the first 16kB of a 10kB
> file, then we can store into offset 0-12287, but stores to offsets
> 12288-16383 get a signal (I forget if it's SEGV or BUS).  Thus far,
> we've declined to even create folios in the page cache that would let us
> create PTEs for offset 12288-16383, so I haven't paid too much attention
> to this.  Now we're going to have folios that extend into that range, so
> we need to be sure that when we mmap(), we only create PTEs that go as
> far as 12287.
> 
> Can you check that we have such an fstest, and that we still pass it
> with your patches applied and a suitably large block size?
> 

So the mmap is giving the correct SIGBUS error when we try to do this:
dd if=/dev/zero of=./test bs=10k count=1; 
xfs_io -c "mmap -w 0 16384" -c "mwrite 13000 10" test

Logs on bs=64k ps=4k system:
root@...ian:/media/test# dd if=/dev/zero of=./test bs=10k count=1;
root@...ian:/media/test# du -sh test 
64K     test
root@...ian:/media/test# ls -l --block-size=k test 
-rw-r--r-- 1 root root 10K Apr 22 10:42 test
root@...ian:/media/test# xfs_io -c "mmap  0 16384" -c "mwrite 13000 10" test
Bus error

The check in filemap_fault takes care of this:

max_idx = DIV_ROUND_UP(i_size_read(inode), PAGE_SIZE);
if (unlikely(index >= max_idx))
        return VM_FAULT_SIGBUS;

The same operation for read should also give a bus error, but it didn't.
Further investigation pointed out that the fault_around() does not take
this condition into account for LBS configuration. When I set fault_around_bytes
to 4096, things worked as expected as we skip fault_around for reads. 

I have a patch that return SIGBUS also for the following read operation:
dd if=/dev/zero of=./test bs=10k count=1; 
xfs_io -c "mmap -r 0 16384" -c "mread 13000 10" test

This is the patch I have for now that fixes fault_around() logic for LBS
configuration:

diff --git a/mm/filemap.c b/mm/filemap.c
index f0c0cfbbd134..259531dd297b 100644
--- a/mm/filemap.c
+++ b/mm/filemap.c
@@ -3600,12 +3600,15 @@ vm_fault_t filemap_map_pages(struct vm_fault *vmf,
        }
        do {
                unsigned long end;
+               unsigned long i_size;
 
                addr += (xas.xa_index - last_pgoff) << PAGE_SHIFT;
                vmf->pte += xas.xa_index - last_pgoff;
                last_pgoff = xas.xa_index;
                end = folio_next_index(folio) - 1;
-               nr_pages = min(end, end_pgoff) - xas.xa_index + 1;
+               i_size = DIV_ROUND_UP(i_size_read(mapping->host),
+                                     PAGE_SIZE) - 1;
+               nr_pages = min3(end, end_pgoff, i_size) - xas.xa_index + 1;
 
                if (!folio_test_large(folio))
                        ret |= filemap_map_order0_folio(vmf,

I will send a new version of the series this week after doing some more
testing.