Intel-XE Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: Sasha Levin <sashal@kernel.org>
To: patches@lists.linux.dev, stable@vger.kernel.org
Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>,
	Lucas De Marchi <lucas.demarchi@intel.com>,
	Sasha Levin <sashal@kernel.org>,
	thomas.hellstrom@linux.intel.com, rodrigo.vivi@intel.com,
	intel-xe@lists.freedesktop.org
Subject: [PATCH AUTOSEL 6.17] drm/xe/configfs: Enforce canonical device names
Date: Sat, 25 Oct 2025 11:55:50 -0400	[thread overview]
Message-ID: <20251025160905.3857885-119-sashal@kernel.org> (raw)
In-Reply-To: <20251025160905.3857885-1-sashal@kernel.org>

From: Michal Wajdeczko <michal.wajdeczko@intel.com>

[ Upstream commit 400a6da1e967c4f117e4757412df06dcfaea0e6a ]

While we expect config directory names to match PCI device name,
currently we are only scanning provided names for domain, bus,
device and function numbers, without checking their format.
This would pass slightly broken entries like:

  /sys/kernel/config/xe/
  ├── 0000:00:02.0000000000000
  │   └── ...
  ├── 0000:00:02.0x
  │   └── ...
  ├──  0: 0: 2. 0
  │   └── ...
  └── 0:0:2.0
      └── ...

To avoid such mistakes, check if the name provided exactly matches
the canonical PCI device address format, which we recreated from
the parsed BDF data. Also simplify scanf format as it can't really
catch all formatting errors.

Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Lucas De Marchi <lucas.demarchi@intel.com>
Reviewed-by: Lucas De Marchi <lucas.demarchi@intel.com>
Link: https://lore.kernel.org/r/20250722141059.30707-3-michal.wajdeczko@intel.com
Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---

LLM Generated explanations, may be completely bogus:

YES

**Why It’s A Bug**
- Current code accepts any string that scans into
  domain/bus/slot/function, even if not in canonical PCI BDF format. See
  parsing in drivers/gpu/drm/xe/xe_configfs.c:264.
- The driver later looks up the configfs group by constructing the
  canonical BDF name, so a misnamed directory cannot be found and
  settings silently don’t apply. See lookup formatting in
  drivers/gpu/drm/xe/xe_configfs.c:310-311.
- The in-file docs already prescribe canonical names (for example
  0000:03:00.0), reinforcing that non-canonical input is unintended; see
  example path in drivers/gpu/drm/xe/xe_configfs.c:41.

**Fix Details**
- Parsing is relaxed to read numbers generically, then the code
  synthesizes the canonical BDF string and requires an exact match:
  - Replace strict-width `sscanf(name, "%04x:%02x:%02x.%x", ...)` with
    `sscanf(name, "%x:%x:%x.%x", ...)` to get the values.
  - Build the canonical name and enforce exact equality via
    `scnprintf(canonical, ..., "%04x:%02x:%02x.%d", ...)` and
    `strcmp(name, canonical) == 0`. Returns `-EINVAL` if it differs.
- The canonical composition uses `PCI_SLOT(PCI_DEVFN(slot, function))`
  and `PCI_FUNC(...)`, implicitly constraining slot/function to valid
  bit widths and preventing odd encodings from slipping through.
- The change is localized to group creation in
  drivers/gpu/drm/xe/xe_configfs.c:256 (the function where the new
  `canonical` buffer, `scnprintf`, and `strcmp` checks are added).

**User Impact Fixed**
- Prevents creating “broken” directories like 0000:00:02.0000000000000,
  0000:00:02.0x, 0:0:2.0, or with spaces/uppercase hex. Previously these
  were accepted, but the driver’s later lookup (by canonical name) would
  not find them, so configfs settings were ignored.
- With this patch, such inputs fail fast with `-EINVAL` instead of
  failing silently later.

**Risk and Scope**
- Small, contained change to a single function in the Xe configfs code;
  no architectural changes or core subsystem impact.
- Maintains existing error behavior for non-existent devices (`-ENODEV`
  remains).
- Behavior change is strictly tighter validation; correct canonical
  names continue to work. Scripts relying on non-canonical names would
  not have worked reliably anyway (lookups failed), so rejecting them is
  safer.

**Stable Backport Considerations**
- Meets stable criteria: fixes a real user-visible misbehavior (silent
  no-op config), minimal patch size, confined to the Xe driver’s
  configfs path, no API/ABI changes.
- No dependency on other features; headers used (`string.h`, PCI macros)
  are already present. Applies cleanly around
  drivers/gpu/drm/xe/xe_configfs.c:256-266.
- Applicable to stable series that include Xe configfs (the file and
  functions present in this tree: drivers/gpu/drm/xe/xe_configfs.c:256
  and drivers/gpu/drm/xe/xe_configfs.c:310).

In summary, this is a low-risk input validation fix that prevents silent
misconfiguration and aligns behavior with documented usage. It is
suitable for backporting to stable kernels that ship the Xe configfs
interface.

 drivers/gpu/drm/xe/xe_configfs.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/xe/xe_configfs.c b/drivers/gpu/drm/xe/xe_configfs.c
