xfrm: prevent ipcomp scratch buffer race condition

In ipcomp_compress(), sortirq is enabled too early, allowing the
per-cpu scratch buffer to be rewritten by ipcomp_decompress()
(called on the same CPU in softirq context) between populating
the buffer and copying the compressed data to the skb.

v2: as pointed out by Steffen Klassert, if we also move the
local_bh_disable() before reading the per-cpu pointers, we can
get rid of get_cpu()/put_cpu().

v3: removed ipcomp_decompress part (as explained by Herbert Xu,
it cannot be called from process context), get rid of cpu
variable (thanks to Eric Dumazet)

Signed-off-by: Michal Kubecek <mkubecek@suse.cz>
Reviewed-by: Eric Dumazet <edumazet@google.com>
Acked-by: Herbert Xu <herbert@gondor.apana.org.au>
Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>

diff --git a/net/xfrm/xfrm_ipcomp.c b/net/xfrm/xfrm_ipcomp.c
index 2906d520eea7c2b7636fc94f3a60f5131701f57d..3be02b680268d1ccb7e3672a92777b12963b8562 100644 (file)
--- a/net/xfrm/xfrm_ipcomp.c
+++ b/net/xfrm/xfrm_ipcomp.c
@@ -141,14 +141,14 @@ static int ipcomp_compress(struct xfrm_state *x, struct sk_buff *skb)
        const int plen = skb->len;
        int dlen = IPCOMP_SCRATCH_SIZE;
        u8 *start = skb->data;
-       const int cpu = get_cpu();
-       u8 *scratch = *per_cpu_ptr(ipcomp_scratches, cpu);
-       struct crypto_comp *tfm = *per_cpu_ptr(ipcd->tfms, cpu);
+       struct crypto_comp *tfm;
+       u8 *scratch;
        int err;
 
        local_bh_disable();
+       scratch = *this_cpu_ptr(ipcomp_scratches);
+       tfm = *this_cpu_ptr(ipcd->tfms);
        err = crypto_comp_compress(tfm, start, plen, scratch, &dlen);
-       local_bh_enable();
        if (err)
                goto out;
 
@@ -158,13 +158,13 @@ static int ipcomp_compress(struct xfrm_state *x, struct sk_buff *skb)
        }
 
        memcpy(start + sizeof(struct ip_comp_hdr), scratch, dlen);
-       put_cpu();
+       local_bh_enable();
 
        pskb_trim(skb, dlen + sizeof(struct ip_comp_hdr));
        return 0;
 
 out:
-       put_cpu();
+       local_bh_enable();
        return err;
 }