Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:18 as permitted sender) client-ip=2620:137:e000::1:18;
Date:   Wed, 9 Feb 2022 21:48:12 +0100
From:   Michal =?iso-8859-1?Q?Such=E1nek?= <msuchanek@suse.de>
To:     20220209170358.3266629-1-atomlin@redhat.com
Cc:     mcgrof@kernel.org, cl@linux.com, pmladek@suse.com, mbenes@suse.cz,
        akpm@linux-foundation.org, jeyu@kernel.org,
        linux-kernel@vger.kernel.org, linux-modules@vger.kernel.org,
        live-patching@vger.kernel.org, atomlin@atomlin.com,
        ghalat@redhat.com, allen.lkml@gmail.com, void@manifault.com,
        joe@perches.com, christophe.leroy@csgroup.eu,
        oleksandr@natalenko.name
Subject: Re: [PATCH v5 07/13] module: Move extra signature support out of
 core code
Message-ID: <20220209204812.GD3113@kunlun.suse.cz>
References: <20220209170814.3268487-1-atomlin@redhat.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20220209170814.3268487-1-atomlin@redhat.com>
User-Agent: Mutt/1.10.1 (2018-07-13)
Precedence: bulk

Hello,

On Wed, Feb 09, 2022 at 05:08:08PM +0000, Aaron Tomlin wrote:
> No functional change.

There is functional change.


> @@ -2565,70 +2542,6 @@ static inline void kmemleak_load_module(const struct module *mod,
>  }
>  #endif
>  
> -#ifdef CONFIG_MODULE_SIG
> -static int module_sig_check(struct load_info *info, int flags)
> -{
> -	int err = -ENODATA;
> -	const unsigned long markerlen = sizeof(MODULE_SIG_STRING) - 1;
> -	const char *reason;
> -	const void *mod = info->hdr;
> -	bool mangled_module = flags & (MODULE_INIT_IGNORE_MODVERSIONS |
> -				       MODULE_INIT_IGNORE_VERMAGIC);
> -	/*
> -	 * Do not allow mangled modules as a module with version information
> -	 * removed is no longer the module that was signed.
> -	 */
> -	if (!mangled_module &&
             ^^^^^^^^^^^^^
> -	    info->len > markerlen &&
> -	    memcmp(mod + info->len - markerlen, MODULE_SIG_STRING, markerlen) == 0) {
> -		/* We truncate the module to discard the signature */
> -		info->len -= markerlen;
> -		err = mod_verify_sig(mod, info);
> -		if (!err) {
> -			info->sig_ok = true;
> -			return 0;
> -		}
> -	}

> diff --git a/kernel/module/signing.c b/kernel/module/signing.c
> index 8aeb6d2ee94b..ff41541e982a 100644
> --- a/kernel/module/signing.c
> +++ b/kernel/module/signing.c

> @@ -43,3 +62,59 @@ int mod_verify_sig(const void *mod, struct load_info *info)
>  				      VERIFYING_MODULE_SIGNATURE,
>  				      NULL, NULL);
>  }
> +
> +int module_sig_check(struct load_info *info, int flags)
> +{
> +	int err = -ENODATA;
> +	const unsigned long markerlen = sizeof(MODULE_SIG_STRING) - 1;
> +	const char *reason;
> +	const void *mod = info->hdr;
> +
> +	/*
> +	 * Require flags == 0, as a module with version information
> +	 * removed is no longer the module that was signed
> +	 */
> +	if (flags == 0 &&
            ^^^^^^

This reverts a97ac8cb24a3c3ad74794adb83717ef1605d1b47

Please re-apply.

Thanks

Michal
> +	    info->len > markerlen &&
> +	    memcmp(mod + info->len - markerlen, MODULE_SIG_STRING, markerlen) == 0) {
> +		/* We truncate the module to discard the signature */
> +		info->len -= markerlen;
> +		err = mod_verify_sig(mod, info);
> +		if (!err) {
> +			info->sig_ok = true;
> +			return 0;
> +		}
> +	}