Skip to contentSkip to conversation

iommu/vt-d: Fix NULL pointer dereference in prq_event_thread()

PATCH New Sasha Levin 1 participant 13 messages Nov 29, 2018

Subsystems (5)
lore
Share & tools
lore

Sasha Levin

author maintainer
#0 lore raw

PATCH 1/13 iommu/vt-d: Fix NULL pointer dereference in prq_event_thread()

From: Lu Baolu <[email protected]>

[ Upstream commit 19ed3e2dd8549c1a34914e8dad01b64e7837645a ]

When handling page request without pasid event, go to "no_pasid" branch instead of "bad_req". Otherwise, a NULL pointer deference will happen there.

Cc Ashok Raj <[email protected]>
Cc Jacob Pan <[email protected]>
Cc Sohil Mehta <[email protected]>
Fixes a222a7f0bb6c9 'iommu/vt-d: Implement page request handling'
---
 drivers/iommu/intel-svm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/iommu/intel-svm.c +1 -1
index 10068a481e22..cbde03e509c1 100644
@@ -558,7 +558,7 @@ static irqreturn_t prq_event_thread(int irq, void *d)
 			pr_err("%s: Page request without PASID: %08llx %08llx\n",
 			       iommu->name, ((unsigned long long *)req)[0],
 			       ((unsigned long long *)req)[1]);
