From: Beau Belgrave <beaub@linux.microsoft.com>
To: Yifan Zhang <yfzhang23@stu.pku.edu.cn>
Cc: Steven Rostedt <rostedt@goodmis.org>,
Masami Hiramatsu <mhiramat@kernel.org>,
linux-trace-kernel@vger.kernel.org,
Mathieu Desnoyers <mathieu.desnoyers@efficios.com>,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2 1/2] tracing/user_events: validate dynamic cpumask size
Date: Wed, 4 Nov 2026 21:39:29 +0000 [thread overview]
Message-ID: <20261104213929.GA65-beaub@linux.microsoft.com> (raw)
In-Reply-To: <179367391951.8187.4749000539954371762@stu.pku.edu.cn>
On Tue, Nov 03, 2026 at 10:45:19AM +0800, Yifan Zhang wrote:
> user_event_validate() checks that a dynamic field lies inside the event but
> does not verify the minimum size required by its filter type. A one-byte
> dynamic cpumask can therefore reach filter_pred_cpumask(), which consumes
> cpumask_size() bytes beyond the event buffer.
>
> KASAN reports:
>
> BUG: KASAN: slab-use-after-free in filter_match_preds
> Read of size 8
> filter_match_preds
Hey Yifan,
In the V1 there was a repro case where this value was a __data_loc,
which defines where the actual data (and it's size) is in the payload.
There isn't a type min-size requirement when __data_loc is used, this
seems like the bug resides possibly in filter_match_preds for
__data_loc cases? See comments futher down.
Steve, do you think this is the case?
If not, then I guess we would have to validate min-size requirements of
all types in the dynamic case, and I hope we could do that centrally to
avoid fixing these in various spots.
> trace_event_buffer_commit
> user_event_ftrace
> user_events_write_core
>
> Mark dynamic cpumask fields and reject locations shorter than
> cpumask_size().
>
> The patched kernel rejects the short event and accepts a full-size control
> event without a KASAN report.
>
> Fixes: 2467cda1b5c9 ("user_events: Validate user payloads for size and null termination")
> Signed-off-by: Yifan Zhang <yfzhang23@stu.pku.edu.cn>
> ---
> kernel/trace/trace_events_user.c | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
> diff --git a/kernel/trace/trace_events_user.c b/kernel/trace/trace_events_user.c
> index f658c3a77aa7a..85938b45b7618 100644
> --- a/kernel/trace/trace_events_user.c
> +++ b/kernel/trace/trace_events_user.c
> @@ -170,6 +170,7 @@ struct user_event_file_info {
>
> #define VALIDATOR_ENSURE_NULL (1 << 0)
> #define VALIDATOR_REL (1 << 1)
> +#define VALIDATOR_CPUMASK BIT(2)
>
> struct user_event_validator {
> struct list_head user_event_link;
> @@ -1159,6 +1160,9 @@ static int user_event_add_field(struct user_event *user, const char *type,
> if (strstr(type, "char") != NULL)
> validator_flags |= VALIDATOR_ENSURE_NULL;
>
> + if (strstr(type, "cpumask_t"))
> + validator_flags |= VALIDATOR_CPUMASK;
> +
You can only get to this case if you put struct, __rel_loc, or __data_loc.
Those have their own size requirements and are dynamic in nature, IE:
Each write describes the actual size in the __rel_loc and __data_loc
cases, and statically for the struct case.
user_field_size() checks if a type name has a known size, if not, it
gets rejected. The only time this isn't the case is if you put struct in
front and then have to give a size that's expected. The other case is if
you put __data_loc or __rel_loc, both require the payload to dynamically
have a size associated.
In the dynamic case, the filter should be ensuring these values are
minimally sized.
> validator = kmalloc_obj(*validator, GFP_KERNEL_ACCOUNT);
>
> if (!validator) {
> @@ -1598,6 +1602,10 @@ static int user_event_validate(struct user_event *user, void *data, int len)
> offset = loc & 0xffff;
> size = loc >> 16;
>
> + if (unlikely((validator->flags & VALIDATOR_CPUMASK) &&
> + size < cpumask_size()))
> + return -EFAULT;
> +
> if (likely(validator->flags & VALIDATOR_REL))
> pos += offset + sizeof(loc);
> else
> --
> 2.39.5 (Apple Git-154)
Thanks,
-Beau
prev parent reply other threads:[~2026-11-04 21:39 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-11-03 2:45 [PATCH v2 0/2] tracing/user_events: validate dynamic field bounds Yifan Zhang
2026-11-03 2:45 ` [PATCH v2 2/2] tracing/user_events: reject overflowing field offsets Yifan Zhang
2026-11-03 2:45 ` [PATCH v2 1/2] tracing/user_events: validate dynamic cpumask size Yifan Zhang
2026-11-04 21:39 ` Beau Belgrave [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20261104213929.GA65-beaub@linux.microsoft.com \
--to=beaub@linux.microsoft.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-trace-kernel@vger.kernel.org \
--cc=mathieu.desnoyers@efficios.com \
--cc=mhiramat@kernel.org \
--cc=rostedt@goodmis.org \
--cc=yfzhang23@stu.pku.edu.cn \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.