lkml.org 
[lkml]   [2022]   [Oct]   [25]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    Date
    From
    Subject[v3 PATCH] af_key: Fix send_acquire race with pfkey_register
    On Mon, Oct 24, 2022 at 09:20:00AM +0200, Sabrina Dubroca wrote:
    > 2022-10-24, 14:06:12 +0800, Herbert Xu wrote:
    > > @@ -1697,11 +1699,11 @@ static int pfkey_register(struct sock *sk, struct sk_buff *skb, const struct sad
    > > pfk->registered |= (1<<hdr->sadb_msg_satype);
    > > }
    > >
    > > - mutex_lock(&pfkey_mutex);
    > > + spin_lock_bh(&pfkey_alg_lock);
    > > xfrm_probe_algs();
    >
    > I don't think we can do that:
    >
    > void xfrm_probe_algs(void)
    > {
    > int i, status;
    >
    > BUG_ON(in_softirq());

    Indeed. I was also wrong in stating that this bug was created by
    namespaces. This race has always existed since this code was first
    added.

    ---8<---
    The function pfkey_send_acquire may race with pfkey_register
    (which could even be in a different name space). This may result
    in a buffer overrun.

    Allocating the maximum amount of memory that could be used prevents
    this.

    Reported-by: syzbot+1e9af9185d8850e2c2fa@syzkaller.appspotmail.com
    Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
    Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>

    diff --git a/net/key/af_key.c b/net/key/af_key.c
    index c85df5b958d2..213287814328 100644
    --- a/net/key/af_key.c
    +++ b/net/key/af_key.c
    @@ -2905,7 +2905,7 @@ static int count_ah_combs(const struct xfrm_tmpl *t)
    break;
    if (!aalg->pfkey_supported)
    continue;
    - if (aalg_tmpl_set(t, aalg) && aalg->available)
    + if (aalg_tmpl_set(t, aalg))
    sz += sizeof(struct sadb_comb);
    }
    return sz + sizeof(struct sadb_prop);
    @@ -2923,7 +2923,7 @@ static int count_esp_combs(const struct xfrm_tmpl *t)
    if (!ealg->pfkey_supported)
    continue;

    - if (!(ealg_tmpl_set(t, ealg) && ealg->available))
    + if (!(ealg_tmpl_set(t, ealg)))
    continue;

    for (k = 1; ; k++) {
    @@ -2934,16 +2934,17 @@ static int count_esp_combs(const struct xfrm_tmpl *t)
    if (!aalg->pfkey_supported)
    continue;

    - if (aalg_tmpl_set(t, aalg) && aalg->available)
    + if (aalg_tmpl_set(t, aalg))
    sz += sizeof(struct sadb_comb);
    }
    }
    return sz + sizeof(struct sadb_prop);
    }

    -static void dump_ah_combs(struct sk_buff *skb, const struct xfrm_tmpl *t)
    +static int dump_ah_combs(struct sk_buff *skb, const struct xfrm_tmpl *t)
    {
    struct sadb_prop *p;
    + int sz = 0;
    int i;

    p = skb_put(skb, sizeof(struct sadb_prop));
    @@ -2971,13 +2972,17 @@ static void dump_ah_combs(struct sk_buff *skb, const struct xfrm_tmpl *t)
    c->sadb_comb_soft_addtime = 20*60*60;
    c->sadb_comb_hard_usetime = 8*60*60;
    c->sadb_comb_soft_usetime = 7*60*60;
    + sz += sizeof(*c);
    }
    }
    +
    + return sz + sizeof(*p);
    }

    -static void dump_esp_combs(struct sk_buff *skb, const struct xfrm_tmpl *t)
    +static int dump_esp_combs(struct sk_buff *skb, const struct xfrm_tmpl *t)
    {
    struct sadb_prop *p;
    + int sz = 0;
    int i, k;

    p = skb_put(skb, sizeof(struct sadb_prop));
    @@ -3019,8 +3024,11 @@ static void dump_esp_combs(struct sk_buff *skb, const struct xfrm_tmpl *t)
    c->sadb_comb_soft_addtime = 20*60*60;
    c->sadb_comb_hard_usetime = 8*60*60;
    c->sadb_comb_soft_usetime = 7*60*60;
    + sz += sizeof(*c);
    }
    }
    +
    + return sz + sizeof(*p);
    }

    static int key_notify_policy_expire(struct xfrm_policy *xp, const struct km_event *c)
    @@ -3150,6 +3158,7 @@ static int pfkey_send_acquire(struct xfrm_state *x, struct xfrm_tmpl *t, struct
    struct sadb_x_sec_ctx *sec_ctx;
    struct xfrm_sec_ctx *xfrm_ctx;
    int ctx_size = 0;
    + int alg_size = 0;

    sockaddr_size = pfkey_sockaddr_size(x->props.family);
    if (!sockaddr_size)
    @@ -3161,16 +3170,16 @@ static int pfkey_send_acquire(struct xfrm_state *x, struct xfrm_tmpl *t, struct
    sizeof(struct sadb_x_policy);

    if (x->id.proto == IPPROTO_AH)
    - size += count_ah_combs(t);
    + alg_size = count_ah_combs(t);
    else if (x->id.proto == IPPROTO_ESP)
    - size += count_esp_combs(t);
    + alg_size = count_esp_combs(t);

    if ((xfrm_ctx = x->security)) {
    ctx_size = PFKEY_ALIGN8(xfrm_ctx->ctx_len);
    size += sizeof(struct sadb_x_sec_ctx) + ctx_size;
    }

    - skb = alloc_skb(size + 16, GFP_ATOMIC);
    + skb = alloc_skb(size + alg_size + 16, GFP_ATOMIC);
    if (skb == NULL)
    return -ENOMEM;

    @@ -3224,10 +3233,13 @@ static int pfkey_send_acquire(struct xfrm_state *x, struct xfrm_tmpl *t, struct
    pol->sadb_x_policy_priority = xp->priority;

    /* Set sadb_comb's. */
    + alg_size = 0;
    if (x->id.proto == IPPROTO_AH)
    - dump_ah_combs(skb, t);
    + alg_size = dump_ah_combs(skb, t);
    else if (x->id.proto == IPPROTO_ESP)
    - dump_esp_combs(skb, t);
    + alg_size = dump_esp_combs(skb, t);
    +
    + hdr->sadb_msg_len += alg_size / 8;

    /* security context */
    if (xfrm_ctx) {
    --
    Email: Herbert Xu <herbert@gondor.apana.org.au>
    Home Page: http://gondor.apana.org.au/~herbert/
    PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
    \
     
     \ /
      Last update: 2022-10-25 08:07    [W:5.065 / U:0.036 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site