Received-SPF: pass (google.com: domain of linux-kernel+bounces-82324-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1;
From: Michael Kelley <mhklinux@outlook.com>
To: Robin Murphy <robin.murphy@arm.com>, Will Deacon <will@kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
CC: "kernel-team@android.com" <kernel-team@android.com>,
	"iommu@lists.linux.dev" <iommu@lists.linux.dev>, Christoph Hellwig
	<hch@lst.de>, Marek Szyprowski <m.szyprowski@samsung.com>, Petr Tesarik
	<petr.tesarik1@huawei-partners.com>, Dexuan Cui <decui@microsoft.com>,
	Nicolin Chen <nicolinc@nvidia.com>
Subject: RE: [PATCH v4 5/5] iommu/dma: Force swiotlb_max_mapping_size on an
 untrusted device
Thread-Topic: [PATCH v4 5/5] iommu/dma: Force swiotlb_max_mapping_size on an
 untrusted device
Thread-Index: AQHaZLoRyKgMK9jLMUWPn0GpqcfvobEdDJiAgAAUxOA=
Date: Mon, 26 Feb 2024 21:11:22 +0000
Message-ID:
 <SN6PR02MB4157CD26C1D9BAC64208D9D8D45A2@SN6PR02MB4157.namprd02.prod.outlook.com>
References: <20240221113504.7161-1-will@kernel.org>
 <20240221113504.7161-6-will@kernel.org>
 <6be819d8-f1f9-4833-81c3-32220617f0c5@arm.com>
In-Reply-To: <6be819d8-f1f9-4833-81c3-32220617f0c5@arm.com>
Accept-Language: en-US
Content-Language: en-US
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0:
 =?us-ascii?Q?4rx1fnkY4QPsenAHewF0kUDnOcECoP9JZ2jbCGoMzm7TNY0LPRasH1pWe/GO?=
 =?us-ascii?Q?kenB8ZJYyd5h7F7GqPARQxYILUgtXjoiDvOVxwwwXmKfwCHi+A1+2KMwUFZR?=
 =?us-ascii?Q?UdeTiRN5fDNrbW3gTfn7Kycp7CzIR2ZTGaSZMMxGnD0F6l/vK6cER5QAR+Uv?=
 =?us-ascii?Q?5w9RzC895WXL7D90w41TxaxAxwo+pdKCy0+npRatAjg84CIKC212d4h+AFS9?=
 =?us-ascii?Q?86KlO2Vcxr42pEJCxrxUPu8lxusx0TS5CFzondOdtaAcmcauOg2MOqu42Cxt?=
 =?us-ascii?Q?ggugTMomawmoEIDH1WOxVf/osQONLv0M2RJHcp0DfCT3YvbdkZW6dB2CIlle?=
 =?us-ascii?Q?d0rdwsrIWQazniuk+YLFkn9OmSUZBe0Phlv+8veDuYN70uwkfakTl/zr+Ouw?=
 =?us-ascii?Q?pFvtzq24KAmGv91aCLs/6r9nlMgGiQad1x7XTQt0tVBQoBCXFavlx/GSXsm4?=
 =?us-ascii?Q?QiRD1vrUyBoC1bPiL6x1jx9rHEIAtMX0+c4X5n9UM8he/nvsz8lJgyrtm5S1?=
 =?us-ascii?Q?mLAIa/IWLYUAJxdDk2xGeLSQNF1XXdLv0ElLXSgWDnBaJxmZ7CgGpi5TnqxR?=
 =?us-ascii?Q?3McTZokDJBeesgQWFfbzqaZ9pZl/VywWJnyVurAKDxdmNMw+BVAwFNKfZ2Wy?=
 =?us-ascii?Q?v+rIzfqjQsEpbo3LkQXkGLKtJRPXa1WMZGir6FjZWpXi1iKBbW9tvZ1Snz6s?=
 =?us-ascii?Q?6r21wHF/wPjgW3uJYoIw9uTvRjKw7Ud73Wh5wCxbZXkFdCrdfDclWmaOVwlo?=
 =?us-ascii?Q?bVaHX49KahxYD5blTEAaoUVmVA0NntlC0PZgKdApOTlFB0UpWeiXrKjHyyh2?=
 =?us-ascii?Q?shLzJN9md3PEWQIRMkd/MGxkTubtA4SlNjXOf5E4XDyLcX6E+s/J0Q49lNH9?=
 =?us-ascii?Q?es5CyxoRDQRE7gQ4eIkxR1e7/12az5My9wn/t6Dm1+rCwL1vL0yojaiRrx30?=
 =?us-ascii?Q?nB3VAKLNlgHsYeqdM6bU5rygQFWWd0JfvT37HoLOBTnxK9JSzqEiSEa4PUeW?=
 =?us-ascii?Q?TZF2L+5PSjs37at0KnZhlxiF7uXZkuZELDCp4dQx+YpkaAy8mk6vwbLVMPXk?=
 =?us-ascii?Q?EB8Vi5zj3aguZwQss2/CMSNONP9ZLsrl0Oscx8646adQK+wZA7PA4bsPE/JC?=
 =?us-ascii?Q?KesuXHugXRF6DNPL7wEcp1M0UKuXKznWmf1rL/bU8y8EHiR/m1YAMdoFtNeP?=
 =?us-ascii?Q?wv/F2JTPb4XFML2KFq8iLg5RIKkbT4WeTLEouw=3D=3D?=
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk
X-Mailing-List: linux-kernel@vger.kernel.org
List-Id: <linux-kernel.vger.kernel.org>
List-Subscribe: <mailto:linux-kernel+subscribe@vger.kernel.org>
List-Unsubscribe: <mailto:linux-kernel+unsubscribe@vger.kernel.org>
MIME-Version: 1.0
X-OriginatorOrg: outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-AuthSource: SN6PR02MB4157.namprd02.prod.outlook.com
X-MS-Exchange-CrossTenant-RMS-PersistedConsumerOrg: 00000000-0000-0000-0000-000000000000
X-MS-Exchange-CrossTenant-Network-Message-Id: 67e2f709-9929-44a3-02e5-08dc370f7c65
X-MS-Exchange-CrossTenant-rms-persistedconsumerorg: 00000000-0000-0000-0000-000000000000
X-MS-Exchange-CrossTenant-originalarrivaltime: 26 Feb 2024 21:11:23.0505
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 84df9e7f-e9f6-40af-b435-aaaaaaaaaaaa
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CH0PR02MB8150

From: Robin Murphy <robin.murphy@arm.com> Sent: Monday, February 26, 2024 1=
1:36 AM
>=20
> On 21/02/2024 11:35 am, Will Deacon wrote:
> > From: Nicolin Chen <nicolinc@nvidia.com>
> >
> > The swiotlb does not support a mapping size > swiotlb_max_mapping_size(=
).
> > On the other hand, with a 64KB PAGE_SIZE configuration, it's observed t=
hat
> > an NVME device can map a size between 300KB~512KB, which certainly fail=
ed
> > the swiotlb mappings, though the default pool of swiotlb has many slots=
:
> >      systemd[1]: Started Journal Service.
> >   =3D> nvme 0000:00:01.0: swiotlb buffer is full (sz: 327680 bytes), to=
tal 32768 (slots), used 32 (slots)
> >      note: journal-offline[392] exited with irqs disabled
> >      note: journal-offline[392] exited with preempt_count 1
> >
> > Call trace:
> > [    3.099918]  swiotlb_tbl_map_single+0x214/0x240
> > [    3.099921]  iommu_dma_map_page+0x218/0x328
> > [    3.099928]  dma_map_page_attrs+0x2e8/0x3a0
> > [    3.101985]  nvme_prep_rq.part.0+0x408/0x878 [nvme]
> > [    3.102308]  nvme_queue_rqs+0xc0/0x300 [nvme]
> > [    3.102313]  blk_mq_flush_plug_list.part.0+0x57c/0x600
> > [    3.102321]  blk_add_rq_to_plug+0x180/0x2a0
> > [    3.102323]  blk_mq_submit_bio+0x4c8/0x6b8
> > [    3.103463]  __submit_bio+0x44/0x220
> > [    3.103468]  submit_bio_noacct_nocheck+0x2b8/0x360
> > [    3.103470]  submit_bio_noacct+0x180/0x6c8
> > [    3.103471]  submit_bio+0x34/0x130
> > [    3.103473]  ext4_bio_write_folio+0x5a4/0x8c8
> > [    3.104766]  mpage_submit_folio+0xa0/0x100
> > [    3.104769]  mpage_map_and_submit_buffers+0x1a4/0x400
> > [    3.104771]  ext4_do_writepages+0x6a0/0xd78
> > [    3.105615]  ext4_writepages+0x80/0x118
> > [    3.105616]  do_writepages+0x90/0x1e8
> > [    3.105619]  filemap_fdatawrite_wbc+0x94/0xe0
> > [    3.105622]  __filemap_fdatawrite_range+0x68/0xb8
> > [    3.106656]  file_write_and_wait_range+0x84/0x120
> > [    3.106658]  ext4_sync_file+0x7c/0x4c0
> > [    3.106660]  vfs_fsync_range+0x3c/0xa8
> > [    3.106663]  do_fsync+0x44/0xc0
> >
> > Since untrusted devices might go down the swiotlb pathway with dma-iomm=
u,
> > these devices should not map a size larger than swiotlb_max_mapping_siz=
e.
> >
> > To fix this bug, add iommu_dma_max_mapping_size() for untrusted devices=
 to
> > take into account swiotlb_max_mapping_size() v.s. iova_rcache_range() f=
rom
> > the iommu_dma_opt_mapping_size().
>=20
> On the basis that this is at least far closer to correct than doing nothi=
ng,
>=20
> Acked-by: Robin Murphy <robin.murphy@arm.com>
>=20
> TBH I'm scared to think about theoretical correctness for all the
> interactions between the IOVA granule and min_align_mask, since just the
> SWIOTLB stuff is bad enough, even before you realise the ways that the
> IOVA allocation isn't necessarily right either. However I reckon as long
> as we don't ever see a granule smaller than IO_TLB_SIZE, and/or a
> min_align_mask larger than a granule, then this should probably work
> well enough as-is.
>=20

I'm not convinced.  The conditions you describe are reasonable
and reflect upstream code today. But there can still be a failure
due to attempting to allocate a "too large" swiotlb buffer. It
happens with a granule of 64K and min_align_mask of 4K - 1 (the
NVMe case) when the offset passed to iommu_dma_map_page()
is non-zero modulo 4K.  With this patch, the size passed into
iommu_dma_map_page() is limited to 252K, but it gets rounded
up to 256K.  Then swiotlb_tbl_map_single() adds the offset
modulo 4K.  The result exceeds the 256K limit in swiotlb and
the mapping fails.

The case I describe is a reasonable case that happens in the real
world.  As is, this patch will work most of the time because for
large xfers the offset is typically at least 4K aligned. But not always.

Michael