Commit 7d86aa41 authored by Chengfeng Ye's avatar Chengfeng Ye Committed by Paolo Abeni
Browse files

mctp: route: hold key->lock in mctp_flow_prepare_output()



mctp_flow_prepare_output() checks key->dev and may call
mctp_dev_set_key(), but it does not hold key->lock while doing so.

mctp_dev_set_key() and mctp_dev_release_key() are annotated with
__must_hold(&key->lock), so key->dev access is intended to be
serialized by key->lock. The mctp_sendmsg() transmit path reaches
mctp_flow_prepare_output() via mctp_local_output() -> mctp_dst_output()
without holding key->lock, so the check-and-set sequence is racy.

Example interleaving:

  CPU0                                  CPU1
  ----                                  ----
  mctp_flow_prepare_output(key, devA)
    if (!key->dev)  // sees NULL
                                        mctp_flow_prepare_output(
                                            key, devB)
                                          if (!key->dev)  // still NULL
                                          mctp_dev_set_key(devB, key)
                                            mctp_dev_hold(devB)
                                            key->dev = devB
    mctp_dev_set_key(devA, key)
      mctp_dev_hold(devA)
      key->dev = devA   // overwrites devB

Now both devA and devB references were acquired, but only the final
key->dev value is tracked for release. One reference can be lost,
causing a resource leak as mctp_dev_release_key() would only decrease
the reference on one dev.

Fix by taking key->lock around the key->dev check and
mctp_dev_set_key() call.

Fixes: 67737c45 ("mctp: Pass flow data & flow release events to drivers")
Signed-off-by: default avatarChengfeng Ye <dg573847474@gmail.com>
Link: https://patch.msgid.link/20260306031402.857224-1-dg573847474@gmail.com


Signed-off-by: default avatarPaolo Abeni <pabeni@redhat.com>
parent 950803f7
Loading
Loading
Loading
Loading
+7 −4
Original line number Diff line number Diff line
@@ -359,6 +359,7 @@ static void mctp_flow_prepare_output(struct sk_buff *skb, struct mctp_dev *dev)
{
	struct mctp_sk_key *key;
	struct mctp_flow *flow;
	unsigned long flags;

	flow = skb_ext_find(skb, SKB_EXT_MCTP);
	if (!flow)
@@ -366,12 +367,14 @@ static void mctp_flow_prepare_output(struct sk_buff *skb, struct mctp_dev *dev)

	key = flow->key;

	if (key->dev) {
		WARN_ON(key->dev != dev);
		return;
	}
	spin_lock_irqsave(&key->lock, flags);

	if (!key->dev)
		mctp_dev_set_key(dev, key);
	else
		WARN_ON(key->dev != dev);

	spin_unlock_irqrestore(&key->lock, flags);
}
#else
static void mctp_skb_set_flow(struct sk_buff *skb, struct mctp_sk_key *key) {}