mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Andrea Righi <arighi@nvidia.com>
To: Tejun Heo <tj@kernel.org>
Cc: sched-ext@lists.linux.dev, David Vernet <void@manifault.com>,
	Changwoo Min <changwoo@igalia.com>,
	Emil Tsalapatis <emil@etsalapatis.com>,
	David Dai <david.dai@linux.dev>,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH 1/4] sched_ext: Don't sign-extend unsigned enums in __COMPAT_read_enum()
Date: Sun, 25 Oct 2026 11:20:53 +0100	[thread overview]
Message-ID: <at3YBauZxfWw8su4@gpd4> (raw)
In-Reply-To: <20261023232102.1124938-2-tj@kernel.org>

Hi Tejun,

On Fri, Oct 23, 2026 at 01:20:59PM -1000, Tejun Heo wrote:
> __COMPAT_read_enum() looks an enumerator up in the running kernel's BTF. For
> a 4-byte enum, it assigns struct btf_enum.val, an __s32, to the u64 result,
> which sign-extends. BTF records the enum's signedness in the type's kflag.
> The code ignores it, so an unsigned enumerator with bit 31 set is
> sign-extended too.
> 
> b1c2a6b0d90d ("sched_ext: scx_qmap: Read SCX_RESCUE_DISABLE through the enum
> lookup tables") added SCX_RESCUE_DISABLE to the enum lookup tables. It is
> U32_MAX in enum scx_consts and the only such enumerator in the tables. BPF
> code sees it as 0xffffffffffffffff instead of 0xffffffff, so a comparison
> against a u32 holding the real value never matches.
> 
> scx_qmap, its only user, narrows the value to u32 in userspace and is
> unaffected.
> 
> Sign-extend only when the kflag says the enum is signed and zero-extend
> otherwise. A signed enum keeps the sign extension, which is what C gives
> when the enumerator converts to u64. SCX_TASK_CURSOR, 1 << 31 in a signed
> enum, still reads 0xffffffff80000000. Verified with a probe against a built
> kernel's BTF.
> 
> Signed-off-by: Tejun Heo <tj@kernel.org>
> ---
>  tools/sched_ext/include/scx/compat.h | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/sched_ext/include/scx/compat.h b/tools/sched_ext/include/scx/compat.h
> index 07fb55e63c57..c8d7280ceffa 100644
> --- a/tools/sched_ext/include/scx/compat.h
> +++ b/tools/sched_ext/include/scx/compat.h
> @@ -143,7 +143,11 @@ static inline bool __COMPAT_read_enum(const char *type, const char *name, u64 *v
>  				if (t->size == 8 && !strncmp(type, "scx_", 4))
>  					return __COMPAT_recover_truncated_enum64(type, name,
>  										 (u32)e[i].val, v);
> -				*v = e[i].val;
> +				/* kflag marks a signed enum */
> +				if (btf_kflag(t))
> +					*v = (s64)e[i].val;
> +				else
> +					*v = (u32)e[i].val;

nit: BTF generated by pahole < 1.24 never sets the enum kflag, so on
those kernels signed enums are zero-extended as well. It's harmless
today (SCX_TASK_CURSOR is the only affected entry and it's only used as
a u32 flag), but maybe it's worth mentioning it in the comment?
Something like:

	/*
	 * kflag marks a signed enum. BTF from pahole < 1.24 never sets it,
	 * so signed enums are zero-extended there too.
	 */

Reviewed-by: Andrea Righi <arighi@nvidia.com>

Thanks,
-Andrea

>  				return true;
>  			}
>  		}
> -- 
> 2.56.0
> 

  reply	other threads:[~2026-10-25 10:21 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-10-23 23:20 [PATCHSET sched_ext/for-7.5] sched_ext: Sync tools headers with scx and fix unsigned enum reads Tejun Heo
2026-10-23 23:20 ` [PATCH 1/4] sched_ext: Don't sign-extend unsigned enums in __COMPAT_read_enum() Tejun Heo
2026-10-25 10:20   ` Andrea Righi [this message]
2026-10-26 17:33   ` [PATCH v2 " Tejun Heo
2026-10-23 23:21 ` [PATCH 2/4] sched_ext: Sync tools autogen enum lookup tables from the scx repo Tejun Heo
2026-10-23 23:21 ` [PATCH 3/4] sched_ext: Sync common and compat headers " Tejun Heo
2026-10-23 23:21 ` [PATCH 4/4] sched_ext: Drop stale entries from the enum lookup tables Tejun Heo
2026-10-25 10:21 ` [PATCHSET sched_ext/for-7.5] sched_ext: Sync tools headers with scx and fix unsigned enum reads Andrea Righi
2026-10-26 17:33 ` Tejun Heo

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=at3YBauZxfWw8su4@gpd4 \
    --to=arighi@nvidia.com \
    --cc=changwoo@igalia.com \
    --cc=david.dai@linux.dev \
    --cc=emil@etsalapatis.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=sched-ext@lists.linux.dev \
    --cc=tj@kernel.org \
    --cc=void@manifault.com \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®