| From: Paolo Abeni <pabeni@redhat.com> |
| Date: Thu, 6 Jun 2019 15:45:03 +0200 |
| Subject: pktgen: do not sleep with the thread lock held. |
| |
| commit 720f1de4021f09898b8c8443f3b3e995991b6e3a upstream. |
| |
| Currently, the process issuing a "start" command on the pktgen procfs |
| interface, acquires the pktgen thread lock and never release it, until |
| all pktgen threads are completed. The above can blocks indefinitely any |
| other pktgen command and any (even unrelated) netdevice removal - as |
| the pktgen netdev notifier acquires the same lock. |
| |
| The issue is demonstrated by the following script, reported by Matteo: |
| |
| ip -b - <<'EOF' |
| link add type dummy |
| link add type veth |
| link set dummy0 up |
| EOF |
| modprobe pktgen |
| echo reset >/proc/net/pktgen/pgctrl |
| { |
| echo rem_device_all |
| echo add_device dummy0 |
| } >/proc/net/pktgen/kpktgend_0 |
| echo count 0 >/proc/net/pktgen/dummy0 |
| echo start >/proc/net/pktgen/pgctrl & |
| sleep 1 |
| rmmod veth |
| |
| Fix the above releasing the thread lock around the sleep call. |
| |
| Additionally we must prevent racing with forcefull rmmod - as the |
| thread lock no more protects from them. Instead, acquire a self-reference |
| before waiting for any thread. As a side effect, running |
| |
| rmmod pktgen |
| |
| while some thread is running now fails with "module in use" error, |
| before this patch such command hanged indefinitely. |
| |
| Note: the issue predates the commit reported in the fixes tag, but |
| this fix can't be applied before the mentioned commit. |
| |
| v1 -> v2: |
| - no need to check for thread existence after flipping the lock, |
| pktgen threads are freed only at net exit time |
| - |
| |
| Fixes: 6146e6a43b35 ("[PKTGEN]: Removes thread_{un,}lock() macros.") |
| Reported-and-tested-by: Matteo Croce <mcroce@redhat.com> |
| Signed-off-by: Paolo Abeni <pabeni@redhat.com> |
| Signed-off-by: David S. Miller <davem@davemloft.net> |
| Signed-off-by: Ben Hutchings <ben@decadent.org.uk> |
| --- |
| net/core/pktgen.c | 11 +++++++++++ |
| 1 file changed, 11 insertions(+) |
| |
| --- a/net/core/pktgen.c |
| +++ b/net/core/pktgen.c |
| @@ -3055,7 +3055,13 @@ static int pktgen_wait_thread_run(struct |
| |
| if_unlock(t); |
| |
| + /* note: 't' will still be around even after the unlock/lock |
| + * cycle because pktgen_thread threads are only cleared at |
| + * net exit |
| + */ |
| + mutex_unlock(&pktgen_thread_lock); |
| msleep_interruptible(100); |
| + mutex_lock(&pktgen_thread_lock); |
| |
| if (signal_pending(current)) |
| goto signal; |
| @@ -3072,6 +3078,10 @@ static int pktgen_wait_all_threads_run(s |
| struct pktgen_thread *t; |
| int sig = 1; |
| |
| + /* prevent from racing with rmmod */ |
| + if (!try_module_get(THIS_MODULE)) |
| + return sig; |
| + |
| mutex_lock(&pktgen_thread_lock); |
| |
| list_for_each_entry(t, &pn->pktgen_threads, th_list) { |
| @@ -3085,6 +3095,7 @@ static int pktgen_wait_all_threads_run(s |
| t->control |= (T_STOP); |
| |
| mutex_unlock(&pktgen_thread_lock); |
| + module_put(THIS_MODULE); |
| return sig; |
| } |
| |