Linux-rt-users archive on lore.kernel.org
 help / color / Atom feed
* [RT PATCH v2] net/xfrm/xfrm_ipcomp: Protect scratch buffer with local_lock
@ 2019-08-19 12:27 Juri Lelli
  2019-08-19 19:57 ` Steven Rostedt
                   ` (2 more replies)
  0 siblings, 3 replies; 9+ messages in thread
From: Juri Lelli @ 2019-08-19 12:27 UTC (permalink / raw)
  To: tglx, bigeasy, rostedt; +Cc: linux-rt-users, linux-kernel, williams, Juri Lelli

The following BUG has been reported while running ipsec tests.

 BUG: scheduling while atomic: irq/78-eno3-rx-/12023/0x00000002
 Modules linked in: ipcomp xfrm_ipcomp ...
 Preemption disabled at:
 [<ffffffffc0b29730>] ipcomp_input+0xd0/0x9a0 [xfrm_ipcomp]
 CPU: 1 PID: 12023 Comm: irq/78-eno3-rx- Kdump: loaded Not tainted [...] #1
 Hardware name: [...]
 Call Trace:
  dump_stack+0x5c/0x80
  ? ipcomp_input+0xd0/0x9a0 [xfrm_ipcomp]
  __schedule_bug.cold.81+0x44/0x51
  __schedule+0x5bf/0x6a0
  schedule+0x39/0xd0
  rt_spin_lock_slowlock_locked+0x10e/0x2b0
  rt_spin_lock_slowlock+0x50/0x80
  get_page_from_freelist+0x609/0x1560
  ? zlib_updatewindow+0x5a/0xd0
  __alloc_pages_nodemask+0xd9/0x280
  ipcomp_input+0x299/0x9a0 [xfrm_ipcomp]
  xfrm_input+0x5e3/0x960
  xfrm4_ipcomp_rcv+0x34/0x50
  ip_local_deliver_finish+0x22d/0x250
  ip_local_deliver+0x6d/0x110
  ? ip_rcv_finish+0xac/0x480
  ip_rcv+0x28e/0x3f9
  ? packet_rcv+0x43/0x4c0
  __netif_receive_skb_core+0xb7c/0xd10
  ? inet_gro_receive+0x8e/0x2f0
  netif_receive_skb_internal+0x4a/0x160
  napi_gro_receive+0xee/0x110
  tg3_rx+0x2a8/0x810 [tg3]
  tg3_poll_work+0x3b3/0x830 [tg3]
  tg3_poll_msix+0x3b/0x170 [tg3]
  net_rx_action+0x1ff/0x470
  ? __switch_to_asm+0x41/0x70
  do_current_softirqs+0x223/0x3e0
  ? irq_thread_check_affinity+0x20/0x20
  __local_bh_enable+0x51/0x60
  irq_forced_thread_fn+0x5e/0x80
  ? irq_finalize_oneshot.part.45+0xf0/0xf0
  irq_thread+0x13d/0x1a0
  ? wake_threads_waitq+0x30/0x30
  kthread+0x112/0x130
  ? kthread_create_worker_on_cpu+0x70/0x70
  ret_from_fork+0x35/0x40

The problem resides in the fact that get_cpu(), called from
ipcomp_input() disables preemption, and that triggers the scheduling
while atomic BUG further down the callpath chain of
get_page_from_freelist(), i.e.,

  ipcomp_input
    ipcomp_decompress
      <-- get_cpu()
      alloc_page(GFP_ATOMIC)
        alloc_pages(GFP_ATOMIC, 0)
          alloc_pages_current
            __alloc_pages_nodemask
              get_page_from_freelist
                (try_this_zone:) rmqueue
                  rmqueue_pcplist
                    __rmqueue_pcplist
                      rmqueue_bulk
                        <-- spin_lock(&zone->lock) - BUG

Fix this by replacing get_cpu() with a local lock to protect
ipcomp_scratches buffers used by ipcomp_(de)compress().

Suggested-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Signed-off-by: Juri Lelli <juri.lelli@redhat.com>

--
This v2 applies to v4.19.59-rt24.

v1 -> v2: Use a local lock instead of {get,put}_cpu_light(), as the
latter doesn't protect against multiple CPUs invoking (de)compress
function at the same time, thus concurently working on the same scratch
buffer.
---
 net/xfrm/xfrm_ipcomp.c | 19 +++++++++++++------
 1 file changed, 13 insertions(+), 6 deletions(-)

