Linux PCI subsystem development
 help / color / mirror / Atom feed
From: "Lorenzo Stoakes (ARM)" <ljs@kernel.org>
To: "Krzysztof Wilczyński" <kwilczynski@kernel.org>
Cc: Andrew Morton <akpm@linux-foundation.org>,
	 David Hildenbrand <david@kernel.org>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	 Tejun Heo <tj@kernel.org>, Bjorn Helgaas <bhelgaas@google.com>,
	 Bjorn Helgaas <helgaas@kernel.org>,
	Manivannan Sadhasivam <mani@kernel.org>,
	 Lorenzo Pieralisi <lpieralisi@kernel.org>,
	"Liam R . Howlett" <liam@infradead.org>,
	 Baoquan He <baoquan.he@linux.dev>,
	Pratyush Yadav <pratyush@kernel.org>,
	 Pasha Tatashin <pasha.tatashin@soleen.com>,
	Jaroslav Kysela <perex@perex.cz>, Takashi Iwai <tiwai@suse.com>,
	 Michal Hocko <mhocko@suse.com>, Mike Rapoport <rppt@kernel.org>,
	 Simona Vetter <simona.vetter@ffwll.ch>,
	Suren Baghdasaryan <surenb@google.com>,
	 Vlastimil Babka <vbabka@kernel.org>,
	Dave Young <ruirui.yang@linux.dev>,
	linux-mm@kvack.org,  linux-pci@vger.kernel.org,
	linux-sound@vger.kernel.org, kexec@lists.infradead.org,
	 driver-core@lists.linux.dev
Subject: Re: [PATCH v2 0/3] mm,kernfs,proc: Unmap mmaps of removed files via file->f_mapping
Date: Thu, 30 Jul 2026 09:24:00 +0100	[thread overview]
Message-ID: <amsJ_T3qTJ-TJcwk@lucifer> (raw)
In-Reply-To: <20260726001049.GA2219014@rocinante>

On Sun, Jul 26, 2026 at 10:22:52AM +0900, Krzysztof Wilczyński wrote:
> Hello,
>
> > > The PCI resource files in sysfs and the /proc/bus/pci device files swap
> > > their f_mapping to the shared iomem address space at open time, so that
> > > revoke_iomem() can unmap userspace mappings when a driver claims a
> > > region, see commit 636b21b50152 ("PCI: Revoke mappings like devmem").
> > >
> > > Their VMAs are therefore attached to the shared address space, which
> > > neither removal path reaches: kernfs_drain_open_files() unmaps the
> > > sysfs inode's own mapping, which contains none of them, and
> > > proc_entry_rundown() does not unmap anything at all.
> > >
> > > As a result, userspace mappings of PCI BARs survive device removal,
> > > and also survive a BAR resize on the sysfs side, keeping stale PTEs
> > > into physical address space that the kernel may have reassigned since.
> > > A mapping made before the device is removed still returns the previous
> > > register value after the device has been released, through both
> > > interfaces.
> >
> > Thanks.  Can we please have full description of the userspace-visible
> > effects of this?
>
> Definitely.  From the PCI side, to put this into perspective:
>
> Without this series, a process that maps a BAR keeps a fully working
> mapping after the device is removed or its VFs torn down.  The same
> applies when a BAR is resized, as the resize removes and recreates the
> resource groups.  Nothing faults, so the process cannot tell the device
> has gone.  A read returns the register contents as they were, because
> removal clears Bus Master but leaves Memory Space Enable set, so the
> device still decodes its BARs, and, as these attributes are writable,
> a write is still accepted and reaches it.  Those addresses are released
> on removal and can be assigned to another device, which a BAR resize
> does explicitly, and the mapping then reaches whatever occupies them.
>
> On the sysfs side this most likely has been a regression as the
> kernfs_drain_open_files() unmapped these mappings correctly until
> commit 636b21b50152 ("PCI: Revoke mappings like devmem") swapped
> f_mapping to the shared iomem address space, after which the drain
> walked an interval tree the VMAs were no longer on.  The procfs side
> never unmapped anything, so there the behaviour is new.
>
> A read here returns another device's register contents, and as read to
> clear registers are common, it can also acknowledge an interrupt or clear
> an error the owning driver has not seen.  A write can be worse, as an
> offset that was a control register on the old device may be a doorbell,
> a reset bit or a DMA descriptor address on the new one.
>
> With this series the mapping is torn down as part of the removal, and
> the next access raises SIGBUS, so userspace gets an error instead of
> silently reading stale or unrelated registers.  Both interfaces behave
> the same way afterwards.
>
> Tested on 7.2-rc1 with and without the series applied.  In each case
> the file is mmap'd, the device is removed while the mapping is held,
> and the mapping is then accessed:
>
>   - sysfs resource read after removal:
>        before: returns 0x18140241, the value read before the removal
>        after:  SIGBUS
>
>    - sysfs resource written after removal:
>        before: the write is accepted and reads back 0x00000000
>        after:  SIGBUS on the write
>
>    - sysfs resource mmap, fd closed, then read after removal:
>        before: returns 0x18140241
>        after:  SIGBUS
>
>    - sysfs resource mmap, moved with mremap and split with munmap, then forked, both read after removal:
>        before: parent and child both return 0x18140241
>        after:  SIGBUS in both
>
>    - two sysfs resources of different devices mmap, one device removed, both read:
>        before: neither is unmapped, both return their values
>        after:  the removed device raises SIGBUS, the other still returns 0x48140240
>
>    - /proc/bus/pci used to mmap resource, read after removal:
>        before: returns 0x18140241
>        after:  SIGBUS
>
> I hope this helps!
>
> > AI review might have found a few things:
> > 	https://sashiko.dev/#/patchset/20260725210549.3716546-1-kwilczynski@kernel.org
>
> I have seen the reviews.  Will reply to each.
>
> Thank you!
>
> 	Krzysztof

