Re: [PATCH v1 11/11] scsi: ufs: ufs-qcom: support ICE keyslot partitioning for guest VMs
flat view
From: sashiko-bot@kernel.org
Date: 2026-08-27 16:26:13
Also in:
linux-scsi
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider: - [High] Integer overflow in ICE keyslot boundary checks allows out-of-bounds hardware register access. - [High] Missing overlap validation for keyslot partitions causes silent data corruption across VMs. - [High] Inability to distinguish between a missing DT node and a 0-slot host reservation overrides explicit 0-slot allocations, breaking VM partitioning. -- commit f3bdc5333b93892b7c3cc455d6a3fd8e3b213cb4 Author: linlzhan [off-list ref] scsi: ufs: ufs-qcom: support ICE keyslot partitioning for guest VMs This commit adds support for partitioning ICE hardware keyslots so that the host and guest VMs can operate within their own physical slot ranges. It introduces a new function to parse the keyslot map from the device tree and initializes the crypto profile with the host's specific slot count.
quoted hunk ↗ jump to hunk
diff --git a/drivers/ufs/host/ufs-qcom.c b/drivers/ufs/host/ufs-qcom.c index 62396212a0a73..0611ab50f4cc9 100644 --- a/drivers/ufs/host/ufs-qcom.c +++ b/drivers/ufs/host/ufs-qcom.c
[ ... ]
quoted hunk ↗ jump to hunk
+static int ufs_qcom_ice_parse_slot_table(struct device *dev, + unsigned int hw_max_slots, + unsigned int *num_slots, + unsigned int *slot_offset) +{ + struct device_node *slots_np, *child; + unsigned int total_slots = 0; + bool first = true; + int ret = 0; + + *num_slots = 0; + *slot_offset = 0; + + slots_np = of_parse_phandle(dev->of_node, "qcom,ice-keyslot-map", 0); + if (!slots_np) + return 0; + + for_each_child_of_node(slots_np, child) { + u32 off, max; + + if (of_property_read_u32(child, "qcom,ice-slot-offset", &off) || + of_property_read_u32(child, "qcom,max-ice-slots", &max)) + continue; + + if (off + max > hw_max_slots) {
[Severity: High] If a malformed device tree provides excessively large values for qcom,ice-slot-offset and qcom,max-ice-slots, can off + max wrap around the 32-bit limit? This could defeat the bounds check and potentially allow out-of-bounds hardware register accesses.
quoted hunk ↗ jump to hunk
+ dev_err(dev, + "ice-keyslot-map: slots [%u..%u) exceed hw max %u\n", + off, off + max, hw_max_slots); + of_node_put(child); + ret = -EINVAL; + break; + } + + if (first) { + *num_slots = max; + *slot_offset = off; + first = false; + } + total_slots += max;
[Severity: High] Can total_slots overflow a 32-bit integer when adding max? This could defeat the subsequent check that ensures the total allocated slots do not exceed the hardware maximum. [Severity: High] Does this logic only sum the capacities and miss bounds collision detection between different VM partition ranges? If the explicit slot intervals overlap in the device tree configuration, could the host and guests silently overwrite each other's keys because the total sum still fits within the hardware maximum?
quoted hunk ↗ jump to hunk
+ } + + of_node_put(slots_np);
[ ... ]
quoted hunk ↗ jump to hunk
@@ -192,7 +262,23 @@ static int ufs_qcom_ice_init(struct ufs_qcom_host *host)
[ ... ]
quoted hunk ↗ jump to hunk
+ /* + * Parse the qcom,ice-keyslot-map DT node: validate all entries against + * the hardware slot count and read the host's own reservation. If no + * partitioning is configured (num_slots == 0), the profile manages the + * full hardware slot range. + */ + err = ufs_qcom_ice_parse_slot_table(dev, hw_max_slots, + &num_slots, &slot_offset); + if (err) { + dev_err(dev, "failed to parse ICE slot table: %d\n", err); + return err; + } + + err = devm_blk_crypto_profile_init(dev, profile, + num_slots ? num_slots : hw_max_slots);
[Severity: High] Since ufs_qcom_ice_parse_slot_table() initializes num_slots to 0 and returns 0 when the device tree node is absent, is it possible to distinguish between a missing node and a valid 0-slot host allocation? If a device tree explicitly allocates 0 slots to the host to dedicate all physical keyslots to VMs, does this ternary operation conflate the valid empty partition with a missing configuration and forcefully assign all hardware slots to the host's crypto profile? -- Sashiko AI review · https://sashiko.dev/#/patchset/20260827160806.1295313-1-linlin.zhang@oss.qualcomm.com?part=11