-			goto bad_req;
+			goto no_pasid;
 		}
 
 		if (!svm || svm->pasid != req->pasid) {
Signature · 1 line
2.17.1
author maintainer
#1 lore raw

PATCH 2/13 iommu/ipmmu-vmsa: Fix crash on early domain free

From: Geert Uytterhoeven <[email protected]>

[ Upstream commit e5b78f2e349eef5d4fca5dc1cf5a3b4b2cc27abd ]

If iommu_ops.add_device() fails, iommu_ops.domain_free() is still called, leading to a crash, as the domain was only partially initialized:

    ipmmu-vmsa e67b0000.mmu: Cannot accommodate DMA translation for IOMMU page tables
    sata_rcar ee300000.sata: Unable to initialize IPMMU context
    iommu: Failed to add device ee300000.sata to group 0: -22
    Unable to handle kernel NULL pointer dereference at virtual address 0000000000000038
    ...
    Call trace:
     ipmmu_domain_free+0x1c/0xa0
     iommu_group_release+0x48/0x68
     kobject_put+0x74/0xe8
     kobject_del.part.0+0x3c/0x50
     kobject_put+0x60/0xe8
     iommu_group_get_for_dev+0xa8/0x1f0
     ipmmu_add_device+0x1c/0x40
     of_iommu_configure+0x118/0x190

Fix this by checking if the domain's context already exists, before trying to destroy it.

Fixes d25a2a16f0889 ('iommu: Add driver for Renesas VMSA-compatible IPMMU')
---
 drivers/iommu/ipmmu-vmsa.c | 3 +++
 1 file changed, 3 insertions(+)
drivers/iommu/ipmmu-vmsa.c +3 -0
index 624e7ff76166..9101be1a6b59 100644
@@ -372,6 +372,9 @@ static int ipmmu_domain_init_context(struct ipmmu_vmsa_domain *domain)
 
 static void ipmmu_domain_destroy_context(struct ipmmu_vmsa_domain *domain)
 {
+	if (!domain->mmu)
+		return;
+
 	/*
 	 * Disable the context. Flush the TLB as required when modifying the
 	 * context registers.
Signature · 1 line
2.17.1
author maintainer
#2 lore raw

PATCH 3/13 can: rcar_can: Fix erroneous registration

In reply to Sasha Levin

From: Fabrizio Castro <[email protected]>

[ Upstream commit 68c8d209cd4337da4fa04c672f0b62bb735969bc ]

Assigning 2 to "renesas,can-clock-select" tricks the driver into registering the CAN interface, even though we don't want that. This patch improves one of the checks to prevent that from happening.

Fixes 862e2b6af9413b43 ("can: rcar_can: support all input clocks")
---
 drivers/net/can/rcar_can.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)
drivers/net/can/rcar_can.c +4 -1
index bc46be39549d..9d93492ddfcc 100644
@@ -24,6 +24,9 @@
 
 #define RCAR_CAN_DRV_NAME	"rcar_can"
 
+#define RCAR_SUPPORTED_CLOCKS	(BIT(CLKR_CLKP1) | BIT(CLKR_CLKP2) | \
+				 BIT(CLKR_CLKEXT))
+
 /* Mailbox configuration:
  * mailbox 60 - 63 - Rx FIFO mailboxes
  * mailbox 56 - 59 - Tx FIFO mailboxes
@@ -789,7 +792,7 @@ static int rcar_can_probe(struct platform_device *pdev)
 		goto fail_clk;
 	}
 
-	if (clock_select >= ARRAY_SIZE(clock_names)) {
+	if (!(BIT(clock_select) & RCAR_SUPPORTED_CLOCKS)) {
 		err = -EINVAL;
 		dev_err(&pdev->dev, "invalid CAN clock selected\n");
 		goto fail_clk;
Signature · 1 line
2.17.1
author maintainer
#3 lore raw

PATCH 4/13 batman-adv: Expand merged fragment buffer for full packet

In reply to Sasha Levin

From: Sven Eckelmann <[email protected]>

[ Upstream commit d7d8bbb40a5b1f682ee6589e212934f4c6b8ad60 ]

The complete size ("total_size") of the fragmented packet is stored in the fragment header and in the size of the fragment chain. When the fragments are ready for merge, the skbuff's tail of the first fragment is expanded to have enough room after the data pointer for at least total_size. This means that it gets expanded by total_size - first_skb->len.

But this is ignoring the fact that after expanding the buffer, the fragment header is pulled by from this buffer. Assuming that the tailroom of the buffer was already 0, the buffer after the data pointer of the skbuff is now only total_size - len(fragment_header) large. When the merge function is then processing the remaining fragments, the code to copy the data over to the merged skbuff will cause an skb_over_panic when it tries to actually put enough data to fill the total_size bytes of the packet.

The size of the skb_pull must therefore also be taken into account when the buffer's tailroom is expanded.

Fixes 610bfc6bc99b ("batman-adv: Receive fragmented packets and merge")
---
 net/batman-adv/fragmentation.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
net/batman-adv/fragmentation.c +1 -1
index 5d2f9d4879b2..d50c3b003dc9 100644
@@ -266,7 +266,7 @@ batadv_frag_merge_packets(struct hlist_head *chain)
 	kfree(entry);
 
 	packet = (struct batadv_frag_packet *)skb_out->data;
-	size = ntohs(packet->total_size);
+	size = ntohs(packet->total_size) + hdr_size;
 
 	/* Make room for the rest of the fragments. */
 	if (pskb_expand_head(skb_out, 0, size - skb_out->len, GFP_ATOMIC) < 0) {
Signature · 1 line
2.17.1
author maintainer
#4 lore raw

PATCH 5/13 bnx2x: Assign unique DMAE channel number for FW DMAE transactions.

In reply to Sasha Levin

From: Sudarsana Reddy Kalluru <[email protected]>

[ Upstream commit 77e461d14ed141253573eeeb4d34eccc51e38328 ]

Driver assigns DMAE channel 0 for FW as part of START_RAMROD command. FW uses this channel for DMAE operations (e.g., TIME_SYNC implementation). Driver also uses the same channel 0 for DMAE operations for some of the PFs (e.g., PF0 on Port0). This could lead to concurrent access to the DMAE channel by FW and driver which is not legal. Hence need to assign unique DMAE id for FW.
Currently following DMAE channels are used by the clients,
MFW - OCBB/OCSD functionality uses DMAE channel 14/15
Driver 0-3 and 8-11 (for PF dmae operations)
4 and 12 (for stats requests)
Assigning unique dmae_id '13' to the FW.

Changes from previous version:
------------------------------
v2: Incorporated the review comments.

---
 drivers/net/ethernet/broadcom/bnx2x/bnx2x.h    | 7 +++++++
 drivers/net/ethernet/broadcom/bnx2x/bnx2x_sp.c | 1 +
 2 files changed, 8 insertions(+)
drivers/net/ethernet/broadcom/bnx2x/bnx2x.h +7 -0
index 1ea068815419..2491cdc2535c 100644
@@ -2291,6 +2291,13 @@ void bnx2x_igu_clear_sb_gen(struct bnx2x *bp, u8 func, u8 idu_sb_id,
 #define PMF_DMAE_C(bp)			(BP_PORT(bp) * MAX_DMAE_C_PER_PORT + \
 					 E1HVN_MAX)
 
+/* Following is the DMAE channel number allocation for the clients.
+ *   MFW: OCBB/OCSD implementations use DMAE channels 14/15 respectively.
+ *   Driver: 0-3 and 8-11 (for PF dmae operations)
+ *           4 and 12 (for stats requests)
+ */
+#define BNX2X_FW_DMAE_C                 13 /* Channel for FW DMAE operations */
+
 /* PCIE link and speed */
 #define PCICFG_LINK_WIDTH		0x1f00000
 #define PCICFG_LINK_WIDTH_SHIFT		20
drivers/net/ethernet/broadcom/bnx2x/bnx2x_sp.c +1 -0
index ff702a707a91..343e3366d751 100644
@@ -5931,6 +5931,7 @@ static inline int bnx2x_func_send_start(struct bnx2x *bp,
 	rdata->sd_vlan_tag	= cpu_to_le16(start_params->sd_vlan_tag);
 	rdata->path_id		= BP_PATH(bp);
 	rdata->network_cos_mode	= start_params->network_cos_mode;
+	rdata->dmae_cmd_id	= BNX2X_FW_DMAE_C;
 
 	rdata->vxlan_dst_port	= cpu_to_le16(start_params->vxlan_dst_port);
 	rdata->geneve_dst_port	= cpu_to_le16(start_params->geneve_dst_port);
Signature · 1 line
2.17.1
author maintainer
#5 lore raw

PATCH 6/13 qed: Fix PTT leak in qed_drain()

In reply to Sasha Levin

From: Denis Bolotin <[email protected]>

[ Upstream commit 9aaa4e8ba12972d674caeefbc5f88d83235dd697 ]

Release PTT before entering error flow.

---
 drivers/net/ethernet/qlogic/qed/qed_main.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/net/ethernet/qlogic/qed/qed_main.c +1 -1
index 35e1468d8196..b8ae6ed5c7ba 100644
@@ -1124,9 +1124,9 @@ static int qed_drain(struct qed_dev *cdev)
 			return -EBUSY;
 		}
 		rc = qed_mcp_drain(hwfn, ptt);
+		qed_ptt_release(hwfn, ptt);
 		if (rc)
 			return rc;
-		qed_ptt_release(hwfn, ptt);
 	}
 
 	return 0;
Signature · 1 line
2.17.1
author maintainer
#6 lore raw

PATCH 7/13 qed: Fix reading wrong value in loop condition

In reply to Sasha Levin

From: Denis Bolotin <[email protected]>

[ Upstream commit ed4eac20dcffdad47709422e0cb925981b056668 ]

The value of "sb_index" is written by the hardware. Reading its value and writing it to "index" must finish before checking the loop condition.

---
 drivers/net/ethernet/qlogic/qed/qed_int.c | 2 ++
 1 file changed, 2 insertions(+)
drivers/net/ethernet/qlogic/qed/qed_int.c +2 -0
index 9cc9d62c1fec..8b15a018d625 100644
@@ -177,6 +177,8 @@ static int qed_int_attentions(struct qed_hwfn *p_hwfn)
 	 */
 	do {
 		index = p_sb_attn->sb_index;
+		/* finish reading index before the loop condition */
+		dma_rmb();
 		attn_bits = le32_to_cpu(p_sb_attn->atten_bits);
 		attn_acks = le32_to_cpu(p_sb_attn->atten_ack);
 	} while (index != p_sb_attn->sb_index);
Signature · 1 line
2.17.1
author maintainer
#7 lore raw

PATCH 8/13 net/mlx4_core: Zero out lkey field in SW2HW_MPT fw command

In reply to Sasha Levin

From: Jack Morgenstein <[email protected]>

[ Upstream commit bd85fbc2038a1bbe84990b23ff69b6fc81a32b2c ]

When re-registering a user mr, the mpt information for the existing mr when running SRIOV is obtained via the QUERY_MPT fw command. The returned information includes the mpt's lkey.

This retrieved mpt information is used to move the mpt back to hardware ownership in the rereg flow (via the SW2HW_MPT fw command when running SRIOV).

The fw API spec states that for SW2HW_MPT, the lkey field must be zero. Any ConnectX-3 PF driver which checks for strict spec adherence will return failure for SW2HW_MPT if the lkey field is not zero (although the fw in practice ignores this field for SW2HW_MPT).

Thus, in order to conform to the fw API spec, set the lkey field to zero before invoking SW2HW_MPT when running SRIOV.

Fixes e630664c8383 ("mlx4_core: Add helper functions to support MR re-registration")
---
 drivers/net/ethernet/mellanox/mlx4/mr.c | 1 +
 1 file changed, 1 insertion(+)
drivers/net/ethernet/mellanox/mlx4/mr.c +1 -0
index 93195191f45b..53833c06696f 100644
@@ -366,6 +366,7 @@ int mlx4_mr_hw_write_mpt(struct mlx4_dev *dev, struct mlx4_mr *mmr,
 			container_of((void *)mpt_entry, struct mlx4_cmd_mailbox,
 				     buf);
 
+		(*mpt_entry)->lkey = 0;
 		err = mlx4_SW2HW_MPT(dev, mailbox, key);
 	}
 
Signature · 1 line
2.17.1
author maintainer
#8 lore raw

PATCH 9/13 net/mlx4_core: Fix uninitialized variable compilation warning

In reply to Sasha Levin

From: Tariq Toukan <[email protected]>

[ Upstream commit 3ea7e7ea53c9f6ee41cb69a29c375fe9dd9a56a7 ]

Initialize the uid variable to zero to avoid the compilation warning.

Fixes 7a89399ffad7 ("net/mlx4: Add mlx4_bitmap zone allocator")
---
 drivers/net/ethernet/mellanox/mlx4/alloc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/net/ethernet/mellanox/mlx4/alloc.c +1 -1
index 0c51c69f802f..a4912b11e54f 100644
@@ -339,7 +339,7 @@ void mlx4_zone_allocator_destroy(struct mlx4_zone_allocator *zone_alloc)
 static u32 __mlx4_alloc_from_zone(struct mlx4_zone_entry *zone, int count,
 				  int align, u32 skip_mask, u32 *puid)
 {
-	u32 uid;
+	u32 uid = 0;
 	u32 res;
 	struct mlx4_zone_allocator *zone_alloc = zone->allocator;
 	struct mlx4_zone_entry *curr_node;
Signature · 1 line
2.17.1
author maintainer
#9 lore raw

PATCH 10/13 net/mlx4: Fix UBSAN warning of signed integer overflow

In reply to Sasha Levin

From: Aya Levin <[email protected]>

[ Upstream commit a463146e67c848cbab5ce706d6528281b7cded08 ]

UBSAN: Undefined behavior in

drivers/net/ethernet/mellanox/mlx4/resource_tracker.c:626:29 signed integer overflow: 1802201963 + 1802201963 cannot be represented in type 'int'

The union of res_reserved and res_port_rsvd[MLX4_MAX_PORTS] monitors granting of reserved resources. The grant operation is calculated and protected, thus both members of the union cannot be negative. Changed type of res_reserved and of res_port_rsvd[MLX4_MAX_PORTS] from signed int to unsigned int, allowing large value.

Fixes 5a0d0a6161ae ("mlx4: Structures and init/teardown for VF resource quotas")
---
 drivers/net/ethernet/mellanox/mlx4/mlx4.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/net/ethernet/mellanox/mlx4/mlx4.h +2 -2
index f5fdbd53d052..db40387ffaf6 100644
@@ -537,8 +537,8 @@ struct slave_list {
 struct resource_allocator {
 	spinlock_t alloc_lock; /* protect quotas */
 	union {
-		int res_reserved;
-		int res_port_rsvd[MLX4_MAX_PORTS];
+		unsigned int res_reserved;
+		unsigned int res_port_rsvd[MLX4_MAX_PORTS];
 	};
 	union {
 		int res_free;
Signature · 1 line
2.17.1
author maintainer
#10 lore raw

PATCH 11/13 net: faraday: ftmac100: remove netif_running(netdev) check before disabling interrupts

In reply to Sasha Levin

From: Vincent Chen <[email protected]>

[ Upstream commit 426a593e641ebf0d9288f0a2fcab644a86820220 ]

In the original ftmac100_interrupt(), the interrupts are only disabled when the condition "netif_running(netdev)" is true. However, this condition causes kerenl hang in the following case. When the user requests to disable the network device, kernel will clear the bit __LINK_STATE_START from the dev->state and then call the driver's ndo_stop function. Network device interrupts are not blocked during this process. If an interrupt occurs between clearing __LINK_STATE_START and stopping network device, kernel cannot disable the interrupts due to the condition "netif_running(netdev)" in the ISR. Hence, kernel will hang due to the continuous interruption of the network device.

In order to solve the above problem, the interrupts of the network device should always be disabled in the ISR without being restricted by the condition "netif_running(netdev)".

[V2]
Remove unnecessary curly braces.

---
 drivers/net/ethernet/faraday/ftmac100.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)
drivers/net/ethernet/faraday/ftmac100.c +3 -4
index dce5f7b7f772..05e1f923f49e 100644
@@ -865,11 +865,10 @@ static irqreturn_t ftmac100_interrupt(int irq, void *dev_id)
 	struct net_device *netdev = dev_id;
 	struct ftmac100 *priv = netdev_priv(netdev);
 
-	if (likely(netif_running(netdev))) {
-		/* Disable interrupts for polling */
-		ftmac100_disable_all_int(priv);
+	/* Disable interrupts for polling */
+	ftmac100_disable_all_int(priv);
+	if (likely(netif_running(netdev)))
 		napi_schedule(&priv->napi);
-	}
 
 	return IRQ_HANDLED;
 }
Signature · 1 line
2.17.1
author maintainer
#11 lore raw

PATCH 12/13 iommu/vt-d: Use memunmap to free memremap

In reply to Sasha Levin

From: Pan Bian <[email protected]>

[ Upstream commit 829383e183728dec7ed9150b949cd6de64127809 ]

memunmap() should be used to free the return of memremap(), not iounmap().

Fixes dfddb969edf0 ('iommu/vt-d: Switch from ioremap_cache to memremap')
---
 drivers/iommu/intel-iommu.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/iommu/intel-iommu.c +1 -1
index 49b266433f4c..7feaa82f8c7c 100644
@@ -2977,7 +2977,7 @@ static int copy_context_table(struct intel_iommu *iommu,
 			}
 
 			if (old_ce)
-				iounmap(old_ce);
+				memunmap(old_ce);
 
 			ret = 0;
 			if (devfn < 0x80)
Signature · 1 line
2.17.1
author maintainer
#12 lore raw

PATCH 13/13 net: amd: add missing of_node_put()

In reply to Sasha Levin

From: Yangtao Li <[email protected]>

[ Upstream commit c44c749d3b6fdfca39002e7e48e03fe9f9fe37a3 ]

of_find_node_by_path() acquires a reference to the node returned by it and that reference needs to be dropped by its caller. This place doesn't do that, so fix it.

---
 drivers/net/ethernet/amd/sunlance.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)
drivers/net/ethernet/amd/sunlance.c +3 -1
index 7847638bdd22..8914170fccff 100644
@@ -1419,7 +1419,7 @@ static int sparc_lance_probe_one(struct platform_device *op,
 
 			prop = of_get_property(nd, "tpe-link-test?", NULL);
 			if (!prop)
-				goto no_link_test;
+				goto node_put;
 
 			if (strcmp(prop, "true")) {
 				printk(KERN_NOTICE "SunLance: warning: overriding option "
@@ -1428,6 +1428,8 @@ static int sparc_lance_probe_one(struct platform_device *op,
 				       "to [email protected]\n");
 				auxio_set_lte(AUXIO_LTE_ON);
 			}
+node_put:
+			of_node_put(nd);
 no_link_test:
 			lp->auto_select = 1;
 			lp->tpe = 0;
Signature · 1 line
2.17.1