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 27446CDD54D for ; Wed, 18 Sep 2024 17:14:33 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id A2D676B007B; Wed, 18 Sep 2024 13:14:32 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 9DDC26B0083; Wed, 18 Sep 2024 13:14:32 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 87E466B00A6; Wed, 18 Sep 2024 13:14:32 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0013.hostedemail.com [216.40.44.13]) by kanga.kvack.org (Postfix) with ESMTP id 674F76B007B for ; Wed, 18 Sep 2024 13:14:32 -0400 (EDT) Received: from smtpin08.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay01.hostedemail.com (Postfix) with ESMTP id 163481C4DA9 for ; Wed, 18 Sep 2024 17:14:32 +0000 (UTC) X-FDA: 82578507984.08.99304A4 Received: from mail-pf1-f169.google.com (mail-pf1-f169.google.com [209.85.210.169]) by imf04.hostedemail.com (Postfix) with ESMTP id 15C004001A for ; Wed, 18 Sep 2024 17:14:29 +0000 (UTC) Authentication-Results: imf04.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=i0+lhHIT; dmarc=pass (policy=none) header.from=gmail.com; spf=pass (imf04.hostedemail.com: domain of shankerwangmiao@gmail.com designates 209.85.210.169 as permitted sender) smtp.mailfrom=shankerwangmiao@gmail.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1726679587; a=rsa-sha256; cv=none; b=Y74zNqPE9PgCkliyXqcm4nEEmB2GgS87sUB/KX3l6Ax9lR2/TaktqtnrCZhRr40dXGRRP3 /MkinfarTmt6bV7dEkyFSkEaZ0pj8F8HRUxMvEbzFXy+QAVws/CnGvKtTGQ4gqadTAjlUQ BcGTVzFRIJVvdpC16YCsFchMj1pyT8E= ARC-Authentication-Results: i=1; imf04.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=i0+lhHIT; dmarc=pass (policy=none) header.from=gmail.com; spf=pass (imf04.hostedemail.com: domain of shankerwangmiao@gmail.com designates 209.85.210.169 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=1726679587; 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=L8XWEjshIeUyAJDr89/XlRuNEb4mgSG3Wk3IyKgdD2M=; b=bPPhknJL8XQEReYbSECMgSuG+ZTfTBxY8bd6i1aSfbxcmlG3biS1l4mchMRuaS44v/FYP6 fxrAFJlbI93+BwbFnkmmwVTQOZgb7HjOQsZKSm7kBsrQcI/nVSkLc0xN97WB0RvWlCFDIx GKubGn2aJoKxqkZaAGhsAz/x9k77Vlw= Received: by mail-pf1-f169.google.com with SMTP id d2e1a72fcca58-717839f9eb6so1219922b3a.3 for ; Wed, 18 Sep 2024 10:14:29 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1726679669; x=1727284469; 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=L8XWEjshIeUyAJDr89/XlRuNEb4mgSG3Wk3IyKgdD2M=; b=i0+lhHITFnAVxl3Pa2nKWdWMd3aJliCDRF9h2ybclX0dGxqwIx9dK+g9/TU3iPNKzS uem48Zwy0eMYSKLLsTf9upipMvHyDJNnILIO1sw3M3qQuPi4nHWQL7jMUUG/gkl0T+bE v3T6Fnz8EfiWK9/RbSxSRr1TqRFSQEKvN+n67GEY2OizI1N5FFPx20wZv/NX26P9J8/X u4epH/oRwv5jX/aJcqlDSRg0Uh55c0ogzRxw21vi0ls0Y9jtBJBpvGy3qMeEacj9Gu+u mO73Ofqd6kknJRdPQxYKKsXjnhPyNNRkubPAXUmv983QlHdeFAvk6+0P6sUfBpXsd3Tg EOAw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1726679669; x=1727284469; 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=L8XWEjshIeUyAJDr89/XlRuNEb4mgSG3Wk3IyKgdD2M=; b=CnA8EgVbMV1NM1TtGGtGQiM5pITqb3i6ksTZKSW56YDJ2+AE5+uoFl26Lsf0MgTLDw 4Zy5UsjZlyPLctDCmth8J1/4J7qrrwkMNDMZdNwepVVZVK4zHsbGzT/6VPPSbI/TDfBX zxOFmATcfLTkV4OKz6CYVXxS9EPi4nR/dEccTWbGUK5gi8BapLWahqVpq129nj0gUlg3 4pX4AhDTpwc6ZqCEUoxPTRr5muT2CUDrmoGkpSVsMOEpf0HmWNWbfi19hWqeKiAyWn96 X2/FAYhYQUyCUrPcGxWs4RZL6erV6PPxXYpVEdEVIJD1txOUDfiFhECE9kbIf1tea+e2 uxEg== X-Forwarded-Encrypted: i=1; AJvYcCWUXEibEaZ8Yuw1QY2lgKbz+6ySwhp3n3vkES0R+HsHqptMhAwXgAzA7o/CqS3OfdI5LP46niPJoA==@kvack.org X-Gm-Message-State: AOJu0Yz4irxJK1odyNYhTcaHkgf4uVv/HMiBLprbNvoJLspMS9vTfoVG ulX7efCUtSfxnUAcUIuM4f06bkD0bscBrKn4yzBav0+PkHZ6rLmH X-Google-Smtp-Source: AGHT+IH6UrksX18YRMK53M8BYR7liw/I9T3GTtpyZHqa/05mpaP5k4izxrS5OWeY4OfGxRz7cwV6hg== X-Received: by 2002:a05:6a21:6da2:b0:1cf:2be2:5e5a with SMTP id adf61e73a8af0-1cf764cdd7bmr16397918637.10.1726679668417; Wed, 18 Sep 2024 10:14:28 -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 41be03b00d2f7-7db4a5a8cf3sm6769905a12.85.2024.09.18.10.14.24 (version=TLS1_2 cipher=ECDHE-ECDSA-AES128-GCM-SHA256 bits=128/128); Wed, 18 Sep 2024 10:14:27 -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: Date: Thu, 19 Sep 2024 01:14:11 +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: <9CBE412E-25B3-4C36-80F6-5EA9248B9085@gmail.com> References: <20240814163711.GA351420@bhelgaas> <86348A3F-6AF4-4DC0-ACF5-08EC52E3828C@gmail.com> To: Bjorn Helgaas X-Mailer: Apple Mail (2.3776.700.51) X-Rspamd-Queue-Id: 15C004001A X-Rspam-User: X-Rspamd-Server: rspam05 X-Stat-Signature: 9noz898hpwba684sqb91os6cm5ani518 X-HE-Tag: 1726679669-404984 X-HE-Meta: U2FsdGVkX1///bnDpdbM58VTxvcF1g/qieL5SoTELjT5rjT0WEz1ec1A25ivr4pWS5PH9zZ7I2E9+DEGRTkuv5MZBtP/XQTRy2ICRyDxYN3dzWee17sp/BJvZBXsFA6pZ6Igfp9Zfi2YVYmy6lrwux5bTYgEls7/ak3U2il91vOTuKg1ocYjHsc/QT9v67lY3CTVa+6FUQJ4V6fA8udcacqvDgDBuCY7o+ah6Ju6QcruPLyI8g8bvjuxrUS8eHzNE+dgR/m3ttwl/OehoWQrFCeAyzmXcVtQiTwVgGPRTBmV98cdSqyESynH5K7Paxd37kBJgrgcc5eNEKyjuvaVel589KVsnsl5n2yXcOH1jVDxHjs3CNHEAsEA/xl8g7q22tFsyHNIto3y6meHuwS/fJM8tHtLKqW37DjAaCKvWjfZKD+gyZhhC/gqZGnAVKVBg14/VqLYtYA9YVU8RSo4I8AoB/6rpmDnE44YQ3iZSVWaDlhLVlg3fr8JbVA2TVb0ZZQt3tEQuqwNNmbrbAzWHcMfOsPfRgKw85D6myP6y3Cmy30myyfPtzQtM+n6/F5n9XOExDqSJbnp14VbUrTbVWcbdkBxFcBVO96rTwuZbnBun9X5XCGxSl5DYkj0J8VoQoqXpQHpNVnQ282SkkNBpoJ9cJzHtN7dCQuhpfjOeWsGMTLnygEnyd73lATgzUO/cZpbd9tzp04a4Hnk7cmwEfrXFpYnIi1xTEGDzekQWFdVPRKF0+53Ho0VUDUqc0Ik7uzB6EtqKurgjDaf32TOfYjnNT6R+3/s6njPzA/K5ZVfg/gVTE0MswE5o8q5WzvQogW23UIwUHDq6gkGQ7Fesc6fPyoLLTSPlSxwU9QNo6eXU2CzTb8hFuvY0Pv0laZMBSEwmyHU6dHZFZe2jwyXJMPbR+A3ro3oWkuMCLuPH/FETiZYi89qVNNe0GQWzquMiZ9IQZV+ipVr6ORTcxC 6UtLXwBc CXnGRadCINKr4Wb80M6YTefimaZv6oVz+DHKM5yT33zK9rX0z3yIGK3a0vI6pWIHm1aTUyeV5b7118BWBlHdGToI3Nxa8sjSO+7a5jKMjX/9Q3ETBqQ+STc5LxzSiHAV7xpFKl8gdKMGMtD2kX7I9WL6CLOsyrnoe7/7tzZ5I7m1VvofmaCW/j/GE1ok2FIjjuaQTbBEMa0fvgPJ23+Iq4yBHn0QL81jMtQQhbxELuta5rO6rRB4WSU0EoIxtO/QFIdb935bUv4zrfPJLhqXkPKNbjbeu5nHuk/IrHlcyWH5q0Y5DXY+WyU+L7jP+wKiMpT7nRnpQHqwaK7PK1iwUzkXwtqjPqsLQBZeV2kw0vF1l6mTFHqvAFBQAQZU2+awbAhMQ8ZFl8K2BzcacyEucIRU5g+SXxRockLH8KVkLb0q5OVC0IjDgmABlpHWwDasY991AwxlGi8wpQ0TIn20vCTQMs7bW3T6VgUqc 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: > 2024=E5=B9=B48=E6=9C=8821=E6=97=A5 12:43=EF=BC=8CMiao Wang = =E5=86=99=E9=81=93=EF=BC=9A >=20 > Sorry for directly looping Andrew in. I think Andrew may be familiar = with > the code in question. >=20 > 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. >=20 >> 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. It seems that there is no reply from linux-mm about this. I believe even = if there might be a better place for the mm subsystem to check the = alignment, it is still necessary to do the check here, because details about which = ACPI entry is causing the problem is only available here. If in the future, = we would developed another better place to do the alignment check, we may = refactor the code here. >>>=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 >=20 >=20