diff --git a/net/xfrm/xfrm_ipcomp.c b/net/xfrm/xfrm_ipcomp.c
index a00ec715aa46..3b4a38febf0a 100644
--- a/net/xfrm/xfrm_ipcomp.c
+++ b/net/xfrm/xfrm_ipcomp.c
@@ -18,6 +18,7 @@
 #include <linux/crypto.h>
 #include <linux/err.h>
 #include <linux/list.h>
+#include <linux/locallock.h>
 #include <linux/module.h>
 #include <linux/mutex.h>
 #include <linux/percpu.h>
@@ -35,6 +36,7 @@ struct ipcomp_tfms {
 };
 
 static DEFINE_MUTEX(ipcomp_resource_mutex);
+static DEFINE_LOCAL_IRQ_LOCK(ipcomp_scratches_lock);
 static void * __percpu *ipcomp_scratches;
 static int ipcomp_scratch_users;
 static LIST_HEAD(ipcomp_tfms_list);
@@ -45,12 +47,14 @@ 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);
-	int len;
+	u8 *scratch;
+	struct crypto_comp *tfm;
+	int err, len;
 
+	local_lock(ipcomp_scratches_lock);
+	scratch = *this_cpu_ptr(ipcomp_scratches);
+	tfm = *this_cpu_ptr(ipcd->tfms);
+	err = crypto_comp_decompress(tfm, start, plen, scratch, &dlen);
 	if (err)
 		goto out;
 
@@ -103,7 +107,7 @@ static int ipcomp_decompress(struct xfrm_state *x, struct sk_buff *skb)
 	err = 0;
 
 out:
-	put_cpu();
+	local_unlock(ipcomp_scratches_lock);
 	return err;
 }
 
@@ -146,6 +150,7 @@ static int ipcomp_compress(struct xfrm_state *x, struct sk_buff *skb)
 	int err;
 
 	local_bh_disable();
+	local_lock(ipcomp_scratches_lock);
 	scratch = *this_cpu_ptr(ipcomp_scratches);
 	tfm = *this_cpu_ptr(ipcd->tfms);
 	err = crypto_comp_compress(tfm, start, plen, scratch, &dlen);
@@ -158,12 +163,14 @@ static int ipcomp_compress(struct xfrm_state *x, struct sk_buff *skb)
 	}
 
 	memcpy(start + sizeof(struct ip_comp_hdr), scratch, dlen);
+	local_unlock(ipcomp_scratches_lock);
 	local_bh_enable();
 
 	pskb_trim(skb, dlen + sizeof(struct ip_comp_hdr));
 	return 0;
 
 out:
+	local_unlock(ipcomp_scratches_lock);
 	local_bh_enable();
 	return err;
 }
-- 
2.17.2


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [RT PATCH v2] net/xfrm/xfrm_ipcomp: Protect scratch buffer with local_lock
  2019-08-19 12:27 [RT PATCH v2] net/xfrm/xfrm_ipcomp: Protect scratch buffer with local_lock Juri Lelli
@ 2019-08-19 19:57 ` Steven Rostedt
  2019-08-20  6:43   ` Juri Lelli
  2019-08-20  5:35 ` kbuild test robot
  2019-08-20  8:28 ` Sebastian Andrzej Siewior
  2 siblings, 1 reply; 9+ messages in thread
From: Steven Rostedt @ 2019-08-19 19:57 UTC (permalink / raw)
  To: Juri Lelli; +Cc: tglx, bigeasy, linux-rt-users, linux-kernel, williams

On Mon, 19 Aug 2019 14:27:31 +0200
Juri Lelli <juri.lelli@redhat.com> wrote:

> The following BUG has been reported while running ipsec tests.

Thanks!

I'm still in the process of backporting patches to fix some bugs that
showed up with the latest merge of upstream stable. I'll add this to
the queue to add.

-- Steve


