[PATCH 3.2 66/95] af_key: Add lock to key dump

From: Ben Hutchings
Date: Sun Jul 16 2017 - 10:55:46 EST


3.2.91-rc1 review patch. If anyone has any objections, please let me know.

------------------

From: Yuejie Shi <syjcnss@xxxxxxxxx>

commit 89e357d83c06b6fac581c3ca7f0ee3ae7e67109e upstream.

A dump may come in the middle of another dump, modifying its dump
structure members. This race condition will result in NULL pointer
dereference in kernel. So add a lock to prevent that race.

Fixes: 83321d6b9872 ("[AF_KEY]: Dump SA/SP entries non-atomically")
Signed-off-by: Yuejie Shi <syjcnss@xxxxxxxxx>
Signed-off-by: Steffen Klassert <steffen.klassert@xxxxxxxxxxx>
[bwh: Backported to 3.2:
- pfkey_dump() doesn't support filters
- Adjust context]
Signed-off-by: Ben Hutchings <ben@xxxxxxxxxxxxxxx>
---
--- a/net/key/af_key.c
+++ b/net/key/af_key.c
@@ -63,6 +63,7 @@ struct pfkey_sock {
} u;
struct sk_buff *skb;
} dump;
+ struct mutex dump_lock;
};

static inline struct pfkey_sock *pfkey_sk(struct sock *sk)
@@ -139,6 +140,7 @@ static int pfkey_create(struct net *net,
{
struct netns_pfkey *net_pfkey = net_generic(net, pfkey_net_id);
struct sock *sk;
+ struct pfkey_sock *pfk;
int err;

if (!capable(CAP_NET_ADMIN))
@@ -153,6 +155,9 @@ static int pfkey_create(struct net *net,
if (sk == NULL)
goto out;

+ pfk = pfkey_sk(sk);
+ mutex_init(&pfk->dump_lock);
+
sock->ops = &pfkey_ops;
sock_init_data(sock, sk);

@@ -283,13 +288,23 @@ static int pfkey_do_dump(struct pfkey_so
struct sadb_msg *hdr;
int rc;

+ mutex_lock(&pfk->dump_lock);
+ if (!pfk->dump.dump) {
+ rc = 0;
+ goto out;
+ }
+
rc = pfk->dump.dump(pfk);
- if (rc == -ENOBUFS)
- return 0;
+ if (rc == -ENOBUFS) {
+ rc = 0;
+ goto out;
+ }

if (pfk->dump.skb) {
- if (!pfkey_can_dump(&pfk->sk))
- return 0;
+ if (!pfkey_can_dump(&pfk->sk)) {
+ rc = 0;
+ goto out;
+ }

hdr = (struct sadb_msg *) pfk->dump.skb->data;
hdr->sadb_msg_seq = 0;
@@ -300,6 +315,9 @@ static int pfkey_do_dump(struct pfkey_so
}

pfkey_terminate_dump(pfk);
+
+out:
+ mutex_unlock(&pfk->dump_lock);
return rc;
}

@@ -1791,18 +1809,24 @@ static int pfkey_dump(struct sock *sk, s
u8 proto;
struct pfkey_sock *pfk = pfkey_sk(sk);

- if (pfk->dump.dump != NULL)
+ mutex_lock(&pfk->dump_lock);
+ if (pfk->dump.dump != NULL) {
+ mutex_unlock(&pfk->dump_lock);
return -EBUSY;
+ }

proto = pfkey_satype2proto(hdr->sadb_msg_satype);
- if (proto == 0)
+ if (proto == 0) {
+ mutex_unlock(&pfk->dump_lock);
return -EINVAL;
+ }

pfk->dump.msg_version = hdr->sadb_msg_version;
pfk->dump.msg_pid = hdr->sadb_msg_pid;
pfk->dump.dump = pfkey_dump_sa;
pfk->dump.done = pfkey_dump_sa_done;
xfrm_state_walk_init(&pfk->dump.u.state, proto);
+ mutex_unlock(&pfk->dump_lock);

return pfkey_do_dump(pfk);
}
@@ -2661,14 +2685,18 @@ static int pfkey_spddump(struct sock *sk
{
struct pfkey_sock *pfk = pfkey_sk(sk);

- if (pfk->dump.dump != NULL)
+ mutex_lock(&pfk->dump_lock);
+ if (pfk->dump.dump != NULL) {
+ mutex_unlock(&pfk->dump_lock);
return -EBUSY;
+ }

pfk->dump.msg_version = hdr->sadb_msg_version;
pfk->dump.msg_pid = hdr->sadb_msg_pid;
pfk->dump.dump = pfkey_dump_sp;
pfk->dump.done = pfkey_dump_sp_done;
xfrm_policy_walk_init(&pfk->dump.u.policy, XFRM_POLICY_TYPE_MAIN);
+ mutex_unlock(&pfk->dump_lock);

return pfkey_do_dump(pfk);
}