linux-kernel - Re: [PATCH 05/11] touchscreen: elants: Use octal permissions

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

Date:   Mon, 23 Jul 2018 11:24:12 -0700
From:   Guenter Roeck <linux@...ck-us.net>
To:     Joe Perches <joe@...ches.com>
Cc:     Dmitry Torokhov <dmitry.torokhov@...il.com>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        dev-harsh1998 <harshitjain6751@...il.com>, trivial@...nel.org,
        Simon Budig <simon.budig@...nelconcepts.de>,
        Andi Shyti <andi@...zian.org>,
        Luca Ceresoli <luca@...aceresoli.net>,
        linux-input@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 05/11] touchscreen: elants: Use octal permissions

On Mon, Jul 23, 2018 at 10:25:10AM -0700, Joe Perches wrote:
> On Mon, 2018-07-23 at 10:18 -0700, Dmitry Torokhov wrote:
> > On Mon, Jul 23, 2018 at 03:32:00PM +0200, Greg Kroah-Hartman wrote:
> > > On Mon, Jul 23, 2018 at 06:49:20PM +0530, dev-harsh1998 wrote:
> > > > WARNING: Symbolic permissions 'S_IRUGO' are not preferred. Consider using octal permissions '0444'.
> > > > +static DEVICE_ATTR(iap_mode, S_IRUGO, show_iap_mode, NULL);
> > > > 
> > > > WARNING: Symbolic permissions 'S_IWUSR' are not preferred. Consider using octal permissions '0200'.
> > > > +static DEVICE_ATTR(update_fw, S_IWUSR, NULL, write_update_fw)
> > > > 
> > > > WARNING: Symbolic permissions 'S_IRUGO' are not preferred. Consider using octal permissions '0444'.
> > > > +		.dattr = __ATTR(_field, S_IRUGO,			\
> > > > 
> > > > Signed-off-by: Harshit Jain <harshitjain6751@...il.com>
> > > 
> > > This name doesn't match up with the From: line above :(
> > > 
> > > Please fix up and try again.
> > 
> > dtor@...r-ws:~/kernel/linux-next$ git grep S_IRU | wc -l
> > 7605
> > 
> > We either need to run a tree-wide script or leave this alone. FWIW I am
> > perfectly fine with either octals or symbolic names so I do not see
> > benefit of doing conversion for code that is not known to be broken.
> 
> About half of those are in one subsystem (drivers/hwmon)
> 
> $ git grep -w S_IRUGO | cut -f1,2 -d'/' | \
>   sort | uniq -c | sort -rn | head -10 | cat -n
>      1	   3846 drivers/hwmon
>      2	    748 drivers/scsi
>      3	    215 drivers/infiniband
>      4	    168 drivers/usb
>      5	    109 drivers/media
>      6	    106 drivers/input
>      7	    102 drivers/platform
>      8	    101 drivers/misc
>      9	    101 drivers/gpu
>     10	     91 drivers/edac
> 
> The generic reason is octal is readable and S_<FOO> is unintelligible.
> 
> https://lkml.org/lkml/2016/8/2/1945
> 

That is Linus' opinion, but not mine. Granted, my opinion doesn't count
much, but enough that I won't waste my time updating the hwmon subsystem
to match the new rules. Rules change all the time, and I am quite sure
checkpatch will find a lot of stuff to complain about (again) in the
hwmon subsystem, after my last spout of cleanup a couple of years ago.
There are much more urgent issues to fix there (such as, for example,
converting the "offending" drivers to the latest API, which would
magically cause most of the offenders to disappear).
No one but me cares about those, so bothering about the use of S_<FOO>
in hwmon seems to be quite pointless. Except, of course, it is quite
useful to have something to point to as bad citizen.

Guenter