lkml.org 
[lkml]   [2014]   [Mar]   [19]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
From
SubjectRe: [PATCH] virtio-blk: make the queue depth configurable
Date
Joe Perches <joe@perches.com> writes:
> On Sun, 2014-03-16 at 22:00 -0700, Joe Perches wrote:
>> On Mon, 2014-03-17 at 14:25 +1030, Rusty Russell wrote:
>>
>> > Erk, our tests are insufficient. Testbuilding an allmodconfig with this
>> > now:
>>
>> Good idea.
>>
>> > diff --git a/include/linux/moduleparam.h b/include/linux/moduleparam.h
>> []
>> > @@ -188,6 +188,9 @@ struct kparam_array
>> > /* Default value instead of permissions? */ \
>> > static int __param_perm_check_##name __attribute__((unused)) = \
>> > BUILD_BUG_ON_ZERO((perm) < 0 || (perm) > 0777 || ((perm) & 2)) \
>> > + /* User perms >= group perms >= other perms. */ \
>> > + + BUILD_BUG_ON_ZERO(((perm) >> 6) < (((perm) >> 3) & 7)) \
>> > + + BUILD_BUG_ON_ZERO((((perm) >> 3) & 7) < ((perm) & 7)) \
>> > + BUILD_BUG_ON_ZERO(sizeof(""prefix) > MAX_PARAM_PREFIX_LEN); \
>> > static const char __param_str_##name[] = prefix #name; \
>> > static struct kernel_param __moduleparam_const __param_##name \
>>
>> It might make sense to separate this octal permissions
>> test into a new macro for other checks in macros like
>> CLASS_ATTR, DEVICE_ATTR, SENSOR_ATTR and SENSOR_ATTR_2.

OK, I took your bikeshed and re-painted it below.

> #define VERIFY_OCTAL_PERMISSIONS(perms) \
> ({ \
> if (__builtin_constant_p(perms)) { \
> BUILD_BUG_ON((perms) < 0); \
> BUILD_BUG_ON((perms) > 0777); \
> /* User perms >= group perms >= other perms */ \
> BUILD_BUG_ON(((perms) >> 6) < (((perms) >> 3) & 7)); \
> BUILD_BUG_ON((((perms) >> 3) & 7) < ((perms) & 7)); \
> } \
> ; \
> })

Subject: 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 drivers/pci/slot.c.

Suggested-by: Joe Perches <joe@perches.com>
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>

diff --git a/drivers/pci/slot.c b/drivers/pci/slot.c
index 7dd62fa9d0bd..396c200b9ddb 100644
--- a/drivers/pci/slot.c
+++ b/drivers/pci/slot.c
@@ -116,11 +116,11 @@ static void pci_slot_release(struct kobject *kobj)
}

static struct pci_slot_attribute pci_slot_attr_address =
- __ATTR(address, (S_IFREG | S_IRUGO), address_read_file, NULL);
+ __ATTR(address, S_IRUGO, address_read_file, NULL);
static struct pci_slot_attribute pci_slot_attr_max_speed =
- __ATTR(max_bus_speed, (S_IFREG | S_IRUGO), max_speed_read_file, NULL);
+ __ATTR(max_bus_speed, S_IRUGO, max_speed_read_file, NULL);
static struct pci_slot_attribute pci_slot_attr_cur_speed =
- __ATTR(cur_bus_speed, (S_IFREG | S_IRUGO), cur_speed_read_file, NULL);
+ __ATTR(cur_bus_speed, S_IRUGO, cur_speed_read_file, NULL);

static struct attribute *pci_slot_default_attrs[] = {
&pci_slot_attr_address.attr,
diff --git a/include/linux/kernel.h b/include/linux/kernel.h
index 471090093c67..945afeb3058a 100644
--- a/include/linux/kernel.h
+++ b/include/linux/kernel.h
@@ -842,4 +842,13 @@ static inline void ftrace_dump(enum ftrace_dump_mode oops_dump_mode) { }
# define REBUILD_DUE_TO_FTRACE_MCOUNT_RECORD
#endif

+/* Permissions on a sysfs file: you didn't miss the 0 prefix did you? */
+#define VERIFY_OCTAL_PERMISSIONS(perms) \
+ ((__builtin_constant_p(perms) ? \
+ BUILD_BUG_ON_ZERO((perms) < 0) + \
+ BUILD_BUG_ON_ZERO((perms) > 0777) + \
+ /* User perms >= group perms >= other perms */ \
+ BUILD_BUG_ON_ZERO(((perms) >> 6) < (((perms) >> 3) & 7)) + \
+ BUILD_BUG_ON_ZERO((((perms) >> 3) & 7) < ((perms) & 7)): 0) + \
+ (perms))
#endif
diff --git a/include/linux/moduleparam.h b/include/linux/moduleparam.h
index 175f6995d1af..204a67743804 100644
--- a/include/linux/moduleparam.h
+++ b/include/linux/moduleparam.h
@@ -186,14 +186,12 @@ struct kparam_array
parameters. */
#define __module_param_call(prefix, name, ops, arg, perm, level) \
/* Default value instead of permissions? */ \
- static int __param_perm_check_##name __attribute__((unused)) = \
- BUILD_BUG_ON_ZERO((perm) < 0 || (perm) > 0777 || ((perm) & 2)) \
- + BUILD_BUG_ON_ZERO(sizeof(""prefix) > MAX_PARAM_PREFIX_LEN); \
- static const char __param_str_##name[] = prefix #name; \
+ static const char __param_str_##name[] = prefix #name; \
static struct kernel_param __moduleparam_const __param_##name \
__used \
__attribute__ ((unused,__section__ ("__param"),aligned(sizeof(void *)))) \
- = { __param_str_##name, ops, perm, level, { arg } }
+ = { __param_str_##name, ops, VERIFY_OCTAL_PERMISSIONS(perm), \
+ level, { arg } }

/* Obsolete - use module_param_cb() */
#define module_param_call(name, set, get, arg, perm) \
diff --git a/include/linux/sysfs.h b/include/linux/sysfs.h
index 30b2ebee6439..f517e6e488c8 100644
--- a/include/linux/sysfs.h
+++ b/include/linux/sysfs.h
@@ -71,7 +71,8 @@ struct attribute_group {
*/

#define __ATTR(_name, _mode, _show, _store) { \
- .attr = {.name = __stringify(_name), .mode = _mode }, \
+ .attr = {.name = __stringify(_name), \
+ .mode = VERIFY_OCTAL_PERMISSIONS(_mode) }, \
.show = _show, \
.store = _store, \
}

\
 
 \ /
  Last update: 2014-03-19 08:21    [W:0.088 / U:75.124 seconds]
©2003-2018 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site