Thank you for your contribution! Sashiko AI review found 1 potential issue(s) 
to consider:
- [High] Incomplete tracefs gating: Caller-created tracefs files bypass the 
newly introduced `trace_remote_tracefs_open` gating, leaving them fully 
accessible if registration fails.
--

--- Patch [1]: [PATCH v5 01/18] tracing/remotes: Gate tracefs files opening on 
trace remote registration ---
commit 63ef30c302e1341f240b4f5b82d4e65d421da4cf
Author: Vincent Donnefort <[email protected]>
tracing/remotes: Gate tracefs files opening on trace remote registration

This commit adds a global list of registered trace remotes and a gating
function to prevent access to tracefs files for remotes that are being
destroyed or failed to register. Standard trace remote file operations
are wrapped to check this list before allowing open().

> --- a/kernel/trace/trace_remote.c
> +++ b/kernel/trace/trace_remote.c

[ ... ]

> @@ -932,8 +999,12 @@ int trace_remote_register(const char *name, struct 
> trace_remote_callbacks *cbs,
>       }
>  
>       ret = cbs->init ? cbs->init(remote->dentry, priv) : 0;
> -     if (ret)
> +     if (ret) {
>               pr_err("Init failed for trace remote '%s' (%d)\n", name, ret);
> +     } else {
> +             guard(mutex)(&trace_remotes_lock);
> +             list_add(&remote->node, &trace_remotes);
> +     }
>  
>       return ret;
>  }

[Severity: High]
Does this leave caller-created tracefs files exposed if cbs->init() fails?

If a remote's initialization fails halfway through, any custom tracefs files
already created by cbs->init() will leak and remain accessible. Because
trace_remote_tracefs_open() is static to trace_remote.c, the caller cannot
wrap their custom files' open() callbacks with this new gating mechanism.

For example, if KVM's hyp_trace_init_tracefs() successfully creates
a 'write_event' file but fails later, trace_remote_register() returns an
error and the remote is not added to the trace_remotes list. 

Userspace could then open the leaked 'write_event' file, bypassing the
gating entirely, and trigger kvm_call_hyp_nvhe() against the uninitialized
hypervisor stub.

Should trace_remote_tracefs_open() be exported so that subsystems can gate
their custom files, or should the leaked files be cleaned up when
trace_remote_register() returns an error?

-- 
Sashiko AI review ยท 
https://sashiko.dev/#/patchset/[email protected]?part=1

Reply via email to