linux-kernel - Re: [PATCH] treewide: const qualify ctl

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>] [day] [month] [year] [list]

Message-ID: <hrxhjywchs7kbovfvlpkqlazz4gakmntrwq74xhuno5ti6ye43@4rqev6uweu4p>
Date: Tue, 18 Feb 2025 11:35:27 +0100
From: Joel Granados <joel.granados@...nel.org>
To: Petr Mladek <pmladek@...e.com>
Cc: bot+bpf-ci@...nel.org, Douglas Anderson <dianders@...omium.org>, 
	Andrew Morton <akpm@...ux-foundation.org>, kernel-ci@...a.com, andrii@...nel.org, daniel@...earbox.net, 
	martin.lau@...ux.dev, "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] treewide: const qualify ctl_tables where applicable

On Tue, Feb 18, 2025 at 11:29:53AM +0100, Joel Granados wrote:
> On Mon, Jan 13, 2025 at 11:13:57AM +0100, Petr Mladek wrote:
> > On Fri 2025-01-10 14:32:30, Joel Granados wrote:
> > > On Thu, Jan 09, 2025 at 01:38:33PM +0000, bot+bpf-ci@...nel.org wrote:
> > > > Dear patch submitter,
> > > > 
> > > > CI has tested the following submission:
> > > > Status:     FAILURE
> > > > Name:       treewide: const qualify ctl_tables where applicable
> > > > Patchwork:  https://patchwork.kernel.org/project/netdevbpf/list/?series=923743&state=*
> > > > Matrix:     https://github.com/kernel-patches/bpf/actions/runs/12690795270
> > > > 
> > > > Failed jobs:
> > > > build-x86_64-gcc: https://github.com/kernel-patches/bpf/actions/runs/12690795270/job/35372434718
> > > > build-x86_64-llvm-17: https://github.com/kernel-patches/bpf/actions/runs/12690795270/job/35372434997
> > > > build-x86_64-llvm-17-O2: https://github.com/kernel-patches/bpf/actions/runs/12690795270/job/35372435294
> > > > build-x86_64-llvm-18: https://github.com/kernel-patches/bpf/actions/runs/12690795270/job/35372435638
> > > > build-x86_64-llvm-18-O2: https://github.com/kernel-patches/bpf/actions/runs/12690795270/job/35372435949
> > > We can't make watchdog_hardlockup_sysctl const here because it is
> > > changing the ctl_talbe.mode to 0644 if watchdog_hardlockup_available is
> > > true. I'll remove this sysctl array from my patchset to move forward
> > > with the general constification, but I still don't fully understand the
> > > need for the modification of the permissions. 
> > > 
> > > My main question is: Cant we just leave the permissions as they where
> > > originally (before the this commit [1])? The problem touched by [1] is
> > > when the user writes to nmi_watchdog and watchdog_hardlockup_available
> > > is false, they will receive a -ENOTSUPP error from proc_nmi_watchdog.
> > > But wont they get an error anyway if they try to write to a read-only
> > > file? Does this fix target some specific user-space application?
> > > 
> > > I have added the original to:/from: contacts from [1]. Please correct me
> > > if I have miss-read the situation.
> > > 
> > > Best
> > > 
> > > [1] https://lore.kernel.org/lkml/20230526184139.1.I0d75971cc52a7283f495aac0bd5c3041aadc734e@changeid/
> > 
> > My understanding is that adding the const is going to help
> > security. It will move the structure into .rodata section.
> > Do I get it correctly, please?
> > 
> > On the other hand, the manipulation of the access rights just affects
> > the error code which users might see: -ENOTSUPP vs. -EPERM.
> > I agree that this is not important. Some people might even consider
> > -ENOTSUPP as more informative.
> > 
> > From my POV, the commit 9ec272c586b07d1 ("watchdog/hardlockup: keep
> > kernel.nmi_watchdog sysctl as 0444 if probe fails") can be reverted.
> Perfect. I'll prepare a V1 with the revert and add some other
> "straglers" that also need to be const qualified.

Adding the list for visibility
> 
> Best
> 
> 
> > 
> > Best Regards,
> > Petr
> 
> -- 
> 
> Joel Granados

-- 

Joel Granados