6.18.50-rc1 review

6.18-stable review patch. If anyone has any objections, please let me know.

------------------

From: Max Lee <[email protected]>

commit ec3d987fcaf92516d13ee18c305c82281557046d upstream.

The Realtek RTS525A PCIe card reader reports an AER Correctable Replay Timer Timeout storm when ASPM L0s is enabled on its link. On an affected HP ZBook Power 16 inch G11, the Root Port received tens of millions of AER interrupts from the RTS525A even when the rtsx_pci driver was blacklisted and the endpoint was not enabled by a driver.

For example:

pcieport 0000:00:1c.6: AER: Multiple Correctable error message received from 0000:58:00.0 rtsx_pci 0000:58:00.0: PCIe Bus Error: severity=Correctable, type=Data Link Layer, (Transmitter ID)
rtsx_pci 0000:58:00.0: device [10ec:525a] error status/mask=00001000/00006000
rtsx_pci 0000:58:00.0: [12] Timeout
pcieport 0000:00:1c.6: AER: Correctable error message received from 0000:58:00.0

Testing with OS-native AER control showed that disabling only L0s on the RTS525A link stops new AER interrupt and counter growth while leaving L1 enabled. Disabling L1, L1 substates, or Clock PM alone did not stop the storm.

Prevent the broken L0s configuration by removing L0s from the RTS525A advertised ASPM capability. This avoids enabling the non-working ASPM state instead of masking the resulting AER Replay Timer Timeout reports.

---
 drivers/pci/quirks.c |    3 +++
 1 file changed, 3 insertions(+)

@@ -2506,6 +2506,9 @@ DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_I
 DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x10f4, quirk_disable_aspm_l0s);
 DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x1508, quirk_disable_aspm_l0s);
 
