Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues: - [High] Missing RCU read lock or refcounting causes a Use-After-Free of `drm_ras_node` in netlink callbacks. -- --- Patch [1]: [PATCH v5 1/5] drm/ras: Cancel and free message on get counter failure --- commit a7d45a29dc348f6234c9ec9f5c9ccdf5d73188b7 Author: Raag Jadav <[email protected]> drm/ras: Cancel and free message on get counter failure doit_reply_value() directly returns on get counter failure, which results in stale sk_buff and genetlink header that aren't cleaned up. Fix it and while at it, consolidate error handling using goto. > diff --git a/drivers/gpu/drm/drm_ras.c b/drivers/gpu/drm/drm_ras.c > index d6eab29a1394c..467a169026fc1 100644 > --- a/drivers/gpu/drm/drm_ras.c > +++ b/drivers/gpu/drm/drm_ras.c > @@ -201,25 +201,28 @@ static int doit_reply_value(struct genl_info *info, u32 > node_id, > > hdr = genlmsg_iput(msg, info); > if (!hdr) { > - nlmsg_free(msg); > - return -EMSGSIZE; > + ret = -EMSGSIZE; > + goto free_msg; > } > > ret = get_node_error_counter(node_id, error_id, > &error_name, &value); [Severity: High] This is a pre-existing issue, but is there a missing RCU read lock around these node accesses? Since drm_ras_nl_family uses parallel_ops = true, callbacks execute concurrently without the global genl_lock(). Looking at get_node_error_counter(), it retrieves the node using xa_load() and then dereferences it: get_node_error_counter() { node = xa_load(&drm_ras_xa, node_id); if (!node || !node->query_error_counter) return -ENOENT; ... return node->query_error_counter(node, error_id, name, value); } Because xa_load() only holds the RCU read lock internally and drops it before returning, the node pointer is unprotected here. If a driver concurrently unregisters and frees the node memory, could this lead to a use-after-free when the callback dereferences node->query_error_counter? A similar issue appears to exist in drm_ras_nl_list_nodes_dumpit() where xa_for_each_start() is used without an encompassing rcu_read_lock(). > if (ret) > - return ret; > + goto cancel_msg; > > ret = msg_reply_value(msg, error_id, error_name, value); > - if (ret) { > - genlmsg_cancel(msg, hdr); > - nlmsg_free(msg); > - return ret; > - } > + if (ret) > + goto cancel_msg; > > genlmsg_end(msg, hdr); > > return genlmsg_reply(msg, info); > + > +cancel_msg: > + genlmsg_cancel(msg, hdr); > +free_msg: > + nlmsg_free(msg); > + return ret; > } > > /** -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=1