> 
>  BUG: scheduling while atomic: irq/78-eno3-rx-/12023/0x00000002
>  Modules linked in: ipcomp xfrm_ipcomp ...
>  Preemption disabled at:
>  [<ffffffffc0b29730>] ipcomp_input+0xd0/0x9a0 [xfrm_ipcomp]
>  CPU: 1 PID: 12023 Comm: irq/78-eno3-rx- Kdump: loaded Not tainted [...] #1
>  Hardware name: [...]
>  Call Trace:
>   dump_stack+0x5c/0x80
>   ? ipcomp_input+0xd0/0x9a0 [xfrm_ipcomp]
>   __schedule_bug.cold.81+0x44/0x51
>   __schedule+0x5bf/0x6a0
>   schedule+0x39/0xd0
>   rt_spin_lock_slowlock_locked+0x10e/0x2b0
>   rt_spin_lock_slowlock+0x50/0x80
>   get_page_from_freelist+0x609/0x1560
>   ? zlib_updatewindow+0x5a/0xd0
>   __alloc_pages_nodemask+0xd9/0x280
>   ipcomp_input+0x299/0x9a0 [xfrm_ipcomp]
>   xfrm_input+0x5e3/0x960
>   xfrm4_ipcomp_rcv+0x34/0x50
>   ip_local_deliver_finish+0x22d/0x250
>   ip_local_deliver+0x6d/0x110
>   ? ip_rcv_finish+0xac/0x480
>   ip_rcv+0x28e/0x3f9
>   ? packet_rcv+0x43/0x4c0
>   __netif_receive_skb_core+0xb7c/0xd10
>   ? inet_gro_receive+0x8e/0x2f0
>   netif_receive_skb_internal+0x4a/0x160
>   napi_gro_receive+0xee/0x110
>   tg3_rx+0x2a8/0x810 [tg3]
>   tg3_poll_work+0x3b3/0x830 [tg3]
>   tg3_poll_msix+0x3b/0x170 [tg3]
>   net_rx_action+0x1ff/0x470
>   ? __switch_to_asm+0x41/0x70
>   do_current_softirqs+0x223/0x3e0
>   ? irq_thread_check_affinity+0x20/0x20
>   __local_bh_enable+0x51/0x60
>   irq_forced_thread_fn+0x5e/0x80
>   ? irq_finalize_oneshot.part.45+0xf0/0xf0
>   irq_thread+0x13d/0x1a0
>   ? wake_threads_waitq+0x30/0x30
>   kthread+0x112/0x130
>   ? kthread_create_worker_on_cpu+0x70/0x70
>   ret_from_fork+0x35/0x40
> 



^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [RT PATCH v2] net/xfrm/xfrm_ipcomp: Protect scratch buffer with local_lock
  2019-08-19 12:27 [RT PATCH v2] net/xfrm/xfrm_ipcomp: Protect scratch buffer with local_lock Juri Lelli
  2019-08-19 19:57 ` Steven Rostedt
@ 2019-08-20  5:35 ` kbuild test robot
  2019-08-20  6:42   ` Juri Lelli
  2019-08-20  8:28 ` Sebastian Andrzej Siewior
  2 siblings, 1 reply; 9+ messages in thread
From: kbuild test robot @ 2019-08-20  5:35 UTC (permalink / raw)
  To: Juri Lelli
  Cc: kbuild-all, tglx, bigeasy, rostedt, linux-rt-users, linux-kernel,
	williams, Juri Lelli

[-- Attachment #1: Type: text/plain, Size: 1558 bytes --]

Hi Juri,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on linus/master]
[cannot apply to v5.3-rc5 next-20190819]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Juri-Lelli/net-xfrm-xfrm_ipcomp-Protect-scratch-buffer-with-local_lock/20190820-113542
config: ia64-allmodconfig (attached as .config)
compiler: ia64-linux-gcc (GCC) 7.4.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        GCC_VERSION=7.4.0 make.cross ARCH=ia64 

If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

>> net//xfrm/xfrm_ipcomp.c:17:10: fatal error: linux/locallock.h: No such file or directory
    #include <linux/locallock.h>
             ^~~~~~~~~~~~~~~~~~~
   compilation terminated.

