LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
To: Juri Lelli <juri.lelli@redhat.com>
Cc: rostedt@goodmis.org, tglx@linutronix.de,
	linux-rt-users@vger.kernel.org, linux-kernel@vger.kernel.org,
	williams@redhat.com
Subject: Re: [PATCH] net/xfrm/xfrm_ipcomp: Use {get,put}_cpu_light
Date: Tue, 13 Aug 2019 17:58:58 +0200	[thread overview]
Message-ID: <20190813155858.e4bcbkom2vsiafhh@linutronix.de> (raw)
In-Reply-To: <20190717072019.13681-1-juri.lelli@redhat.com>

On 2019-07-17 09:20:19 [+0200], Juri Lelli wrote:
> The following BUG has been reported while running ipsec tests.
> Hi,
> 
> This has been found on a 4.19.x-rt kernel, but 5.x-rt(s) are affected as
> well.
> 
> Best,
> 
> Juri
> ---
>  net/xfrm/xfrm_ipcomp.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/net/xfrm/xfrm_ipcomp.c b/net/xfrm/xfrm_ipcomp.c
> index a00ec715aa46..39d9e663384f 100644
> --- a/net/xfrm/xfrm_ipcomp.c
> +++ b/net/xfrm/xfrm_ipcomp.c
> @@ -45,7 +45,7 @@ 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();
> +	const int cpu = get_cpu_light();

By using get_cpu_light() you don't forbid another function to invoke
ipcomp_decompress() on the same CPU. That means that

>  	u8 *scratch = *per_cpu_ptr(ipcomp_scratches, cpu);

scratch buffer here could be used by two tasks on the same CPU. You are
aware of that right?
According to your backtrace you get here from NAPI which means BH which
means it is enough to use smp_processor_id() in such a case.

ipcomp_compress() is using the very same buffer while invoking
local_bh_disable() before using the buffer to ensure nothing else is
using the buffer on this CPU. This will work in v5.2-RT because the new
softirq code uses a local_lock() as part of local_bh_disable(). This
does not work on v4.19-RT and earlier. 

For v4.19 and earlier I suggest to use a local_lock().
For v5.2 and later I suggest to replace get_cpu() with
smp_processor_id(). Ideally a with a lockdep annotation to ensure that
BH is disabled (which we don't have).

>  	struct crypto_comp *tfm = *per_cpu_ptr(ipcd->tfms, cpu);
>  	int err = crypto_comp_decompress(tfm, start, plen, scratch, &dlen);

Sebastian

      parent reply	other threads:[~2019-08-13 15:59 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-07-17  7:20 Juri Lelli
2019-07-17  7:22 ` Daniel Bristot de Oliveira
2019-07-17 13:58 ` Pankaj Gupta
2019-08-13 15:58 ` Sebastian Andrzej Siewior [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190813155858.e4bcbkom2vsiafhh@linutronix.de \
    --to=bigeasy@linutronix.de \
    --cc=juri.lelli@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-rt-users@vger.kernel.org \
    --cc=rostedt@goodmis.org \
    --cc=tglx@linutronix.de \
    --cc=williams@redhat.com \
    --subject='Re: [PATCH] net/xfrm/xfrm_ipcomp: Use {get,put}_cpu_light' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).