index 58c1f397c68c9..797508cc6eb17 100644
--- a/drivers/gpu/drm/xe/xe_configfs.c
+++ b/drivers/gpu/drm/xe/xe_configfs.c
@@ -259,12 +259,19 @@ static struct config_group *xe_config_make_device_group(struct config_group *gro
 	unsigned int domain, bus, slot, function;
 	struct xe_config_device *dev;
 	struct pci_dev *pdev;
+	char canonical[16];
 	int ret;
 
-	ret = sscanf(name, "%04x:%02x:%02x.%x", &domain, &bus, &slot, &function);
+	ret = sscanf(name, "%x:%x:%x.%x", &domain, &bus, &slot, &function);
 	if (ret != 4)
 		return ERR_PTR(-EINVAL);
 
+	ret = scnprintf(canonical, sizeof(canonical), "%04x:%02x:%02x.%d", domain, bus,
+			PCI_SLOT(PCI_DEVFN(slot, function)),
+			PCI_FUNC(PCI_DEVFN(slot, function)));
+	if (ret != 12 || strcmp(name, canonical))
+		return ERR_PTR(-EINVAL);
+
 	pdev = pci_get_domain_bus_and_slot(domain, bus, PCI_DEVFN(slot, function));
 	if (!pdev)
 		return ERR_PTR(-ENODEV);
-- 
2.51.0


  parent reply	other threads:[~2025-10-25 16:14 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <20251025160905.3857885-1-sashal@kernel.org>
2025-10-25 15:54 ` [PATCH AUTOSEL 6.17] drm/xe/pcode: Initialize data0 for pcode read routine Sasha Levin
2025-10-25 15:54 ` [PATCH AUTOSEL 6.17] drm/xe: improve dma-resv handling for backup object Sasha Levin
2025-10-25 15:54 ` [PATCH AUTOSEL 6.17] drm/xe: Extend wa_13012615864 to additional Xe2 and Xe3 platforms Sasha Levin
2025-10-25 15:54 ` [PATCH AUTOSEL 6.17] drm/xe/ptl: Apply Wa_16026007364 Sasha Levin
2025-10-25 15:55 ` [PATCH AUTOSEL 6.17] drm/xe: Set GT as wedged before sending wedged uevent Sasha Levin
2025-10-25 15:55 ` [PATCH AUTOSEL 6.17] drm/xe/i2c: Enable bus mastering Sasha Levin
2025-10-25 15:55 ` Sasha Levin [this message]
2025-10-25 15:56 ` [PATCH AUTOSEL 6.17] drm/xe: Extend Wa_22021007897 to Xe3 platforms Sasha Levin
2025-10-25 15:56 ` [PATCH AUTOSEL 6.17] drm/xe: Cancel pending TLB inval workers on teardown Sasha Levin
2025-10-25 15:57 ` [PATCH AUTOSEL 6.17-6.12] drm/xe/guc: Increase GuC crash dump buffer size Sasha Levin
2025-10-25 15:57 ` [PATCH AUTOSEL 6.17] drm/xe/wcl: Extend L3bank mask workaround Sasha Levin
2025-10-25 15:57 ` [PATCH AUTOSEL 6.17-6.12] drm/xe/guc: Set upper limit of H2G retries over CTB Sasha Levin
2025-10-25 15:57 ` [PATCH AUTOSEL 6.17] drm/xe: Make page size consistent in loop Sasha Levin
2025-10-25 15:57 ` [PATCH AUTOSEL 6.17] drm/xe/guc: Add devm release action to safely tear down CT Sasha Levin
2025-10-25 15:57 ` [PATCH AUTOSEL 6.17] drm/xe/pf: Program LMTT directory pointer on all GTs within a tile Sasha Levin
2025-10-25 15:58 ` [PATCH AUTOSEL 6.17] drm/xe/guc: Always add CT disable action during second init step Sasha Levin
2025-10-25 15:58 ` [PATCH AUTOSEL 6.17] drm/xe/pf: Don't resume device from restart worker Sasha Levin
2025-10-25 15:59 ` [PATCH AUTOSEL 6.17-6.12] drm/xe/guc: Return an error code if the GuC load fails Sasha Levin
2025-10-25 15:59 ` [PATCH AUTOSEL 6.17] drm/xe: Ensure GT is in C0 during resumes Sasha Levin
2025-10-25 15:59 ` [PATCH AUTOSEL 6.17] drm/xe: rework PDE PAT index selection Sasha Levin
2025-10-25 16:01 ` [PATCH AUTOSEL 6.17-6.12] drm/xe/guc: Add more GuC load error status codes Sasha Levin
2025-10-25 16:01 ` [PATCH AUTOSEL 6.17-6.12] drm/xe: Fix oops in xe_gem_fault when running core_hotunplug test Sasha Levin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20251025160905.3857885-119-sashal@kernel.org \
    --to=sashal@kernel.org \
    --cc=intel-xe@lists.freedesktop.org \
    --cc=lucas.demarchi@intel.com \
    --cc=michal.wajdeczko@intel.com \
    --cc=patches@lists.linux.dev \
    --cc=rodrigo.vivi@intel.com \
    --cc=stable@vger.kernel.org \
    --cc=thomas.hellstrom@linux.intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox