linux-kernel - Re: [PATCH v2 1/2] vfio: Prevent open

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <20250614000926.GQ1174925@nvidia.com>
Date: Fri, 13 Jun 2025 21:09:26 -0300
From: Jason Gunthorpe <jgg@...dia.com>
To: Alex Williamson <alex.williamson@...hat.com>
Cc: Jacob Pan <jacob.pan@...ux.microsoft.com>, linux-kernel@...r.kernel.org,
	"iommu@...ts.linux.dev" <iommu@...ts.linux.dev>,
	"Liu, Yi L" <yi.l.liu@...el.com>, Zhang Yu <zhangyu1@...rosoft.com>,
	Easwar Hariharan <eahariha@...ux.microsoft.com>,
	Saurabh Sengar <ssengar@...ux.microsoft.com>
Subject: Re: [PATCH v2 1/2] vfio: Prevent open_count decrement to negative

On Fri, Jun 13, 2025 at 04:31:00PM -0600, Alex Williamson wrote:
> Hi Jacob,
> 
> On Tue,  3 Jun 2025 08:23:42 -0700
> Jacob Pan <jacob.pan@...ux.microsoft.com> wrote:
> 
> > When vfio_df_close() is called with open_count=0, it triggers a warning in
> > vfio_assert_device_open() but still decrements open_count to -1. This
> > allows a subsequent open to incorrectly pass the open_count == 0 check,
> > leading to unintended behavior, such as setting df->access_granted = true.
> > 
> > For example, running an IOMMUFD compat no-IOMMU device with VFIO tests
> > (https://github.com/awilliam/tests/blob/master/vfio-noiommu-pci-device-open.c)
> > results in a warning and a failed VFIO_GROUP_GET_DEVICE_FD ioctl on the
> > first run, but the second run succeeds incorrectly.
> > 
> > Add checks to avoid decrementing open_count below zero.
> 
> The example above suggests to me that this is a means by which we could
> see this, but in reality it seems it is the only means by which we can
> create this scenario, right?

I understood this as an assertion hit because of the bug fixed in
patch 2 and thus the missed assertion error handling flow was noticed.

Obviously the assertion should never happen, but if it does we should
try to recover better than we currently do.

Jason