+/* Realtek RTS525A generates a Replay Timer Timeout storm when L0s is enabled. */
+DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_REALTEK, 0x525a, quirk_disable_aspm_l0s);
+
 static void quirk_disable_aspm_l0s_l1(struct pci_dev *dev)
 {
 	pcie_aspm_remove_cap(dev,

6.18-stable review patch. If anyone has any objections, please let me know.

------------------

From: Darshit Shah <[email protected]>

commit 97ca178c899d0049210d325b123ed024eb5ac000 upstream.

PCIe r7.0, sec 6.2.11, "Implementation Note: Determination of DPC Control", recommends that "... operating systems always link control of DPC to the control of Advanced Error Reporting."

Any PCIe device may advertise AER, but only Root Ports and Root Complex Event Collectors can generate AER interrupts, so the AER driver only binds to RPs and RCECs.

Any Root Port or Switch Downstream Port may advertise Downstream Port Containment (DPC), but previously the DPC driver was limited to devices the AER driver could bind to, i.e., only RPs that advertised AER.

Since any Port with DPC can generate DPC interrupts, allow the DPC driver to bind to such a Port as long as the OS controls AER, regardless of whether the AER driver binds to it.

---
 drivers/pci/pcie/portdrv.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

@@ -264,7 +264,7 @@ static int get_port_device_capability(st
 	 */
 	if (pci_find_ext_capability(dev, PCI_EXT_CAP_ID_DPC) &&
 	    pci_aer_available() &&
-	    (pcie_ports_dpc_native || (services & PCIE_PORT_SERVICE_AER)))
+	    (pcie_ports_dpc_native || host->native_aer))
 		services |= PCIE_PORT_SERVICE_DPC;
 
 	/* Enable bandwidth control if more than one speed is supported. */

6.18-stable review patch. If anyone has any objections, please let me know.

------------------

From: Farhan Ali <[email protected]>

commit 231c7a57d19304beb0931e6cbe3a4929daf49747 upstream.

The current MSI-X restoration path assumes the Command register Memory bit is enabled when writing MSI-X messages. But it's possible the last saved and restored state of a device may not have the Memory bit enabled, even if a device driver later enables Memory bit and MSI-X. Attempting to access Memory space without Memory bit enabled can lead to Unsupported Request (UR) from the device. Fix this by enabling Memory bit and restore it afterwards.

---
 drivers/pci/msi/msi.c |   10 ++++++++++
 1 file changed, 10 insertions(+)

@@ -860,6 +860,7 @@ void __pci_restore_msix_state(struct pci
 {
 	struct msi_desc *entry;
 	bool write_msg;
+	u16 cmd;
 
 	if (!dev->msix_enabled)
 		return;

@@ -869,6 +870,14 @@ void __pci_restore_msix_state(struct pci
 	pci_msix_clear_and_set_ctrl(dev, 0,
 				PCI_MSIX_FLAGS_ENABLE | PCI_MSIX_FLAGS_MASKALL);
 
+	/*
+	 * The restored device state may not have Memory Space enabled.
+	 * Since the MSI-X Table and PBA are in Memory Space, enable it
+	 * while restoring them.
+	 */
+	pci_read_config_word(dev, PCI_COMMAND, &cmd);
+	pci_write_config_word(dev, PCI_COMMAND, cmd | PCI_COMMAND_MEMORY);
+
 	write_msg = arch_restore_msi_irqs(dev);
 
 	scoped_guard (msi_descs_lock, &dev->dev) {

@@ -879,6 +888,7 @@ void __pci_restore_msix_state(struct pci
 		}
 	}
 
+	pci_write_config_word(dev, PCI_COMMAND, cmd);
 	pci_msix_clear_and_set_ctrl(dev, PCI_MSIX_FLAGS_MASKALL, 0);
 }
 

6.18-stable review patch. If anyone has any objections, please let me know.

------------------

From: Krzysztof Wilczyński <[email protected]>

commit 4ff664a81d729b37f2eb65de80a670abfb61c9a0 upstream.

Currently, proc_bus_pci_read() and proc_bus_pci_write() do not return early for zero-length configuration space accesses at valid offsets.

Such an access invokes pci_config_pm_runtime_get() and pci_config_pm_runtime_put() around transfer blocks that do nothing.

This is a problem because pci_config_pm_runtime_get() synchronously resumes the upstream bridge through pm_runtime_get_sync(), and resumes the device itself through pm_runtime_resume() when it is in D3cold, only for the handler to return zero immediately afterwards. Such a spurious wakeup wastes power and adds needless resume latency.

The sysfs core already returns early for in-range zero-length binary attribute accesses before pci_read_config() or pci_write_config() is invoked. In contrast, the VFS forwards zero-length requests to the procfs callbacks, where they continue into runtime PM handling.

Return early from proc_bus_pci_read() and proc_bus_pci_write() when nbytes is zero, before any runtime PM involvement.

The value returned to userspace at these offsets remains zero, so the change is not visible to userspace.

---
 drivers/pci/proc.c |    6 ++++++
 1 file changed, 6 insertions(+)

@@ -45,6 +45,9 @@ static ssize_t proc_bus_pci_read(struct
 	else
 		size = 64;
 
+	if (!nbytes)
+		return 0;
+
 	if (pos >= size)
 		return 0;
 	if (nbytes >= size)

@@ -121,6 +124,9 @@ static ssize_t proc_bus_pci_write(struct
 	if (ret)
 		return ret;
 
+	if (!nbytes)
+		return 0;
+
 	if (pos >= size)
 		return 0;
 	if (nbytes >= size)

6.18-stable review patch. If anyone has any objections, please let me know.

------------------

From: Krzysztof Wilczyński <[email protected]>

commit f82f53e75eff382fc8f56b73279b54f7cf5a5c65 upstream.

proc_bus_pci_read() decides how much of the config space is readable based on capable(CAP_SYS_ADMIN), which checks the credentials of the task calling read(), not the credentials of the process that opened the file.

The sysfs equivalent, pci_read_config(), has checked the credentials of the opening process since commit de139a339395 ("pci: check caps from sysfs file open to read device dependent config space"), so a privileged process can open the config space file and pass the file descriptor to an unprivileged process (for example, a process running a KVM guest with an assigned device), which can then read the entire config space. The check was subsequently routed through the LSM framework in commit 47970b1b2aa6 ("pci:
use security_capable() when checking capablities during config space read") and converted to the dedicated helper in commit ab0fa82b2df9 ("pci-sysfs:
use proper file capability helper function").

Thus, the two interfaces check the same capability against different credentials. Checking the credentials of the task calling read() makes the outcome depend on who reads rather than who opened, so the restriction is bypassed whenever a more privileged process reads through the descriptor. Checking the credentials recorded in file->f_cred settles the decision at open() time and ties it to the file, where it cannot change with the caller.

Use file_ns_capable() to check CAP_SYS_ADMIN against the credentials in effect when the file was opened, bringing the procfs interface in line with the sysfs behaviour.

As a result, a file descriptor opened by a privileged process and passed to an unprivileged one now allows the entire config space to be read through procfs, matching sysfs.

---
 drivers/pci/proc.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

@@ -38,7 +38,7 @@ static ssize_t proc_bus_pci_read(struct
 	 * undefined locations (think of Intel PIIX4 as a typical example).
 	 */
 
-	if (capable(CAP_SYS_ADMIN))
+	if (file_ns_capable(file, &init_user_ns, CAP_SYS_ADMIN))
 		size = dev->cfg_size;
 	else if (dev->hdr_type == PCI_HEADER_TYPE_CARDBUS)
 		size = 128;

6.18-stable review patch. If anyone has any objections, please let me know.

------------------

From: Krzysztof Wilczyński <[email protected]>

commit 3359e044d597dd5344f17613e4be6b6e12067f60 upstream.

Currently, a driver can claim a region of a device's config space as exclusive using pci_request_config_region_exclusive(), after which a write to that region originating from user space is expected to emit a warning and taint the kernel. The check is advisory only, as the write itself is still allowed to proceed.

Since commit 278294798ac9 ("PCI: Allow drivers to request exclusive config regions"), the sysfs config space attribute performs this check in pci_write_config(), but the procfs interface was never updated. A write performed through /proc/bus/pci/BB/DD.F therefore bypasses the detection entirely, even though both interfaces offer the same level of access.

Add the same resource_is_exclusive() check to proc_bus_pci_write().

---
 drivers/pci/proc.c |    8 ++++++++
 1 file changed, 8 insertions(+)

@@ -14,6 +14,8 @@
 #include <linux/capability.h>
 #include <linux/uaccess.h>
 #include <linux/security.h>
+#include <linux/panic.h>
+#include <linux/sched.h>
 #include <asm/byteorder.h>
 #include "pci.h"
 

@@ -127,6 +129,12 @@ static ssize_t proc_bus_pci_write(struct
 	if (!nbytes)
 		return 0;
 
+	if (resource_is_exclusive(&dev->driver_exclusive_resource, pos, nbytes)) {
+		pci_warn_once(dev, "%s: Unexpected write to kernel-exclusive config offset %x",
+			      current->comm, pos);
+		add_taint(TAINT_USER, LOCKDEP_STILL_OK);
+	}
+
 	if (pos >= size)
 		return 0;
 	if (nbytes >= size)

6.18-stable review patch. If anyone has any objections, please let me know.

------------------

From: Shuai Xue <[email protected]>

commit af3b69b16383fbc8fe5f61b5b0150d2e41ede71f upstream.

iommu_call_iopf_notifier() looks up the requester with pci_get_domain_bus_and_slot(), which returns a PCI device with its reference count incremented.

Neither the successful iommu_report_device_fault() path nor the abort path drops that reference, so every handled PPR request leaks a PCI device reference.

This is the same ownership rule that was fixed for the old iommu_v2 ppr_notifier() path by commit 6cf0981c2233 ("iommu/amd: Fix pci device refcount leak in ppr_notifier()"), but iommu_call_iopf_notifier() was added later as a separate PPR/IOPF notifier path.

Drop the PCI device reference after handling the PPR entry.

---
 drivers/iommu/amd/ppr.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

@@ -151,7 +151,7 @@ static void iommu_call_iopf_notifier(str
 
 	/* Submit event */
 	iommu_report_device_fault(&pdev->dev, &event);
-
+	pci_dev_put(pdev);
 	return;
 
 out:

@@ -159,6 +159,7 @@ out:
 	amd_iommu_complete_ppr(&pdev->dev, PPR_PASID(raw[0]),
 			       IOMMU_PAGE_RESP_FAILURE,
 			       PPR_TAG(raw[0]) & 0x1FF);
+	pci_dev_put(pdev);
 }
 
 void amd_iommu_poll_ppr_log(struct amd_iommu *iommu)

6.18-stable review patch. If anyone has any objections, please let me know.

------------------

From: Weimin Xiong <[email protected]>

commit 535a200220ca2c83bc8bf54bd2cbe045d6ee70c4 upstream.

msm_iommu_probe() adds its devm-managed IOMMU object to qcom_iommu_devices before adding the IOMMU sysfs device and registering it with the IOMMU core.

If iommu_device_sysfs_add() fails, probe returns with the object still on qcom_iommu_devices. The driver core then releases the devm allocation, leaving a dangling list entry that later list walks may dereference.

If iommu_device_register() fails, the same dangling list entry remains and the sysfs device is left registered as well.

Unwind the sysfs device and global list entry in reverse setup order on the corresponding failure paths.

---
 drivers/iommu/msm_iommu.c |   10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

@@ -783,19 +783,25 @@ static int msm_iommu_probe(struct platfo
 				     "msm-smmu.%pa", &ioaddr);
 	if (ret) {
 		pr_err("Could not add msm-smmu at %pa to sysfs\n", &ioaddr);
-		return ret;
+		goto err_remove_list;
 	}
 
 	ret = iommu_device_register(&iommu->iommu, &msm_iommu_ops, &pdev->dev);
 	if (ret) {
 		pr_err("Could not register msm-smmu at %pa\n", &ioaddr);
-		return ret;
+		goto err_remove_sysfs;
 	}
 
 	pr_info("device mapped at %p, irq %d with %d ctx banks\n",
 		iommu->base, iommu->irq, iommu->ncb);
 
 	return ret;
+
+err_remove_sysfs:
+	iommu_device_sysfs_remove(&iommu->iommu);
+err_remove_list:
+	list_del(&iommu->dev_node);
+	return ret;
 }
 
 static const struct of_device_id msm_iommu_dt_match[] = {

6.18-stable review patch. If anyone has any objections, please let me know.

------------------

From: Shuai Xue <[email protected]>

commit 530f8f9c3546cb3ebee1b135375aaee08a073ebb upstream.

iommu_attach_device_pasid() installs the new SVA attach handle in the group PASID lookup before iommu_sva_bind_device() returns. A concurrent bind can therefore find and reuse the same handle after iommu_sva_lock is dropped.

handle->dev was initialized after dropping iommu_sva_lock. This leaves a window where a racing bind can return a handle whose dev pointer is still NULL. A subsequent iommu_sva_unbind_device() can then dereference it via handle->dev->iommu_group.

Initialize handle->dev before releasing iommu_sva_lock so any visible SVA handle is fully initialized.

---
 drivers/iommu/iommu-sva.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

@@ -145,8 +145,8 @@ struct iommu_sva *iommu_sva_bind_device(
 	list_add(&domain->next, &iommu_mm->sva_domains);
 out:
 	refcount_set(&handle->users, 1);
-	mutex_unlock(&iommu_sva_lock);
 	handle->dev = dev;
+	mutex_unlock(&iommu_sva_lock);
 	return handle;
 
 out_free_domain:

6.18-stable review patch. If anyone has any objections, please let me know.

------------------

From: Nicolin Chen <[email protected]>

commit 4379610c79bd88ddbea10e7f6c21e16d4b338c6b upstream.

tegra241_vintf_init_vsid() programs the guest-provided vSID into SID_MATCH, whose VIRT_SID field spans bits [20:1] with bit 0 as the match-enable flag. The HW therefore matches only a 20-bit Stream ID.

The bound check rejects only virt_sid > UINT_MAX, which admits a value far wider than the field. The write "virt_sid << 1 | 0x1" then drops every bit above 20: a virt_sid of 0x80000000 lands as SID_MATCH = 0x1, a valid match on vSID 0, so the entry aliases the wrong Stream ID. Because vdev->virt_id is guest-controlled, a VMM can trigger it.

Validate virt_sid against the field width with FIELD_MAX(), and program the register with FIELD_PREP() so the value and the field stay consistent.

---
 drivers/iommu/arm/arm-smmu-v3/tegra241-cmdqv.c |    8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

@@ -58,6 +58,8 @@
 #define  VINTF_ENABLED			BIT(0)
 
 #define TEGRA241_VINTF_SID_MATCH(s)	(0x0040 + 0x4*(s))
+#define  VINTF_SID_MATCH_VIRT_SID	GENMASK(20, 1)
+#define  VINTF_SID_MATCH_ENABLE		BIT(0)
 #define TEGRA241_VINTF_SID_REPLACE(s)	(0x0080 + 0x4*(s))
 
 #define TEGRA241_VINTF_LVCMDQ_ERR_MAP_64(m) \

@@ -1249,7 +1251,7 @@ static int tegra241_vintf_init_vsid(stru
 	u64 virt_sid = vdev->virt_id;
 	int sidx;
 
-	if (virt_sid > UINT_MAX)
+	if (virt_sid > FIELD_MAX(VINTF_SID_MATCH_VIRT_SID))
 		return -EINVAL;
 
 	WARN_ON_ONCE(master->num_streams != 1);

@@ -1261,7 +1263,9 @@ static int tegra241_vintf_init_vsid(stru
 		return sidx;
 
 	writel(stream->id, REG_VINTF(vintf, SID_REPLACE(sidx)));
-	writel(virt_sid << 1 | 0x1, REG_VINTF(vintf, SID_MATCH(sidx)));
+	writel(FIELD_PREP(VINTF_SID_MATCH_VIRT_SID, virt_sid) |
+		       VINTF_SID_MATCH_ENABLE,
+	       REG_VINTF(vintf, SID_MATCH(sidx)));
 	dev_dbg(vintf->cmdqv->dev,
 		"VINTF%u: allocated SID_REPLACE%d for pSID=%x, vSID=%x\n",
 		vintf->idx, sidx, stream->id, (u32)virt_sid);

6.18-stable review patch. If anyone has any objections, please let me know.

------------------

From: Shameer Kolothum <[email protected]>

commit 2bd22a0d40503a65d243b011de146603c8ce1cbc upstream.

arm_smmu_device_remove() manually frees the IOPF queue, destroys the vmid_map and disables the device, while the IRQs and queues are devm managed. devm unwinds only after remove() returns, so the cleanup runs in the wrong order. The IOPF queue is freed before the event-queue IRQ whose handler uses it.

Manage all of it with devm so the unwind order is correct. Free the IOPF queue and vmid_map via devm actions, and disable the device from one registered after arm_smmu_device_reset().

This is also a prerequisite for fixing a Tegra241 CMDQV CMD_SYNC use-after-free in the subsequent patch.

---
 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c |   56 ++++++++++++++++++++--------
 1 file changed, 40 insertions(+), 16 deletions(-)

@@ -3837,6 +3837,20 @@ int arm_smmu_cmdq_init(struct arm_smmu_d
 	return 0;
 }
 
+static void arm_smmu_free_iopf_action(void *data)
+{
+	struct iopf_queue *queue = data;
+
+	iopf_queue_free(queue);
+}
+
+static void arm_smmu_destroy_vmid_map(void *data)
+{
+	struct ida *ida = data;
+
+	ida_destroy(ida);
+}
+
 static int arm_smmu_init_queues(struct arm_smmu_device *smmu)
 {
 	int ret;

@@ -3864,6 +3878,11 @@ static int arm_smmu_init_queues(struct a
 		smmu->evtq.iopf = iopf_queue_alloc(dev_name(smmu->dev));
 		if (!smmu->evtq.iopf)
 			return -ENOMEM;
+		ret = devm_add_action_or_reset(smmu->dev,
+					       arm_smmu_free_iopf_action,
+					       smmu->evtq.iopf);
+		if (ret)
+			return ret;
 	}
 
 	/* priq */

@@ -3942,7 +3961,8 @@ static int arm_smmu_init_strtab(struct a
 
 	ida_init(&smmu->vmid_map);
 
-	return 0;
+	return devm_add_action_or_reset(smmu->dev, arm_smmu_destroy_vmid_map,
+					&smmu->vmid_map);
 }
 
 static int arm_smmu_init_structures(struct arm_smmu_device *smmu)

@@ -4155,6 +4175,13 @@ static int arm_smmu_device_disable(struc
 	return ret;
 }
 
+static void arm_smmu_disable_action(void *data)
+{
+	struct arm_smmu_device *smmu = data;
+
+	arm_smmu_device_disable(smmu);
+}
+
 static void arm_smmu_write_strtab(struct arm_smmu_device *smmu)
 {
 	struct arm_smmu_strtab_cfg *cfg = &smmu->strtab_cfg;

@@ -4878,7 +4905,7 @@ static int arm_smmu_device_probe(struct
 	/* Initialise in-memory data structures */
 	ret = arm_smmu_init_structures(smmu);
 	if (ret)
-		goto err_free_iopf;
+		return ret;
 
 	/* Record our private device structure */
 	platform_set_drvdata(pdev, smmu);

@@ -4888,30 +4915,30 @@ static int arm_smmu_device_probe(struct
 
 	/* Reset the device */
 	ret = arm_smmu_device_reset(smmu);
+	if (ret) {
+		arm_smmu_device_disable(smmu);
+		return ret;
+	}
+
+	/* Register last so it unwinds first, while the CMDQ is still up. */
+	ret = devm_add_action_or_reset(smmu->dev, arm_smmu_disable_action, smmu);
 	if (ret)
-		goto err_disable;
+		return ret;
 
 	/* And we're up. Go go go! */
 	ret = iommu_device_sysfs_add(&smmu->iommu, dev, NULL,
 				     "smmu3.%pa", &ioaddr);
 	if (ret)
-		goto err_disable;
+		return ret;
 
 	ret = iommu_device_register(&smmu->iommu, &arm_smmu_ops, dev);
 	if (ret) {
 		dev_err(dev, "Failed to register iommu\n");
-		goto err_free_sysfs;
+		iommu_device_sysfs_remove(&smmu->iommu);
+		return ret;
 	}
 
 	return 0;
-
-err_free_sysfs:
-	iommu_device_sysfs_remove(&smmu->iommu);
-err_disable:
-	arm_smmu_device_disable(smmu);
-err_free_iopf:
-	iopf_queue_free(smmu->evtq.iopf);
-	return ret;
 }
 
 static void arm_smmu_device_remove(struct platform_device *pdev)

@@ -4920,9 +4947,6 @@ static void arm_smmu_device_remove(struc
 
 	iommu_device_unregister(&smmu->iommu);
 	iommu_device_sysfs_remove(&smmu->iommu);
-	arm_smmu_device_disable(smmu);
-	iopf_queue_free(smmu->evtq.iopf);
-	ida_destroy(&smmu->vmid_map);
 }
 
 static void arm_smmu_device_shutdown(struct platform_device *pdev)

6.18-stable review patch. If anyone has any objections, please let me know.

------------------

From: Peiyang He <[email protected]>

commit b7b0b3851474883d4aba6ed72da87141204b23e5 upstream.

iommu_mock_device_add() first calls iommu_fwspec_init(), which on success allocates both dev->iommu (via dev_iommu_get()) and dev->iommu->fwspec. If the subsequent device_add(dev) call fails, the error path only calls iommu_fwspec_free(dev), which frees fwspec but leaves dev->iommu still allocated.

This triggers the following kmemleak report when fuzzing with Syzkaller:

BUG: memory leak
unreferenced object 0xffff888011e0a200 (size 192):
  comm "syz.1.1695", pid 24885, jiffies 4295222527
  hex dump (first 32 bytes):
    00 00 00 00 00 00 00 00 00 00 00 00 ad 4e ad de  .............N..
    ff ff ff ff 00 00 00 00 ff ff ff ff ff ff ff ff  ................
  backtrace (crc 25df5bb3):
    kmemleak_alloc_recursive include/linux/kmemleak.h:44 [inline]
    slab_post_alloc_hook mm/slub.c:4575 [inline]
    slab_alloc_node mm/slub.c:4899 [inline]
    __kmalloc_cache_noprof+0x47a/0x710 mm/slub.c:5415
    kmalloc_noprof include/linux/slab.h:950 [inline]
    kzalloc_noprof include/linux/slab.h:1188 [inline]
    dev_iommu_get+0x10c/0x1a0 drivers/iommu/iommu.c:408
    iommu_fwspec_init+0x288/0x4d0 drivers/iommu/iommu.c:3087
    iommu_mock_device_add+0x46/0xb0 drivers/iommu/iommu.c:385
    mock_dev_create drivers/iommu/iommufd/selftest.c:1025 [inline]
    iommufd_test_mock_domain drivers/iommu/iommufd/selftest.c:1066 [inline]
    iommufd_test+0x2f8a/0x6190 drivers/iommu/iommufd/selftest.c:2072
    iommufd_fops_ioctl+0x367/0x540 drivers/iommu/iommufd/main.c:533
    vfs_ioctl fs/ioctl.c:51 [inline]
    __do_sys_ioctl fs/ioctl.c:597 [inline]
    __se_sys_ioctl fs/ioctl.c:583 [inline]
    __x64_sys_ioctl+0x18e/0x210 fs/ioctl.c:583
    do_syscall_x64 arch/x86/entry/syscall_64.c:63 [inline]
    do_syscall_64+0x116/0x800 arch/x86/entry/syscall_64.c:94
    entry_SYSCALL_64_after_hwframe+0x77/0x7f

Fix this by calling dev_iommu_free(dev) instead of iommu_fwspec_free(dev) in the device_add() failure path. dev_iommu_free() frees both fwspec and the outer dev_iommu struct and clears dev->iommu.

---
 drivers/iommu/iommu.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

@@ -360,7 +360,7 @@ int iommu_mock_device_add(struct device
 
 	rc = device_add(dev);
 	if (rc)
-		iommu_fwspec_free(dev);
+		dev_iommu_free(dev);
 	return rc;
 }
 EXPORT_SYMBOL_GPL(iommu_mock_device_add);

6.18-stable review patch. If anyone has any objections, please let me know.

------------------

From: Kevin Tian <[email protected]>

commit 219cc978d69ce9b538d0d73936c569d4ca5b0a24 upstream.

If user explicitly requests to disable iommu (via "iommu=off" or "intel_iommu=off"), there is no reason to force enabling it due to platform opt-in (for external-facing devices). User should be aware of any security implication of doing so.

"intel_iommu=off" implements this policy by setting no_platform_optin to skip platform opt-in in platform_optin_force_iommu().

However, "iommu=off" (no_iommu=1) doesn't set no_platform_optin hence is broken in this aspect:

- detect_intel_iommu() doesn't request ACS if no_iommu=1
- platform_optin_force_iommu() forces iommu on if external-facing devices exist and no_platform_optin is not set

This leads to a bad configuration with ACS disabled while DMA remapping is enabled.

Instead of setting no_platform_optin (will soon be removed) for no_iommu=1, directly check no_iommu in platform_optin_force_iommu().

---
 drivers/iommu/intel/iommu.c |    6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

@@ -2956,10 +2956,11 @@ static bool has_external_pci(void)
 
 static int __init platform_optin_force_iommu(void)
 {
-	if (!dmar_platform_optin() || no_platform_optin || !has_external_pci())
+	if (no_iommu || !dmar_platform_optin() || no_platform_optin ||
+	    !has_external_pci())
 		return 0;
 
-	if (no_iommu || dmar_disabled)
+	if (dmar_disabled)
 		pr_info("Intel-IOMMU force enabled due to platform opt in\n");
 
 	/*

@@ -2970,7 +2971,6 @@ static int __init platform_optin_force_i
 		iommu_set_default_passthrough(false);
 
 	dmar_disabled = 0;
-	no_iommu = 0;
 
 	return 1;
 }

6.18-stable review patch. If anyone has any objections, please let me know.

------------------

From: Kevin Tian <[email protected]>

commit 607432b2618b61df81134be0ef2562b8300c1216 upstream.

Currently the conditions of requesting ACS in detect_intel_iommu() don't include tboot, leading to a possible misconfiguration with ACS disabled (e.g. due to user opts) while iommu is later forced on by tboot_force_iommu().

Fix it by checking tboot in detect_intel_iommu().

---
 drivers/iommu/intel/dmar.c  |   15 +++++++++++++--
 drivers/iommu/intel/iommu.c |    2 +-
 drivers/iommu/intel/iommu.h |    2 ++
 3 files changed, 16 insertions(+), 3 deletions(-)

@@ -915,6 +915,18 @@ dmar_validate_one_drhd(struct acpi_dmar_
 	return 0;
 }
 
+static bool dmar_required(void)
+{
+	/* tboot supersedes any user/platform opt */
+	if (!intel_iommu_tboot_noforce && tboot_enabled())
+		return true;
+
+	if (!no_iommu && (!dmar_disabled || dmar_platform_optin()))
+		return true;
+
+	return false;
+}
+
 void __init detect_intel_iommu(void)
 {
 	int ret;

@@ -928,8 +940,7 @@ void __init detect_intel_iommu(void)
 	if (!ret)
 		ret = dmar_walk_dmar_table((struct acpi_table_dmar *)dmar_tbl,
 					   &validate_drhd_cb);
-	if (!ret && !no_iommu && !iommu_detected &&
-	    (!dmar_disabled || dmar_platform_optin())) {
+	if (!ret && !iommu_detected && dmar_required()) {
 		iommu_detected = 1;
 		/* Make sure ACS will be enabled */
 		pci_request_acs();

@@ -64,7 +64,7 @@ static int rwbf_quirk;
  * (used when kernel is launched w/ TXT)
  */
 static int force_on = 0;
-static int intel_iommu_tboot_noforce;
+int intel_iommu_tboot_noforce;
 static int no_platform_optin;
 
 #define ROOT_ENTRY_NR (VTD_PAGE_SIZE/sizeof(struct root_entry))

@@ -1434,6 +1434,7 @@ static inline bool ecmd_has_pmu_essentia
 
 extern int dmar_disabled;
 extern int intel_iommu_enabled;
+extern int intel_iommu_tboot_noforce;
 #else
 static inline int iommu_calculate_agaw(struct intel_iommu *iommu)
 {

@@ -1446,6 +1447,7 @@ static inline int iommu_calculate_max_sa
 #define dmar_disabled	(1)
 #define intel_iommu_enabled (0)
 #define intel_iommu_sm (0)
+#define intel_iommu_tboot_noforce (0)
 #endif
 
 static inline const char *decode_prq_descriptor(char *str, size_t size,

6.18-stable review patch. If anyone has any objections, please let me know.

------------------

From: Shuai Xue <[email protected]>

commit 0dbcdf4473a614adbd732d567c9b39ac0e040e0c upstream.

iommufd_access_notify_unmap() skips internal accesses because they do not have an external unmap callback to invoke.

However, the current test calls iommufd_lock_obj() before checking whether the access is internal. If iommufd_lock_obj() succeeds, the loop then sees the internal access and continues, bypassing the matching iommufd_put_object() used by the normal unmap path. This leaks the object reference taken by iommufd_lock_obj().

Check for internal accesses first so skipped entries are never locked.

---
 drivers/iommu/iommufd/device.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

index 170a7005f0bc..e4537db724d5 100644
@@ -1307,8 +1307,8 @@ void iommufd_access_notify_unmap(struct io_pagetable *iopt, unsigned long iova,
 
 	xa_lock(&ioas->iopt.access_list);
 	xa_for_each(&ioas->iopt.access_list, index, access) {
-		if (!iommufd_lock_obj(&access->obj) ||
-		    iommufd_access_is_internal(access))
+		if (iommufd_access_is_internal(access) ||
+		    !iommufd_lock_obj(&access->obj))
 			continue;
 		xa_unlock(&ioas->iopt.access_list);
 

2.55.0

6.18-stable review patch. If anyone has any objections, please let me know.

------------------

From: Shuai Xue <[email protected]>

commit 4ac2ce123824d5f885c868fa1f9f4d463141a2ba upstream.

iommufd_take_all_iova_rwsem() takes an object reference and the iova_rwsem write lock before storing the IOAS in the temporary ioas_list xarray.

If xa_store() fails, the current IOAS has not been inserted into ioas_list yet. iommufd_release_all_iova_rwsem() only unwinds IOAS objects already present in that xarray, so it cannot release the current IOAS.

Release the current IOAS rwsem and object reference before unwinding the previously stored entries.

---
 drivers/iommu/iommufd/ioas.c |    2 ++
 1 file changed, 2 insertions(+)

@@ -433,6 +433,8 @@ static int iommufd_take_all_iova_rwsem(s
 
 		rc = xa_err(xa_store(ioas_list, index, ioas, GFP_KERNEL));
 		if (rc) {
+			up_write(&ioas->iopt.iova_rwsem);
+			refcount_dec(&ioas->obj.users);
 			iommufd_release_all_iova_rwsem(ictx, ioas_list);
 			return rc;
 		}

6.18-stable review patch. If anyone has any objections, please let me know.

------------------

From: Peiyang He <[email protected]>

commit 8c07df7cdfcf52f1ff276c588612aabc6c6b8399 upstream.

IOMMUFD selftest TRIGGER_IOPF borrows an attach handle from group->pasid_array without synchronizing against PASID detach, then a concurrent iommu_report_device_fault() can dereference that borrowed handle's domain pointer after the detach erases the handle and frees the backing struct iommufd_attach_handle. TRIGGER_IOPF then dereferences the freed handle, causing a UAF.

Fix by adding a iopf_rwsem in mock_dev to follow the expected design of a real driver. Hold its read side across the whole iommu_report_device_fault() call, and its write side around every path that attaches, detaches, or replaces a device domain. This can block new reports and drains in-flight reports before an old attach handle or the IOPF fault parameter can be removed. Also take the write side while registering a mock device, since it can invoke the mock driver's default-domain attach callback.

---
 drivers/iommu/iommufd/selftest.c |   31 ++++++++++++++++++++++++++++++-
 1 file changed, 30 insertions(+), 1 deletion(-)

@@ -183,6 +183,7 @@ struct mock_dev {
 	struct device dev;
 	struct mock_viommu *viommu;
 	struct rw_semaphore viommu_rwsem;
+	struct rw_semaphore iopf_rwsem;
 	unsigned long flags;
 	unsigned long vdev_id;
 	int id;

@@ -1101,6 +1102,7 @@ static struct mock_dev *mock_dev_create(
 		return ERR_PTR(-ENOMEM);
 
 	init_rwsem(&mdev->viommu_rwsem);
+	init_rwsem(&mdev->iopf_rwsem);
 	device_initialize(&mdev->dev);
 	mdev->flags = dev_flags;
 	mdev->dev.release = mock_dev_release;

@@ -1126,7 +1128,9 @@ static struct mock_dev *mock_dev_create(
 		goto err_put;
 	}
 
+	down_write(&mdev->iopf_rwsem);
 	rc = iommu_mock_device_add(&mdev->dev, &mock_iommu.iommu_dev);
+	up_write(&mdev->iopf_rwsem);
 	if (rc)
 		goto err_put;
 	return mdev;

@@ -1181,7 +1185,9 @@ static int iommufd_test_mock_domain(stru
 	}
 	sobj->idev.idev = idev;
 
+	down_write(&sobj->idev.mock_dev->iopf_rwsem);
 	rc = iommufd_device_attach(idev, IOMMU_NO_PASID, &pt_id);
+	up_write(&sobj->idev.mock_dev->iopf_rwsem);
 	if (rc)
 		goto out_unbind;
 

@@ -1196,7 +1202,9 @@ static int iommufd_test_mock_domain(stru
 	return 0;
 
 out_detach:
+	down_write(&sobj->idev.mock_dev->iopf_rwsem);
 	iommufd_device_detach(idev, IOMMU_NO_PASID);
+	up_write(&sobj->idev.mock_dev->iopf_rwsem);
 out_unbind:
 	iommufd_device_unbind(idev);
 out_mdev:

@@ -1240,7 +1248,9 @@ static int iommufd_test_mock_domain_repl
 	if (IS_ERR(sobj))
 		return PTR_ERR(sobj);
 
+	down_write(&sobj->idev.mock_dev->iopf_rwsem);
 	rc = iommufd_device_replace(sobj->idev.idev, IOMMU_NO_PASID, &pt_id);
+	up_write(&sobj->idev.mock_dev->iopf_rwsem);
 	if (rc)
 		goto out_sobj;
 

@@ -1852,10 +1862,16 @@ static int iommufd_test_trigger_iopf(str
 {
 	struct iopf_fault event = {};
 	struct iommufd_device *idev;
+	struct mock_dev *mdev;
 
 	idev = iommufd_get_device(ucmd, cmd->trigger_iopf.dev_id);
 	if (IS_ERR(idev))
 		return PTR_ERR(idev);
+	if (!iommufd_selftest_is_mock_dev(idev->dev)) {
+		iommufd_put_object(ucmd->ictx, &idev->obj);
+		return -EINVAL;
+	}
+	mdev = to_mock_dev(idev->dev);
 
 	event.fault.prm.flags = IOMMU_FAULT_PAGE_REQUEST_LAST_PAGE;
 	if (cmd->trigger_iopf.pasid != IOMMU_NO_PASID)

@@ -1866,7 +1882,9 @@ static int iommufd_test_trigger_iopf(str
 	event.fault.prm.grpid = cmd->trigger_iopf.grpid;
 	event.fault.prm.perm = cmd->trigger_iopf.perm;
 
+	down_read(&mdev->iopf_rwsem);
 	iommu_report_device_fault(idev->dev, &event);
+	up_read(&mdev->iopf_rwsem);
 	iommufd_put_object(ucmd->ictx, &idev->obj);
 
 	return 0;

@@ -1974,14 +1992,19 @@ static int iommufd_test_pasid_attach(str
 	if (IS_ERR(sobj))
 		return PTR_ERR(sobj);
 
+	down_write(&sobj->idev.mock_dev->iopf_rwsem);
 	rc = iommufd_device_attach(sobj->idev.idev, cmd->pasid_attach.pasid,
 				   &cmd->pasid_attach.pt_id);
+	up_write(&sobj->idev.mock_dev->iopf_rwsem);
 	if (rc)
 		goto out_sobj;
 
 	rc = iommufd_ucmd_respond(ucmd, sizeof(*cmd));
-	if (rc)
+	if (rc) {
+		down_write(&sobj->idev.mock_dev->iopf_rwsem);
 		iommufd_device_detach(sobj->idev.idev, cmd->pasid_attach.pasid);
+		up_write(&sobj->idev.mock_dev->iopf_rwsem);
+	}
 
 out_sobj:
 	iommufd_put_object(ucmd->ictx, &sobj->obj);

@@ -1998,8 +2021,10 @@ static int iommufd_test_pasid_replace(st
 	if (IS_ERR(sobj))
 		return PTR_ERR(sobj);
 
+	down_write(&sobj->idev.mock_dev->iopf_rwsem);
 	rc = iommufd_device_replace(sobj->idev.idev, cmd->pasid_attach.pasid,
 				    &cmd->pasid_attach.pt_id);
+	up_write(&sobj->idev.mock_dev->iopf_rwsem);
 	if (rc)
 		goto out_sobj;
 

@@ -2019,7 +2044,9 @@ static int iommufd_test_pasid_detach(str
 	if (IS_ERR(sobj))
 		return PTR_ERR(sobj);
 
+	down_write(&sobj->idev.mock_dev->iopf_rwsem);
 	iommufd_device_detach(sobj->idev.idev, cmd->pasid_detach.pasid);
+	up_write(&sobj->idev.mock_dev->iopf_rwsem);
 	iommufd_put_object(ucmd->ictx, &sobj->obj);
 	return 0;
 }

@@ -2030,7 +2057,9 @@ void iommufd_selftest_destroy(struct iom
 
 	switch (sobj->type) {
 	case TYPE_IDEV:
+		down_write(&sobj->idev.mock_dev->iopf_rwsem);
 		iommufd_device_detach(sobj->idev.idev, IOMMU_NO_PASID);
+		up_write(&sobj->idev.mock_dev->iopf_rwsem);
 		iommufd_device_unbind(sobj->idev.idev);
 		mock_dev_destroy(sobj->idev.mock_dev);
 		break;

6.18-stable review patch. If anyone has any objections, please let me know.

------------------

From: HyeongJun An <[email protected]>

commit 83c80495e45eddf64c6525fb582d8db68f256b71 upstream.

set_attribute() populates the security area of the BIOS attribute request buffer with the current admin password via populate_security_buffer(), then dumps the whole request buffer with print_hex_dump_bytes(). This can expose the plaintext admin password in the kernel log.

The same issue was fixed for the password attribute path by commit d1a196e0a6dc ("platform/x86: dell-wmi-sysman: Don't hex dump plaintext password data"). Remove the remaining dump from the BIOS attribute path.

---
 drivers/platform/x86/dell/dell-wmi-sysman/biosattr-interface.c |    1 -
 1 file changed, 1 deletion(-)

@@ -84,7 +84,6 @@ int set_attribute(const char *a_name, co
 	if (ret < 0)
 		goto out;
 
-	print_hex_dump_bytes("set attribute data: ", DUMP_PREFIX_NONE, buffer, buffer_size);
 	ret = call_biosattributes_interface(wmi_priv.bios_attr_wdev,
 					    buffer, buffer_size,
 					    SETATTRIBUTE_METHOD_ID);

6.18-stable review patch. If anyone has any objections, please let me know.

------------------

From: HyeongJun An <[email protected]>

commit 80e0d353c86a9a168ad6d213f494796294381538 upstream.

isst_if_get_perf_level_mask() and isst_if_get_base_freq_mask() use the user-provided level as an index into perf_levels[] via _read_pp_level_info() and _read_bf_level_info(), but neither helper validates it first.

The adjacent level-info helpers reject levels above max_level before reading the same per-level register block. Add the same bounds checks to the mask helpers, and reject disabled SST-PP levels in isst_if_get_perf_level_mask() to match isst_if_get_perf_level_info().

This prevents out-of-bounds reads from the per-level offset table on invalid ioctl input.

---
 drivers/platform/x86/intel/speed_select_if/isst_tpmi_core.c |    9 +++++++++
 1 file changed, 9 insertions(+)

@@ -1260,6 +1260,12 @@ static int isst_if_get_perf_level_mask(v
 	if (!power_domain_info)
 		return -EINVAL;
 
+	if (cpumask.level > power_domain_info->max_level)
+		return -EINVAL;
+
+	if (!(power_domain_info->pp_header.level_en_mask & BIT(cpumask.level)))
+		return -EINVAL;
+
 	_read_pp_level_info("mask", mask, cpumask.level, SST_PP_INFO_2_OFFSET,
 			    SST_PP_RSLVD_CORE_MASK_START, SST_PP_RSLVD_CORE_MASK_WIDTH,
 			    SST_MUL_FACTOR_NONE)

@@ -1341,6 +1347,9 @@ static int isst_if_get_base_freq_mask(vo
 	if (!power_domain_info)
 		return -EINVAL;
 
+	if (cpumask.level > power_domain_info->max_level)
+		return -EINVAL;
+
 	_read_bf_level_info("BF-cpumask", mask, cpumask.level, SST_BF_INFO_1_OFFSET,
 			    P1_HI_CORE_MASK_START, P1_HI_CORE_MASK_WIDTH,
 			    SST_MUL_FACTOR_NONE)

6.18-stable review patch. If anyone has any objections, please let me know.

------------------

From: HyeongJun An <[email protected]>

commit a89f07db0cb95c54dac4a8406c79a04e44a73c3c upstream.

isst_if_clos_assoc() validates the user-supplied socket_id with 'socket_id > topology_max_packages()', but isst_common.sst_inst[] is allocated with topology_max_packages() entries, so the valid index range is [0, topology_max_packages()). The '>' comparison lets socket_id == topology_max_packages() pass and index one entry past the array.

In addition, isst_common.sst_inst[socket_id] is NULL for an in-range package that has no bound TPMI SST instance, and the pointer is used without a NULL check. Both the out-of-bounds entry and the NULL pointer are then dereferenced by map_partition_power_domain_id() and the following power_domain_info access.

Reject socket_id >= topology_max_packages() and a NULL sst_inst, matching the checks already performed by get_instance().

---
 drivers/platform/x86/intel/speed_select_if/isst_tpmi_core.c |    4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

@@ -729,7 +729,7 @@ static long isst_if_clos_assoc(void __us
 		if (copy_from_user(&clos_assoc, ptr, sizeof(clos_assoc)))
 			return -EFAULT;
 
-		if (clos_assoc.socket_id > topology_max_packages())
+		if (clos_assoc.socket_id >= topology_max_packages())
 			return -EINVAL;
 
 		cpu = clos_assoc.logical_cpu;

@@ -747,6 +747,8 @@ static long isst_if_clos_assoc(void __us
 		pkg_id = clos_assoc.socket_id;
 
 		sst_inst = isst_common.sst_inst[pkg_id];
+		if (!sst_inst)
+			return -EINVAL;
 
 		punit_id = map_partition_power_domain_id(sst_inst, punit_id, &part);
 		if (punit_id < 0)

6.18-stable review patch. If anyone has any objections, please let me know.

------------------

From: Fan Wu <[email protected]>

commit 57e5d877f898d5e5c9d672a77bb6bdd24f0d9bf5 upstream.

Disabling the device interrupt and freeing the IRQ prevents new card-detect work from being queued, but carddet_work already queued by the handler can still run after via_sd_remove() returns. via_sdc_card_detect() recovers the host through container_of() and dereferences its MMIO base; once remove() returns the host can be freed, so that work would touch freed memory.

Cancel carddet_work after freeing the IRQ and before cancelling finish_bh_work, which the card-detect handler can also queue. carddet_work can re-enable the interrupt through via_reset_pcictrl(); mask it again afterwards.

This issue was found by an in-house static analysis tool and confirmed by manual code review.

---
 drivers/mmc/host/via-sdmmc.c |    4 ++++
 1 file changed, 4 insertions(+)

@@ -1200,6 +1200,10 @@ static void via_sd_remove(struct pci_dev
 
 	free_irq(pcidev->irq, sdhost);
 
+	cancel_work_sync(&sdhost->carddet_work);
+	/* carddet_work may re-enable the interrupt via via_reset_pcictrl(). */
+	writeb(0x0, sdhost->pcictrl_mmiobase + VIA_CRDR_PCIINTCTRL);
+
 	timer_delete_sync(&sdhost->timer);
 
 	cancel_work_sync(&sdhost->finish_bh_work);

6.18-stable review patch. If anyone has any objections, please let me know.

------------------

From: Fan Wu <[email protected]>

commit 088eaa92fcebaa6b957ccf9635afdf39643a577d upstream.

request_irq() registers the SD card-detect interrupt and the probe enables it before mmc_add_host() runs. If mmc_add_host() fails, the error path only unmaps the registers and returns: the interrupt stays registered, so the handler keeps running against the host once it is freed. via_sdc_isr() dereferences sdhost and its MMIO base and schedules carddet_work, which via_sdc_card_detect() also runs against freed memory through its container_of() dereference.

Add a probe-error path that disables and frees the interrupt and cancels carddet_work before unmapping. carddet_work can re-enable the device interrupt via via_reset_pcictrl(), which restores PCIINTCTRL, so mask it again after cancelling the work.

This issue was found by an in-house static analysis tool and confirmed by manual code review.

---
 drivers/mmc/host/via-sdmmc.c |    8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

@@ -1154,10 +1154,16 @@ static int via_sd_probe(struct pci_dev *
 
 	ret = mmc_add_host(mmc);
 	if (ret)
-		goto unmap;
+		goto free_irq;
 
 	return 0;
 
+free_irq:
+	writeb(0x0, sdhost->pcictrl_mmiobase + VIA_CRDR_PCIINTCTRL);
+	free_irq(pcidev->irq, sdhost);
+	cancel_work_sync(&sdhost->carddet_work);
+	/* carddet_work may re-enable the interrupt via via_reset_pcictrl(). */
+	writeb(0x0, sdhost->pcictrl_mmiobase + VIA_CRDR_PCIINTCTRL);
 unmap:
 	iounmap(sdhost->mmiobase);
 release:

6.18-stable review patch. If anyone has any objections, please let me know.

------------------

From: Srinivas Pandruvada <[email protected]>

commit 3de2776e9d7073765c10c2326c2bda5926811ea6 upstream.

To be consistent with other places, add a NULL check for failed socket loading by checking isst_common.sst_inst[].

---
 drivers/platform/x86/intel/speed_select_if/isst_tpmi_core.c |    2 ++
 1 file changed, 2 insertions(+)

@@ -1380,6 +1380,8 @@ static int isst_if_get_tpmi_instance_cou
 		return -EINVAL;
 
 	sst_inst = isst_common.sst_inst[tpmi_inst.socket_id];
+	if (!sst_inst)
+		return -EINVAL;
 
 	tpmi_inst.count = isst_instance_count(sst_inst);
 

6.18-stable review patch. If anyone has any objections, please let me know.

------------------

From: Srinivas Pandruvada <[email protected]>

commit 0f377f2b47646abe6ec3616ae6a8670d9ff7eb86 upstream.

Currently only 2 features SST-TF and SST-BF are supported, so only allow bit 0 and bit 1.

---
 drivers/platform/x86/intel/speed_select_if/isst_tpmi_core.c |    4 ++++
 1 file changed, 4 insertions(+)

@@ -846,6 +846,7 @@ static long isst_if_clos_assoc(void __us
 
 #define SST_PP_FEATURE_STATE_START	8
 #define SST_PP_FEATURE_STATE_WIDTH	8
+#define SST_PP_FEATURE_STATE_VALID_MASK	GENMASK(1, 0)
 
 #define SST_BF_FEATURE_SUPPORTED_START	12
 #define SST_BF_FEATURE_SUPPORTED_WIDTH	1

@@ -996,6 +997,9 @@ static int isst_if_set_perf_feature(void
 	if (power_domain_info->write_blocked)
 		return -EPERM;
 
+	if (perf_feature.feature & ~SST_PP_FEATURE_STATE_VALID_MASK)
+		return -EINVAL;
+
 	_write_pp_info("perf_feature", perf_feature.feature, SST_PP_CONTROL_OFFSET,
 		       SST_PP_FEATURE_STATE_START, SST_PP_FEATURE_STATE_WIDTH,
 		       SST_MUL_FACTOR_NONE)

6.18-stable review patch. If anyone has any objections, please let me know.

------------------

From: Srinivas Pandruvada <[email protected]>

commit 9b9026943b19d06ebf520b1f4786621947cf43c8 upstream.

Add check for enabled levels only when reading MMIO. Some levels can be disabled by BIOS. If the level is not enabled, return an error.

Reset the enable and allowed level masks if there is a failure to add a perf level.

---
 drivers/platform/x86/intel/speed_select_if/isst_tpmi_core.c |   16 ++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)

@@ -336,8 +336,11 @@ static int sst_add_perf_profiles(struct
 	int i;
 
 	pd_info->perf_levels = devm_kcalloc(dev, levels, sizeof(struct perf_level), GFP_KERNEL);
-	if (!pd_info->perf_levels)
+	if (!pd_info->perf_levels) {
+		pd_info->pp_header.allowed_level_mask = 0;
+		pd_info->pp_header.level_en_mask = 0;
 		return 0;
+	}
 
 	pd_info->ratio_unit = pd_info->pp_header.ratio_unit;
 	pd_info->avx_levels = SST_MAX_AVX_LEVELS;

@@ -879,7 +882,7 @@ static int isst_if_get_perf_level(void _
 		      SST_PP_FEATURE_STATE_START, SST_PP_FEATURE_STATE_WIDTH, SST_MUL_FACTOR_NONE)
 	perf_level.enabled = !!(power_domain_info->sst_header.cap_mask & BIT(1));
 
-	level_mask = perf_level.level_mask;
+	level_mask = perf_level.level_mask & power_domain_info->pp_header.level_en_mask;
 	perf_level.sst_bf_support = 0;
 	for_each_set_bit(level, &level_mask, BITS_PER_BYTE) {
 		/*

@@ -1317,6 +1320,9 @@ static int isst_if_get_base_freq_info(vo
 	if (base_freq.level > power_domain_info->max_level)
 		return -EINVAL;
 
+	if (!(power_domain_info->pp_header.level_en_mask & BIT(base_freq.level)))
+		return -EINVAL;
+
 	_read_bf_level_info("p1_high", base_freq.high_base_freq_mhz, base_freq.level,
 			    SST_BF_INFO_0_OFFSET, SST_BF_P1_HIGH_START, SST_BF_P1_HIGH_WIDTH,
 			    SST_MUL_FACTOR_FREQ)

@@ -1356,6 +1362,9 @@ static int isst_if_get_base_freq_mask(vo
 	if (cpumask.level > power_domain_info->max_level)
 		return -EINVAL;
 
+	if (!(power_domain_info->pp_header.level_en_mask & BIT(cpumask.level)))
+		return -EINVAL;
+
 	_read_bf_level_info("BF-cpumask", mask, cpumask.level, SST_BF_INFO_1_OFFSET,
 			    P1_HI_CORE_MASK_START, P1_HI_CORE_MASK_WIDTH,
 			    SST_MUL_FACTOR_NONE)

@@ -1453,6 +1462,9 @@ static int isst_if_get_turbo_freq_info(v
 	if (turbo_freq.level > power_domain_info->max_level)
 		return -EINVAL;
 
+	if (!(power_domain_info->pp_header.level_en_mask & BIT(turbo_freq.level)))
+		return -EINVAL;
+
 	turbo_freq.max_buckets = TRL_MAX_BUCKETS;
 	turbo_freq.max_trl_levels = TRL_MAX_LEVELS;
 	turbo_freq.max_clip_freqs = SST_TF_MAX_LP_CLIP_RATIOS;

6.18-stable review patch. If anyone has any objections, please let me know.

------------------

From: Srinivas Pandruvada <[email protected]>

commit 124e2dbabe460c2a6e7440f4ad8af560131295c9 upstream.

Validate max CLOS ID and logical CPU ID for core power feature. Reject any clos level or logical CPU number greater than the supported maximum. These are used to calculate MMIO offset.

---
 drivers/platform/x86/intel/speed_select_if/isst_tpmi_core.c |   13 ++++++++++++
 1 file changed, 13 insertions(+)

@@ -652,6 +652,8 @@ static long isst_if_core_power_state(voi
 #define SST_CLOS_CONFIG_MAX_START	16
 #define SST_CLOS_CONFIG_MAX_WIDTH	8
 
+#define SST_MAX_CLOS			3
+
 static long isst_if_clos_param(void __user *argp)
 {
 	struct tpmi_per_power_domain_info *power_domain_info;

@@ -660,6 +662,9 @@ static long isst_if_clos_param(void __us
 	if (copy_from_user(&clos_param, argp, sizeof(clos_param)))
 		return -EFAULT;
 
+	if (clos_param.clos > SST_MAX_CLOS)
+		return -EINVAL;
+
 	power_domain_info = get_instance(clos_param.socket_id, clos_param.power_domain_id);
 	if (!power_domain_info)
 		return -EINVAL;

@@ -706,6 +711,8 @@ static long isst_if_clos_param(void __us
 #define SST_CLOS_ASSOC_CPUS_PER_REG	16
 #define SST_CLOS_ASSOC_BITS_PER_CPU	4
 
+#define SST_CLOS_ASSOC_MAX_LOGICAL_CPU	63
+
 static long isst_if_clos_assoc(void __user *argp)
 {
 	struct isst_if_clos_assoc_cmds assoc_cmds;

@@ -732,9 +739,15 @@ static long isst_if_clos_assoc(void __us
 		if (copy_from_user(&clos_assoc, ptr, sizeof(clos_assoc)))
 			return -EFAULT;
 
+		if (clos_assoc.clos > SST_MAX_CLOS)
+			return -EINVAL;
+
 		if (clos_assoc.socket_id >= topology_max_packages())
 			return -EINVAL;
 
+		if (clos_assoc.logical_cpu > SST_CLOS_ASSOC_MAX_LOGICAL_CPU)
+			return -EINVAL;
+
 		cpu = clos_assoc.logical_cpu;
 		clos = clos_assoc.clos;
 

6.18-stable review patch. If anyone has any objections, please let me know.

------------------

From: Srinivas Pandruvada <[email protected]>

commit 1700b4f804555467b7eff58dff7acc11d508b3a1 upstream.

Allow only 0 or 1 for core_power enable and priority_type parameters.

---
 drivers/platform/x86/intel/speed_select_if/isst_tpmi_core.c |    7 +++++++
 1 file changed, 7 insertions(+)

@@ -602,6 +602,9 @@ static bool disable_dynamic_sst_features
 #define SST_CP_PRIORITY_TYPE_START	1
 #define SST_CP_PRIORITY_TYPE_WIDTH	1
 
+#define SST_CP_MAX_ENABLE		1
+#define SST_CP_MAX_PRIORITY_TYPE	1
+
 static long isst_if_core_power_state(void __user *argp)
 {
 	struct tpmi_per_power_domain_info *power_domain_info;

@@ -621,6 +624,10 @@ static long isst_if_core_power_state(voi
 		if (power_domain_info->write_blocked)
 			return -EPERM;
 
+		if (core_power.enable > SST_CP_MAX_ENABLE ||
+		    core_power.priority_type > SST_CP_MAX_PRIORITY_TYPE)
+			return -EINVAL;
+
 		_write_cp_info("cp_enable", core_power.enable, SST_CP_CONTROL_OFFSET,
 			       SST_CP_ENABLE_START, SST_CP_ENABLE_WIDTH, SST_MUL_FACTOR_NONE)
 		_write_cp_info("cp_prio_type", core_power.priority_type, SST_CP_CONTROL_OFFSET,

6.18-stable review patch. If anyone has any objections, please let me know.

------------------

From: Srinivas Pandruvada <[email protected]>

commit 574b59bb4b6bfcfd1f639d02f1041b314d43a2e6 upstream.

Validate range for frequency and proportional priority while setting CLOS parameters.

---
 drivers/platform/x86/intel/speed_select_if/isst_tpmi_core.c |   12 ++++++++++++
 1 file changed, 12 insertions(+)

@@ -661,6 +661,9 @@ static long isst_if_core_power_state(voi
 
 #define SST_MAX_CLOS			3
 
+#define SST_MAX_FREQ			0xff
+#define SST_CLOS_MAX_PRIORITY		0x0f
+
 static long isst_if_clos_param(void __user *argp)
 {
 	struct tpmi_per_power_domain_info *power_domain_info;

@@ -680,6 +683,15 @@ static long isst_if_clos_param(void __us
 		if (power_domain_info->write_blocked)
 			return -EPERM;
 
+		if (!in_range(clos_param.min_freq_mhz / SST_MUL_FACTOR_FREQ, 0, SST_MAX_FREQ + 1))
+			return -EINVAL;
+
+		if (!in_range(clos_param.max_freq_mhz / SST_MUL_FACTOR_FREQ, 0, SST_MAX_FREQ + 1))
+			return -EINVAL;
+
+		if (!in_range(clos_param.prop_prio, 0, SST_CLOS_MAX_PRIORITY + 1))
+			return -EINVAL;
+
 		_write_cp_info("clos.min_freq", clos_param.min_freq_mhz,
 			       (SST_CLOS_CONFIG_0_OFFSET + clos_param.clos * SST_REG_SIZE),
 			       SST_CLOS_CONFIG_MIN_START, SST_CLOS_CONFIG_MIN_WIDTH,

6.18-stable review patch. If anyone has any objections, please let me know.

------------------

From: Srinivas Pandruvada <[email protected]>

commit f9a647cb8d90c09633a49a1e766e140e78012444 upstream.

If sst_add_perf_profiles() fails for memory allocation, it continues to allow SST-CP (core-power) feature. But in practice this is not very useful as to achieve some frequencies via SST-CP, an SST-PP (perf-profile) level change is required.

---
 drivers/platform/x86/intel/speed_select_if/isst_tpmi_core.c |   10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

@@ -339,7 +339,7 @@ static int sst_add_perf_profiles(struct
 	if (!pd_info->perf_levels) {
 		pd_info->pp_header.allowed_level_mask = 0;
 		pd_info->pp_header.level_en_mask = 0;
-		return 0;
+		return -ENOMEM;
 	}
 
 	pd_info->ratio_unit = pd_info->pp_header.ratio_unit;

@@ -370,7 +370,7 @@ static int sst_add_perf_profiles(struct
 static int sst_main(struct auxiliary_device *auxdev, struct tpmi_per_power_domain_info *pd_info)
 {
 	struct device *dev = &auxdev->dev;
-	int i, mask, levels;
+	int i, ret, mask, levels;
 
 	*((u64 *)&pd_info->sst_header) = readq(pd_info->sst_base);
 	pd_info->sst_header.cp_offset *= 8;

@@ -402,8 +402,12 @@ static int sst_main(struct auxiliary_dev
 			levels = i;
 		mask <<= 1;
 	}
+
+	ret = sst_add_perf_profiles(auxdev, pd_info, levels + 1);
+	if (ret)
+		return ret;
+
 	pd_info->max_level = levels;
-	sst_add_perf_profiles(auxdev, pd_info, levels + 1);
 
 	return 0;
 }

6.18-stable review patch. If anyone has any objections, please let me know.

------------------

From: Ma Ke <[email protected]>

commit 62b57396c26a1ce54963709928ea0d01fa522eea upstream.

ecl_ishtp_cl_probe() acquires a reference to an ACPI device via acpi_find_eclite_device() but fails to release it in the error path when acpi_opregion_init() fails. This results in a reference count leak, preventing proper cleanup of the ACPI device.

Calling path: acpi_find_eclite_device() -> acpi_dev_get_first_match_dev() -> acpi_dev_get_next_match_dev() -> bus_find_device() -> get_device().

Found by code review.

---
 drivers/platform/x86/intel/ishtp_eclite.c |    5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

@@ -600,13 +600,16 @@ static int ecl_ishtp_cl_probe(struct ish
 	rv = acpi_opregion_init(opr_dev);
 	if (rv) {
 		dev_err(cl_data_to_dev(opr_dev), "ACPI opregion init failed\n");
-		goto err_exit;
+		goto err_put;
 	}
 
 	/* Reprobe devices depending on ECLite - battery, fan, etc. */
 	acpi_dev_clear_dependencies(opr_dev->adev);
 
 	return 0;
+
+err_put:
+	acpi_dev_put(opr_dev->adev);
 err_exit:
 	ishtp_set_connection_state(ecl_ishtp_cl, ISHTP_CL_DISCONNECTING);
 	ishtp_cl_disconnect(ecl_ishtp_cl);

6.18-stable review patch. If anyone has any objections, please let me know.

------------------

From: Julian Haarmann <[email protected]>

commit bbc497b5231829d32c3a53b6e66be1add76c0064 upstream.

On newer Lenovo Yoga devices like the "Yoga 9 2-in-1 14IPH11 - Type 83SE", the hinge switch WMI query returns extra data in the upper bits (e.g. 0x50001 laptop mode, 0x50002 tablet mode, ect.).

The driver previously checked for exact matches (0x01 laptop, 0x02 tablet, ect.) causing newer switches to not work.

Mask the WMI query result to only match the lower byte and ignore upper bits.

---
 drivers/platform/x86/lenovo/ymc.c |    8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

@@ -8,6 +8,8 @@
 #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
 
 #include <linux/acpi.h>
+#include <linux/bitfield.h>
+#include <linux/bits.h>
 #include <linux/dmi.h>
 #include <linux/input.h>
 #include <linux/input/sparse-keymap.h>

@@ -20,6 +22,8 @@
 #define LENOVO_YMC_QUERY_INSTANCE 0
 #define LENOVO_YMC_QUERY_METHOD 0x01
 
+#define LENOVO_YMC_STATE_MASK GENMASK(7, 0)
+
 static bool force;
 module_param(force, bool, 0444);
 MODULE_PARM_DESC(force, "Force loading on boards without a convertible DMI chassis-type");

@@ -85,7 +89,9 @@ static void lenovo_ymc_notify(struct wmi
 			"WMI event data is not an integer\n");
 		goto free_obj;
 	}
-	code = obj->integer.value;
+
+	/* strip upper bits (e.g. 0x50000) on newer devices */
+	code = FIELD_GET(LENOVO_YMC_STATE_MASK, obj->integer.value);
 
 	if (!sparse_keymap_report_event(priv->input_dev, code, 1, true))
 		dev_warn(&wdev->dev, "Unknown key %d pressed\n", code);

6.18-stable review patch. If anyone has any objections, please let me know.

------------------

From: Thorsten Blum <[email protected]>

commit 4f3183f5ae9b8ddfe338d79a96146a05342bbe50 upstream.

cert_thumbprint() already returns the accumulated output length, but certificate_thumbprint_show() adds that value to count again, making the next line use the wrong offset. Errors returned by cert_thumbprint() are also ignored and their negative values added to count.

Assign the total length to count instead and propagate errors correctly.

---
 drivers/platform/x86/lenovo/think-lmi.c |    8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

@@ -741,6 +741,8 @@ static ssize_t certificate_thumbprint_sh
 		return -EOPNOTSUPP;
 
 	for (i = 0; i < ARRAY_SIZE(thumbtypes); i++) {
+		ssize_t ret;
+
 		if (tlmi_priv.pwdcfg.core.password_mode >= TLMI_PWDCFG_MODE_MULTICERT) {
 			/* Format: 'SVC | SMC, Thumbtype' */
 			wmistr = kasprintf(GFP_KERNEL, "%s,%s",

@@ -752,8 +754,12 @@ static ssize_t certificate_thumbprint_sh
 		}
 		if (!wmistr)
 			return -ENOMEM;
-		count += cert_thumbprint(buf, wmistr, count);
+
+		ret = cert_thumbprint(buf, wmistr, count);
 		kfree(wmistr);
+		if (ret < 0)
+			return ret;
+		count = ret;
 	}
 
 	return count;

6.18-stable review patch. If anyone has any objections, please let me know.

------------------

From: Thorsten Blum <[email protected]>

commit abca989604f60fe29d7170431f819e28ec7d868a upstream.

Multi-certificate support also allows the system authentication object to store ->signature and ->save_signature, which leak when the driver is removed. Free the signatures to avoid leaking memory.

---
 drivers/platform/x86/lenovo/think-lmi.c |    4 ++++
 1 file changed, 4 insertions(+)

@@ -1458,6 +1458,10 @@ static void tlmi_release_attr(void)
 	/* Free up any saved signatures */
 	kfree(tlmi_priv.pwd_admin->signature);
 	kfree(tlmi_priv.pwd_admin->save_signature);
+	if (tlmi_priv.pwd_system) {
+		kfree(tlmi_priv.pwd_system->signature);
+		kfree(tlmi_priv.pwd_system->save_signature);
+	}
 
 	/* Authentication structures */
 	list_for_each_entry_safe(pos, n, &tlmi_priv.authentication_kset->list, entry)

6.18-stable review patch. If anyone has any objections, please let me know.

------------------

From: Thorsten Blum <[email protected]>

commit 54745d563114b74f6fecebce68cd020d06c1772b upstream.

current_password_store() checks the password length before removing the trailing newline, which can reject valid passwords that are exactly ->maxlen bytes long.

It also passes ->maxlen to strscpy(), which truncates passwords without a newline.

Use strchrnul() to measure the password length up to the newline, then copy that many bytes and add a trailing NUL terminator using strscpy().

---
 drivers/platform/x86/lenovo/think-lmi.c |    7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

@@ -438,14 +438,13 @@ static ssize_t current_password_store(st
 	struct tlmi_pwd_setting *setting = to_tlmi_pwd_setting(kobj);
 	size_t pwdlen;
 
-	pwdlen = strlen(buf);
+	/* Strip newline; setting password won't work if one is present. */
+	pwdlen = strchrnul(buf, '\n') - buf;
 	/* pwdlen == 0 is allowed to clear the password */
 	if (pwdlen && ((pwdlen < setting->minlen) || (pwdlen > setting->maxlen)))
 		return -EINVAL;
 
-	strscpy(setting->password, buf, setting->maxlen);
-	/* Strip out CR if one is present, setting password won't work if it is present */
-	strreplace(setting->password, '\n', '\0');
+	strscpy(setting->password, buf, pwdlen + 1);
 	return count;
 }
 

6.18-stable review patch. If anyone has any objections, please let me know.

------------------

From: Bryam Vargas <[email protected]>

commit 833740a2333c2e4db4e02e3d0ffba04e8718a5f3 upstream.

Each EC FIFO event carries an 8-bit sensor number (in->sensor_num). cros_ec_sensorhub_ring_handler() validates the FIFO event count, the per-read count and the ring bound, but not the sensor number, which cros_ec_sensor_ring_process_event() then uses unchecked to index sensorhub->batch_state[] - allocated with only sensorhub->sensor_num entries. A sensor number of sensor_num or larger is an out-of-bounds read and write of batch_state[].

Validate the sensor number in the ring handler, where each event is read from the EC, and drop a malformed event before it is used.

---
 drivers/platform/chrome/cros_ec_sensorhub_ring.c |    8 ++++++++
 1 file changed, 8 insertions(+)

@@ -879,6 +879,14 @@ static void cros_ec_sensorhub_ring_handl
 
 		for (in = sensorhub->resp->fifo_read.data, j = 0;
 		     j < number_data; j++, in++) {
+			/* Skip event if sensor_num from EC is out of bounds. */
+			if (in->sensor_num >= sensorhub->sensor_num) {
+				dev_warn_ratelimited(sensorhub->dev,
+						     "Invalid sensor number %u from EC\n",
+						     in->sensor_num);
+				continue;
+			}
+
 			if (cros_ec_sensor_ring_process_event(
 						sensorhub, fifo_info,
 						fifo_timestamp,

6.18-stable review patch. If anyone has any objections, please let me know.

------------------

From: Mario Limonciello <[email protected]>

commit 9cef693bce96bb4c6952f48d855284cf7fa4f367 upstream.

dev->msg_port is switched to MSG_PORT_S2D before issuing the S2D SMU commands but is only restored to MSG_PORT_PMC on the success path. The early "return -EIO" and "return -ENOMEM" leave the port stuck on MSG_PORT_S2D, so all subsequent SMU communication - including the s2idle prepare/restore handlers - is directed at the wrong mailbox.

Consolidate the exit path through a single label so the message port is always restored.

---
 drivers/platform/x86/amd/pmc/mp1_stb.c |   24 +++++++++++++++---------
 1 file changed, 15 insertions(+), 9 deletions(-)

@@ -289,7 +289,7 @@ int amd_stb_s2d_init(struct amd_pmc_dev
 	u32 phys_addr_low, phys_addr_hi;
 	u64 stb_phys_addr;
 	u32 size = 0;
-	int ret;
+	int ret = 0;
 
 	if (!enable_stb)
 		return 0;

@@ -307,8 +307,10 @@ int amd_stb_s2d_init(struct amd_pmc_dev
 	dev->msg_port = MSG_PORT_S2D;
 
 	amd_pmc_send_cmd(dev, S2D_TELEMETRY_SIZE, &size, dev->stb_arg.s2d_msg_id, true);
-	if (size != S2D_TELEMETRY_BYTES_MAX)
-		return -EIO;
+	if (size != S2D_TELEMETRY_BYTES_MAX) {
+		ret = -EIO;
+		goto out;
+	}
 
 	/* Get DRAM size */
 	ret = amd_pmc_send_cmd(dev, S2D_DRAM_SIZE, &dev->dram_size, dev->stb_arg.s2d_msg_id, true);

@@ -321,12 +323,16 @@ int amd_stb_s2d_init(struct amd_pmc_dev
 
 	stb_phys_addr = ((u64)phys_addr_hi << 32 | phys_addr_low);
 
-	/* Clear msg_port for other SMU operation */
-	dev->msg_port = MSG_PORT_PMC;
-
 	dev->stb_virt_addr = devm_ioremap(dev->dev, stb_phys_addr, dev->dram_size);
-	if (!dev->stb_virt_addr)
-		return -ENOMEM;
+	if (!dev->stb_virt_addr) {
+		ret = -ENOMEM;
+		goto out;
+	}
+
+	ret = 0;
 
-	return 0;
+out:
+	/* Restore the default message port for subsequent SMU operations */
+	dev->msg_port = MSG_PORT_PMC;
+	return ret;
 }

6.18-stable review patch. If anyone has any objections, please let me know.

------------------

From: Mario Limonciello <[email protected]>

commit 0225c1d637687b03726f00ac65b6def843d2c464 upstream.

amd_stb_s2d_init() discards the return value of several S2D SMU commands. When the SMU refuses a command (e.g. "SMU cmd failed. err: 0xff") the failure is only noticed indirectly - if at all - and reported as -EIO, masking the real error.

More seriously, the S2D_PHYS_ADDR_LOW/HIGH return values are ignored, so on failure phys_addr_low/hi are left uninitialised and the assembled address is passed straight to devm_ioremap(). When the SMU leaves them at zero this maps physical address 0 and trips the ioremap-on-RAM warning:

amd_pmc AMDI000B:00: SMU cmd failed. err: 0xff ioremap on RAM at 0x0000000000000000 - 0x0000000000ffffff

  WARNING: CPU: 13 PID: 4592 at arch/x86/mm/ioremap.c:...

Check the return value of each SMU command and propagate it, and reject a zero physical address before calling devm_ioremap().

---
 drivers/platform/x86/amd/pmc/mp1_stb.c |   31 ++++++++++++++++++++-----------
 1 file changed, 20 insertions(+), 11 deletions(-)

@@ -306,30 +306,39 @@ int amd_stb_s2d_init(struct amd_pmc_dev
 	/* Spill to DRAM feature uses separate SMU message port */
 	dev->msg_port = MSG_PORT_S2D;
 
-	amd_pmc_send_cmd(dev, S2D_TELEMETRY_SIZE, &size, dev->stb_arg.s2d_msg_id, true);
+	ret = amd_pmc_send_cmd(dev, S2D_TELEMETRY_SIZE, &size, dev->stb_arg.s2d_msg_id, true);
+	if (ret)
+		goto out;
 	if (size != S2D_TELEMETRY_BYTES_MAX) {
 		ret = -EIO;
 		goto out;
 	}
 
-	/* Get DRAM size */
-	ret = amd_pmc_send_cmd(dev, S2D_DRAM_SIZE, &dev->dram_size, dev->stb_arg.s2d_msg_id, true);
-	if (ret || !dev->dram_size)
+	/* Get DRAM size; fall back to the default if the query fails */
+	if (amd_pmc_send_cmd(dev, S2D_DRAM_SIZE, &dev->dram_size, dev->stb_arg.s2d_msg_id, true) ||
+	    !dev->dram_size)
 		dev->dram_size = S2D_TELEMETRY_DRAMBYTES_MAX;
 
 	/* Get STB DRAM address */
-	amd_pmc_send_cmd(dev, S2D_PHYS_ADDR_LOW, &phys_addr_low, dev->stb_arg.s2d_msg_id, true);
-	amd_pmc_send_cmd(dev, S2D_PHYS_ADDR_HIGH, &phys_addr_hi, dev->stb_arg.s2d_msg_id, true);
+	ret = amd_pmc_send_cmd(dev, S2D_PHYS_ADDR_LOW, &phys_addr_low,
+			       dev->stb_arg.s2d_msg_id, true);
+	if (ret)
+		goto out;
+	ret = amd_pmc_send_cmd(dev, S2D_PHYS_ADDR_HIGH, &phys_addr_hi,
+			       dev->stb_arg.s2d_msg_id, true);
+	if (ret)
+		goto out;
 
 	stb_phys_addr = ((u64)phys_addr_hi << 32 | phys_addr_low);
-
-	dev->stb_virt_addr = devm_ioremap(dev->dev, stb_phys_addr, dev->dram_size);
-	if (!dev->stb_virt_addr) {
-		ret = -ENOMEM;
+	if (!stb_phys_addr) {
+		dev_err(dev->dev, "S2D phys addr query returned invalid address\n");
+		ret = -ENXIO;
 		goto out;
 	}
 
-	ret = 0;
+	dev->stb_virt_addr = devm_ioremap(dev->dev, stb_phys_addr, dev->dram_size);
+	if (!dev->stb_virt_addr)
+		ret = -ENOMEM;
 
 out:
 	/* Restore the default message port for subsequent SMU operations */

6.18-stable review patch. If anyone has any objections, please let me know.

------------------

From: Mario Limonciello <[email protected]>

commit 76f650a76d6a36a4bee79d94db90a0e935a95477 upstream.

amd_pmc_probe() registers the LPS0 s2idle handler with acpi_register_lps0_dev() and creates the driver's debugfs directory before calling amd_stb_s2d_init(), which is the last step in probe that can fail.

When amd_stb_s2d_init() fails (for example the S2D telemetry region cannot be ioremapped on a long-running system, or the SMU rejects the S2D setup) the error path only calls pci_dev_put() and returns. This leaves amd_pmc_s2idle_dev_ops on the global lps0_s2idle_devops_head list and leaks the debugfs directory, while the devm-managed resources backing the handler are torn down.

Reloading the module then walks the corrupted list in acpi_register_lps0_dev() and hits:

list_add corruption. next->prev should be prev, but was NULL. kernel BUG at lib/list_debug.c:29!

   acpi_register_lps0_dev+0x44/0x80
   amd_pmc_probe+0x224/0x380 [amd_pmc]
   platform_probe+0x67/0x90

Even without a reload, the stale registration means the next s2idle transition calls into torn-down driver state.

Unwind the debugfs directory and the LPS0 registration on the amd_stb_s2d_init() error path. acpi_unregister_lps0_dev() is safe to call unconditionally here: it is guarded on the same conditions as acpi_register_lps0_dev(), which is exactly what amd_pmc_remove() already relies on.

---
 drivers/platform/x86/amd/pmc/pmc.c |    6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

@@ -870,13 +870,17 @@ static int amd_pmc_probe(struct platform
 	amd_pmc_dbgfs_register(dev);
 	err = amd_stb_s2d_init(dev);
 	if (err)
-		goto err_pci_dev_put;
+		goto err_dbgfs_unregister;
 
 	if (IS_ENABLED(CONFIG_AMD_MP2_STB))
 		amd_mp2_stb_init(dev);
 	pm_report_max_hw_sleep(U64_MAX);
 	return 0;
 
+err_dbgfs_unregister:
+	amd_pmc_dbgfs_unregister(dev);
+	if (IS_ENABLED(CONFIG_SUSPEND))
+		acpi_unregister_lps0_dev(&amd_pmc_s2idle_dev_ops);
 err_pci_dev_put:
 	pci_dev_put(rdev);
 	return err;

6.18-stable review patch. If anyone has any objections, please let me know.

------------------

From: Muhammad Bilal <[email protected]>

commit 40e10e6cc8f70c041431a1e30186807e28ec46e0 upstream.

hp_init_bios_package_attribute() hard-fails when a WMI ACPI package contains fewer elements than the type-specific expected count (e.g. 11 elements instead of 13 for INTEGER or ENUMERATION attributes). This causes the entire hp_bioscfg driver to skip attribute enumeration on older HP hardware whose BIOS returns shortened packages when optional fields like prerequisites or possible values are absent.

Observed on HP EliteBook 840 G2 (BIOS M71 Ver. 01.31):

hp_bioscfg: ACPI-package does not have enough elements: 11 < 13

The element layout has two tiers:
- Elements 0-9 (SECURITY_LEVEL+1 = 10): common to all attribute types
- Elements 10-N: type-specific (bounds, values, encodings, ...)

The per-type populate functions (hp_populate_*_elements_from_package) already handle sparse packages correctly via their own elem < count loop guards and inner-loop bounds checks. The only unsafe case is when we lack even the common elements needed to register the attribute.

Fix by introducing COMMON_ELEM_CNT to mark the hard minimum (10), and splitting the check into two tiers:
- Fewer than COMMON_ELEM_CNT elements: hard fail, can't proceed.
- Fewer than expected type-specific elements: warn, but let the populate function parse what is available.

---
 drivers/platform/x86/hp/hp-bioscfg/bioscfg.c |   11 ++++++++---
 drivers/platform/x86/hp/hp-bioscfg/bioscfg.h |    3 +++
 2 files changed, 11 insertions(+), 3 deletions(-)

@@ -661,12 +661,17 @@ static int hp_init_bios_package_attribut
 	int ret = 0;
 
 	/* Take action appropriate to each ACPI TYPE */
-	if (obj->package.count < min_elements) {
-		pr_err("ACPI-package does not have enough elements: %d < %d\n",
-		       obj->package.count, min_elements);
+	if (obj->package.count < COMMON_ELEM_CNT) {
+		pr_err("ACPI-package is missing common elements: %d < %d\n",
+		       obj->package.count, COMMON_ELEM_CNT);
 		goto pack_attr_exit;
 	}
 
+	if (obj->package.count < min_elements) {
+		pr_warn("ACPI-package has fewer elements than expected: %d < %d, parsing available elements\n",
+			obj->package.count, min_elements);
+	}
+
 	elements = obj->package.elements;
 
 	/* sanity checking */

@@ -279,6 +279,9 @@ enum hp_wmi_data_elements {
 	PSWD_ENCODINGS = 13,
 	PSWD_IS_SET = 14,
 	PSWD_ELEM_CNT = 15,
+
+	/* Minimum elements shared by all attribute types (NAME..SECURITY_LEVEL) */
+	COMMON_ELEM_CNT = SECURITY_LEVEL + 1,
 };
 
 #define GET_INSTANCE_ID(type)						\

6.18-stable review patch. If anyone has any objections, please let me know.

------------------

From: Muhammad Bilal <[email protected]>

commit 05c808362e808e196f75696b8a64f7aa8b2245ce upstream.

The outer parsing loop in each attribute-type parser advances "elem" (the index into the ACPI package element array) by exactly one per iteration, but cases that consume multi-element arrays (PREREQUISITES, ENUM_POSSIBLE_VALUES, PSWD_ENCODINGS) read "size" consecutive elements without adjusting "elem" for the extra entries consumed beyond the first. The next outer iteration then re-reads a leftover element from the array just consumed instead of the next real property, and the type check fails on that stale element, aborting the parse with -EIO.

This produces exactly the failure visible in dmesg on the test hardware, on every boot:

Error expected type 2 for elem 13, but got type 1 instead
hp_bioscfg: Returned error 0x3, "Invalid command value/Feature not supported"

Fix by advancing "elem" by (size - 1) after each array-consuming loop, so the outer loop's own "elem++" lands on the correct next element. "eloc" is intentionally left alone: it indexes the logical property schema, not the physical element array, and each array case is still exactly one logical property regardless of how many physical elements it spans.

The defect is identical across all five attribute-type parsers (enum, integer, string, ordered-list, password), which were copy-pasted from the same template when the driver was introduced.

---
 drivers/platform/x86/hp/hp-bioscfg/enum-attributes.c       |    4 ++++
 drivers/platform/x86/hp/hp-bioscfg/int-attributes.c        |    2 ++
 drivers/platform/x86/hp/hp-bioscfg/order-list-attributes.c |    2 ++
 drivers/platform/x86/hp/hp-bioscfg/passwdobj-attributes.c  |    4 ++++
 drivers/platform/x86/hp/hp-bioscfg/string-attributes.c     |    2 ++
 5 files changed, 14 insertions(+)

@@ -227,6 +227,8 @@ static int hp_populate_enumeration_eleme
 				kfree(str_value);
 				str_value = NULL;
 			}
+			if (size)
+				elem += size - 1;
 			break;
 
 		case SECURITY_LEVEL:

@@ -280,6 +282,8 @@ static int hp_populate_enumeration_eleme
 				kfree(str_value);
 				str_value = NULL;
 			}
+			if (size)
+				elem += (size < MAX_VALUES_SIZE ? size : MAX_VALUES_SIZE) - 1;
 			break;
 		default:
 			pr_warn("Invalid element: %d found in Enumeration attribute or data may be malformed\n", elem);

@@ -243,6 +243,8 @@ static int hp_populate_integer_elements_
 				kfree(str_value);
 				str_value = NULL;
 			}
+			if (size)
+				elem += size - 1;
 			break;
 
 		case SECURITY_LEVEL:

@@ -233,6 +233,8 @@ static int hp_populate_ordered_list_elem
 				kfree(str_value);
 				str_value = NULL;
 			}
+			if (size)
+				elem += size - 1;
 			break;
 
 		case SECURITY_LEVEL:

@@ -321,6 +321,8 @@ static int hp_populate_password_elements
 				str_value = NULL;
 
 			}
+			if (size)
+				elem += size - 1;
 			break;
 		case SECURITY_LEVEL:
 			password_data->common.security_level = int_value;

@@ -362,6 +364,8 @@ static int hp_populate_password_elements
 				str_value = NULL;
 
 			}
+			if (size)
+				elem += size - 1;
 			break;
 		case PSWD_IS_SET:
 			password_data->is_enabled = int_value;

@@ -233,6 +233,8 @@ static int hp_populate_string_elements_f
 				kfree(str_value);
 				str_value = NULL;
 			}
+			if (size)
+				elem += size - 1;
 			break;
 
 		case SECURITY_LEVEL:

6.18-stable review patch. If anyone has any objections, please let me know.

------------------

From: Muhammad Bilal <[email protected]>

commit 1d143d78299d0eb4536698bf98c1815ec69f22a9 upstream.

hp_populate_ordered_list_elements_from_package() differs from the other per-type parsers: its main loop is bounded only by the fixed per-type count and never checks elem against the number of elements actually present in the package,

for (elem = 1, eloc = 1; eloc < ORD_ELEM_CNT; elem++, eloc++)

whereas the string, integer, enumeration and password parsers bound their main loop with "elem < count" as well.

This is safe today because hp_init_bios_package_attribute() rejects any package with fewer than ORD_ELEM_CNT elements before the parser runs. An upcoming change, however, relaxes that check to accept shorter packages.

Bound the loop by the validated element count as well, so it stops at whichever comes first, the per-type count or the real package size,

  for (elem = 1, eloc = 1; eloc < ORD_ELEM_CNT && elem < order_obj_count;
       elem++, eloc++)

order_obj_count is the validated element count, now correctly forwarded from the caller. No functional change for packages that enumerate correctly today.

---
 drivers/platform/x86/hp/hp-bioscfg/order-list-attributes.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

@@ -146,7 +146,7 @@ static int hp_populate_ordered_list_elem
 	if (!order_obj)
 		return -EINVAL;
 
-	for (elem = 1, eloc = 1; eloc < ORD_ELEM_CNT; elem++, eloc++) {
+	for (elem = 1, eloc = 1; eloc < ORD_ELEM_CNT && elem < order_obj_count; elem++, eloc++) {
 
 		switch (order_obj[elem].type) {
 		case ACPI_TYPE_STRING:

6.18-stable review patch. If anyone has any objections, please let me know.

------------------

From: Muhammad Bilal <[email protected]>

commit a7508c7959ff8d037327d377ed21a9c0eabe4674 upstream.

sk_store() and kek_store() strip a trailing newline from the sysfs write before allocating the key buffer:

	length = count;
	if (buf[length - 1] == '\n')
		length--;
	bioscfg_drv.spm_data.signing_key = kmemdup(buf, length, GFP_KERNEL);

but then pass the original "count" (not "length") as the copy size to hp_wmi_perform_query(), which memcpy()s that many bytes out of the "length"-sized allocation, reading one byte past it whenever the write ends in a newline, the normal case for a shell "echo" into sysfs.

KASAN confirms this directly:

  BUG: KASAN: slab-out-of-bounds in hp_wmi_perform_query+0x1e9/0x460 [hp_bioscfg]
  Read of size 28 at addr ffff88813c8e2b80 by task python3/16022
  ...
  sk_store+0xa7/0x240 [hp_bioscfg]
  kernfs_fop_write_iter+0x3e1/0x5d0
  ...
  The buggy address is located 0 bytes inside of
  allocated 27-byte region [ffff88813c8e2b80, ffff88813c8e2b9b)

Reproduced identically for kek_store, and at multiple write sizes (28, 57, 201 bytes), each time reading exactly one byte past a kmemdup() allocation one byte smaller than the write.

Fix by passing "length" instead of "count" to hp_wmi_perform_query() in both functions.

---
 drivers/platform/x86/hp/hp-bioscfg/spmobj-attributes.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

@@ -238,7 +238,7 @@ static ssize_t sk_store(struct kobject *
 	ret = hp_wmi_perform_query(HPWMI_SECUREPLATFORM_SET_SK,
 				   HPWMI_SECUREPLATFORM,
 				   (void *)bioscfg_drv.spm_data.signing_key,
-				   count, 0);
+				   length, 0);
 
 	if (!ret) {
 		bioscfg_drv.spm_data.mechanism = SIGNING_KEY;

@@ -274,7 +274,7 @@ static ssize_t kek_store(struct kobject
 	ret = hp_wmi_perform_query(HPWMI_SECUREPLATFORM_SET_KEK,
 				   HPWMI_SECUREPLATFORM,
 				   (void *)bioscfg_drv.spm_data.endorsement_key,
-				   count, 0);
+				   length, 0);
 
 	if (!ret) {
 		bioscfg_drv.spm_data.mechanism = ENDORSEMENT_KEY;

6.18-stable review patch. If anyone has any objections, please let me know.

------------------

From: Muhammad Bilal <[email protected]>

commit 2b2ec354f905c14e3270e8ec3ab50f7d8ad73bab upstream.

validate_password_input() computes length = strlen(buf) and then checks buf[length - 1] to strip a trailing newline, without checking that length is nonzero first. Writing an empty string (a bare '\n') to current_password or new_password gives length == 0, and buf[length - 1] reads buf[-1], one byte before the heap allocation holding the copied input.

KASAN confirms this directly:

  BUG: KASAN: slab-out-of-bounds in store_password_instance.constprop.0+0x223/0x2a0 [hp_bioscfg]
  Read of size 1 at addr ffff88811bd8da9f by task sh/13740
  ...
  store_password_instance.constprop.0+0x223/0x2a0 [hp_bioscfg]
  current_password_store+0x14/0x20 [hp_bioscfg]
  ...
  The buggy address is located 23 bytes to the right of
  allocated 8-byte region [ffff88811bd8da80, ffff88811bd8da88)

Reproduced identically via new_password_store. Execution continues past the bad read (the garbage byte only affects whether "length" is decremented by one), so the write completes and returns success; this is a pure information read past the buffer, not a crash, but it is still an out-of-bounds access KASAN correctly flags.

Fix by only checking buf[length - 1] when length is nonzero.

---
 drivers/platform/x86/hp/hp-bioscfg/passwdobj-attributes.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

@@ -66,7 +66,7 @@ static int validate_password_input(int i
 	struct password_data *password_data = &bioscfg_drv.password_data[instance_id];
 
 	length = strlen(buf);
-	if (buf[length - 1] == '\n')
+	if (length > 0 && buf[length - 1] == '\n')
 		length--;
 
 	if (length > MAX_PASSWD_SIZE)

6.18-stable review patch. If anyone has any objections, please let me know.

------------------

From: Muhammad Bilal <[email protected]>

commit 2ea12a467a9cb12170417b30784fe26a243a75fe upstream.

current_password_store() and new_password_store() both call store_password_instance() with is_current = true:

	static ssize_t new_password_store(...)
	{
		return store_password_instance(kobj, buf, count, true);
	}

so a write to new_password is routed to current_password instead, and the new_password field is never written by either sysfs entry point.

Fix by passing false from new_password_store(), matching what the is_current parameter is meant to select.

---
 drivers/platform/x86/hp/hp-bioscfg/passwdobj-attributes.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

@@ -123,7 +123,7 @@ static ssize_t new_password_store(struct
 				  struct kobj_attribute *attr,
 				  const char *buf, size_t count)
 {
-	return store_password_instance(kobj, buf, count, true);
+	return store_password_instance(kobj, buf, count, false);
 }
 
 static struct kobj_attribute password_new_password = __ATTR_WO(new_password);

6.18-stable review patch. If anyone has any objections, please let me know.

------------------

From: Muhammad Bilal <[email protected]>

commit dc03f05e419f3460342fb7564884f244622634b6 upstream.

hp_get_string_from_buffer() clamps the converted string length against the destination buffer size with "size > dst_size", so when the converted length is exactly equal to dst_size, conv_dst_size is left at dst_size and the unconditional NUL terminator write

	dst[conv_dst_size] = 0;

lands one byte past the destination buffer. This is the same shape of bug as the previously fixed off-by-one in hp_convert_hexstr_to_str():
the buffer is sized correctly for the content, but the terminator write is never checked against that size.

Fix by changing the comparison to ">=" so conv_dst_size is always left with room for the terminator.

All fixed-size destinations that reach this function (path[512], current_value[512], current_password/current_value[64], and the per-entry buffers in encodings[][512] and prerequisites[][512]) are affected.

---
 drivers/platform/x86/hp/hp-bioscfg/bioscfg.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

@@ -85,7 +85,7 @@ int hp_get_string_from_buffer(u8 **buffe
 	 * bytes.
 	 */
 	conv_dst_size = size;
-	if (size > dst_size)
+	if (size >= dst_size)
 		conv_dst_size = dst_size - 1;
 
 	/*

6.18-stable review patch. If anyone has any objections, please let me know.

------------------

From: Muhammad Bilal <[email protected]>

commit cb6b1b0fb236a9581cae213c2a9182e68cc3ffe5 upstream.

The ACPI_TYPE_STRING case explicitly skips the string conversion for elem == ORD_LIST_ELEMENTS:

	if (elem != PREREQUISITES && elem != ORD_LIST_ELEMENTS) {
		ret = hp_convert_hexstr_to_str(..., &str_value, &value_len);
		if (ret)
			continue;
	}

so by the time the ORD_LIST_ELEMENTS case in the eloc switch runs, str_value is NULL (it was freed and reset to NULL at the end of the previous iteration). That case then does:

	ret = hp_convert_hexstr_to_str(str_value, value_len, &tmpstr, &tmp_len);

hp_convert_hexstr_to_str() rejects a NULL input with -EINVAL, which sends this function to exit_list, and exit_list unconditionally returns 0. The net effect is that any ordered-list attribute with elements present silently ends up with an empty elements list, with no error surfaced anywhere.

Fix by converting the current element directly, order_obj[elem], the same way the PREREQUISITES case already handles its own array elements, instead of reusing the unrelated str_value/value_len left over from earlier processing.

---
 drivers/platform/x86/hp/hp-bioscfg/order-list-attributes.c |    4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

@@ -264,7 +264,9 @@ static int hp_populate_ordered_list_elem
 			 * Ordered list data is stored in hex and comma separated format
 			 * Convert the data and split it to show each element
 			 */
-			ret = hp_convert_hexstr_to_str(str_value, value_len, &tmpstr, &tmp_len);
+			ret = hp_convert_hexstr_to_str(order_obj[elem].string.pointer,
+						       order_obj[elem].string.length,
+						       &tmpstr, &tmp_len);
 			if (ret)
 				goto exit_list;
 

6.18-stable review patch. If anyone has any objections, please let me know.

------------------

From: Muhammad Bilal <[email protected]>

commit e0ddfd77c0c320b7d12b6c9169303b140b798775 upstream.

The per-type package parsers are handed the wrong element count.

hp_init_bios_package_attribute() validates obj->package.count and then calls one of the five hp_populate_*_package_data() wrappers (string, integer, enumeration, ordered list, password). Each wrapper forwards a count to its hp_populate_*_elements_from_package() parser, but instead of forwarding the validated obj->package.count it derives the count from elements[0]. elements[0] is the NAME field and is always an ACPI_TYPE_STRING, so reading ->package.count from it in fact reads ->string.length through the union acpi_object. The parsers thus bound themselves against the length of the name string rather than against the real number of elements in the package.

This is safe today because hp_init_bios_package_attribute() refuses any package that has fewer than the type's element count, so a parser only ever runs on a full package and never reads past it regardless of the bogus bound.

An upcoming change relaxes that check to accept shorter packages. Once a parser can receive fewer elements than its per-type count, a bound taken from the name length no longer reflects the array size, and the "elem < count" loop conditions and "elem + n >= count" sub-loop guards read past the end of elements[] - an out-of-bounds heap read.

Forward the validated obj->package.count to every *_package_data() wrapper so the parsers bound themselves against the real package size. This does not change behaviour for the packages that enumerate correctly today and is a prerequisite for accepting shorter packages safely.

---
 drivers/platform/x86/hp/hp-bioscfg/bioscfg.c               |    5 +++++
 drivers/platform/x86/hp/hp-bioscfg/bioscfg.h               |    5 +++++
 drivers/platform/x86/hp/hp-bioscfg/enum-attributes.c       |    4 +++-
 drivers/platform/x86/hp/hp-bioscfg/int-attributes.c        |    4 +++-
 drivers/platform/x86/hp/hp-bioscfg/order-list-attributes.c |    6 ++++--
 drivers/platform/x86/hp/hp-bioscfg/passwdobj-attributes.c  |    6 ++++--
 drivers/platform/x86/hp/hp-bioscfg/string-attributes.c     |    4 +++-
 7 files changed, 27 insertions(+), 7 deletions(-)

@@ -736,26 +736,31 @@ static int hp_init_bios_package_attribut
 	switch (attr_type) {
 	case HPWMI_STRING_TYPE:
 		ret = hp_populate_string_package_data(elements,
+						      obj->package.count,
 						      instance_id,
 						      attr_name_kobj);
 		break;
 	case HPWMI_INTEGER_TYPE:
 		ret = hp_populate_integer_package_data(elements,
+						       obj->package.count,
 						       instance_id,
 						       attr_name_kobj);
 		break;
 	case HPWMI_ENUMERATION_TYPE:
 		ret = hp_populate_enumeration_package_data(elements,
+							   obj->package.count,
 							   instance_id,
 							   attr_name_kobj);
 		break;
 	case HPWMI_ORDERED_LIST_TYPE:
 		ret = hp_populate_ordered_list_package_data(elements,
+							    obj->package.count,
 							    instance_id,
 							    attr_name_kobj);
 		break;
 	case HPWMI_PASSWORD_TYPE:
 		ret = hp_populate_password_package_data(elements,
+							obj->package.count,
 							instance_id,
 							attr_name_kobj);
 		break;

@@ -404,6 +404,7 @@ int hp_populate_string_buffer_data(u8 *b
 int hp_alloc_string_data(void);
 void hp_exit_string_attributes(void);
 int hp_populate_string_package_data(union acpi_object *str_obj,
+				    int str_obj_count,
 				    int instance_id,
 				    struct kobject *attr_name_kobj);
 

@@ -414,6 +415,7 @@ int hp_populate_integer_buffer_data(u8 *
 int hp_alloc_integer_data(void);
 void hp_exit_integer_attributes(void);
 int hp_populate_integer_package_data(union acpi_object *integer_obj,
+				     int integer_obj_count,
 				     int instance_id,
 				     struct kobject *attr_name_kobj);
 

@@ -424,6 +426,7 @@ int hp_populate_enumeration_buffer_data(
 int hp_alloc_enumeration_data(void);
 void hp_exit_enumeration_attributes(void);
 int hp_populate_enumeration_package_data(union acpi_object *enum_obj,
+					 int enum_obj_count,
 					 int instance_id,
 					 struct kobject *attr_name_kobj);
 

@@ -435,6 +438,7 @@ int hp_populate_ordered_list_buffer_data
 int hp_alloc_ordered_list_data(void);
 void hp_exit_ordered_list_attributes(void);
 int hp_populate_ordered_list_package_data(union acpi_object *order_obj,
+					  int order_obj_count,
 					  int instance_id,
 					  struct kobject *attr_name_kobj);
 

@@ -443,6 +447,7 @@ int hp_populate_password_buffer_data(u8
 				     int instance_id,
 				     struct kobject *attr_name_kobj);
 int hp_populate_password_package_data(union acpi_object *password_obj,
+				      int password_obj_count,
 				      int instance_id,
 				      struct kobject *attr_name_kobj);
 int hp_alloc_password_data(void);

@@ -304,10 +304,12 @@ exit_enumeration_package:
  * Populate all properties of an instance under enumeration attribute
  *
  * @enum_obj: ACPI object with enumeration data
+ * @enum_obj_count: Number of elements in @enum_obj
  * @instance_id: The instance to enumerate
  * @attr_name_kobj: The parent kernel object
  */
 int hp_populate_enumeration_package_data(union acpi_object *enum_obj,
+					 int enum_obj_count,
 					 int instance_id,
 					 struct kobject *attr_name_kobj)
 {

@@ -316,7 +318,7 @@ int hp_populate_enumeration_package_data
 	enum_data->attr_name_kobj = attr_name_kobj;
 
 	hp_populate_enumeration_elements_from_package(enum_obj,
-						      enum_obj->package.count,
+						      enum_obj_count,
 						      instance_id);
 	hp_update_attribute_permissions(enum_data->common.is_readonly,
 					&enumeration_current_val);

@@ -277,10 +277,12 @@ exit_integer_package:
  * Populate all properties of an instance under integer attribute
  *
  * @integer_obj: ACPI object with integer data
+ * @integer_obj_count: Number of elements in @integer_obj
  * @instance_id: The instance to enumerate
  * @attr_name_kobj: The parent kernel object
  */
 int hp_populate_integer_package_data(union acpi_object *integer_obj,
+				     int integer_obj_count,
 				     int instance_id,
 				     struct kobject *attr_name_kobj)
 {

@@ -288,7 +290,7 @@ int hp_populate_integer_package_data(uni
 
 	integer_data->attr_name_kobj = attr_name_kobj;
 	hp_populate_integer_elements_from_package(integer_obj,
-						  integer_obj->package.count,
+						  integer_obj_count,
 						  instance_id);
 	hp_update_attribute_permissions(integer_data->common.is_readonly,
 					&integer_current_val);

@@ -303,10 +303,12 @@ exit_list:
  * Populate all properties of an instance under ordered_list attribute
  *
  * @order_obj: ACPI object with ordered_list data
+ * @order_obj_count: Number of elements in @order_obj
  * @instance_id: The instance to enumerate
  * @attr_name_kobj: The parent kernel object
  */
-int hp_populate_ordered_list_package_data(union acpi_object *order_obj, int instance_id,
+int hp_populate_ordered_list_package_data(union acpi_object *order_obj, int order_obj_count,
+					  int instance_id,
 					  struct kobject *attr_name_kobj)
 {
 	struct ordered_list_data *ordered_list_data = &bioscfg_drv.ordered_list_data[instance_id];

@@ -314,7 +316,7 @@ int hp_populate_ordered_list_package_dat
 	ordered_list_data->attr_name_kobj = attr_name_kobj;
 
 	hp_populate_ordered_list_elements_from_package(order_obj,
-						       order_obj->package.count,
+						       order_obj_count,
 						       instance_id);
 	hp_update_attribute_permissions(ordered_list_data->common.is_readonly,
 					&ordered_list_current_val);

@@ -389,10 +389,12 @@ exit_package:
  *	Populate all properties for an instance under password attribute
  *
  * @password_obj: ACPI object with password data
+ * @password_obj_count: Number of elements in @password_obj
  * @instance_id: The instance to enumerate
  * @attr_name_kobj: The parent kernel object
  */
-int hp_populate_password_package_data(union acpi_object *password_obj, int instance_id,
+int hp_populate_password_package_data(union acpi_object *password_obj, int password_obj_count,
+				      int instance_id,
 				      struct kobject *attr_name_kobj)
 {
 	struct password_data *password_data = &bioscfg_drv.password_data[instance_id];

@@ -400,7 +402,7 @@ int hp_populate_password_package_data(un
 	password_data->attr_name_kobj = attr_name_kobj;
 
 	hp_populate_password_elements_from_package(password_obj,
-						   password_obj->package.count,
+						   password_obj_count,
 						   instance_id);
 
 	hp_friendly_user_name_update(password_data->common.path,

@@ -265,10 +265,12 @@ exit_string_package:
  * Populate all properties of an instance under string attribute
  *
  * @string_obj: ACPI object with string data
+ * @string_obj_count: Number of elements in @string_obj
  * @instance_id: The instance to enumerate
  * @attr_name_kobj: The parent kernel object
  */
 int hp_populate_string_package_data(union acpi_object *string_obj,
+				    int string_obj_count,
 				    int instance_id,
 				    struct kobject *attr_name_kobj)
 {

@@ -277,7 +279,7 @@ int hp_populate_string_package_data(unio
 	string_data->attr_name_kobj = attr_name_kobj;
 
 	hp_populate_string_elements_from_package(string_obj,
-						 string_obj->package.count,
+						 string_obj_count,
 						 instance_id);
 
 	hp_update_attribute_permissions(string_data->common.is_readonly,

6.18-stable review patch. If anyone has any objections, please let me know.

------------------

From: Muhammad Bilal <[email protected]>

commit b0e2af3ec94e0431adb59d9f249ebbd3b7285158 upstream.

hp_populate_enumeration_elements_from_package() returns -EIO and aborts enumeration of the entire attribute when any single element has an unexpected ACPI type. This is observed on HP EliteBook 840 G2 when the BIOS returns malformed ACPI data following a failed WMI query:

ACPI BIOS Error (bug): AE_AML_BUFFER_LIMIT, Index (0x000000032)

    is beyond end of object (length 0x32)

ACPI Error: Aborting method \_SB.WMID.WQBE due to previous error Error expected type 2 for elem 13, but got type 1 instead
hp_bioscfg: Returned error 0x3,
"Invalid command value/Feature not supported"

Aborting immediately discards the attribute entirely.

Warn about the unexpected element type, free the temporary string, skip the offending element, and continue parsing the remaining package instead of failing the whole attribute.

---
 drivers/platform/x86/hp/hp-bioscfg/enum-attributes.c |    7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

@@ -163,10 +163,11 @@ static int hp_populate_enumeration_eleme
 
 		/* Check that both expected and read object type match */
 		if (expected_enum_types[eloc] != enum_obj[elem].type) {
-			pr_err("Error expected type %d for elem %d, but got type %d instead\n",
-			       expected_enum_types[eloc], elem, enum_obj[elem].type);
+			pr_warn("Unexpected element type at elem %d: expected %d, got %d, skipping\n",
+				elem, expected_enum_types[eloc], enum_obj[elem].type);
 			kfree(str_value);
-			return -EIO;
+			str_value = NULL;
+			continue;
 		}
 
 		/* Assign appropriate element value to corresponding field */

6.18-stable review patch. If anyone has any objections, please let me know.

------------------

From: Laxman Acharya Padhya <[email protected]>

commit ba77efee1b95b4ad7559b1cdbe7cd7fa36dca95b upstream.

io_handle_query_entry() clamps hdr.size for the inbound copy_from_user() but keeps the original user value as usize. copy_struct_to_user() uses that usize and, when it is larger than the kernel result, clear_user()s the trailing bytes.

As hdr.size is a __u32, a query can request nearly 4 GiB of zeroing, including on the error path where res_size stays 0. The interface is reachable without a ring via IORING_REGISTER_QUERY.

Reject sizes larger than PAGE_SIZE, as recommended for copy_struct_* interfaces.

---
 io_uring/query.c |    3 +++
 1 file changed, 3 insertions(+)

@@ -36,6 +36,9 @@ static int io_handle_query_entry(struct
 
 	if (copy_from_user(&hdr, uhdr, sizeof(hdr)))
 		return -EFAULT;
+	/* copy_struct_to_user() zeros up to usize bytes */
+	if (hdr.size > PAGE_SIZE)
+		return -E2BIG;
 	usize = hdr.size;
 	hdr.size = min(hdr.size, IO_MAX_QUERY_SIZE);
 	udata = u64_to_user_ptr(hdr.query_data);

6.18-stable review patch. If anyone has any objections, please let me know.

------------------

From: Kuan-Wei Chiu <[email protected]>

commit 25c7e242aca084fdc1098248194032317dca625d upstream.

In of_icc_get_by_index() and icc_get(), if the dynamic allocation for path->name fails via kasprintf(), the error handling path directly calls kfree(path) to free the path object and returns an error.

However, prior to this point, path_find() calls path_init(), which already links the path's requests into the req_list of the respective interconnect nodes via hlist_add_head(). Directly invoking kfree(path) leaves dangling pointers in the hlist. A subsequent call to icc_get() or icc_set_bw() will traverse or modify these corrupted lists, triggering a slab use afterfree.

KASAN report showing the vulnerability when reproducing via debugfs:

  BUG: KASAN: slab-use-after-free in path_find+0x6f8/0xcfc
  Write of size 8 at addr fff000000d43f748 by task sh/1
  ...
  Call trace:
   kasan_report+0xac/0xfc
   path_find+0x6f8/0xcfc
   icc_get+0x148/0x380
   icc_get_set+0xf8/0x2d0
  ...
  Freed by task 1:
   kfree+0x1a0/0x4a4
   icc_get+0x2cc/0x380
   icc_get_set+0xf8/0x2d0

Fix this by replacing kfree(path) with the proper teardown function, icc_put(path), which safely removes the requests from the req_list using hlist_del() and drops the provider usage references before freeing the memory.

Additionally, in icc_get(), ensure that the icc_lock mutex is released prior to calling icc_put(path) to avoid a deadlock, as icc_put() internally acquires the same lock.

---
 drivers/interconnect/core.c |    7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

@@ -528,7 +528,7 @@ struct icc_path *of_icc_get_by_index(str
 	path->name = kasprintf(GFP_KERNEL, "%s-%s",
 			       src_data->node->name, dst_data->node->name);
 	if (!path->name) {
-		kfree(path);
+		icc_put(path);
 		path = ERR_PTR(-ENOMEM);
 	}
 

@@ -626,8 +626,9 @@ struct icc_path *icc_get(struct device *
 
 	path->name = kasprintf(GFP_KERNEL, "%s-%s", src_node->name, dst_node->name);
 	if (!path->name) {
-		kfree(path);
-		path = ERR_PTR(-ENOMEM);
+		mutex_unlock(&icc_lock);
+		icc_put(path);
+		return ERR_PTR(-ENOMEM);
 	}
 out:
 	mutex_unlock(&icc_lock);

6.18-stable review patch. If anyone has any objections, please let me know.

------------------

From: Yousef Alhouseen <[email protected]>

commit 53637506884dbd5c91a89b1a3547d99d80f8ed2c upstream.

ipmb_write() read message fields before validating the length byte.

A zero or short write can read uninitialized stack bytes.

A length smaller than the SMBus header underflows the block write length.

Require a non-empty buffer and the minimum IPMB request length.

Also require the length byte plus payload before parsing the message.

Message-ID: <[email protected]>

---
 drivers/char/ipmi/ipmb_dev_int.c |    5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

@@ -141,13 +141,14 @@ static ssize_t ipmb_write(struct file *f
 	u8 msg[MAX_MSG_LEN];
 	ssize_t ret;
 
-	if (count > sizeof(msg))
+	if (!count || count > sizeof(msg))
 		return -EINVAL;
 
 	if (copy_from_user(&msg, buf, count))
 		return -EFAULT;
 
-	if (count < msg[0])
+	if (msg[IPMB_MSG_LEN_IDX] < IPMB_REQUEST_LEN_MIN ||
+	    count < (size_t)msg[IPMB_MSG_LEN_IDX] + 1)
 		return -EINVAL;
 
 	rq_sa = GET_7BIT_ADDR(msg[RQ_SA_8BIT_IDX]);

6.18-stable review patch. If anyone has any objections, please let me know.

------------------

From: Yuho Choi <[email protected]>

commit b6c46ab0bdee90c238e96ea4a74972118c97900d upstream.

ipmi_add_smi() creates the nr_users and nr_msgs files before trying to create the maintenance_mode file. If that last creation fails, the error path removes only nr_users before dropping the final reference to the interface.

Remove nr_msgs as well so no sysfs attribute embedded in the freed interface remains registered.

Message-ID: <[email protected]>

---
 drivers/char/ipmi/ipmi_msghandler.c |    1 +
 1 file changed, 1 insertion(+)

@@ -3741,6 +3741,7 @@ int ipmi_add_smi(struct module         *
 	sysfs_attr_init(&intf->maintenance_mode_devattr.attr);
 	rv = device_create_file(intf->si_dev, &intf->maintenance_mode_devattr);
 	if (rv) {
+		device_remove_file(intf->si_dev, &intf->nr_msgs_devattr);
 		device_remove_file(intf->si_dev, &intf->nr_users_devattr);
 		goto out_err_bmc_reg;
 	}

6.18-stable review patch. If anyone has any objections, please let me know.

------------------

From: Seiji Nishikawa <[email protected]>

commit 6d920a75df9a83ab096b3cde7a643b656e4fdfeb upstream.

try_smi_init() allocates new_smi->si_sm and later calls ipmi_register_smi_mod(), which maps to ipmi_add_smi().

During ipmi_add_smi(), the upper IPMI message handler obtains the initial BMC device information through __bmc_get_device_id(). This can fail if the BMC does not return a successful response to the Get Device ID command.

When the BMC returns a nonzero completion code, the device-id helper retries the command and eventually returns -EIO if the device ID still cannot be fetched.

On this failure path, ipmi_add_smi() logs "Unable to get the device id" and goes to out_err_started, where it invokes the lower driver's shutdown callback. try_smi_init() then logs the returned registration failure:

ipmi_si IPI0001:00: IPMI message handler: Unable to get the device id: -5 ipmi_si IPI0001:00: Unable to register device: error -5

For ipmi_si, the shutdown callback is shutdown_smi(), which cleans up the SI state machine data, frees smi_info->si_sm, and sets smi_info->si_sm and smi_info->intf to NULL.

However, intf->in_shutdown is not set on this failed-registration rollback path. Therefore, the asynchronous redo_bmc_reg work item can still retry BMC device-id probing after the lower driver has already cleared its SI state machine data. In the observed case, that retry path reached start_next_msg(), which passed the NULL smi_info->si_sm pointer to the selected KCS state machine handler:

BUG: unable to handle kernel NULL pointer dereference at 0000000000000000
Workqueue: events redo_bmc_reg [ipmi_msghandler]
RIP: start_kcs_transaction+0x2c/0x190 [ipmi_si]
Call Trace:
 start_next_msg+0x50/0x80 [ipmi_si]
 check_start_timer_thread.part.9+0x3b/0x50 [ipmi_si]
 sender+0x69/0x80 [ipmi_si]
 i_ipmi_request+0x2ac/0x9d0 [ipmi_msghandler]
 __get_device_id.isra.29+0xaa/0x180 [ipmi_msghandler]
 __bmc_get_device_id+0xef/0x950 [ipmi_msghandler]
 redo_bmc_reg+0x52/0x60 [ipmi_msghandler]
 process_one_work+0x1a7/0x360

Set intf->in_shutdown on the out_err_started path before invoking the lower driver's shutdown callback. This prevents later redo_bmc_reg retries from using an interface whose lower driver state has been cleaned up, and applies the same shutdown state to other IPMI interfaces as well.

Message-ID: <[email protected]>

---
 drivers/char/ipmi/ipmi_msghandler.c |    1 +
 1 file changed, 1 insertion(+)

@@ -3759,6 +3759,7 @@ int ipmi_add_smi(struct module         *
  out_err_bmc_reg:
 	ipmi_bmc_unregister(intf);
  out_err_started:
+	intf->in_shutdown = true;
 	if (intf->handlers->shutdown)
 		intf->handlers->shutdown(intf->send_info);
  out_err:

6.18-stable review patch. If anyone has any objections, please let me know.

------------------

From: Corey Minyard <[email protected]>

commit ae84a2536577057e97f23f75a202e26d0e86cf01 upstream.

If an error occurs during startup of an IPMI interface, it may have scheduled work to run. The work needs to be canceled before the interface can be freed.

---
 drivers/char/ipmi/ipmi_msghandler.c |    1 +
 1 file changed, 1 insertion(+)

@@ -3766,6 +3766,7 @@ int ipmi_add_smi(struct module         *
 	list_del(&intf->link);
 	mutex_unlock(&ipmi_interfaces_mutex);
 	mutex_unlock(&smi_watchers_mutex);
+	cancel_work_sync(&intf->smi_work);
 	kref_put(&intf->refcount, intf_free);
 
 	return rv;

6.18-stable review patch. If anyone has any objections, please let me know.

------------------

From: Alexandra Winter <[email protected]>

commit 80230a18c164a4b5bbc048fe2768b219ac17bc5a upstream.

afiucv_hs_rcv() selects a socket from iucv_sk_list by matching four 8-byte name fields in the transport header alone. No check is made against the net_device the frame arrived on.

This can cause a frame arriving on any netdev to be delivered to an AF_IUCV socket. Three problems follow.

First, a frame arriving over HiperSockets can be delivered to a socket bound to the classic z/VM IUCV transport, which has iucv->hs_dev == NULL. iucv_sock_bind() takes the classic path whenever the requested userid matches iucv_userid, even on a guest that also has a HiperSockets device carrying the same identifier. The child socket created by afiucv_hs_callback_syn() for such a match inherits hs_dev = NULL and transport = AF_IUCV_TRANS_HIPER, so the first send() on it returns -ENODEV. The socket delivered to accept() is unusable.

Second, a frame arriving on one netdev can be delivered to a socket bound to a different IQD device. Which can lead to
- Accept-queue exhaustion (DoS)
- Attacker-controlled peer identity in the child socket
- Data injection into existing sockets
- Fabric noise on the IQD fabric, where bogus replies are sent
- killing established connections

Third, all AF_IUCV sockets live in init_net, as iucv_sock_alloc() calls sk_alloc(&init_net, ...). But even frames arriving on netdev devices in a namespace can be delivered to an IUCV socket. So a process in an unprivileged user and network namespace holding only the CAP_NET_RAW capability valid within that namespace can send a raw ETH_P_AF_IUCV frame on its own lo device and have it matched against init_net sockets.

Fix all three by skipping any socket whose hs_dev does not match the ingress device. A classic z/VM IUCV socket has hs_dev == NULL; the ingress dev is never NULL, so classic sockets are skipped automatically. An unbound HIPER socket also has hs_dev == NULL and is skipped. A bound HIPER socket is only reachable from the exact IQD device it was bound to. Because hs_dev is always a device in init_net (iucv_sock_bind() scans for_each_netdev_rcu(&init_net, ...) exclusively), a frame whose ingress device belongs to another namespace never matches any socket.

Note that AF_IUCV over HiperSockets provides no per-connection
authentication: no sequence numbers, no TLS, no nonce. The four name fields identifying a connection are exchanged in plaintext on the shared HiperSockets segment (VCHID). Any host on the same HiperSockets segment could spoof any frame type against an existing connection. That is a protocol-level property unchanged by this patch. The fix reduces the attack surface to peers present on the same HiperSockets segment.

---
 net/iucv/af_iucv.c |    2 ++
 1 file changed, 2 insertions(+)

@@ -2079,6 +2079,8 @@ static int afiucv_hs_rcv(struct sk_buff
 	sk = NULL;
 	read_lock(&iucv_sk_list.lock);
 	sk_for_each(sk, &iucv_sk_list.head) {
+		if (iucv_sk(sk)->hs_dev != dev)
+			continue;
 		if (trans_hdr->flags == AF_IUCV_FLAG_SYN) {
 			if ((!memcmp(&iucv_sk(sk)->src_name,
 				     trans_hdr->destAppName, 8)) &&

6.18-stable review patch. If anyone has any objections, please let me know.

------------------

From: Weiming Shi <[email protected]>

commit 71283aaa6c65b3cec84caf1dc78560985737641f upstream.

xdp_convert_zc_to_xdp_frame() clones an XSK packet into an order-0 page and advertises PAGE_SIZE as its frame size. It allows the copied frame to occupy the page tail needed by skb_shared_info and records zero headroom even when metadata separates the frame header from packet data. An AF_XDP zero-copy packet redirected through cpumap can therefore make the skb overlap skb_shared_info or place it beyond the allocated page.

Limit the copied layout to SKB_WITH_OVERHEAD(PAGE_SIZE) and include the metadata length in frame headroom. Redirect callers already handle a NULL conversion result.

BUG: KASAN: slab-out-of-bounds in skb_gro_receive
Write of size 4 at addr ffff88800cf37004 by task cpumap/1/map:1/146
Call Trace:
 skb_gro_receive (net/core/gro.c:174)
 udp_gro_receive (net/ipv4/udp_offload.c:812)
 inet_gro_receive (net/ipv4/af_inet.c:1539)
 dev_gro_receive (net/core/gro.c:515)
 gro_receive_skb (net/core/gro.c:633)
 cpu_map_kthread_run (kernel/bpf/cpumap.c:395)
 kthread (kernel/kthread.c:436)
 ret_from_fork (arch/x86/kernel/process.c:164)
 ret_from_fork_asm (arch/x86/entry/entry_64.S:255)
Kernel panic - not syncing: KASAN: panic_on_warn set ...

---
 net/core/xdp.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

@@ -585,7 +585,7 @@ struct xdp_frame *xdp_convert_zc_to_xdp_
 		   xdp->data - xdp->data_meta;
 	totsize = xdp->data_end - xdp->data + metasize;
 
-	if (sizeof(*xdpf) + totsize > PAGE_SIZE)
+	if (sizeof(*xdpf) + totsize > SKB_WITH_OVERHEAD(PAGE_SIZE))
 		return NULL;
 
 	page = dev_alloc_page();

@@ -602,7 +602,7 @@ struct xdp_frame *xdp_convert_zc_to_xdp_
 
 	xdpf->data = addr + metasize;
 	xdpf->len = totsize - metasize;
-	xdpf->headroom = 0;
+	xdpf->headroom = metasize;
 	xdpf->metasize = metasize;
 	xdpf->frame_sz = PAGE_SIZE;
 	xdpf->mem_type = MEM_TYPE_PAGE_ORDER0;

6.18-stable review patch. If anyone has any objections, please let me know.

------------------

From: Aleksandr Khromov <[email protected]>

commit 2c4e7c42d77e78ad595dbb9e4b5886b58b45d89d upstream.

slip_devs[] stores bare net_device pointers and takes no reference on them. sl_sync() and sl_alloc() walk that table from slip_open() under rtnl_lock(), while an entry is dropped by sl_free_netdev(), which sl_setup() installs as dev->priv_destructor.

priv_destructor is called from netdev_run_todo(), which deliberately runs with the RTNL semaphore released so that it can sleep while waiting for the device refcount to drop:

	/* Snapshot list, allow later requests */
	list_replace_init(&net_todo_list, &list);

	__rtnl_unlock();
	...
		if (dev->priv_destructor)
			dev->priv_destructor(dev);	/* slip_devs[i] = NULL */
		if (dev->needs_free_netdev)
			free_netdev(dev);
		...
		/* Free network device */
		kobject_put(&dev->dev.kobj);

So rtnl_lock() does not serialise slip_open() against the teardown at all. sl_sync() can load slip_devs[i] while the entry is still published and dereference it after netdev_run_todo() has run the destructor and released the device:

  CPU0 (slip_open)                 CPU1 (slip_close)
                                   unregister_netdev()
                                     rtnl_unlock()
                                       netdev_run_todo()
                                         __rtnl_unlock()
  rtnl_lock()
  sl_sync()
    dev = slip_devs[i]
                                         priv_destructor(dev)
                                           slip_devs[i] = NULL
                                         kobject_put(&dev->dev.kobj)
                                           /* dev is freed */
    sl = netdev_priv(dev)
    if (sl->tty || sl->leased)     /* use-after-free */

  BUG: KASAN: use-after-free in sl_sync drivers/net/slip/slip.c:730 [inline]
  BUG: KASAN: use-after-free in slip_open+0xef4/0x1210 drivers/net/slip/slip.c:806
  Read of size 1 at addr ffff8880712dac71 by task syz-executor.2/6506

  CPU: 2 PID: 6506 Comm: syz-executor.2 Not tainted 6.1.134-syzkaller-00260-g0c8fc3469765 #0
  Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.12.0-1 04/01/2014
  Call Trace:
   sl_sync drivers/net/slip/slip.c:730 [inline]
   slip_open+0xef4/0x1210 drivers/net/slip/slip.c:806
   tty_ldisc_open+0xa2/0x120 drivers/tty/tty_ldisc.c:433
   tty_set_ldisc+0x324/0x720 drivers/tty/tty_ldisc.c:564
   tiocsetd drivers/tty/tty_io.c:2428 [inline]
   tty_ioctl+0x5f0/0x1530 drivers/tty/tty_io.c:2712

  Allocated by task 6502:
   alloc_netdev_mqs+0x98/0xfe0 net/core/dev.c:10719
   sl_alloc drivers/net/slip/slip.c:756 [inline]
   slip_open+0x36d/0x1210 drivers/net/slip/slip.c:817
   tty_ldisc_open+0xa2/0x120 drivers/tty/tty_ldisc.c:433
   tty_set_ldisc+0x324/0x720 drivers/tty/tty_ldisc.c:564

  Freed by task 6497:
   device_release+0xa2/0x240 drivers/base/core.c:2507
   kobject_put+0x179/0x280 lib/kobject.c:729
   netdev_run_todo+0x6c8/0xef0 net/core/dev.c:10509
   slip_close+0x166/0x1c0 drivers/net/slip/slip.c:906
   tty_ldisc_close+0x113/0x1a0 drivers/tty/tty_ldisc.c:456
   tty_ldisc_kill+0x94/0x160 drivers/tty/tty_ldisc.c:614
   tty_ldisc_release+0xe3/0x2b0 drivers/tty/tty_ldisc.c:782
   tty_release+0xbcc/0xe70 drivers/tty/tty_io.c:1860

Commit e58c19124189 ("slip: Fix use-after-free Read in slip_open") fixed a different source of stale entries - a device left in slip_devs[] after slip_open() freed it on the registration error path - and does not address this race, which is why the report survives it.

Drop the entry from ndo_uninit instead. unregister_netdevice() calls ndo_uninit under RTNL, before the device is queued to netdev_run_todo(), so an entry that sl_sync() can still see while holding RTNL belongs to a device that cannot be freed until RTNL is dropped. sl_free_netdev() stays only for the slip_open() error path, where register_netdevice() may have failed before ndo_init and ndo_uninit is then not called either. Both running for the same device is harmless: they run under the same RTNL section, so the slot cannot have been reused in between.

This also removes the second symptom of the missing exclusion: a destructor running after sl_alloc() had already handed the slot out to another channel used to clear a live entry, so sl_sync() stopped at that NULL, sl_alloc() returned the same index again, and register_netdevice() failed with -EEXIST because slN was still there.

Reproduced on x86_64 with several threads looping over open("/dev/ptmx") + ioctl(TIOCSETD, N_SLIP) + close().

Found by Linux Verification Center (linuxtesting.org) with Syzkaller.

---
 drivers/net/slip/slip.c |   11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

@@ -628,9 +628,15 @@ static void sl_uninit(struct net_device
 	struct slip *sl = netdev_priv(dev);
 
 	sl_free_bufs(sl);
+	/* Drop the slip_devs[] entry here rather than from the destructor:
+	 * ndo_uninit runs under RTNL, so it cannot race sl_sync().
+	 */
+	slip_devs[dev->base_addr] = NULL;
 }
 
-/* Hook the destructor so we can free slip devices at the right point in time */
+/* Only for the slip_open() error path: register_netdevice() can fail before
+ * ndo_init, and then ndo_uninit is not called either.
+ */
 static void sl_free_netdev(struct net_device *dev)
 {
 	int i = dev->base_addr;

@@ -657,7 +663,6 @@ static void sl_setup(struct net_device *
 {
 	dev->netdev_ops		= &sl_netdev_ops;
 	dev->needs_free_netdev	= true;
-	dev->priv_destructor	= sl_free_netdev;
 
 	dev->hard_header_len	= 0;
 	dev->addr_len		= 0;

@@ -908,7 +913,7 @@ static void slip_close(struct tty_struct
 #endif
 	/* Flush network side */
 	unregister_netdev(sl->dev);
-	/* This will complete via sl_free_netdev */
+	/* sl_uninit() has dropped the slip_devs[] entry by now */
 }
 
 static void slip_hangup(struct tty_struct *tty)

6.18-stable review patch. If anyone has any objections, please let me know.

------------------

From: Fabio Porcedda <[email protected]>

commit 1056e79fffd0841f43c6a1b25664b196b3caf1c6 upstream.

Add the followin Telit Cinterion FE990D50 composition:

0x0991: rmnet + tty (AT/NMEA) + tty (AT) + tty (AT) + tty (AT) +
tty (diag) + ADPL + adb

T:  Bus=01 Lev=01 Prnt=01 Port=06 Cnt=03 Dev#= 10 Spd=480  MxCh= 0
D:  Ver= 2.10 Cls=00(>ifc ) Sub=00 Prot=00 MxPS=64 #Cfgs=  1
P:  Vendor=1bc7 ProdID=0991 Rev=06.06
S:  Manufacturer=Telit Cinterion
S:  Product=FE990
S:  SerialNumber=2aa802d2
C:  #Ifs= 9 Cfg#= 1 Atr=e0 MxPwr=500mA
I:  If#= 0 Alt= 0 #EPs= 3 Cls=ff(vend.) Sub=ff Prot=50 Driver=qmi_wwan
E:  Ad=01(O) Atr=02(Bulk) MxPS= 512 Ivl=0ms
E:  Ad=81(I) Atr=02(Bulk) MxPS= 512 Ivl=0ms
E:  Ad=82(I) Atr=03(Int.) MxPS=   8 Ivl=32ms
I:  If#= 1 Alt= 0 #EPs= 3 Cls=ff(vend.) Sub=ff Prot=60 Driver=option
E:  Ad=02(O) Atr=02(Bulk) MxPS= 512 Ivl=0ms
E:  Ad=83(I) Atr=02(Bulk) MxPS= 512 Ivl=0ms
E:  Ad=84(I) Atr=03(Int.) MxPS=  10 Ivl=32ms
I:  If#= 2 Alt= 0 #EPs= 3 Cls=ff(vend.) Sub=ff Prot=40 Driver=option
E:  Ad=03(O) Atr=02(Bulk) MxPS= 512 Ivl=0ms
E:  Ad=85(I) Atr=02(Bulk) MxPS= 512 Ivl=0ms
E:  Ad=86(I) Atr=03(Int.) MxPS=  10 Ivl=32ms
I:  If#= 3 Alt= 0 #EPs= 3 Cls=ff(vend.) Sub=ff Prot=40 Driver=option
E:  Ad=04(O) Atr=02(Bulk) MxPS= 512 Ivl=0ms
E:  Ad=87(I) Atr=02(Bulk) MxPS= 512 Ivl=0ms
E:  Ad=88(I) Atr=03(Int.) MxPS=  10 Ivl=32ms
I:  If#= 4 Alt= 0 #EPs= 3 Cls=ff(vend.) Sub=ff Prot=40 Driver=option
E:  Ad=05(O) Atr=02(Bulk) MxPS= 512 Ivl=0ms
E:  Ad=89(I) Atr=02(Bulk) MxPS= 512 Ivl=0ms
E:  Ad=8a(I) Atr=03(Int.) MxPS=  10 Ivl=32ms
I:  If#= 5 Alt= 0 #EPs= 2 Cls=ff(vend.) Sub=ff Prot=30 Driver=option
E:  Ad=06(O) Atr=02(Bulk) MxPS= 512 Ivl=0ms
E:  Ad=8b(I) Atr=02(Bulk) MxPS= 512 Ivl=0ms
I:  If#= 6 Alt= 0 #EPs= 1 Cls=ff(vend.) Sub=ff Prot=80 Driver=(none)
E:  Ad=8c(I) Atr=02(Bulk) MxPS= 512 Ivl=0ms
I:  If#= 7 Alt= 0 #EPs= 1 Cls=ff(vend.) Sub=ff Prot=70 Driver=(none)
E:  Ad=8d(I) Atr=02(Bulk) MxPS= 512 Ivl=0ms
I:  If#= 8 Alt= 0 #EPs= 2 Cls=ff(vend.) Sub=42 Prot=01 Driver=(none)
E:  Ad=07(O) Atr=02(Bulk) MxPS= 512 Ivl=0ms
E:  Ad=8e(I) Atr=02(Bulk) MxPS= 512 Ivl=0ms

---
 drivers/net/usb/qmi_wwan.c |    1 +
 1 file changed, 1 insertion(+)

@@ -1360,6 +1360,7 @@ static const struct usb_device_id produc
 	{QMI_FIXED_INTF(0x1bbb, 0x0203, 2)},	/* Alcatel L800MA */
 	{QMI_FIXED_INTF(0x2357, 0x0201, 4)},	/* TP-LINK HSUPA Modem MA180 */
 	{QMI_FIXED_INTF(0x2357, 0x9000, 4)},	/* TP-LINK MA260 */
+	{QMI_QUIRK_SET_DTR(0x1bc7, 0x0991, 0)}, /* Telit FE990D50 */
 	{QMI_QUIRK_SET_DTR(0x1bc7, 0x1031, 3)}, /* Telit LE910C1-EUX */
 	{QMI_QUIRK_SET_DTR(0x1bc7, 0x1034, 2)}, /* Telit LE910C4-WWX */
 	{QMI_QUIRK_SET_DTR(0x1bc7, 0x1037, 4)}, /* Telit LE910C4-WWX */

6.18-stable review patch. If anyone has any objections, please let me know.

------------------

From: Asim Viladi Oglu Manizada <[email protected]>

commit 447c9303942c439a117d9b76ce6d6e2116b38ee7 upstream.

tun_get_user() uses tun->align both as skb headroom and when choosing how much packet data to keep linear. OVS can propagate an oversized headroom request from another port to TUN or TAP.

When align is larger than the usable space in a one-page skb head, SKB_MAX_HEAD(align) underflows and the result becomes negative when stored in good_linear. That value later wraps when assigned to the size_t linear variable, and tun_alloc_skb() can place skb->data outside the allocated head.

Bound the headroom stored by TUN to the one-page skb-head budget and the largest non-sentinel 16-bit skb header offset. Leave one linear byte for raw TUN and a complete Ethernet header for TAP, including NET_IP_ALIGN.

Also pull the raw-TUN protocol byte and the TAP Ethernet header before accessing them, so these checks remain safe for nonlinear skbs supplied by other allocation paths.

---
 drivers/net/tun.c |   21 ++++++++++++++++-----
 1 file changed, 16 insertions(+), 5 deletions(-)

@@ -1105,11 +1105,16 @@ static netdev_features_t tun_net_fix_fea
 static void tun_set_headroom(struct net_device *dev, int new_hr)
 {
 	struct tun_struct *tun = netdev_priv(dev);
+	size_t max_headroom;
 
-	if (new_hr < NET_SKB_PAD)
-		new_hr = NET_SKB_PAD;
+	max_headroom = min_t(size_t, SKB_MAX_HEAD(0), U16_MAX - 1);
 
-	tun->align = new_hr;
+	if ((tun->flags & TUN_TYPE_MASK) == IFF_TAP)
+		max_headroom -= ETH_HLEN + NET_IP_ALIGN;
+	else
+		max_headroom -= 1;
+
+	tun->align = clamp_t(int, new_hr, NET_SKB_PAD, max_headroom);
 }
 
 static void