linux-kernel - Re: [GIT PULL] 9p fixes for 6.12-rc4

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] [day] [month] [year] [list]
Message-ID: <CAEf4BzaAjpcGfFahFcYavBtiKJC6LHf55Q_y6i5MDfCWkU-mZQ@mail.gmail.com>
Date: Wed, 23 Oct 2024 16:37:37 -0700
From: Andrii Nakryiko <andrii.nakryiko@...il.com>
To: Dominique Martinet <asmadeus@...ewreck.org>
Cc: David Howells <dhowells@...hat.com>, Matthew Wilcox <willy@...radead.org>, 
	Andrii Nakryiko <andrii@...nel.org>, ericvh@...nel.org, linux-kernel@...r.kernel.org, 
	linux_oss@...debyte.com, pedro.falcato@...il.com, regressions@...mhuis.info, 
	torvalds@...ux-foundation.org, v9fs@...ts.linux.dev, bpf@...r.kernel.org
Subject: Re: [GIT PULL] 9p fixes for 6.12-rc4

On Wed, Oct 23, 2024 at 4:24 PM Dominique Martinet
<asmadeus@...ewreck.org> wrote:
>
> Adding David/Willy to recpients as I'm not 100% up to date on folios
>
> Andrii Nakryiko wrote on Wed, Oct 23, 2024 at 09:56:06AM -0700:
> > > The following changes since commit 98f7e32f20d28ec452afb208f9cffc08448a2652:
> > >
> > >   Linux 6.11 (2024-09-15 16:57:56 +0200)
> > >
> > > are available in the Git repository at:
> > >
> > >   https://github.com/martinetd/linux tags/9p-for-6.12-rc4
> > >
> > > for you to fetch changes up to 79efebae4afc2221fa814c3cae001bede66ab259:
> > >
> > >   9p: Avoid creating multiple slab caches with the same name (2024-09-23 05:51:27 +0900)
> > >
> > > ----------------------------------------------------------------
> > > Mashed-up update that I sat on too long:
> > >
> > > - fix for multiple slabs created with the same name
> > > - enable multipage folios
> > > - theorical fix to also look for opened fids by inode if none
> > > was found by dentry
> > >
> > > ----------------------------------------------------------------
> > > David Howells (1):
> > >      9p: Enable multipage folios
> >
> > Are there any known implications of this change on madvise()'s MADV_PAGEOUT
> > behavior? After most recent pull from Linus's tree, one of BPF selftests
> > started failing. Bisection points to:
> >
> >   9197b73fd7bb ("Merge tag '9p-for-6.12-rc4' of https://github.com/martinetd/linux")
> >
> > ... which is just an empty merge commit. So the "9p: Enable multipage folios"
> > by itself doesn't cause any regression, but when merged with the rest of the
> > code it does. I confirmed by reverting
> > 1325e4a91a40 ("9p: Enable multipage folios"), after which the test in question
> > is succeeding again.
>
> (looks like 3c217a182018 ("selftests/bpf: add build ID tests") wasn't in
> yet on the 9p multipage folios commit)
>
> > The test in question itself is a bit involved, but what it ultimately tries to
> > do is to ensure that part of ELF file containing build ID is paged out to cause
> > BPF helper to fail to retrieve said build ID (due to non-faulable context).
> >
> > For that, we use the following sequence in target binary and process:
> >
> > madvise(addr, page_sz, MADV_POPULATE_READ);
> > madvise(addr, page_sz, MADV_PAGEOUT);
> >
> > First making sure page is paged in, then paged out. We make sure that build ID
> > is memory mapped in a separate segment with its own single-page memory mapping.
> > No changes or regressions there. No huge pages seem to be involved.
>
> That's probably obvious but I guess the selftest runs the binary
> directly from a 9p mount?

Yep, should have pointed that out explicitly.

>
> > It used to work reliably, now it doesn't work. Any clue why or what should we
> > do differently to make sure that memory page with build ID information is not
> > paged in (reliably)?
>
> Unless David/Willy has a solution immediately I'd say let's take the time to
> sort this out and revert that commit for now -- I'll send a revert patch
> immediately and submit it to Linus on Saturday.
>
> Conceptually I guess something is broken with MADV_PAGEOUT on >1 page
> folio, perhaps it's only evicting folios if the whole folio is in range
> but it should evict any folio that touches the range or something?

Could be, yeah. It's not necessarily a bug of 9P itself, but it would
be nice to have some way to page out memory. Maybe we need some extra
flags or a new MADV_PAGEOUT_OVERLAPPING command for madvise(), or
something along those lines?

>
> Sorry I don't have time to dig further here, hopefully that's not too
> difficult to handle and we can try again in rc1 proper of another cycle,
> I shouldn't have sent that this late.
>

No worries, thanks for a quick reply!

>
> (leaving full text below for new recipients)
> > Thanks!
> >
> > P.S. The target binary and madvise() manipulations are at:
> >
> >   tools/testing/selftests/bpf/uprobe_multi.c, see trigger_uprobe()
> > The test itself in BPF selftest is at:
> >
> >   tools/testing/selftests/bpf/prog_tests/build_id.c, see subtest_nofault(),
> >   build_id_resident is false in this case.
> >
> > >
> > > Dominique Martinet (1):
> > >       9p: v9fs_fid_find: also lookup by inode if not found dentry
> > >
> > > Pedro Falcato (1):
> > >       9p: Avoid creating multiple slab caches with the same name
> > >
> > >  fs/9p/fid.c       |  5 ++---
> > >  fs/9p/vfs_inode.c |  1 +
> > >  net/9p/client.c   | 10 +++++++++-
> > >  3 files changed, 12 insertions(+), 4 deletions(-)
> > >
> >
>
> Thanks,
> --
> Dominique Martinet | Asmadeus