Re: [PATCH] perf/hw_breakpoint: Avoid leaking private x86 breakpoint ranges

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] perf/hw_breakpoint: memcmp validates uninitialized flag-gated fields
--

commit 0cb8c384abc8b74fee8923d87b586e123993d285
Author: Kyle Zeng <kylebot@openai.com>

perf/hw_breakpoint: Avoid leaking private x86 breakpoint ranges

This commit addresses a security issue where private x86 breakpoint ranges
can be leaked to unprivileged callers by validating immutable attributes
before parsing a modify request. It also checks x86 kernel-breakpoint
access before consulting private blacklists and validates instruction
lengths early.

> diff --git a/kernel/events/hw_breakpoint.c b/kernel/events/hw_breakpoint.c
> index 789add0c185a0..1f0e7f63ba82c 100644
> --- a/kernel/events/hw_breakpoint.c
> +++ b/kernel/events/hw_breakpoint.c
[ ... ]
> @@ -765,10 +765,6 @@ modify_user_hw_breakpoint_check(struct perf_event *bp, struct perf_event_attr *a
>  	struct arch_hw_breakpoint hw = { };
>  	int err;
>  
> -	err = hw_breakpoint_parse(bp, attr, &hw);
> -	if (err)
> -		return err;
> -
>  	if (check) {
>  		struct perf_event_attr old_attr;
>  
> @@ -778,6 +774,10 @@ modify_user_hw_breakpoint_check(struct perf_event *bp, struct perf_event_attr *a
>  			return -EINVAL;

[Severity: Medium]
Does this memcmp() inappropriately enforce equality on uninitialized, unused,
and flag-gated fields?

Userspace is not required to zero unused fields in the perf_event_attr struct.
If a user opens a hw_breakpoint event with uninitialized stack garbage in
unused fields (e.g., sample_regs_user, config2), and later calls
PERF_EVENT_IOC_MODIFY_ATTRIBUTES with a new perf_event_attr to modify bp_addr,
the new struct may contain different stack garbage.

Since perf_copy_attr() copies the struct verbatim from userspace, this
memcmp() fails when comparing the original and new structs due to the
differing uninitialized bytes. This will spuriously reject valid modification
requests with -EINVAL.

>  	}
>  
> +	err = hw_breakpoint_parse(bp, attr, &hw);
> +	if (err)
> +		return err;
> +
>  	if (bp->attr.bp_type != attr->bp_type) {
>  		err = modify_bp_slot(bp, bp->attr.bp_type, attr->bp_type);
>  		if (err)

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261006222519.43193-1-kylebot@openai.com?part=1