From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id EC975C52D6F for ; Wed, 21 Aug 2024 04:43:54 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id E82536B0083; Wed, 21 Aug 2024 00:43:53 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id E31A46B0085; Wed, 21 Aug 2024 00:43:53 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id CFA116B0088; Wed, 21 Aug 2024 00:43:53 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0012.hostedemail.com [216.40.44.12]) by kanga.kvack.org (Postfix) with ESMTP id AF3C96B0083 for ; Wed, 21 Aug 2024 00:43:53 -0400 (EDT) Received: from smtpin07.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay03.hostedemail.com (Postfix) with ESMTP id 013D6A0815 for ; Wed, 21 Aug 2024 04:43:52 +0000 (UTC) X-FDA: 82475009946.07.856B477 Received: from mail-pg1-f175.google.com (mail-pg1-f175.google.com [209.85.215.175]) by imf19.hostedemail.com (Postfix) with ESMTP id 2414C1A0006 for ; Wed, 21 Aug 2024 04:43:50 +0000 (UTC) Authentication-Results: imf19.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=ggyYs53d; dmarc=pass (policy=none) header.from=gmail.com; spf=pass (imf19.hostedemail.com: domain of shankerwangmiao@gmail.com designates 209.85.215.175 as permitted sender) smtp.mailfrom=shankerwangmiao@gmail.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1724215372; a=rsa-sha256; cv=none; b=1EAocQdC9M+1aNGCZs1KJ5dXBRXQa7A+ILU8AH/TH4kZcaFOnqIMXjEhlmw9UX0uByQ+RL nFEhZH6zze3+LvHJbXAgKyquLObhAVRpuaItZNheXeeBlBkU8qpPiUS+7ngSSGdnw9f+QC bP0m9snClF5nOb8huD85J82VrqtIxmc= ARC-Authentication-Results: i=1; imf19.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=ggyYs53d; dmarc=pass (policy=none) header.from=gmail.com; spf=pass (imf19.hostedemail.com: domain of shankerwangmiao@gmail.com designates 209.85.215.175 as permitted sender) smtp.mailfrom=shankerwangmiao@gmail.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1724215372; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=3P3Hk9XNXX67Z9XxlANZXs0svzlSvZih1xysLj6CbmM=; b=XlFT6teXWRHWDxuoS08AKALaZucpxJaQWLu7gx5dtG88t2rFe4UGAjKX+zNjUmcQeMlc+J sNWJUs58vf49QsKv0ALhWwQC3f+3VBAUCTkvbBTuB245Xd1WaDzYLnZJS9uU9TFU1G9dXg oFKVWOCKO/soDiD5FexB2JGOH2W1zVY= Received: by mail-pg1-f175.google.com with SMTP id 41be03b00d2f7-7c3d415f85eso416125a12.0 for ; Tue, 20 Aug 2024 21:43:50 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1724215430; x=1724820230; darn=kvack.org; h=to:references:message-id:content-transfer-encoding:cc:date :in-reply-to:from:subject:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=3P3Hk9XNXX67Z9XxlANZXs0svzlSvZih1xysLj6CbmM=; b=ggyYs53dfjHOBLW8re9Pbdd7O1CC7LbfQrHSDBgjUU407o26F+dpjbWciLP77x+nQk edNiUjkJ6DBng4Iu2g6b/VoHqq4DnB55Hx2XdLtSC52WJOyCgZZWYfwucJEXZgOCS7QU CxiA4O9HOeGX0VreyxuVuj5EwETA4FRavyLLq4fJNOhgQXrXidfvA4oFTY5InJ7rXHvc 1EbXKae2SzD9iqwvsc4XgiaUAGsvBwelc4VCx3FZeCYYZ3xZ349b5KwuS6mwP3qlbLUD BLfvW71wp0Oft/zI4cpl+WxyccoQpUB6wtptP2UMiahzHeyVJVUPIjDjdOPfzeRfJFwd El6w== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1724215430; x=1724820230; h=to:references:message-id:content-transfer-encoding:cc:date :in-reply-to:from:subject:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=3P3Hk9XNXX67Z9XxlANZXs0svzlSvZih1xysLj6CbmM=; b=Uq4ny+ByotvM1qnMxrOJgeDI/RsK9TFM7Z88DMtGcIfJ5+YPI4PhS87ELlwfKmCv/5 CsxCvLKxPOyydIG3v1sxcpb/SH6THMPWmAhO1yDw2oNVXAzPP9QHf39+sXC/zMkffWCP O8nnfQ7K7dTW4nz3GxXnaQQWeFn+x1qbgbRLQu1XQYYdmxEPU3jWHwyiHpoab8U/MxRT 5CZbBWipzAZJKJkuprJeW/WXXbHlmDLI1gGMLw9Nnkk92qa7wDCXbfyFQwOB//eI9K7y sJyVn17ZsTh7oibggqwyi5JCBq2cz7+6f7APRu4DWktXu1WqirE2BkKAlg69cOVIhOMz sihQ== X-Forwarded-Encrypted: i=1; AJvYcCXJZjCbF7oj5UMPvOY3uX0nY/1s+n19EeB1avzRbfKuWAULIWf7vcP9S4sy+YIHm+9GBtplcdxxZg==@kvack.org X-Gm-Message-State: AOJu0Yy3iAx76Cce05cHtlmMdCK2jp3MZJwx4D/vlVIRC4PiIl0CKVc0 Lmmw29VSnH0ala2JL5hW9h6TmWGFfIA52StuzFdXO1FbKhCAB2zx X-Google-Smtp-Source: AGHT+IH+ozAZ+jz6m72kC+HtpaDsVUaOzExvPIGhiOIYtJTBStrUt/otPT/jHSUZPA+86wGWbG9jHQ== X-Received: by 2002:a05:6a20:3951:b0:1c4:84ee:63d1 with SMTP id adf61e73a8af0-1cad82e8cf0mr944326637.9.1724215429403; Tue, 20 Aug 2024 21:43:49 -0700 (PDT) Received: from smtpclient.apple (v133-130-115-230.a046.g.tyo1.static.cnode.io. [133.130.115.230]) by smtp.gmail.com with ESMTPSA id d9443c01a7336-201f038b43dsm85164685ad.210.2024.08.20.21.43.45 (version=TLS1_2 cipher=ECDHE-ECDSA-AES128-GCM-SHA256 bits=128/128); Tue, 20 Aug 2024 21:43:48 -0700 (PDT) Content-Type: text/plain; charset=utf-8 Mime-Version: 1.0 (Mac OS X Mail 16.0 \(3776.700.51\)) Subject: Re: [PATCH v3] ACPI: PCI: check if the root io space is page aligned From: Miao Wang In-Reply-To: <86348A3F-6AF4-4DC0-ACF5-08EC52E3828C@gmail.com> Date: Wed, 21 Aug 2024 12:43:32 +0800 Cc: =?utf-8?Q?Ilpo_J=C3=A4rvinen?= , Bjorn Helgaas , "Rafael J. Wysocki" , Len Brown , linux-pci@vger.kernel.org, linux-acpi@vger.kernel.org, linux-mm@kvack.org Content-Transfer-Encoding: quoted-printable Message-Id: References: <20240814163711.GA351420@bhelgaas> <86348A3F-6AF4-4DC0-ACF5-08EC52E3828C@gmail.com> To: Bjorn Helgaas , Andrew Morton X-Mailer: Apple Mail (2.3776.700.51) X-Rspamd-Queue-Id: 2414C1A0006 X-Rspam-User: X-Rspamd-Server: rspam05 X-Stat-Signature: h1ho9h948doq6uuk5oy19npyehpbeskg X-HE-Tag: 1724215430-662425 X-HE-Meta: U2FsdGVkX19AK8XFICNDUfFFxN/Pj3Xda0jQp4p4UgyIx3hIumbv2FFXpHDCArvmUj1F5C4K4z9Layb1DHapUEHDXtQ+P5Cektjla890bOvKnJH9dACL/GXINX3ssHTl0FfQ2/ACbRdQ6IiCOzfByMMTvBBRx+iUmb+HNViCD5uM1ix2/wXjexwnGxLkBuLJEqPR6FidnbbLIqTfLz8r3a6/ae/BoxYpcuIJw8Gz+CWrMbKWmvyN5uBGUNbl7Tc+9DBtc4T5Xfra0I1BkMj31eFxT1bV1TnkN0X+dmKT+pcczz+wQIGjCPEWoXemjS2hzyXaspGlsleV0WCLyPxYnlFH7VuOhX3OSv562PNKfnkP6lyH34GGylvQVcqMjlcxbzobJkhlSrhX42iC1Lmj82YVcx+65J2T9dF2UjRs+2KV01gkmgqbb7Qu1qojXfFNLy6mbCwdgw/ZKCZklQ/0cqpapUqjUGv74gMrMjgYPaszv7Tk2wt6ak0IBw0Z8QfxqAf+Nwvg3N98i9T3ghi5lGXDddoY0yuxWuj11BgGlmKWgZ7BI1ex9aY4dQqyhKBL+TGLkbVWlBAr9fWz0OnmNzU+97kUSwy6crWcWKNDP2uqrCpvJgcAhjMKfV5RJKf/pDJs9VdGhv4OsOJ4sdSsmvMRN8ptuv2Nz7lxxSCFZzB1NtjcQ61ltLw4Ci7z4B6dbbRqtgee16Xk72XV5tumcPhr5RhRroI+nqVCHmSOG8v3dTlU7OrGP9EZ7oZd/JsEafoo5ntu7mhvL5AKu6jggX13Nx8GnDPmx3eV1CipkojR4FjDoz5utubgWjR1XmWebPXSt+voAhtZdSkgUaUuHc0l5zeoN+s43noDcxMZh1w9N8bilQAAbIMvd+4aZPrTUxf2Nlr3eXSaoNCTaFk3Iq4VZp1KTFBjRQ/sX3Da6sVS1QmFKPExGixe2Q4524Yr/t7rJj7Uq1Bwi7BNH1q rhuDraWC 8OiwARu/6dRRGP3l8H6AgcqKT+SGZyfKoSXWL67K117xu448SHj5qaanMSVtU8iK39Y+jOr9tlujQaiVW1giwmf05LIRGDaBG1QGXsNkZoRIphlQGY6pQzFeq2ySSirPlVn3Yvj6P+/qs/4K/piFXQapaqefw5JuO/JPrEhgEj9AWgikFWW0lmUt18CaWkDw+ggLGUXfBTtLv+jnzhMj6gbSITDa89iB3o9J8lgzkxfppvbkomSz5WBzZiLiNVXuOjXFueHRukEDHiblrnFws46skIkEi0TE3Nwpv388CyeyOaaKQMfGTR5/wN1p0SRkZpR1NtdURxq9Oai/bLmnJW0MxTlC+fH4n4nQaiaw/In9qtr8wClo6gt+LCUdK96GSExfUyxWS0QfBdHxAuVMXdpS8XCXgX/An/U7BRto0tvM6hrjDsObHqI8t6IXhD38FErlD7nZ3FYJ0tAcffELWsJcGgdfk7PEYsTRc X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: List-Subscribe: List-Unsubscribe: Sorry for directly looping Andrew in. I think Andrew may be familiar = with the code in question. Some backgrounds: Mis-aligned addresses from ACPI table can pass along=20= pci_remap_iospace() -> vmap_page_range() -> vmap_pte_range() path, = leading to a loop overrun in vmap_pte_range(). Bjorn and I wonder why all those vmap_*_range() functions don't validate the alignment, assuming the = addresses page-aligned. We want to know the best place to do this check. > 2024=E5=B9=B48=E6=9C=8815=E6=97=A5 12:28=EF=BC=8CMiao Wang = =E5=86=99=E9=81=93=EF=BC=9A >=20 > Hi, >=20 >> 2024=E5=B9=B48=E6=9C=8815=E6=97=A5 00:37=EF=BC=8CBjorn Helgaas = =E5=86=99=E9=81=93=EF=BC=9A >>=20 >> [+cc linux-mm for vmap page alignment checking question] >>=20 >> On Wed, Aug 14, 2024 at 08:09:15PM +0800, Miao Wang via B4 Relay = wrote: >>> From: Miao Wang >>>=20 >>> When the IO resource given by _CRS method is not page aligned, = especially >>> when the page size is larger than 4KB, serious problems will happen >>> because the misaligned address is passed down to = pci_remap_iospace(), >>> then to vmap_page_range(), and finally to vmap_pte_range(), where = the >>> length between addr and end is expected to be divisible by = PAGE_SIZE, or >>> the loop will overrun till the pfn_none check fails. >>=20 >> What does this problem look like to a user? Panic, oops, hang, >> warning backtrace? I assume this is not a regression, but maybe >> something you tripped over because of a BIOS defect? Does this need >> to be backported to stable kernels? >=20 > Panic, or actually BUG in vmap_pte_range() at the = !pte_none(ptep_get(pte)) > check, since misaligned addresses will cause the loop in = vmap_pte_range > overrun and finally reach one of the already mapped pages. This = happens on > a LS2k2000 machine, the buggy firmware of which declares the IO space = of > the PCI root controller as follows: >=20 > QWordIO (ResourceProducer, MinFixed, MaxFixed, PosDecode, = EntireRange, > 0x0000000000000000, // Granularity > 0x0000000000004000, // Range Minimum > 0x0000000000009FFF, // Range Maximum > 0x000000FDFC000000, // Translation Offset > 0x0000000000006000, // Length > ,, , TypeStatic, DenseTranslation) >=20 > At first, I thought there might be some overlapping address spaces. = But when > I added some debug output in vmap_page_range(), I realized that it was > because a loop overrun. >=20 > Normally, loongarch64 kernel is compiled using 16K page size, and thus = the > length here is not page aligned. I tested my patch using a virtual = machine > with a deliberately modified DSDT table to reproduce this issue. >=20 >> It seems sort of weird to me that all those vmap_*_range() functions >> take the full page address (not a PFN) and depend on the addr/size >> being page-aligned, but they don't validate the alignment. But I'm >> not a VM person and I suppose there's a reason for passing the full >> address. > Ah, I also have this question. >>=20 >> But it does mean that other users of vmap_page_range() are also >> potentially susceptible to this issue, e.g., vmap(), vm_map_ram(), >> ioremap_page_range(), etc., so I'm not sure that >> acpi_pci_root_remap_iospace() is the best place to check the >> alignment. > My first idea was that the misaligned address is introduced from DSDT > table and the check would be better to be done inside the ACPI system. > However, lets wait for replies from linux-mm to decide where should = be > the best place. >>=20 >>> Signed-off-by: Miao Wang >>> --- >>> Changes in v3: >>> - Adjust code formatting. >>> - Reword the commit message for further description of the possible = reason >>> leading to misaligned IO resource addresses. >>> - Link to v2: = https://lore.kernel.org/r/20240814-check_pci_probe_res-v2-1-a03c8c9b498b@g= mail.com >>>=20 >>> Changes in v2: >>> - Sorry for posting out the draft version in V1, fixed a silly = compiling issue. >>> - Link to v1: = https://lore.kernel.org/r/20240814-check_pci_probe_res-v1-1-122ee07821ab@g= mail.com >>> --- >>> drivers/acpi/pci_root.c | 14 +++++++++++--- >>> 1 file changed, 11 insertions(+), 3 deletions(-) >>>=20 >>> diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c >>> index d0bfb3706801..a425e93024f2 100644 >>> --- a/drivers/acpi/pci_root.c >>> +++ b/drivers/acpi/pci_root.c >>> @@ -858,7 +858,7 @@ static void = acpi_pci_root_validate_resources(struct device *dev, >>> } >>> } >>>=20 >>> -static void acpi_pci_root_remap_iospace(struct fwnode_handle = *fwnode, >>> +static void acpi_pci_root_remap_iospace(struct acpi_device *device, >>> struct resource_entry *entry) >>> { >>> #ifdef PCI_IOBASE >>> @@ -868,7 +868,15 @@ static void acpi_pci_root_remap_iospace(struct = fwnode_handle *fwnode, >>> resource_size_t length =3D resource_size(res); >>> unsigned long port; >>>=20 >>> - if (pci_register_io_range(fwnode, cpu_addr, length)) >>> + if (!PAGE_ALIGNED(cpu_addr) || !PAGE_ALIGNED(length) || >>> + !PAGE_ALIGNED(pci_addr)) { >>> + dev_err(&device->dev, >>> + FW_BUG "I/O resource %pR or its offset %pa is not page aligned\n", >>> + res, &entry->offset); >>> + goto err; >>> + } >>> + >>> + if (pci_register_io_range(&device->fwnode, cpu_addr, length)) >>> goto err; >>=20 >> This change verifies alignment for the ACPI case that leads to the >> pci_remap_iospace() -> vmap_page_range() -> vmap_pte_range() path, = but=20 >> there are others even in drivers/pci/, e.g., pci_remap_iospace() is >> also used in the DT path, where I suppose a defective DT could cause = a >> similar issue. >>=20 >>> port =3D pci_address_to_pio(cpu_addr); >>> @@ -910,7 +918,7 @@ int acpi_pci_probe_root_resources(struct = acpi_pci_root_info *info) >>> else { >>> resource_list_for_each_entry_safe(entry, tmp, list) { >>> if (entry->res->flags & IORESOURCE_IO) >>> - acpi_pci_root_remap_iospace(&device->fwnode, >>> + acpi_pci_root_remap_iospace(device, >>> entry); >>>=20 >>> if (entry->res->flags & IORESOURCE_DISABLED) >>>=20 >>> --- >>> base-commit: 7c626ce4bae1ac14f60076d00eafe71af30450ba >>> change-id: 20240813-check_pci_probe_res-27e3e6df72b2 >>>=20 >>> Best regards, >>> --=20 >>> Miao Wang