Re: module: add an early early_mod_check()

[Jessica Yu]

+++ Luis R. Rodriguez [07/12/17 16:15 -0800]:
> We technically do a bit of sanity check with a local
> struct module with pointers set to passed user data first.
> Only after we do these checks do we embark on allocating
> a struct module based on the passed information.
>
> There's a small set of early checks which help us bail
> out from processing and avoiding memory allocation, we
> currently have these small checks tangled with the allocation
> routine. Split this out so that we can expand on the early
> checks with a local copy first.
>
> This leaves setup_load_info() in an odd place given its
> the one that originally sets up the local struct module
> with pointing to the user data. Just move this out into the
> laod_module() routine to make the early_mod_check() routine
> much simpler.
>
> This change just shuffles code around a bit to make the
> next change easier to read, it technically has no functional
> changes.
>
> Signed-off-by: Luis R. Rodriguez <mcgrof@xxxxxxxxxx>
> ---
> kernel/module.c | 38 ++++++++++++++++++++++++++------------
> 1 file changed, 26 insertions(+), 12 deletions(-)
>
> diff --git a/kernel/module.c b/kernel/module.c
> index 8042b8fcbf14..195ef37e242a 100644
> --- a/kernel/module.c
> +++ b/kernel/module.c
> @@ -3287,23 +3287,28 @@ static bool blacklisted(const char *module_name)
> }
> core_param(module_blacklist, module_blacklist, charp, 0400);
>
> -static struct module *layout_and_allocate(struct load_info *info, int flags)
> +/* Module within temporary copy, this doesn't do any allocation  */
> +static int early_mod_check(struct load_info *info, int flags,
> +			   struct module *mod)
> {
> -	/* Module within temporary copy. */
> -	struct module *mod;
> -	unsigned int ndx;
> 	int err;
>
> -	mod = setup_load_info(info, flags);
> -	if (IS_ERR(mod))
> -		return mod;
> -
> 	if (blacklisted(info->name))
> -		return ERR_PTR(-EPERM);
> +		return -EPERM;
>
> 	err = check_modinfo(mod, info, flags);
> 	if (err)
> -		return ERR_PTR(err);
> +		return err;
> +
> +	return 0;
> +}
> +
> +/* This starts to process the module and finally allocates a copy */
> +static struct module *layout_and_allocate(struct load_info *info,
> +					  struct module *mod)
> +{
> +	unsigned int ndx;
> +	int err;
>
> 	/* Allow arches to frob section contents and sizes.  */
> 	err = module_frob_arch_sections(info->hdr, info->sechdrs,
> @@ -3654,8 +3659,17 @@ static int load_module(struct load_info *info, const char __user *uargs,
> 	if (err)
> 		goto free_copy;
>
> -	/* Figure out module layout, and allocate all the memory. */
> -	mod = layout_and_allocate(info, flags);
> +	mod = setup_load_info(info, flags);
> +	if (IS_ERR(mod))
> +		return -EINVAL;

We need to goto free_copy here too, and propagate the error returned
by setup_load_info() i.e. propagate PTR_ERR(mod).

> +
> +	/* layout and check */
> +	err = early_mod_check(info, flags, mod);
> +	if (err)
> +		goto free_copy;
> +
> +	/* Layout and allocate all the memory. */
> +	mod = layout_and_allocate(info, mod);
> 	if (IS_ERR(mod)) {
> 		err = PTR_ERR(mod);
> 		goto free_copy;
> --
> 2.15.0