param: configurable /sys/module/*/paramaters

PATCH Superseded David Decotigny 2 participants 5 messages Oct 27, 2011 – Oct 31, 2011

Subsystems (2)

Driver core Module support

lore

Share & tools

lore

David Decotigny

author

2011-10-27 02:23 UTC #0 lore raw

cover param: configurable /sys/module/*/paramaters

These changes allow to compile kernels with sysfs but without /sys/module/*/paramaters/. This should allow:
- on small systems: no memory pressure caused by unneeded sysfs attributes.
- on large systems: more modules could be converted to have their perm != 0 in module_param(): better for audit, debug, etc. For example, on my copy, 1658 module attributes have perm == 0, presumably to spare some memory, but it can be interesting to have access to those at runtime.

By default, /sys/module/*/paramaters/ is enabled, but can be disabled in expert mode (CONFIG_SYSFS_MODULE_PARAM is not set).

David Decotigny (3):
param: make destroy_params() private
param: simple refactoring
param: allow to selectively enable /sys/module/MOD/paramaters nodes

 fs/sysfs/Kconfig            |   15 ++++++-
 include/linux/moduleparam.h |   14 +-----
 kernel/module.c             |    9 ++++
 kernel/params.c             |  108 +++++++++++++++++++++----------------------
 4 files changed, 77 insertions(+), 69 deletions(-)

Signature · 1 line

1.7.3.1

David Decotigny

author

2011-10-27 02:23 UTC #1 lore raw

PATCH 1/3 param: make destroy_params() private

This moves params_destroy to module.c, the only place where it is needed and called.

Tested:
make defconfig all

Signed-off-by David Decotigny <[email protected]>

---
 include/linux/moduleparam.h |   10 ----------
 kernel/module.c             |    9 +++++++++
 kernel/params.c             |    8 --------
 3 files changed, 9 insertions(+), 18 deletions(-)

include/linux/moduleparam.h +0 ~~-10~~

index fffb10b..77d09f4 100644
@@ -289,16 +289,6 @@ extern int parse_args(const char *name,
 		      unsigned num,
 		      int (*unknown)(char *param, char *val));
 
-/* Called by module remove. */
-#ifdef CONFIG_SYSFS
-extern void destroy_params(const struct kernel_param *params, unsigned num);
-#else
-static inline void destroy_params(const struct kernel_param *params,
-				  unsigned num)
-{
-}
-#endif /* !CONFIG_SYSFS */
-
 /* All the helper functions */
 /* The macros to do compile-time type checking stolen from Jakub
    Jelinek, who IIRC came up with this idea for the 2.4 module init code. */

kernel/module.c +9 -0

index 93342d9..529e965 100644
@@ -1723,6 +1723,15 @@ void __weak module_arch_cleanup(struct module *mod)
 {
 }
 
+static void destroy_params(const struct kernel_param *params, unsigned num)
+{
+	unsigned int i;
+
+	for (i = 0; i < num; i++)
+		if (params[i].ops->free)
+			params[i].ops->free(params[i].arg);
+}
+
 /* Free a module, remove from lists, etc. */
 static void free_module(struct module *mod)
 {

kernel/params.c +0 -8

index 8217889..75fd7bf 100644
@@ -711,14 +711,6 @@ void module_param_sysfs_remove(struct module *mod)
 }
 #endif
 
-void destroy_params(const struct kernel_param *params, unsigned num)
-{
-	unsigned int i;
-
-	for (i = 0; i < num; i++)
-		if (params[i].ops->free)
-			params[i].ops->free(params[i].arg);
-}
 
 static struct module_kobject * __init locate_module_kobject(const char *name)
 {

Signature · 1 line

1.7.3.1

David Decotigny

author

2011-10-27 02:23 UTC #2 lore raw

PATCH 2/3 param: simple refactoring

In reply to David Decotigny

Moved locate_module_kobject() to limit the number of #ifdef/#endif blocks. Also moved __modinit macro definition at the top.

Tested:
make defconfig all
+ make all with # CONFIG_SYSFS is not set

Signed-off-by David Decotigny <[email protected]>

---
 kernel/params.c |   93 ++++++++++++++++++++++++++----------------------------
 1 files changed, 45 insertions(+), 48 deletions(-)

kernel/params.c +45 ~~-48~~

index 75fd7bf..b0e1668 100644
@@ -31,6 +31,12 @@
 #define DEBUGP(fmt, a...)
 #endif
 
+#ifdef CONFIG_MODULES
+#define __modinit
+#else
+#define __modinit __init
+#endif
+
 /* Protects all parameters, and incidentally kmalloced_param list. */
 static DEFINE_MUTEX(param_lock);

@@ -515,6 +521,44 @@ struct module_param_attrs
 };
 
 #ifdef CONFIG_SYSFS
