Commit 18eb74fa authored by Rusty Russell's avatar Rusty Russell
Browse files

params: cleanup sysfs allocation 



commit 63662139 attempted to patch a
leak (which would only happen on OOM, ie. never), but it didn't quite
work.

This rewrites the code to be as simple as possible.  add_sysfs_param()
adds a parameter.  If it fails, it's the caller's responsibility to
clean up the parameters which already exist.

The kzalloc-then-always-krealloc pattern is perhaps overly simplistic,
but this code has clearly confused people.  It worked on me...

Signed-off-by: default avatarRusty Russell <rusty@rustcorp.com.au>
parent 6da0b565

kernel/params.c

+44 −51
Original line number Diff line number Diff line
@@ -603,73 +603,64 @@ static __modinit int add_sysfs_param(struct module_kobject *mk, 
				     const struct kernel_param *kp,

				     const char *name)

{

	struct module_param_attrs *new;

	struct attribute **attrs;

	int err, num;

	struct module_param_attrs *new_mp;

	struct attribute **new_attrs;

	unsigned int i;



	/* We don't bother calling this with invisible parameters. */

	BUG_ON(!kp->perm);



	if (!mk->mp) {

		num = 0;

		attrs = NULL;

	} else {

		num = mk->mp->num;

		attrs = mk->mp->grp.attrs;

		/* First allocation. */

		mk->mp = kzalloc(sizeof(*mk->mp), GFP_KERNEL);

		if (!mk->mp)

			return -ENOMEM;

		mk->mp->grp.name = "parameters";

		/* NULL-terminated attribute array. */

		mk->mp->grp.attrs = kzalloc(sizeof(mk->mp->grp.attrs[0]),

					    GFP_KERNEL);

		/* Caller will cleanup via free_module_param_attrs */

		if (!mk->mp->grp.attrs)

			return -ENOMEM;

	}



	/* Enlarge. */

	new = krealloc(mk->mp,

		       sizeof(*mk->mp) + sizeof(mk->mp->attrs[0]) * (num+1),

	/* Enlarge allocations. */

	new_mp = krealloc(mk->mp,

			  sizeof(*mk->mp) +

			  sizeof(mk->mp->attrs[0]) * (mk->mp->num + 1),

			  GFP_KERNEL);

	if (!new) {

		kfree(attrs);

		err = -ENOMEM;

		goto fail;

	}

	/* Despite looking like the typical realloc() bug, this is safe.

	 * We *want* the old 'attrs' to be freed either way, and we'll store

	 * the new one in the success case. */

	attrs = krealloc(attrs, sizeof(new->grp.attrs[0])*(num+2), GFP_KERNEL);

	if (!attrs) {

		err = -ENOMEM;

		goto fail_free_new;

	}



	/* Sysfs wants everything zeroed. */

	memset(new, 0, sizeof(*new));

	memset(&new->attrs[num], 0, sizeof(new->attrs[num]));

	memset(&attrs[num], 0, sizeof(attrs[num]));

	new->grp.name = "parameters";

	new->grp.attrs = attrs;

	if (!new_mp)

		return -ENOMEM;

	mk->mp = new_mp;



	/* Extra pointer for NULL terminator */

	new_attrs = krealloc(mk->mp->grp.attrs,

			     sizeof(mk->mp->grp.attrs[0]) * (mk->mp->num + 2),

			     GFP_KERNEL);

	if (!new_attrs)

		return -ENOMEM;

	mk->mp->grp.attrs = new_attrs;



	/* Tack new one on the end. */

	sysfs_attr_init(&new->attrs[num].mattr.attr);

	new->attrs[num].param = kp;

	new->attrs[num].mattr.show = param_attr_show;

	new->attrs[num].mattr.store = param_attr_store;

	new->attrs[num].mattr.attr.name = (char *)name;

	new->attrs[num].mattr.attr.mode = kp->perm;

	new->num = num+1;

	sysfs_attr_init(&mk->mp->attrs[mk->mp->num].mattr.attr);

	mk->mp->attrs[mk->mp->num].param = kp;

	mk->mp->attrs[mk->mp->num].mattr.show = param_attr_show;

	mk->mp->attrs[mk->mp->num].mattr.store = param_attr_store;

	mk->mp->attrs[mk->mp->num].mattr.attr.name = (char *)name;

	mk->mp->attrs[mk->mp->num].mattr.attr.mode = kp->perm;

	mk->mp->num++;



	/* Fix up all the pointers, since krealloc can move us */

	for (num = 0; num < new->num; num++)

		new->grp.attrs[num] = &new->attrs[num].mattr.attr;

	new->grp.attrs[num] = NULL;



	mk->mp = new;

	for (i = 0; i < mk->mp->num; i++)

		mk->mp->grp.attrs[i] = &mk->mp->attrs[i].mattr.attr;

	mk->mp->grp.attrs[mk->mp->num] = NULL;

	return 0;



fail_free_new:

	kfree(new);

fail:

	mk->mp = NULL;

	return err;

}



#ifdef CONFIG_MODULES

static void free_module_param_attrs(struct module_kobject *mk)

{

	if (mk->mp)

		kfree(mk->mp->grp.attrs);

	kfree(mk->mp);

	mk->mp = NULL;
@@ -695,8 +686,10 @@ int module_param_sysfs_setup(struct module *mod, 
		if (kparam[i].perm == 0)

			continue;

		err = add_sysfs_param(&mod->mkobj, &kparam[i], kparam[i].name);

		if (err)

		if (err) {

			free_module_param_attrs(&mod->mkobj);

			return err;

		}

		params = true;

	}

CRA Git | Maintained and supported by SUSTech CRA and CCSE