vim +17 net//xfrm/xfrm_ipcomp.c

  > 17	#include <linux/locallock.h>
    18	#include <linux/module.h>
    19	#include <linux/mutex.h>
    20	#include <linux/percpu.h>
    21	#include <linux/slab.h>
    22	#include <linux/smp.h>
    23	#include <linux/vmalloc.h>
    24	#include <net/ip.h>
    25	#include <net/ipcomp.h>
    26	#include <net/xfrm.h>
    27	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 54578 bytes --]

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [RT PATCH v2] net/xfrm/xfrm_ipcomp: Protect scratch buffer with local_lock
  2019-08-20  5:35 ` kbuild test robot
@ 2019-08-20  6:42   ` Juri Lelli
  2019-08-21  1:43     ` Li, Philip
  0 siblings, 1 reply; 9+ messages in thread
From: Juri Lelli @ 2019-08-20  6:42 UTC (permalink / raw)
  To: kbuild test robot
  Cc: Juri Lelli, kbuild-all, tglx, bigeasy, rostedt, linux-rt-users,
	linux-kernel, williams

Hi,

On 20/08/19 13:35, kbuild test robot wrote:
> Hi Juri,
> 
> Thank you for the patch! Yet something to improve:
> 
> [auto build test ERROR on linus/master]
> [cannot apply to v5.3-rc5 next-20190819]
> [if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

This seems to be indeed the case, as this patch is for RT v4.19-rt
stable tree:

git://git.kernel.org/pub/scm/linux/kernel/git/rt/linux-stable-rt.git v4.19-rt

I was under the impression that putting "RT" on the subject line (before
PATCH) would prevent build bot to pick this up, but maybe something
else/different is needed?

Thanks,

Juri

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [RT PATCH v2] net/xfrm/xfrm_ipcomp: Protect scratch buffer with local_lock
  2019-08-19 19:57 ` Steven Rostedt
@ 2019-08-20  6:43   ` Juri Lelli
  0 siblings, 0 replies; 9+ messages in thread
From: Juri Lelli @ 2019-08-20  6:43 UTC (permalink / raw)
  To: Steven Rostedt
  Cc: Juri Lelli, tglx, bigeasy, linux-rt-users, linux-kernel, williams

On 19/08/19 15:57, Steven Rostedt wrote:
> On Mon, 19 Aug 2019 14:27:31 +0200
> Juri Lelli <juri.lelli@redhat.com> wrote:
> 
> > The following BUG has been reported while running ipsec tests.
> 
> Thanks!
> 
> I'm still in the process of backporting patches to fix some bugs that
> showed up with the latest merge of upstream stable. I'll add this to
> the queue to add.

Great, thank you!

Best,

Juri

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [RT PATCH v2] net/xfrm/xfrm_ipcomp: Protect scratch buffer with local_lock
  2019-08-19 12:27 [RT PATCH v2] net/xfrm/xfrm_ipcomp: Protect scratch buffer with local_lock Juri Lelli
  2019-08-19 19:57 ` Steven Rostedt
  2019-08-20  5:35 ` kbuild test robot
@ 2019-08-20  8:28 ` Sebastian Andrzej Siewior
  2 siblings, 0 replies; 9+ messages in thread
From: Sebastian Andrzej Siewior @ 2019-08-20  8:28 UTC (permalink / raw)
  To: Juri Lelli; +Cc: tglx, rostedt, linux-rt-users, linux-kernel, williams

On 2019-08-19 14:27:31 [+0200], Juri Lelli wrote:
> This v2 applies to v4.19.59-rt24.

Looks good, I suggest to apply this to v4.19 and earlier.

For v5.2 and later (including upstream) please send a patch to simply
replace get_cpu() with smp_processor_id(). The reason is that get_cpu()
will not disable BH and according to the call path this function is
invoked in NAPI callback and the other sides does local_bh_disable().
Therefore the caller of this must ensure that BH is disabled and
disabling preemption is not enough.

Sebastian

^ permalink raw reply	[flat|nested] 9+ messages in thread

* RE: [RT PATCH v2] net/xfrm/xfrm_ipcomp: Protect scratch buffer with local_lock
  2019-08-20  6:42   ` Juri Lelli
@ 2019-08-21  1:43     ` Li, Philip
  2019-08-21  6:44       ` Juri Lelli
  0 siblings, 1 reply; 9+ messages in thread
From: Li, Philip @ 2019-08-21  1:43 UTC (permalink / raw)
  To: Juri Lelli, lkp
  Cc: Juri Lelli, kbuild-all, tglx, bigeasy, rostedt, linux-rt-users,
	linux-kernel, williams

> Subject: Re: [RT PATCH v2] net/xfrm/xfrm_ipcomp: Protect scratch buffer with
> local_lock
> 
> Hi,
> 
> On 20/08/19 13:35, kbuild test robot wrote:
> > Hi Juri,
> >
> > Thank you for the patch! Yet something to improve:
> >
> > [auto build test ERROR on linus/master]
> > [cannot apply to v5.3-rc5 next-20190819]
> > [if your patch is applied to the wrong git tree, please drop us a note to help
> improve the system]
> 
> This seems to be indeed the case, as this patch is for RT v4.19-rt
> stable tree:
> 
> git://git.kernel.org/pub/scm/linux/kernel/git/rt/linux-stable-rt.git v4.19-rt
> 
> I was under the impression that putting "RT" on the subject line (before
> PATCH) would prevent build bot to pick this up, but maybe something
> else/different is needed?
Hi Juri, currently if the mail subject has RFC, we will test it but send report
privately to author only.

> 
> Thanks,
> 
> Juri


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [RT PATCH v2] net/xfrm/xfrm_ipcomp: Protect scratch buffer with local_lock
  2019-08-21  1:43     ` Li, Philip
