On 2025-05-07 05:27, Jürgen Groß wrote:
On 06.05.25 23:09, Jason Andryuk wrote:
Marek reported seeing a NULL pointer fault in the xenbus_thread callstack: BUG: kernel NULL pointer dereference, address: 0000000000000000 RIP: e030:__wake_up_common+0x4c/0x180 Call Trace: <TASK> __wake_up_common_lock+0x82/0xd0 process_msg+0x18e/0x2f0 xenbus_thread+0x165/0x1c0
process_msg+0x18e is req->cb(req). req->cb is set to xs_wake_up(), a thin wrapper around wake_up(), or xenbus_dev_queue_reply(). It seems like it was xs_wake_up() in this case.
It seems like req may have woken up the xs_wait_for_reply(), which kfree()ed the req. When xenbus_thread resumes, it faults on the zero-ed data.
Linux Device Drivers 2nd edition states: "Normally, a wake_up call can cause an immediate reschedule to happen, meaning that other processes might run before wake_up returns." ... which would match the behaviour observed.
Change to keeping two krefs on each request. One for the caller, and one for xenbus_thread. Each will kref_put() when finished, and the last will free it.
This use of kref matches the description in Documentation/core-api/kref.rst
Link: https://lore.kernel.org/xen-devel/ZO0WrR5J0xuwDIxW@mail-itl/ Reported-by: "Marek Marczykowski-Górecki" marmarek@invisiblethingslab.com Fixes: fd8aa9095a95 ("xen: optimize xenbus driver for multiple concurrent xenstore accesses") Cc: stable@vger.kernel.org Signed-off-by: Jason Andryuk jason.andryuk@amd.com
Reviewed-by: Juergen Gross jgross@suse.com
Thanks
Kinda RFC-ish as I don't know if it fixes Marek's issue. This does seem like the correct approach if we are seeing req free()ed out from under xenbus_thread.
I think your analysis is correct. When writing this code I didn't think of wake_up() needing to access req->wq _after_ having woken up the waiter.
Yes, this was tricky.
One other thing that makes me think this is correct. If this is the same underlying issue: https://lore.kernel.org/xen-devel/Z_lJTyVipJJEpWg2@mail-itl/
The failure is in the unlock:
pvqspinlock: lock 0xffff8881029af110 has corrupted value 0x0! WARNING: CPU: 1 PID: 118 at kernel/locking/qspinlock_paravirt.h:504 __pv_queued_spin_unlock_slowpath+0xdc/0x120
Which makes me think the req was fine entering wake_up(), and it's only found to be corrupt on the way out.
Regards, Jason