5.4.54-rt33-rc1 stable review patch.
If anyone has any objections, please let me know.
------------------
From: Matt Fleming <[email protected]>
The way user struct reference counting works changed significantly with,
fda31c50292a ("signal: avoid double atomic counter increments for user accounting")
Now user structs are only freed once the last pending signal is
dequeued. Make sigqueue_free_current() follow this new convention to
avoid freeing the user struct multiple times and triggering this
warning:
refcount_t: underflow; use-after-free.
WARNING: CPU: 0 PID: 6794 at lib/refcount.c:288 refcount_dec_not_one+0x45/0x50
Call Trace:
refcount_dec_and_lock_irqsave+0x16/0x60
free_uid+0x31/0xa0
__dequeue_signal+0x17c/0x190
dequeue_signal+0x5a/0x1b0
do_sigtimedwait+0x208/0x250
__x64_sys_rt_sigtimedwait+0x6f/0xd0
do_syscall_64+0x72/0x200
entry_SYSCALL_64_after_hwframe+0x49/0xbe
Signed-off-by: Matt Fleming <[email protected]>
Reported-by: Daniel Wagner <[email protected]>
Signed-off-by: Sebastian Andrzej Siewior <[email protected]>
Signed-off-by: Steven Rostedt (VMware) <[email protected]>
---
kernel/signal.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/kernel/signal.c b/kernel/signal.c
index def2e8e37f1f..aa924f0141cf 100644
--- a/kernel/signal.c
+++ b/kernel/signal.c
@@ -494,8 +494,8 @@ static void sigqueue_free_current(struct sigqueue *q)
up = q->user;
if (rt_prio(current->normal_prio) && !put_task_cache(current, q)) {
- atomic_dec(&up->sigpending);
- free_uid(up);
+ if (atomic_dec_and_test(&up->sigpending))
+ free_uid(up);
} else
__sigqueue_free(q);
}
--
2.28.0
On 13/08/2020 03.45, Steven Rostedt wrote:
> 5.4.54-rt33-rc1 stable review patch.
> If anyone has any objections, please let me know.
>
No objections, quite the contrary. I think this should also be applied
to 4.19-rt:
Commit fda31c50292a is also in 4.19.y (as 797479da0ae9), since 4.19.112
and hence also 4.19.112-rt47. For a while we've tried to track down a
hang that at least sometimes manifests quite similarly
refcount_t: underflow; use-after-free.
WARNING: CPU: 0 PID: 14 at lib/refcount.c:280 refcount_dec_not_one+0xc0/0xd8
...
Call Trace:
[cf45be10] [c0238258] refcount_dec_not_one+0xc0/0xd8 (unreliable)
[cf45be20] [c02383c8] refcount_dec_and_lock_irqsave+0x20/0xa4
[cf45be40] [c0024a70] free_uid+0x2c/0xa0
[cf45be60] [c00384f0] put_cred_rcu+0x58/0x8c
[cf45be70] [c005f048] rcu_cpu_kthread+0x364/0x49c
[cf45bee0] [c003a0d0] smpboot_thread_fn+0x21c/0x29c
[cf45bf10] [c0036464] kthread+0xe0/0x10c
[cf45bf40] [c000f1cc] ret_from_kernel_thread+0x14/0x1c
But our reproducer is rather complicated and involves cutting power to
neighbouring boards, and takes many minutes to trigger. So I tried
Daniel's reproducer
sigwaittest -t -a -p 98
and almost immediately got a trace much more similar to the one in the
commit message
refcount_t: underflow; use-after-free.
WARNING: CPU: 0 PID: 1526 at lib/refcount.c:280
refcount_dec_not_one+0xc0/0xd8
...
Call Trace:
[cebc9e00] [c0238258] refcount_dec_not_one+0xc0/0xd8 (unreliable)
[cebc9e10] [c02383c8] refcount_dec_and_lock_irqsave+0x20/0xa4
[cebc9e30] [c0024a70] free_uid+0x2c/0xa0
[cebc9e50] [c002574c] dequeue_signal+0x90/0x1a4
[cebc9e80] [c0028f74] sys_rt_sigtimedwait+0x24c/0x288
[cebc9f40] [c000f12c] ret_from_syscall+0x0/0x40
With this patch applied, the sigwaittest has now run for 10 minutes
without problems.
I'll have to run some more tests with our reproducer to see if it really
is the same issue, but even if not, the fact that the sigwaittest fails
should be enough to put this in 4.19-rt.
Three requests (in order of importance):
* pull this into 4.19-rt
* add a note about the sigwaittest reproducer to the commit log
* do publish the rt-rcs in some git repository; that makes it a lot
easier to cherry-pick and test patches
Thanks,
Rasmus
On Thu, 13 Aug 2020 10:25:45 +0200
Rasmus Villemoes <[email protected]> wrote:
> On 13/08/2020 03.45, Steven Rostedt wrote:
> > 5.4.54-rt33-rc1 stable review patch.
> > If anyone has any objections, please let me know.
> >
>
> No objections, quite the contrary. I think this should also be applied
> to 4.19-rt:
Yep. We have a rule that no earlier supported stable release should
have a fix that a more recent stable release does not have. So this
needs to be accepted in 5.4-rt before 4.19-rt can have it. And those
maintainers have been waiting patiently for me to push this ;-)
> Three requests (in order of importance):
>
> * pull this into 4.19-rt
> * add a note about the sigwaittest reproducer to the commit log
We don't usually add comments to the commit log for backported patches.
> * do publish the rt-rcs in some git repository; that makes it a lot
> easier to cherry-pick and test patches
This has been talked about before. Perhaps you should bring up posting
stable RT RC releases into git repositories at the RT microconference
at Plumbers ;-) Especially since the last time I asked about it, people
said it wasn't necessary.
https://linuxplumbersconf.org/event/7/page/80-accepted-microconferences#rt-cr
-- Steve