linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [BUG] Crash after module unload if it use DO_ONCE mechanism
@ 2021-06-17  7:51 Kefeng Wang
  2021-06-18 13:25 ` Kefeng Wang
  0 siblings, 1 reply; 2+ messages in thread
From: Kefeng Wang @ 2021-06-17  7:51 UTC (permalink / raw)
  To: David S. Miller, Hannes Frederic Sowa, Alexei Starovoitov,
	Eric Dumazet, Jakub Kicinski, Linux Kernel Network Developers,
	linux-kernel@vger.kernel.org List

Hi all,

We met a crash[3] after module unload if it uses DO_ONCE mechanism,
also we could reproduce by the demo module[1] and the hack patch[2].

The DO_ONCE mechanism could be use directly(eg, testmgr.c), and there 
are some macro which is used by lots of net drivers,
"prandom_init_once"
"get_random_once/get_random_once_wait"
"net_get_random_once/net_get_random_once_wait"

The analysis of crash is as follows,

init_module
  get_random_once
   DO_ONCE
   DEFINE_STATIC_KEY_TRUE(___once_key);
   __do_once_done
     once_disable_jump(once_key);
       INIT_WORK(&w->work, once_deferred);
       struct once_work *w;
       w->key = key;
       schedule_work(&w->work);                    cleanup_module
                                                    *the key is destroy*
process_one_work
  once_deferred
    BUG_ON(!static_key_enabled(work->key));
       static_key_count((struct static_key *)x)   //*access key, crash*

I can't find a good way to fix the issue, any suggestion?

Thanks.



[1] test module
static int test;
int init_module(void) {
	pr_info("Hello\n");
	get_random_once(&test, sizeof(int));
	return 0;
}
void cleanup_module(void) {
	pr_info("Bye %x!\n", test);
}
[2] hack to add some delay
diff --git a/lib/once.c b/lib/once.c
index 8b7d6235217e..b56b8ced4bab 100644
--- a/lib/once.c
+++ b/lib/once.c
@@ -14,6 +14,7 @@ static void once_deferred(struct work_struct *w)
         struct once_work *work;

         work = container_of(w, struct once_work, work);
+       msleep(8000);
         BUG_ON(!static_key_enabled(work->key));
         static_branch_disable(work->key);
         kfree(work);

[3] crash log
[  253.560859] Hello
[  253.562851] Bye 92bbb335!
[  261.585813] Unable to handle kernel paging request at virtual address 
ffff000001293018
[  261.585815] Mem abort info:
[  261.585816]   ESR = 0x96000007
[  261.585817]   Exception class = DABT (current EL), IL = 32 bits
[  261.585818]   SET = 0, FnV = 0
[  261.585818]   EA = 0, S1PTW = 0
[  261.585819] Data abort info:
[  261.585820]   ISV = 0, ISS = 0x00000007
[  261.585821]   CM = 0, WnR = 0
[  261.585822] swapper pgtable: 4k pages, 48-bit VAs, pgdp = 
00000000e45c016c
[  261.585823] [ffff000001293018] pgd=000000023fffe003, 
pud=000000023354b003, pmd=00000001d4099003, pte=0000000000000000
[  261.585827] Internal error: Oops: 96000007 [#1] SMP
[  261.586458] Process kworker/25:1 (pid: 291, stack limit = 
0xffff0000841b0000)
[  261.586880] CPU: 25 PID: 291 Comm: kworker/25:1 Kdump: loaded 
Tainted: P        W  OE     4.19.90+ #14
[  261.587415] Hardware name: QEMU KVM Virtual Machine, BIOS 0.0.0 
02/06/2015
[  261.587819] Workqueue: events once_deferred
[  261.588062] pstate: 60c00005 (nZCv daif +PAN +UAO)
[  261.588341] pc : static_key_count+0x18/0x30
[  261.588584] lr : once_deferred+0x30/0x80
[  261.588810] sp : ffff0000841b3d70
[  261.589000] x29: ffff0000841b3d70 x28: 0000000000000000
[  261.589308] x27: 0000000000000000 x26: ffff00008131f330
[  261.589615] x25: 0000000000000000 x24: ffff8001defd1c08
[  261.590025] x23: 0000000000000000 x22: ffff8001ff4d3000
[  261.590414] x21: ffff8001ff4cee80 x20: ffff8001f3bbd100
[  261.590868] x19: ffff000001293018 x18: ffffffffffffffff
[  261.591254] x17: 0000000000000000 x16: 0000000000000000
[  261.591638] x15: ffff0000812fa748 x14: ffff0000814f1d50
[  261.592026] x13: ffff0000814f1996 x12: ffffffffffffffac
[  261.592409] x11: 0000000000000000 x10: 0000000000000b80
[  261.592794] x9 : ffff0000841b3bf0 x8 : 3535303030303030
[  261.593179] x7 : 303078302079656b x6 : ffff0000814f0f80
[  261.593564] x5 : 00ffffffffffffff x4 : 0000000000000000
[  261.593978] x3 : 0000000000000000 x2 : 173087582665d800
[  261.594362] x1 : 0000000000000000 x0 : ffff00008055a888
[  261.594748] Call trace:
[  261.594928]  static_key_count+0x18/0x30
[  261.595207]  once_deferred+0x30/0x80
[  261.595469]  process_one_work+0x1b8/0x458
[  261.595762]  worker_thread+0x158/0x498
[  261.596034]  kthread+0x134/0x138
[  261.596271]  ret_from_fork+0x10/0x18
[  261.596531] Code: f9000bf3 aa0003f3 aa1e03e0 d503201f (b9400260)


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

* Re: [BUG] Crash after module unload if it use DO_ONCE mechanism
  2021-06-17  7:51 [BUG] Crash after module unload if it use DO_ONCE mechanism Kefeng Wang
@ 2021-06-18 13:25 ` Kefeng Wang
  0 siblings, 0 replies; 2+ messages in thread
