From patchwork Tue Oct 15 21:40:30 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Michal Kubecek X-Patchwork-Id: 283791 X-Patchwork-Delegate: davem@davemloft.net Return-Path: X-Original-To: patchwork-incoming@ozlabs.org Delivered-To: patchwork-incoming@ozlabs.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 8E2B52C0345 for ; Wed, 16 Oct 2013 08:40:36 +1100 (EST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S933696Ab3JOVkd (ORCPT ); Tue, 15 Oct 2013 17:40:33 -0400 Received: from cantor2.suse.de ([195.135.220.15]:36297 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S933181Ab3JOVkc (ORCPT ); Tue, 15 Oct 2013 17:40:32 -0400 Received: from relay2.suse.de (unknown [195.135.220.254]) by mx2.suse.de (Postfix) with ESMTP id 18D1EA598D; Tue, 15 Oct 2013 23:40:31 +0200 (CEST) Received: by unicorn.suse.cz (Postfix, from userid 1000) id B0D06E8A60; Tue, 15 Oct 2013 23:40:30 +0200 (CEST) In-Reply-To: <20131015083348.GW7660@secunet.com> References: <20131015083348.GW7660@secunet.com> From: Michal Kubecek Subject: [PATCH ipsec v2] xfrm: prevent ipcomp scratch buffer race condition To: Steffen Klassert Cc: Herbert Xu , "David S. Miller" , netdev@vger.kernel.org Message-Id: <20131015214030.B0D06E8A60@unicorn.suse.cz> Date: Tue, 15 Oct 2013 23:40:30 +0200 (CEST) Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org 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 --- 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); + 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; }