| Message ID | 20131015214030.B0D06E8A60@unicorn.suse.cz |
|---|---|
| State | Superseded, archived |
| Delegated to: | David Miller |
| Headers | show |
On Tue, 2013-10-15 at 23:40 +0200, Michal Kubecek wrote: > 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. > > Add similar protection into ipcomp_decompress() as it can be > called from process context as well (even if such scenario seems > a bit artificial). > > 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(). > > Signed-off-by: Michal Kubecek <mkubecek@suse.cz> > --- > net/xfrm/xfrm_ipcomp.c | 29 ++++++++++++++++++----------- > 1 file changed, 18 insertions(+), 11 deletions(-) > > diff --git a/net/xfrm/xfrm_ipcomp.c b/net/xfrm/xfrm_ipcomp.c > index 2906d52..9fe4f42 100644 > --- a/net/xfrm/xfrm_ipcomp.c > +++ b/net/xfrm/xfrm_ipcomp.c > @@ -45,12 +45,17 @@ static int ipcomp_decompress(struct xfrm_state *x, struct sk_buff *skb) > const int plen = skb->len; > int dlen = IPCOMP_SCRATCH_SIZE; > const 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); > - int err = crypto_comp_decompress(tfm, start, plen, scratch, &dlen); > + struct crypto_comp *tfm; > + u8 *scratch; > + int cpu; > + int err; > int len; > > + local_bh_disable(); > + cpu = smp_processor_id(); > + scratch = *per_cpu_ptr(ipcomp_scratches, cpu); > + tfm = *per_cpu_ptr(ipcd->tfms, cpu); Have you tried this_cpu_ptr() instead ? -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/net/xfrm/xfrm_ipcomp.c b/net/xfrm/xfrm_ipcomp.c index 2906d52..9fe4f42 100644 --- a/net/xfrm/xfrm_ipcomp.c +++ b/net/xfrm/xfrm_ipcomp.c @@ -45,12 +45,17 @@ static int ipcomp_decompress(struct xfrm_state *x, struct sk_buff *skb) const int plen = skb->len; int dlen = IPCOMP_SCRATCH_SIZE; const 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); - int err = crypto_comp_decompress(tfm, start, plen, scratch, &dlen); + struct crypto_comp *tfm; + u8 *scratch; + int cpu; + int err; int len; + local_bh_disable(); + cpu = smp_processor_id(); + scratch = *per_cpu_ptr(ipcomp_scratches, cpu); + tfm = *per_cpu_ptr(ipcd->tfms, cpu); + err = crypto_comp_decompress(tfm, start, plen, scratch, &dlen); if (err) goto out; @@ -103,7 +108,7 @@ static int ipcomp_decompress(struct xfrm_state *x, struct sk_buff *skb) err = 0; out: - put_cpu(); + local_bh_enable(); return err; } @@ -141,14 +146,16 @@ 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 cpu; int err; local_bh_disable(); + cpu = smp_processor_id(); + scratch = *per_cpu_ptr(ipcomp_scratches, cpu); + tfm = *per_cpu_ptr(ipcd->tfms, cpu); err = crypto_comp_compress(tfm, start, plen, scratch, &dlen); - local_bh_enable(); if (err) goto out; @@ -158,13 +165,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; }
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. Add similar protection into ipcomp_decompress() as it can be called from process context as well (even if such scenario seems a bit artificial). 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(). Signed-off-by: Michal Kubecek <mkubecek@suse.cz> --- net/xfrm/xfrm_ipcomp.c | 29 ++++++++++++++++++----------- 1 file changed, 18 insertions(+), 11 deletions(-)