Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755245AbaA1MU4 (ORCPT ); Tue, 28 Jan 2014 07:20:56 -0500 Received: from mail-ee0-f50.google.com ([74.125.83.50]:57120 "EHLO mail-ee0-f50.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755229AbaA1MUy (ORCPT ); Tue, 28 Jan 2014 07:20:54 -0500 Date: Tue, 28 Jan 2014 13:20:48 +0100 From: Ingo Molnar To: Geert Uytterhoeven Cc: Joe Perches , Linus Torvalds , David Howells , Linux Kernel Mailing List , linux-afs@lists.infradead.org, Pali =?iso-8859-1?Q?Roh=E1r?= , Alexey Dobriyan Subject: Re: [PATCH] afs: proc cells and rootcell are writeable Message-ID: <20140128122048.GA26767@gmail.com> References: <20140126122729.32113.19659.stgit@warthog.procyon.org.uk> <20140126201928.GA8224@gmail.com> <20140126202509.GA10275@gmail.com> <20140128120432.GA26347@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org * Geert Uytterhoeven wrote: > On Tue, Jan 28, 2014 at 1:04 PM, Ingo Molnar wrote: > > * Geert Uytterhoeven wrote: > >> On Sun, Jan 26, 2014 at 9:25 PM, Ingo Molnar wrote: > >> > * Ingo Molnar wrote: > >> >> * Linus Torvalds wrote: > >> >> > On Sun, Jan 26, 2014 at 4:27 AM, David Howells wrote: > >> >> > > - p = proc_create("cells", 0, proc_afs, &afs_proc_cells_fops); > >> >> > > + p = proc_create("cells", S_IFREG | S_IRUGO | S_IWUSR, proc_afs, &afs_proc_cells_fops); > >> >> > > - p = proc_create("rootcell", 0, proc_afs, &afs_proc_rootcell_fops); > >> >> > > + p = proc_create("rootcell", S_IFREG | S_IRUGO | S_IWUSR, proc_afs, &afs_proc_rootcell_fops); > >> >> > > >> >> > So the S_IFREG isn't necessary. > >> >> > > >> >> > And quite frankly, I personally think S_IRUGO | S_IWUSR is _less_ > >> >> > readable than 0644. It's damn hard to parse those random letter > >> >> > combinations, and at least I have to really think about it, in a way > >> >> > that the octal representation does *not* make me go "I have to think > >> >> > about that". > >> >> > > >> >> > So my personal preference would be to just see that simple 0644 in > >> >> > proc_create. Hmm? > >> >> > >> >> Perhaps we could also generate the most common variants as: > >> >> > >> >> #define PERM__rw_r__r__ 0644 > >> >> #define PERM__r________ 0400 > >> >> #define PERM__r__r__r__ 0444 > >> >> #define PERM__r_xr_xr_x 0555 > >> > >> I like it (also without the PERM prefix, cfr. Alexey's old patch). > >> > >> >> or something similar, more or less matching the output of 'ls -l'? > >> > > >> > Another variant of this would be to do the following macro: > >> > > >> > PERM(R_X, R_X, R_X) > >> > PERM(R__, R__, R__) > >> > PERM(RW_, R__, R__) > >> > >> IMHO, this is again less outstanding. > >> > >> > With the advantage of separating the groups better and reducing the > >> > number of constants needed. > >> > >> Only a limited number of combinations is in active use, right? > > > > Correct - and in fact that kind of limitation is also a security > > feature: using patterns _outside_ of the typical, already defined > > group of permission patterns would in itself be a 'is that really > > justified?' red flag during review. > > Then Joe (CCed :-) can write a checkpatch rule to flag all new users of the > I_S[RWX}* flags.., I'd also flag raw octal use: they are easily mixed up and only a relatively small group of people will read them as-is, most other kernel/Linux people will only recognize anomalies in the rwxrwxrwx notation. So the number of reviewers who might spot bugs, either during patch submission or later on reading the code increases. Thanks, Ingo -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/