Thanks for the details, be good to put this concisely in the cover
letter/relevant commit messages on respin.

Cheers, Lorenzo

      reply	other threads:[~2026-07-30  8:24 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-25 21:05 [PATCH v2 0/3] mm,kernfs,proc: Unmap mmaps of removed files via file->f_mapping Krzysztof Wilczyński
2026-07-25 21:05 ` [PATCH v2 1/3] mm: Add unmap_mapping_file() helper Krzysztof Wilczyński
2026-07-25 21:35   ` sashiko-bot
2026-07-27 16:36   ` David Hildenbrand (Arm)
2026-07-30  8:23   ` Lorenzo Stoakes (ARM)
2026-07-25 21:05 ` [PATCH v2 2/3] kernfs: Unmap mmaps of removed files via file->f_mapping Krzysztof Wilczyński
2026-07-25 21:22   ` sashiko-bot
2026-07-25 21:05 ` [PATCH v2 3/3] proc: " Krzysztof Wilczyński
2026-07-25 21:19   ` sashiko-bot
2026-07-25 21:37 ` [PATCH v2 0/3] mm,kernfs,proc: " Andrew Morton
2026-07-26  1:22   ` Krzysztof Wilczyński
2026-07-30  8:24     ` Lorenzo Stoakes (ARM) [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=amsJ_T3qTJ-TJcwk@lucifer \
    --to=ljs@kernel.org \
    --cc=akpm@linux-foundation.org \
    --cc=baoquan.he@linux.dev \
    --cc=bhelgaas@google.com \
    --cc=david@kernel.org \
    --cc=driver-core@lists.linux.dev \
    --cc=gregkh@linuxfoundation.org \
    --cc=helgaas@kernel.org \
    --cc=kexec@lists.infradead.org \
    --cc=kwilczynski@kernel.org \
    --cc=liam@infradead.org \
    --cc=linux-mm@kvack.org \
    --cc=linux-pci@vger.kernel.org \
    --cc=linux-sound@vger.kernel.org \
    --cc=lpieralisi@kernel.org \
    --cc=mani@kernel.org \
    --cc=mhocko@suse.com \
    --cc=pasha.tatashin@soleen.com \
    --cc=perex@perex.cz \
    --cc=pratyush@kernel.org \
    --cc=rppt@kernel.org \
    --cc=ruirui.yang@linux.dev \
    --cc=simona.vetter@ffwll.ch \
    --cc=surenb@google.com \
    --cc=tiwai@suse.com \
    --cc=tj@kernel.org \
    --cc=vbabka@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox