mirror of
https://github.com/hardkernel/linux.git
synced 2026-06-05 10:31:46 +09:00
xenbus: Use kref to track req lifetime
commit 1f0304dfd9d217c2f8b04a9ef4b3258a66eedd27 upstream.
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: fd8aa9095a ("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>
Signed-off-by: Juergen Gross <jgross@suse.com>
Message-ID: <20250506210935.5607-1-jason.andryuk@amd.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
This commit is contained in:
committed by
Greg Kroah-Hartman
parent
cf61669c50
commit
8b02f85e84
@@ -77,6 +77,7 @@ enum xb_req_state {
|
|||||||
struct xb_req_data {
|
struct xb_req_data {
|
||||||
struct list_head list;
|
struct list_head list;
|
||||||
wait_queue_head_t wq;
|
wait_queue_head_t wq;
|
||||||
|
struct kref kref;
|
||||||
struct xsd_sockmsg msg;
|
struct xsd_sockmsg msg;
|
||||||
uint32_t caller_req_id;
|
uint32_t caller_req_id;
|
||||||
enum xsd_sockmsg_type type;
|
enum xsd_sockmsg_type type;
|
||||||
@@ -103,6 +104,7 @@ int xb_init_comms(void);
|
|||||||
void xb_deinit_comms(void);
|
void xb_deinit_comms(void);
|
||||||
int xs_watch_msg(struct xs_watch_event *event);
|
int xs_watch_msg(struct xs_watch_event *event);
|
||||||
void xs_request_exit(struct xb_req_data *req);
|
void xs_request_exit(struct xb_req_data *req);
|
||||||
|
void xs_free_req(struct kref *kref);
|
||||||
|
|
||||||
int xenbus_match(struct device *_dev, struct device_driver *_drv);
|
int xenbus_match(struct device *_dev, struct device_driver *_drv);
|
||||||
int xenbus_dev_probe(struct device *_dev);
|
int xenbus_dev_probe(struct device *_dev);
|
||||||
|
|||||||
@@ -309,8 +309,8 @@ static int process_msg(void)
|
|||||||
virt_wmb();
|
virt_wmb();
|
||||||
req->state = xb_req_state_got_reply;
|
req->state = xb_req_state_got_reply;
|
||||||
req->cb(req);
|
req->cb(req);
|
||||||
} else
|
}
|
||||||
kfree(req);
|
kref_put(&req->kref, xs_free_req);
|
||||||
}
|
}
|
||||||
|
|
||||||
mutex_unlock(&xs_response_mutex);
|
mutex_unlock(&xs_response_mutex);
|
||||||
@@ -386,14 +386,13 @@ static int process_writes(void)
|
|||||||
state.req->msg.type = XS_ERROR;
|
state.req->msg.type = XS_ERROR;
|
||||||
state.req->err = err;
|
state.req->err = err;
|
||||||
list_del(&state.req->list);
|
list_del(&state.req->list);
|
||||||
if (state.req->state == xb_req_state_aborted)
|
if (state.req->state != xb_req_state_aborted) {
|
||||||
kfree(state.req);
|
|
||||||
else {
|
|
||||||
/* write err, then update state */
|
/* write err, then update state */
|
||||||
virt_wmb();
|
virt_wmb();
|
||||||
state.req->state = xb_req_state_got_reply;
|
state.req->state = xb_req_state_got_reply;
|
||||||
wake_up(&state.req->wq);
|
wake_up(&state.req->wq);
|
||||||
}
|
}
|
||||||
|
kref_put(&state.req->kref, xs_free_req);
|
||||||
|
|
||||||
mutex_unlock(&xb_write_mutex);
|
mutex_unlock(&xb_write_mutex);
|
||||||
|
|
||||||
|
|||||||
@@ -406,7 +406,7 @@ void xenbus_dev_queue_reply(struct xb_req_data *req)
|
|||||||
mutex_unlock(&u->reply_mutex);
|
mutex_unlock(&u->reply_mutex);
|
||||||
|
|
||||||
kfree(req->body);
|
kfree(req->body);
|
||||||
kfree(req);
|
kref_put(&req->kref, xs_free_req);
|
||||||
|
|
||||||
kref_put(&u->kref, xenbus_file_free);
|
kref_put(&u->kref, xenbus_file_free);
|
||||||
|
|
||||||
|
|||||||
@@ -112,6 +112,12 @@ static void xs_suspend_exit(void)
|
|||||||
wake_up_all(&xs_state_enter_wq);
|
wake_up_all(&xs_state_enter_wq);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
void xs_free_req(struct kref *kref)
|
||||||
|
{
|
||||||
|
struct xb_req_data *req = container_of(kref, struct xb_req_data, kref);
|
||||||
|
kfree(req);
|
||||||
|
}
|
||||||
|
|
||||||
static uint32_t xs_request_enter(struct xb_req_data *req)
|
static uint32_t xs_request_enter(struct xb_req_data *req)
|
||||||
{
|
{
|
||||||
uint32_t rq_id;
|
uint32_t rq_id;
|
||||||
@@ -237,6 +243,12 @@ static void xs_send(struct xb_req_data *req, struct xsd_sockmsg *msg)
|
|||||||
req->caller_req_id = req->msg.req_id;
|
req->caller_req_id = req->msg.req_id;
|
||||||
req->msg.req_id = xs_request_enter(req);
|
req->msg.req_id = xs_request_enter(req);
|
||||||
|
|
||||||
|
/*
|
||||||
|
* Take 2nd ref. One for this thread, and the second for the
|
||||||
|
* xenbus_thread.
|
||||||
|
*/
|
||||||
|
kref_get(&req->kref);
|
||||||
|
|
||||||
mutex_lock(&xb_write_mutex);
|
mutex_lock(&xb_write_mutex);
|
||||||
list_add_tail(&req->list, &xb_write_list);
|
list_add_tail(&req->list, &xb_write_list);
|
||||||
notify = list_is_singular(&xb_write_list);
|
notify = list_is_singular(&xb_write_list);
|
||||||
@@ -261,8 +273,8 @@ static void *xs_wait_for_reply(struct xb_req_data *req, struct xsd_sockmsg *msg)
|
|||||||
if (req->state == xb_req_state_queued ||
|
if (req->state == xb_req_state_queued ||
|
||||||
req->state == xb_req_state_wait_reply)
|
req->state == xb_req_state_wait_reply)
|
||||||
req->state = xb_req_state_aborted;
|
req->state = xb_req_state_aborted;
|
||||||
else
|
|
||||||
kfree(req);
|
kref_put(&req->kref, xs_free_req);
|
||||||
mutex_unlock(&xb_write_mutex);
|
mutex_unlock(&xb_write_mutex);
|
||||||
|
|
||||||
return ret;
|
return ret;
|
||||||
@@ -291,6 +303,7 @@ int xenbus_dev_request_and_reply(struct xsd_sockmsg *msg, void *par)
|
|||||||
req->cb = xenbus_dev_queue_reply;
|
req->cb = xenbus_dev_queue_reply;
|
||||||
req->par = par;
|
req->par = par;
|
||||||
req->user_req = true;
|
req->user_req = true;
|
||||||
|
kref_init(&req->kref);
|
||||||
|
|
||||||
xs_send(req, msg);
|
xs_send(req, msg);
|
||||||
|
|
||||||
@@ -319,6 +332,7 @@ static void *xs_talkv(struct xenbus_transaction t,
|
|||||||
req->num_vecs = num_vecs;
|
req->num_vecs = num_vecs;
|
||||||
req->cb = xs_wake_up;
|
req->cb = xs_wake_up;
|
||||||
req->user_req = false;
|
req->user_req = false;
|
||||||
|
kref_init(&req->kref);
|
||||||
|
|
||||||
msg.req_id = 0;
|
msg.req_id = 0;
|
||||||
msg.tx_id = t.id;
|
msg.tx_id = t.id;
|
||||||
|
|||||||
Reference in New Issue
Block a user