From: Kefeng Wang @ 2021-06-18 13:25 UTC (permalink / raw)
  To: David S. Miller, Hannes Frederic Sowa, Alexei Starovoitov,
	Eric Dumazet, Jakub Kicinski, Linux Kernel Network Developers,
	linux-kernel@vger.kernel.org List
  Cc: wangkefeng.wang



On 2021/6/17 15:51, Kefeng Wang wrote:
> Hi all,
> 
> We met a crash[3] after module unload if it uses DO_ONCE mechanism,
> also we could reproduce by the demo module[1] and the hack patch[2].
> 
> The DO_ONCE mechanism could be use directly(eg, testmgr.c), and there 
> are some macro which is used by lots of net drivers,
> "prandom_init_once"
> "get_random_once/get_random_once_wait"
> "net_get_random_once/net_get_random_once_wait"
> 
> The analysis of crash is as follows,
> 
> init_module
>   get_random_once
>    DO_ONCE
>    DEFINE_STATIC_KEY_TRUE(___once_key);
>    __do_once_done
>      once_disable_jump(once_key);
>        INIT_WORK(&w->work, once_deferred);
>        struct once_work *w;
>        w->key = key;
>        schedule_work(&w->work);                    cleanup_module
>                                                     *the key is destroy*
> process_one_work
>   once_deferred
>     BUG_ON(!static_key_enabled(work->key));
>        static_key_count((struct static_key *)x)   //*access key, crash*
> 
> I can't find a good way to fix the issue, any suggestion?

Here is one solution to avoid the issue, but maybe this is too hack to 
add module into once, is there any better solution, thanks.

diff --git a/include/linux/once.h b/include/linux/once.h
index 9225ee6d96c7..052af082e369 100644
--- a/include/linux/once.h
+++ b/include/linux/once.h
@@ -4,10 +4,11 @@

  #include <linux/types.h>
  #include <linux/jump_label.h>
