netdev - [PATCH net-next] net: mctp-i2c: invalidate flows immediately on TX errors

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-Id: <20240710-mctp-next-v1-1-aefc275966c3@codeconstruct.com.au>
Date: Wed, 10 Jul 2024 10:17:22 +0800
From: Jeremy Kerr <jk@...econstruct.com.au>
To: Matt Johnston <matt@...econstruct.com.au>, 
 "David S. Miller" <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>, 
 Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>
Cc: netdev@...r.kernel.org, Bonnie Lo <Bonnie_Lo@...ynn.com>, 
 Jerry C Chen <Jerry_C_Chen@...ynn.com>, 
 Jeremy Kerr <jk@...econstruct.com.au>
Subject: [PATCH net-next] net: mctp-i2c: invalidate flows immediately on TX
 errors

If we encounter an error on i2c packet transmit, we won't have a valid
flow anymore; since we didn't transmit a valid packet sequence, we'll
have to wait for the key to timeout instead of dropping it on the reply.

This causes the i2c lock to be held for longer than necessary.

Instead, invalidate the flow on TX error, and release the i2c lock
immediately.

Cc: Bonnie Lo <Bonnie_Lo@...ynn.com>
Tested-by: Jerry C Chen <Jerry_C_Chen@...ynn.com>
Signed-off-by: Jeremy Kerr <jk@...econstruct.com.au>
---
 drivers/net/mctp/mctp-i2c.c | 41 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 41 insertions(+)

diff --git a/drivers/net/mctp/mctp-i2c.c b/drivers/net/mctp/mctp-i2c.c
index b37a9e4bade4..4005a41bbd48 100644
--- a/drivers/net/mctp/mctp-i2c.c
+++ b/drivers/net/mctp/mctp-i2c.c
@@ -442,6 +442,42 @@ static void mctp_i2c_unlock_reset(struct mctp_i2c_dev *midev)
 		i2c_unlock_bus(midev->adapter, I2C_LOCK_SEGMENT);
 }
 
+static void mctp_i2c_invalidate_tx_flow(struct mctp_i2c_dev *midev,
+					struct sk_buff *skb)
+{
+	struct mctp_sk_key *key;
+	struct mctp_flow *flow;
+	unsigned long flags;
+	bool release;
+
+	flow = skb_ext_find(skb, SKB_EXT_MCTP);
+	if (!flow)
+		return;
+
+	key = flow->key;
+	if (!key)
+		return;
+
+	spin_lock_irqsave(&key->lock, flags);
+	if (key->manual_alloc) {
+		/* we don't have control over lifetimes for manually-allocated
+		 * keys, so cannot assume we can invalidate all future flows
+		 * that would use this key.
+		 */
+		release = false;
+	} else {
+		release = key->dev_flow_state == MCTP_I2C_FLOW_STATE_ACTIVE;
+		key->dev_flow_state = MCTP_I2C_FLOW_STATE_INVALID;
+	}
+	spin_unlock_irqrestore(&key->lock, flags);
+
+	/* if we have changed state from active, the flow held a reference on
+	 * the lock; release that now.
+	 */
+	if (release)
+		mctp_i2c_unlock_nest(midev);
+}
+
 static void mctp_i2c_xmit(struct mctp_i2c_dev *midev, struct sk_buff *skb)
 {
 	struct net_device_stats *stats = &midev->ndev->stats;
@@ -500,6 +536,11 @@ static void mctp_i2c_xmit(struct mctp_i2c_dev *midev, struct sk_buff *skb)
 	case MCTP_I2C_TX_FLOW_EXISTING:
 		/* existing flow: we already have the lock; just tx */
 		rc = __i2c_transfer(midev->adapter, &msg, 1);
+
+		/* on tx errors, the flow can no longer be considered valid */
+		if (rc)
+			mctp_i2c_invalidate_tx_flow(midev, skb);
+
 		break;
 
 	case MCTP_I2C_TX_FLOW_INVALID:

---
base-commit: 34afb82a3c67f869267a26f593b6f8fc6bf35905
change-id: 20240710-mctp-next-cf7031122060

Best regards,
-- 
Jeremy Kerr <jk@...econstruct.com.au>