Skip to content

Commit

Permalink
Merge branch 'mptcp-fixes'
Browse files Browse the repository at this point in the history
Jeremy Kerr says:

====================
net: mctp: struct sock lifetime fixes

This series is a set of fixes for the sock lifetime handling in the
AF_MCTP code, fixing a uaf reported by Noam Rathaus
<noamr@ssd-disclosure.com>.

The Fixes: tags indicate the original patches affected, but some
tweaking to backport to those commits may be needed; I have a separate
branch with backports to 5.15 if that helps with stable trees.

Of course, any comments/queries most welcome.
====================

Signed-off-by: David S. Miller <davem@davemloft.net>
  • Loading branch information
davem330 committed Jan 25, 2023
2 parents a9e9b78 + b98e1a0 commit ac8d986
Show file tree
Hide file tree
Showing 2 changed files with 28 additions and 16 deletions.
10 changes: 7 additions & 3 deletions net/mctp/af_mctp.c
Original file line number Diff line number Diff line change
Expand Up @@ -544,9 +544,6 @@ static int mctp_sk_init(struct sock *sk)

static void mctp_sk_close(struct sock *sk, long timeout)
{
struct mctp_sock *msk = container_of(sk, struct mctp_sock, sk);

del_timer_sync(&msk->key_expiry);
sk_common_release(sk);
}

Expand Down Expand Up @@ -580,7 +577,14 @@ static void mctp_sk_unhash(struct sock *sk)
spin_lock_irqsave(&key->lock, fl2);
__mctp_key_remove(key, net, fl2, MCTP_TRACE_KEY_CLOSED);
}
sock_set_flag(sk, SOCK_DEAD);
spin_unlock_irqrestore(&net->mctp.keys_lock, flags);

/* Since there are no more tag allocations (we have removed all of the
* keys), stop any pending expiry events. the timer cannot be re-queued
* as the sk is no longer observable
*/
del_timer_sync(&msk->key_expiry);
}

static struct proto mctp_proto = {
Expand Down
34 changes: 21 additions & 13 deletions net/mctp/route.c
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,7 @@ static struct mctp_sk_key *mctp_key_alloc(struct mctp_sock *msk,
key->valid = true;
spin_lock_init(&key->lock);
refcount_set(&key->refs, 1);
sock_hold(key->sk);

return key;
}
Expand All @@ -165,6 +166,7 @@ void mctp_key_unref(struct mctp_sk_key *key)
mctp_dev_release_key(key->dev, key);
spin_unlock_irqrestore(&key->lock, flags);

sock_put(key->sk);
kfree(key);
}

Expand All @@ -177,6 +179,11 @@ static int mctp_key_add(struct mctp_sk_key *key, struct mctp_sock *msk)

spin_lock_irqsave(&net->mctp.keys_lock, flags);

if (sock_flag(&msk->sk, SOCK_DEAD)) {
rc = -EINVAL;
goto out_unlock;
}

hlist_for_each_entry(tmp, &net->mctp.keys, hlist) {
if (mctp_key_match(tmp, key->local_addr, key->peer_addr,
key->tag)) {
Expand All @@ -198,6 +205,7 @@ static int mctp_key_add(struct mctp_sk_key *key, struct mctp_sock *msk)
hlist_add_head(&key->sklist, &msk->keys);
}

out_unlock:
spin_unlock_irqrestore(&net->mctp.keys_lock, flags);

return rc;
Expand Down Expand Up @@ -315,8 +323,8 @@ static int mctp_frag_queue(struct mctp_sk_key *key, struct sk_buff *skb)

static int mctp_route_input(struct mctp_route *route, struct sk_buff *skb)
{
struct mctp_sk_key *key, *any_key = NULL;
struct net *net = dev_net(skb->dev);
struct mctp_sk_key *key;
struct mctp_sock *msk;
struct mctp_hdr *mh;
unsigned long f;
Expand Down Expand Up @@ -361,13 +369,11 @@ static int mctp_route_input(struct mctp_route *route, struct sk_buff *skb)
* key for reassembly - we'll create a more specific
* one for future packets if required (ie, !EOM).
*/
key = mctp_lookup_key(net, skb, MCTP_ADDR_ANY, &f);
if (key) {
msk = container_of(key->sk,
any_key = mctp_lookup_key(net, skb, MCTP_ADDR_ANY, &f);
if (any_key) {
msk = container_of(any_key->sk,
struct mctp_sock, sk);
spin_unlock_irqrestore(&key->lock, f);
mctp_key_unref(key);
key = NULL;
spin_unlock_irqrestore(&any_key->lock, f);
}
}

Expand Down Expand Up @@ -419,14 +425,14 @@ static int mctp_route_input(struct mctp_route *route, struct sk_buff *skb)
* this function.
*/
rc = mctp_key_add(key, msk);
if (rc) {
kfree(key);
} else {
if (!rc)
trace_mctp_key_acquire(key);

/* we don't need to release key->lock on exit */
mctp_key_unref(key);
}
/* we don't need to release key->lock on exit, so
* clean up here and suppress the unlock via
* setting to NULL
*/
mctp_key_unref(key);
key = NULL;

} else {
Expand Down Expand Up @@ -473,6 +479,8 @@ static int mctp_route_input(struct mctp_route *route, struct sk_buff *skb)
spin_unlock_irqrestore(&key->lock, f);
mctp_key_unref(key);
}
if (any_key)
mctp_key_unref(any_key);
out:
if (rc)
kfree_skb(skb);
Expand Down

0 comments on commit ac8d986

Please sign in to comment.