+#include <linux/export.h>

  bool __do_once_start(bool *done, unsigned long *flags);
  void __do_once_done(bool *done, struct static_key_true *once_key,
-		    unsigned long *flags);
+		    unsigned long *flags, struct module *mod);

  /* Call a function exactly once. The idea of DO_ONCE() is to perform
   * a function call such as initialization of random seeds, etc, only
@@ -46,7 +47,7 @@ void __do_once_done(bool *done, struct static_key_true 
*once_key,
  			if (unlikely(___ret)) {				     \
  				func(__VA_ARGS__);			     \
  				__do_once_done(&___done, &___once_key,	     \
-					       &___flags);		     \
+					       &___flags, THIS_MODULE);		\
  			}						     \
  		}							     \
  		___ret;							     \
diff --git a/lib/once.c b/lib/once.c
index 8b7d6235217e..57c6fcf9f694 100644
--- a/lib/once.c
+++ b/lib/once.c
@@ -3,10 +3,12 @@
  #include <linux/spinlock.h>
  #include <linux/once.h>
  #include <linux/random.h>
+#include <linux/module.h>

  struct once_work {
  	struct work_struct work;
  	struct static_key_true *key;
+	struct module *module;
  };

  static void once_deferred(struct work_struct *w)
@@ -16,19 +18,25 @@ static void once_deferred(struct work_struct *w)
  	work = container_of(w, struct once_work, work);
  	BUG_ON(!static_key_enabled(work->key));
  	static_branch_disable(work->key);
+	module_put(work->module);
  	kfree(work);
  }

-static void once_disable_jump(struct static_key_true *key)
+static void once_disable_jump(struct static_key_true *key, struct 
module *mod)
  {
  	struct once_work *w;

+	__module_get(mod);
+
  	w = kmalloc(sizeof(*w), GFP_ATOMIC);
-	if (!w)
+	if (!w) {
+		module_put(mod);
  		return;
+	}

  	INIT_WORK(&w->work, once_deferred);
  	w->key = key;
+	w->module = mod;
  	schedule_work(&w->work);
  }

@@ -53,11 +61,11 @@ bool __do_once_start(bool *done, unsigned long *flags)
  EXPORT_SYMBOL(__do_once_start);

  void __do_once_done(bool *done, struct static_key_true *once_key,
-		    unsigned long *flags)
+		    unsigned long *flags, struct module *mod)
  	__releases(once_lock)
  {
  	*done = true;
  	spin_unlock_irqrestore(&once_lock, *flags);
-	once_disable_jump(once_key);
+	once_disable_jump(once_key, mod);
  }
  EXPORT_SYMBOL(__do_once_done);



> 
> Thanks.
> 
> 
> 
> [1] test module
> static int test;
> int init_module(void) {
>      pr_info("Hello\n");
>      get_random_once(&test, sizeof(int));
>      return 0;
> }
> void cleanup_module(void) {
>      pr_info("Bye %x!\n", test);
> }
> [2] hack to add some delay
> diff --git a/lib/once.c b/lib/once.c
> index 8b7d6235217e..b56b8ced4bab 100644
> --- a/lib/once.c
> +++ b/lib/once.c
> @@ -14,6 +14,7 @@ static void once_deferred(struct work_struct *w)
>          struct once_work *work;
> 
>          work = container_of(w, struct once_work, work);
> +       msleep(8000);
>          BUG_ON(!static_key_enabled(work->key));
>          static_branch_disable(work->key);
>          kfree(work);
> 
> [3] crash log
> [  253.560859] Hello
> [  253.562851] Bye 92bbb335!
> [  261.585813] Unable to handle kernel paging request at virtual address 
> ffff000001293018
> [  261.585815] Mem abort info:
> [  261.585816]   ESR = 0x96000007
> [  261.585817]   Exception class = DABT (current EL), IL = 32 bits
> [  261.585818]   SET = 0, FnV = 0
> [  261.585818]   EA = 0, S1PTW = 0
> [  261.585819] Data abort info:
> [  261.585820]   ISV = 0, ISS = 0x00000007
> [  261.585821]   CM = 0, WnR = 0
> [  261.585822] swapper pgtable: 4k pages, 48-bit VAs, pgdp = 
> 00000000e45c016c
> [  261.585823] [ffff000001293018] pgd=000000023fffe003, 
> pud=000000023354b003, pmd=00000001d4099003, pte=0000000000000000
> [  261.585827] Internal error: Oops: 96000007 [#1] SMP
> [  261.586458] Process kworker/25:1 (pid: 291, stack limit = 
> 0xffff0000841b0000)
> [  261.586880] CPU: 25 PID: 291 Comm: kworker/25:1 Kdump: loaded 
> Tainted: P        W  OE     4.19.90+ #14
> [  261.587415] Hardware name: QEMU KVM Virtual Machine, BIOS 0.0.0 
> 02/06/2015
> [  261.587819] Workqueue: events once_deferred
> [  261.588062] pstate: 60c00005 (nZCv daif +PAN +UAO)
> [  261.588341] pc : static_key_count+0x18/0x30
> [  261.588584] lr : once_deferred+0x30/0x80
> [  261.588810] sp : ffff0000841b3d70
> [  261.589000] x29: ffff0000841b3d70 x28: 0000000000000000
> [  261.589308] x27: 0000000000000000 x26: ffff00008131f330
> [  261.589615] x25: 0000000000000000 x24: ffff8001defd1c08
> [  261.590025] x23: 0000000000000000 x22: ffff8001ff4d3000
> [  261.590414] x21: ffff8001ff4cee80 x20: ffff8001f3bbd100
> [  261.590868] x19: ffff000001293018 x18: ffffffffffffffff
> [  261.591254] x17: 0000000000000000 x16: 0000000000000000
> [  261.591638] x15: ffff0000812fa748 x14: ffff0000814f1d50
> [  261.592026] x13: ffff0000814f1996 x12: ffffffffffffffac
> [  261.592409] x11: 0000000000000000 x10: 0000000000000b80
> [  261.592794] x9 : ffff0000841b3bf0 x8 : 3535303030303030
> [  261.593179] x7 : 303078302079656b x6 : ffff0000814f0f80
> [  261.593564] x5 : 00ffffffffffffff x4 : 0000000000000000
> [  261.593978] x3 : 0000000000000000 x2 : 173087582665d800
> [  261.594362] x1 : 0000000000000000 x0 : ffff00008055a888
> [  261.594748] Call trace:
> [  261.594928]  static_key_count+0x18/0x30
> [  261.595207]  once_deferred+0x30/0x80
> [  261.595469]  process_one_work+0x1b8/0x458
> [  261.595762]  worker_thread+0x158/0x498
> [  261.596034]  kthread+0x134/0x138
> [  261.596271]  ret_from_fork+0x10/0x18
> [  261.596531] Code: f9000bf3 aa0003f3 aa1e03e0 d503201f (b9400260)
> 

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

end of thread, other threads:[~2021-06-18 13:25 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-06-17  7:51 [BUG] Crash after module unload if it use DO_ONCE mechanism Kefeng Wang
2021-06-18 13:25 ` Kefeng Wang

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).