linux-kernel - Re: [PATCH] tools/power turbostat: Fix MSRs with CONFIG

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CAJvTdKkVETa5MC4Y245UamB214U=HK5deadufpHaPnYmm7ywhg@mail.gmail.com>
Date: Wed, 18 Jun 2025 10:32:41 -0400
From: Len Brown <lenb@...nel.org>
To: "Zhang, Rui" <rui.zhang@...el.com>
Cc: "calvin@...nvd.org" <calvin@...nvd.org>, 
	"linux-pm@...r.kernel.org" <linux-pm@...r.kernel.org>, 
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] tools/power turbostat: Fix MSRs with CONFIG_MULTIUSER=n

Thanks for the patch, Calvin, I think it is correct.

Rui,
I'll tweak the comment to explain that the system call doesn't always exist.

I think it is correct to continue on -- the rawio check is basically a
graceful way
to prevent an error in accessing MSRs later.  If we don't have that
graceful mechanism,
then we will or will not hit the access error later, and that's okay.

Interesting configuration you have there Calvin.

thanks,
-Len


On Tue, Jun 17, 2025 at 11:28 PM Zhang, Rui <rui.zhang@...el.com> wrote:
>
> On Sun, 2025-06-15 at 21:38 -0700, Calvin Owens wrote:
> > On Monday 06/16 at 01:30 +0000, Zhang, Rui wrote:
> > > On Fri, 2025-06-13 at 19:20 -0700, Calvin Owens wrote:
> > > > Handle ENOSYS from cap_get_proc() in check_for_cap_sys_rawio(), so
> > > > turbostat can display temperatures when running on kernels compiled
> > > > without multiuser support.
> > > >
> > > > Signed-off-by: Calvin Owens <calvin@...nvd.org>
> > > > ---
> > > >  tools/power/x86/turbostat/turbostat.c | 7 ++++++-
> > > >  1 file changed, 6 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/tools/power/x86/turbostat/turbostat.c
> > > > b/tools/power/x86/turbostat/turbostat.c
> > > > index 925556b90770..f7d665913a52 100644
> > > > --- a/tools/power/x86/turbostat/turbostat.c
> > > > +++ b/tools/power/x86/turbostat/turbostat.c
> > > > @@ -6496,8 +6496,13 @@ int check_for_cap_sys_rawio(void)
> > > >   int ret = 0;
> > > >
> > > >   caps = cap_get_proc();
> > > > - if (caps == NULL)
> > > > + if (caps == NULL) {
> > > > +         /* Support CONFIG_MULTIUSER=n */
> > > > +         if (errno == ENOSYS)
> > >
> > > Can you point me where this knowledge comes from?
> > >
> > > I downloaded the libcap source and didn't see how ENOSYS is set.
> >
> > Hi Rui,
> >
> > When the kernel is built without multiuser support, the capget() et al.
> > syscalls are #ifdef'd out:
> >
> >
> > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/kernel/capability.c#n32
> >
> > ...so userspace will get -ENOSYS when it tries to call them, and that
> > ends up being propagated to errno in userspace.
> >
> > Admittedly it is sort of implicit. Maybe a better way to "fix" this
> > would be to warn the user if the capability check fails, but still
> > attempt to access the MSR devices? I can do that if you prefer.
>
> if cap_get_proc returns -ENOSYS only when CONFIG_MULTIUSER=n, then I
> think the patch is good, maybe we just need some more detailed comment.
>
> and I don't think we should continue when the capability check fails in
> general.
>
> >
> > That is my only problem here: when check_for_cap_sys_rawio() fails, the
> > current code doesn't attempt to access the MSR devices at all, even
> > though in my case it would actually work.
>
> Or maybe we can cover this with the "--force" parameter?
> say, does it make sense to address the problem by running "turbostat --
> force"?
> I'll leave this question to Len.
>
> thanks,
> rui
> >
> > I realize this is very weird: it came up when I was recently including
> > turbostat as part of an extremely tiny bootable utility image.
> >
> > Thanks,
> > Calvin
> >
> > > thanks,
> > > rui
> > > > +                 return 0;
> > > > +
> > > >           return 1;
> > > > + }
> > > >
> > > >   if (cap_get_flag(caps, CAP_SYS_RAWIO, CAP_EFFECTIVE,
> > > > &cap_flag_value)) {
> > > >           ret = 1;
> > >
>


-- 
Len Brown, Intel Open Source Technology Center