+static struct module_kobject * __init locate_module_kobject(const char *name)
+{
+	struct module_kobject *mk;
+	struct kobject *kobj;
+	int err;
+
+	kobj = kset_find_obj(module_kset, name);
+	if (kobj) {
+		mk = to_module_kobject(kobj);
+	} else {
+		mk = kzalloc(sizeof(struct module_kobject), GFP_KERNEL);
+		BUG_ON(!mk);
+
+		mk->mod = THIS_MODULE;
+		mk->kobj.kset = module_kset;
+		err = kobject_init_and_add(&mk->kobj, &module_ktype, NULL,
+					   "%s", name);
+#ifdef CONFIG_MODULES
+		if (!err)
+			err = sysfs_create_file(&mk->kobj, &module_uevent.attr);
+#endif
+		if (err) {
+			kobject_put(&mk->kobj);
+			printk(KERN_ERR
+				"Module '%s' failed add to sysfs, error number %d\n",
+				name, err);
+			printk(KERN_ERR
+				"The system will be unstable now.\n");
+			return NULL;
+		}
+
+		/* So that we hold reference in both cases. */
+		kobject_get(&mk->kobj);
+	}
+
+	return mk;
+}
+
 #define to_param_attr(n) container_of(n, struct param_attribute, mattr)
 
 static ssize_t param_attr_show(struct module_attribute *mattr,

@@ -554,15 +598,7 @@ static ssize_t param_attr_store(struct module_attribute *mattr,
 		return len;
 	return err;
 }
-#endif
-
-#ifdef CONFIG_MODULES
-#define __modinit
-#else
-#define __modinit __init
-#endif
 
-#ifdef CONFIG_SYSFS
 void __kernel_param_lock(void)
 {
 	mutex_lock(&param_lock);

@@ -709,46 +745,7 @@ void module_param_sysfs_remove(struct module *mod)
 		free_module_param_attrs(&mod->mkobj);
 	}
 }
-#endif
-
-
-static struct module_kobject * __init locate_module_kobject(const char *name)
-{
-	struct module_kobject *mk;
-	struct kobject *kobj;
-	int err;
-
-	kobj = kset_find_obj(module_kset, name);
-	if (kobj) {
-		mk = to_module_kobject(kobj);
-	} else {
-		mk = kzalloc(sizeof(struct module_kobject), GFP_KERNEL);
-		BUG_ON(!mk);
-
-		mk->mod = THIS_MODULE;
-		mk->kobj.kset = module_kset;
-		err = kobject_init_and_add(&mk->kobj, &module_ktype, NULL,
-					   "%s", name);
-#ifdef CONFIG_MODULES
-		if (!err)
-			err = sysfs_create_file(&mk->kobj, &module_uevent.attr);
-#endif
-		if (err) {
-			kobject_put(&mk->kobj);
-			printk(KERN_ERR
-				"Module '%s' failed add to sysfs, error number %d\n",
-				name, err);
-			printk(KERN_ERR
-				"The system will be unstable now.\n");
-			return NULL;
-		}
-
-		/* So that we hold reference in both cases. */
-		kobject_get(&mk->kobj);
-	}
-
-	return mk;
-}
+#endif /* CONFIG_MODULES */
 
 static void __init kernel_add_sysfs_param(const char *name,
 					  struct kernel_param *kparam,

Signature · 1 line

1.7.3.1

David Decotigny

author

2011-10-27 02:23 UTC #3 lore raw

PATCH 3/3 param: allow to selectively enable /sys/module/MOD/paramaters nodes

In reply to David Decotigny

This change defines CONFIG_SYSFS_MODULE_PARAM to prevent kernel module parameters from being exposed to user. When unset, /sys/module/MOD is populated as usual, except for the "parameters" sub-directory, which is not created anymore.

Context: by default, when the module_param() attribute perm == 0, the module attribute is not exposed to user in /sys/module/*/parameters. Many module implementations use this strategy, presumably to spare some memory. However, it can be interesting to retrieve how kernel modules are configured at run-time (debug, audit, etc.): it would be nice to see more modules have perm != 0 in order to expose their configuration pararemers to user. Unfortunately, this doesn't play well with memory-constrained systems that need sysfs but don't need this level of introspection. This change allows to support both use cases.

Tested:
- qemu + real x86_64 with defconfig customized:
CONFIG_EXPERT=y
CONFIG_SYSFS=y
# CONFIG_SYSFS_MODULE_PARAM is not set
- qemu + real x86_64 with defconfig
- make all with allyesconfig / allnoconfig / allmodconfig

Signed-off-by David Decotigny <[email protected]>

---
 fs/sysfs/Kconfig            |   15 ++++++++++++++-
 include/linux/moduleparam.h |    4 ++--
 kernel/params.c             |    7 +++++++
 3 files changed, 23 insertions(+), 3 deletions(-)

fs/sysfs/Kconfig +14 -1

index 8c41fea..22ac50c 100644
@@ -1,4 +1,4 @@
-config SYSFS
+menuconfig SYSFS
 	bool "sysfs file system support" if EXPERT
 	default y
 	help

@@ -21,3 +21,16 @@ config SYSFS
 	example, "root=03:01" for /dev/hda1.
 
 	Designers of embedded systems may wish to say N here to conserve space.
+
+if SYSFS
+
+config SYSFS_MODULE_PARAM
+       bool "Module parameters in sysfs" if EXPERT
+       default y
+       help
+         Allow to enable/disable the availability of kernel module
+         parameters in /sys/module/[module_name]/parameters. When
+         unset, this will conserve some memory space. If unsure,
+         say Y.
+
+endif # SYSFS

include/linux/moduleparam.h +2 -2

index 77d09f4..9c42200 100644
@@ -213,7 +213,7 @@ __check_old_set_param(int (*oldset)(const char *, struct kernel_param *))
 		__kernel_param_unlock();		\
 	} while (0)
 
-#ifdef CONFIG_SYSFS
+#ifdef CONFIG_SYSFS_MODULE_PARAM
 extern void __kernel_param_lock(void);
 extern void __kernel_param_unlock(void);
 #else

@@ -400,7 +400,7 @@ extern int param_get_string(char *buffer, const struct kernel_param *kp);
 
 struct module;
 
-#if defined(CONFIG_SYSFS) && defined(CONFIG_MODULES)
+#if defined(CONFIG_SYSFS_MODULE_PARAM) && defined(CONFIG_MODULES)
 extern int module_param_sysfs_setup(struct module *mod,
 				    const struct kernel_param *kparam,
 				    unsigned int num_params);

kernel/params.c +7 -0

index b0e1668..7b2137a 100644
@@ -558,6 +558,9 @@ static struct module_kobject * __init locate_module_kobject(const char *name)
 
 	return mk;
 }
+#endif /* CONFIG_SYSFS */
+
+#ifdef CONFIG_SYSFS_MODULE_PARAM
 
 #define to_param_attr(n) container_of(n, struct param_attribute, mattr)

@@ -805,7 +808,9 @@ static void __init param_sysfs_builtin(void)
 		kernel_add_sysfs_param(modname, kp, name_len);
 	}
 }
