linux-kernel - Re: [PATCH v5 08/14] iommufd/viommu: Add iommufd_viommu_report

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

Message-ID: <Z5Cmvfp4JW9vmMTP@nvidia.com>
Date: Wed, 22 Jan 2025 00:05:17 -0800
From: Nicolin Chen <nicolinc@...dia.com>
To: Jason Gunthorpe <jgg@...dia.com>
CC: <kevin.tian@...el.com>, <corbet@....net>, <will@...nel.org>,
	<joro@...tes.org>, <suravee.suthikulpanit@....com>, <robin.murphy@....com>,
	<dwmw2@...radead.org>, <baolu.lu@...ux.intel.com>, <shuah@...nel.org>,
	<linux-kernel@...r.kernel.org>, <iommu@...ts.linux.dev>,
	<linux-arm-kernel@...ts.infradead.org>, <linux-kselftest@...r.kernel.org>,
	<linux-doc@...r.kernel.org>, <eric.auger@...hat.com>,
	<jean-philippe@...aro.org>, <mdf@...nel.org>, <mshavit@...gle.com>,
	<shameerali.kolothum.thodi@...wei.com>, <smostafa@...gle.com>,
	<ddutile@...hat.com>, <yi.l.liu@...el.com>, <patches@...ts.linux.dev>
Subject: Re: [PATCH v5 08/14] iommufd/viommu: Add iommufd_viommu_report_event
 helper

On Tue, Jan 21, 2025 at 01:40:13PM -0800, Nicolin Chen wrote:
> On Tue, Jan 21, 2025 at 05:14:04PM -0400, Jason Gunthorpe wrote:
> > Since we don't hold the spinlock the whole time there is a race where
> > we could pull the overflow off and then another entry could be pushed
> > while we do the copy_to_user.
> 
> I see. I'll be careful around that. I can imagine that one tricky
> thing can be to restore the overflow node back to the list when a
> copy_to_user fails..

Hmm, it gets trickier because the overflow node is a preallocated
single node per vEVENTQ. We must not only check list_empty for its
restore, but also protect the overflow->header.sequence from races
between atomic_inc and copy_to_user. However, we can't use a mutex
because copy_to_user might DOS...

A simple solution could be to just duplicate overflow nodes, each
of which contains a different sequence, like a regular vEVENT node.
This certainly changes the uAPI for read(). Though the duplication
of overflow nodes doesn't sound optimal, it's acceptable since the
duplicated nodes would have been regular vEVENT nodes if there was
no overflow (i.e. no extra overhead)?

Thanks
Nicolin