On Sat, 2015-03-14 at 07:32 -0700, Guenter Roeck wrote: > On Fri, Mar 13, 2015 at 04:43:43PM -0700, Joe Perches wrote: > > Currently checkpatch will fuss if one uses world writable > > settings in debugfs files and DEVICE_ATTR uses by testing > > S_IWUGO but not testing S_IWOTH, S_IRWXUGO or S_IALLUGO. > > > > Extend the check to catch all cases exporting world writable > > permissions including octal values. > > > > Original-patch-by: Nicholas Mc Guire <[email protected]> > > Signed-off-by: Joe Perches <[email protected]> > > --- > > scripts/checkpatch.pl | 12 ++++++++++-- > > 1 file changed, 10 insertions(+), 2 deletions(-) > > > > diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl > > index 6b79beb..4f07d50 100755 > > --- a/scripts/checkpatch.pl > > +++ b/scripts/checkpatch.pl > > @@ -443,6 +443,14 @@ foreach my $entry (@mode_permission_funcs) { > > $mode_perms_search .= $entry->[0]; > > } > > > > +$our $mode_perms_world_writable = qr{ > > + S_IWUGO | > > + S_IWOTH | > > + S_IRWXUGO | > > + S_IALLUGO | > > + 0[0-7][0-7][2367] > > +}x; > > + > > our $allowed_asm_includes = qr{(?x: > > irq| > > memory| > > @@ -5356,8 +5364,8 @@ sub process { > > } > > } > > > > - if ($line =~ /debugfs_create_file.*S_IWUGO/ || > > - $line =~ /DEVICE_ATTR.*S_IWUGO/ ) { > > + if ($line =~ > > /debugfs_create_\w+.*\b$mode_perms_world_writable\b/ || > > + $line =~ /DEVICE_ATTR.*\b$mode_perms_world_writable\b/) { > > WARN("EXPORTED_WORLD_WRITABLE", > > "Exporting world writable files is usually an > > error. Consider more restrictive permissions.\n" . $herecurr); > > With https://lkml.org/lkml/2015/3/12/412 in mind, maybe this should be > marked as error, at least for sysfs attributes.
Maybe it's time for the debate this commit referenced: commit 58f86cc89c3372d3e61d5b71e5513ec5a0b02848 Author: Rusty Russell <[email protected]> Date: Mon Mar 24 12:00:34 2014 +1030 VERIFY_OCTAL_PERMISSIONS: stricter checking for sysfs perms. Summary of http://lkml.org/lkml/2014/3/14/363 : Ted: module_param(queue_depth, int, 444) Joe: 0444! Rusty: User perms >= group perms >= other perms? Joe: CLASS_ATTR, DEVICE_ATTR, SENSOR_ATTR and SENSOR_ATTR_2? Side effect of stricter permissions means removing the unnecessary S_IFREG from several callers. Note that the BUILD_BUG_ON_ZERO((perm) & 2) test was removed: a fair number of drivers fail this test, so that will be the debate for a future patch. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