+#endif /* CONFIG_SYSFS_MODULE_PARAM */
 
+#ifdef CONFIG_SYSFS
 ssize_t __modver_version_show(struct module_attribute *mattr,
 			      struct module_kobject *mk, char *buf)
 {

@@ -915,7 +920,9 @@ static int __init param_sysfs_init(void)
 	module_sysfs_initialized = 1;
 
 	version_sysfs_builtin();
+#ifdef CONFIG_SYSFS_MODULE_PARAM
 	param_sysfs_builtin();
+#endif /* CONFIG_SYSFS_MODULE_PARAM */
 
 	return 0;
 }

Signature · 1 line

1.7.3.1

Rusty Russell

2011-10-31 06:43 UTC #4 lore raw

Re: [PATCH v1 3/3] param: allow to selectively enable /sys/module/MOD/paramaters nodes

Hi David,

I'm having real trouble parsing your descriptions. I found it
easier to read the patches, and that'd not good.

Quoted 4 lines · David Decotigny

This change defines CONFIG_SYSFS_MODULE_PARAM to prevent kernel module parameters from being exposed to user. When unset, /sys/module/MOD is populated as usual, except for the "parameters" sub-directory, which is not created anymore.

That's backwards. CONFIG_SYSFS_MODULE_PARAM *enables* /sys/module/<modname>/parameters, this implies it disables it. Sure, it allows it to be disabled without disabling all of CONFIG_SYSFS.

Quoted 4 lines

Context: by default, when the module_param() attribute perm == 0, the module attribute is not exposed to user in /sys/module/*/parameters. Many module implementations use this strategy, presumably to spare some memory.

No, they use it because that was the default when I transferred them all from the older module parameter system. It was the safe choice.

Quoted 5 lines

However, it can be
interesting to retrieve how kernel modules are configured at run-time (debug, audit, etc.): it would be nice to see more modules have perm != 0 in order to expose their configuration pararemers to user.

But this patch doesn't address any of that.

Quoted 3 lines

Unfortunately, this doesn't play well with memory-constrained systems that need sysfs but don't need this level of introspection. This change allows to support both use cases.

Do you have any statistics to support your assertion that this has any significant effect on memory usage?

Your patches seem fine, but your descriptions are not straightforward!

Thanks,
Rusty.