From: Ingo Molnar <mingo@kernel.org> To: Linus Torvalds <torvalds@linux-foundation.org> Cc: Pavel Machek <pavel@ucw.cz>, Greg Kroah-Hartman <gregkh@linuxfoundation.org>, Heiko Carstens <heiko.carstens@de.ibm.com>, Baole Ni <baolex.ni@intel.com>, Russell King - ARM Linux <linux@armlinux.org.uk>, "linux-arm-kernel@lists.infradead.org" <linux-arm-kernel@lists.infradead.org>, Linux Kernel Mailing List <linux-kernel@vger.kernel.org>, chuansheng.liu@intel.com Subject: [PATCH] Add file permission mode helpers Date: Wed, 3 Aug 2016 10:11:40 +0200 [thread overview] Message-ID: <20160803081140.GA7833@gmail.com> (raw) In-Reply-To: <CA+55aFw5v23T-zvDZp-MmD_EYxF8WbafwwB59934FV7g21uMGQ@mail.gmail.com> * Linus Torvalds <torvalds@linux-foundation.org> wrote: > [ So I answered similarly to another patch, but I'll just re-iterate > and change the subject line so that it stands out a bit from the > millions of actual patches ] > > On Tue, Aug 2, 2016 at 1:42 PM, Pavel Machek <pavel@ucw.cz> wrote: > > > > Everyone knows what 0644 is, but noone can read S_IRUSR | S_IWUSR | > > S_IRCRP | S_IROTH (*). Please don't do this. > > Absolutely. It's *much* easier to parse and understand the octal > numbers, while the symbolic macro names are just random line noise and > hard as hell to understand. You really have to think about it. > > So we should rather go the other way: convert existing bad symbolic > permission bit macro use to just use the octal numbers. In addition to that I'd love to have something even easier to read, a few common variants of the permissions field of 'ls -l' pre-defined. I did some quick grepping, and collected the main variants that are in use: PERM_r________ 0400 PERM_r__r_____ 0440 PERM_r__r__r__ 0444 PERM_rw_______ 0600 PERM_rw_r_____ 0640 PERM_rw_r__r__ 0644 PERM_rw_rw_r__ 0664 PERM_rw_rw_rw_ 0666 PERM__w_______ 0200 PERM__w__w____ 0220 PERM__w__w__w_ 0222 PERM_r_x______ 0500 PERM_r_xr_x___ 0550 PERM_r_xr_xr_x 0555 PERM_rwx______ 0700 PERM_rwxr_x___ 0750 PERM_rwxr_xr_x 0755 PERM_rwxrwxr_x 0775 PERM_rwxrwxrwx 0777 PERM__wx______ 0300 PERM__wx_wx___ 0330 PERM__wx_wx_wx 0333 Allowing these would be nice too, because there were cases in the past where people messed up the octal representation or our internal symbolic helpers, but this representation is fundamentally self-describing and pretty 'fool proof'. An added advantage would be that during review it would stick out like a sore thumb if anyone used a 'weird' permission variant. For example, if you saw these lines in a driver patch: + __ATTR(l1, 0444, driver_show_l4, NULL); + __ATTR(l3, 0446, driver_show_l4, NULL); + __ATTR(l2, 04444, driver_show_l4, NULL); + __ATTR(l4, 0444, driver_show_l4, NULL); ... would you notice it at a glance that it contains two security holes? While the weird permissions in this: + __ATTR(l1, PERM_r__r__r__, driver_show_l4, NULL); + __ATTR(l3, PERM_r__r__rw_, driver_show_l4, NULL); + __ATTR(l2, PERM_sr__r__r__, driver_show_l4, NULL); + __ATTR(l4, PERM_r__r__r__, driver_show_l4, NULL); Wouln't even build, because the dangerous patterns of PERM_r__r__rw_ or PERM_sr__r__r__ are not defined to begin with. The patch below adds them to stat.h. Thanks, Ingo include/linux/stat.h | 32 ++++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/include/linux/stat.h b/include/linux/stat.h index 075cb0c7eb2a..863d5563427f 100644 --- a/include/linux/stat.h +++ b/include/linux/stat.h @@ -5,6 +5,38 @@ #include <asm/stat.h> #include <uapi/linux/stat.h> +/* + * Human readable symbolic definitions for common + * file permissions: + */ +#define PERM_r________ 0400 +#define PERM_r__r_____ 0440 +#define PERM_r__r__r__ 0444 + +#define PERM_rw_______ 0600 +#define PERM_rw_r_____ 0640 +#define PERM_rw_r__r__ 0644 +#define PERM_rw_rw_r__ 0664 +#define PERM_rw_rw_rw_ 0666 + +#define PERM__w_______ 0200 +#define PERM__w__w____ 0220 +#define PERM__w__w__w_ 0222 + +#define PERM_r_x______ 0500 +#define PERM_r_xr_x___ 0550 +#define PERM_r_xr_xr_x 0555 + +#define PERM_rwx______ 0700 +#define PERM_rwxr_x___ 0750 +#define PERM_rwxr_xr_x 0755 +#define PERM_rwxrwxr_x 0775 +#define PERM_rwxrwxrwx 0777 + +#define PERM__wx______ 0300 +#define PERM__wx_wx___ 0330 +#define PERM__wx_wx_wx 0333 + #define S_IRWXUGO (S_IRWXU|S_IRWXG|S_IRWXO) #define S_IALLUGO (S_ISUID|S_ISGID|S_ISVTX|S_IRWXUGO) #define S_IRUGO (S_IRUSR|S_IRGRP|S_IROTH)
WARNING: multiple messages have this Message-ID (diff)
From: mingo@kernel.org (Ingo Molnar) To: linux-arm-kernel@lists.infradead.org Subject: [PATCH] Add file permission mode helpers Date: Wed, 3 Aug 2016 10:11:40 +0200 [thread overview] Message-ID: <20160803081140.GA7833@gmail.com> (raw) In-Reply-To: <CA+55aFw5v23T-zvDZp-MmD_EYxF8WbafwwB59934FV7g21uMGQ@mail.gmail.com> * Linus Torvalds <torvalds@linux-foundation.org> wrote: > [ So I answered similarly to another patch, but I'll just re-iterate > and change the subject line so that it stands out a bit from the > millions of actual patches ] > > On Tue, Aug 2, 2016 at 1:42 PM, Pavel Machek <pavel@ucw.cz> wrote: > > > > Everyone knows what 0644 is, but noone can read S_IRUSR | S_IWUSR | > > S_IRCRP | S_IROTH (*). Please don't do this. > > Absolutely. It's *much* easier to parse and understand the octal > numbers, while the symbolic macro names are just random line noise and > hard as hell to understand. You really have to think about it. > > So we should rather go the other way: convert existing bad symbolic > permission bit macro use to just use the octal numbers. In addition to that I'd love to have something even easier to read, a few common variants of the permissions field of 'ls -l' pre-defined. I did some quick grepping, and collected the main variants that are in use: PERM_r________ 0400 PERM_r__r_____ 0440 PERM_r__r__r__ 0444 PERM_rw_______ 0600 PERM_rw_r_____ 0640 PERM_rw_r__r__ 0644 PERM_rw_rw_r__ 0664 PERM_rw_rw_rw_ 0666 PERM__w_______ 0200 PERM__w__w____ 0220 PERM__w__w__w_ 0222 PERM_r_x______ 0500 PERM_r_xr_x___ 0550 PERM_r_xr_xr_x 0555 PERM_rwx______ 0700 PERM_rwxr_x___ 0750 PERM_rwxr_xr_x 0755 PERM_rwxrwxr_x 0775 PERM_rwxrwxrwx 0777 PERM__wx______ 0300 PERM__wx_wx___ 0330 PERM__wx_wx_wx 0333 Allowing these would be nice too, because there were cases in the past where people messed up the octal representation or our internal symbolic helpers, but this representation is fundamentally self-describing and pretty 'fool proof'. An added advantage would be that during review it would stick out like a sore thumb if anyone used a 'weird' permission variant. For example, if you saw these lines in a driver patch: + __ATTR(l1, 0444, driver_show_l4, NULL); + __ATTR(l3, 0446, driver_show_l4, NULL); + __ATTR(l2, 04444, driver_show_l4, NULL); + __ATTR(l4, 0444, driver_show_l4, NULL); ... would you notice it at a glance that it contains two security holes? While the weird permissions in this: + __ATTR(l1, PERM_r__r__r__, driver_show_l4, NULL); + __ATTR(l3, PERM_r__r__rw_, driver_show_l4, NULL); + __ATTR(l2, PERM_sr__r__r__, driver_show_l4, NULL); + __ATTR(l4, PERM_r__r__r__, driver_show_l4, NULL); Wouln't even build, because the dangerous patterns of PERM_r__r__rw_ or PERM_sr__r__r__ are not defined to begin with. The patch below adds them to stat.h. Thanks, Ingo include/linux/stat.h | 32 ++++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/include/linux/stat.h b/include/linux/stat.h index 075cb0c7eb2a..863d5563427f 100644 --- a/include/linux/stat.h +++ b/include/linux/stat.h @@ -5,6 +5,38 @@ #include <asm/stat.h> #include <uapi/linux/stat.h> +/* + * Human readable symbolic definitions for common + * file permissions: + */ +#define PERM_r________ 0400 +#define PERM_r__r_____ 0440 +#define PERM_r__r__r__ 0444 + +#define PERM_rw_______ 0600 +#define PERM_rw_r_____ 0640 +#define PERM_rw_r__r__ 0644 +#define PERM_rw_rw_r__ 0664 +#define PERM_rw_rw_rw_ 0666 + +#define PERM__w_______ 0200 +#define PERM__w__w____ 0220 +#define PERM__w__w__w_ 0222 + +#define PERM_r_x______ 0500 +#define PERM_r_xr_x___ 0550 +#define PERM_r_xr_xr_x 0555 + +#define PERM_rwx______ 0700 +#define PERM_rwxr_x___ 0750 +#define PERM_rwxr_xr_x 0755 +#define PERM_rwxrwxr_x 0775 +#define PERM_rwxrwxrwx 0777 + +#define PERM__wx______ 0300 +#define PERM__wx_wx___ 0330 +#define PERM__wx_wx_wx 0333 + #define S_IRWXUGO (S_IRWXU|S_IRWXG|S_IRWXO) #define S_IALLUGO (S_ISUID|S_ISGID|S_ISVTX|S_IRWXUGO) #define S_IRUGO (S_IRUSR|S_IRGRP|S_IROTH)
next prev parent reply other threads:[~2016-08-03 8:12 UTC|newest] Thread overview: 28+ messages / expand[flat|nested] mbox.gz Atom feed top 2016-08-02 20:58 Please don't replace numeric parameter like 0444 with macro Linus Torvalds 2016-08-02 20:58 ` Linus Torvalds 2016-08-02 21:53 ` Rob Landley 2016-08-02 21:53 ` Rob Landley 2016-08-02 23:39 ` [PATCH] checkpatch: Look for symbolic permissions and suggest octal instead Joe Perches 2016-08-03 0:15 ` Al Viro 2016-08-03 0:30 ` Joe Perches 2016-08-15 16:38 ` Joe Perches 2016-08-03 0:42 ` Please don't replace numeric parameter like 0444 with macro Al Viro 2016-08-03 0:42 ` Al Viro 2016-08-03 8:07 ` Konstantin Khlebnikov 2016-08-03 8:07 ` Konstantin Khlebnikov 2016-08-03 8:30 ` Richard Weinberger 2016-08-03 8:30 ` Richard Weinberger 2016-08-03 8:11 ` Ingo Molnar [this message] 2016-08-03 8:11 ` [PATCH] Add file permission mode helpers Ingo Molnar 2016-08-03 8:28 ` Greg Kroah-Hartman 2016-08-03 8:28 ` Greg Kroah-Hartman 2016-08-03 8:39 ` Ingo Molnar 2016-08-03 8:39 ` Ingo Molnar 2016-08-03 9:21 ` Willy Tarreau 2016-08-03 9:21 ` Willy Tarreau 2016-08-03 9:53 ` Marcel Holtmann 2016-08-03 9:53 ` Marcel Holtmann 2016-08-03 15:49 ` Joe Perches 2016-08-03 15:49 ` Joe Perches 2016-08-03 16:38 ` Pavel Machek 2016-08-03 16:38 ` Pavel Machek
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=20160803081140.GA7833@gmail.com \ --to=mingo@kernel.org \ --cc=baolex.ni@intel.com \ --cc=chuansheng.liu@intel.com \ --cc=gregkh@linuxfoundation.org \ --cc=heiko.carstens@de.ibm.com \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux@armlinux.org.uk \ --cc=pavel@ucw.cz \ --cc=torvalds@linux-foundation.org \ /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: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.