@ 2019-08-21  6:44       ` Juri Lelli
  2019-08-21  7:00         ` bigeasy
  0 siblings, 1 reply; 9+ messages in thread
From: Juri Lelli @ 2019-08-21  6:44 UTC (permalink / raw)
  To: Li, Philip
  Cc: Juri Lelli, lkp, kbuild-all, tglx, bigeasy, rostedt,
	linux-rt-users, linux-kernel, williams

On 21/08/19 01:43, Li, Philip wrote:
> > Subject: Re: [RT PATCH v2] net/xfrm/xfrm_ipcomp: Protect scratch buffer with
> > local_lock
> > 
> > Hi,
> > 
> > On 20/08/19 13:35, kbuild test robot wrote:
> > > Hi Juri,
> > >
> > > Thank you for the patch! Yet something to improve:
> > >
> > > [auto build test ERROR on linus/master]
> > > [cannot apply to v5.3-rc5 next-20190819]
> > > [if your patch is applied to the wrong git tree, please drop us a note to help
> > improve the system]
> > 
> > This seems to be indeed the case, as this patch is for RT v4.19-rt
> > stable tree:
> > 
> > git://git.kernel.org/pub/scm/linux/kernel/git/rt/linux-stable-rt.git v4.19-rt
> > 
> > I was under the impression that putting "RT" on the subject line (before
> > PATCH) would prevent build bot to pick this up, but maybe something
> > else/different is needed?
> Hi Juri, currently if the mail subject has RFC, we will test it but send report
> privately to author only.

OK. But, my email had "RT" and not "RFC" in the subject (since it is
meant for one of the PREEMPT_RT stable trees [1]).

Best,

Juri

1 - git://git.kernel.org/pub/scm/linux/kernel/git/rt/linux-stable-rt.git v4.19-rt

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [RT PATCH v2] net/xfrm/xfrm_ipcomp: Protect scratch buffer with local_lock
  2019-08-21  6:44       ` Juri Lelli
@ 2019-08-21  7:00         ` bigeasy
  0 siblings, 0 replies; 9+ messages in thread
From: bigeasy @ 2019-08-21  7:00 UTC (permalink / raw)
  To: Juri Lelli
  Cc: Li, Philip, Juri Lelli, lkp, kbuild-all, tglx, rostedt,
	linux-rt-users, linux-kernel, williams

On 2019-08-21 08:44:22 [+0200], Juri Lelli wrote:
> > Hi Juri, currently if the mail subject has RFC, we will test it but send report
> > privately to author only.
> 
> OK. But, my email had "RT" and not "RFC" in the subject (since it is
> meant for one of the PREEMPT_RT stable trees [1]).

Was the RT tag consider at all at some point? I remember I asked for it
and then the bot did not complain again or I don't remember. At the same
point my rt-devel tree was added to the trees-to-be-tested.

> Best,
> 
> Juri
> 
> 1 - git://git.kernel.org/pub/scm/linux/kernel/git/rt/linux-stable-rt.git v4.19-rt

Sebastian

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, back to index

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-08-19 12:27 [RT PATCH v2] net/xfrm/xfrm_ipcomp: Protect scratch buffer with local_lock Juri Lelli
2019-08-19 19:57 ` Steven Rostedt
2019-08-20  6:43   ` Juri Lelli
2019-08-20  5:35 ` kbuild test robot
2019-08-20  6:42   ` Juri Lelli
2019-08-21  1:43     ` Li, Philip
2019-08-21  6:44       ` Juri Lelli
2019-08-21  7:00         ` bigeasy
2019-08-20  8:28 ` Sebastian Andrzej Siewior

Linux-rt-users archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-rt-users/0 linux-rt-users/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-rt-users linux-rt-users/ https://lore.kernel.org/linux-rt-users \
		linux-rt-users@vger.kernel.org linux-rt-users@archiver.kernel.org
	public-inbox-index linux-rt-users


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-rt-users


AGPL code for this site: git clone https://public-inbox.org/ public-inbox