linux-kernel - Re: [PATCH] x86/umip: Downgrade warning messages to debug loglevel

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20210916002735.GA9330@ranerica-svr.sc.intel.com>
Date:   Wed, 15 Sep 2021 17:27:35 -0700
From:   Ricardo Neri <ricardo.neri-calderon@...ux.intel.com>
To:     Borislav Petkov <bp@...en8.de>
Cc:     X86 ML <x86@...nel.org>, LKML <linux-kernel@...r.kernel.org>,
        Marcus Rückert <mrueckert@...e.com>
Subject: Re: [PATCH] x86/umip: Downgrade warning messages to debug loglevel

On Wed, Sep 15, 2021 at 04:46:20PM +0200, Borislav Petkov wrote:
> On Wed, Sep 15, 2021 at 04:34:10AM -0700, Ricardo Neri wrote:
> > The goal at the time was encourage users to report bugs on the
> > applications and eventually have them fixed. It also meant to warn users
> > about degraded performance due to emulation. To my knowledge, no one has
> > reported the latter thus far.
> 
> Probably because people do not even get to need UMIP a whole lot,
> apparently.
> 
> > Since after almost 4 years, performance degradation does not seem to be a
> > concern, I think it is sensible to remove the warnings.
> 
> Yap.
> 
> > They could also be salvaged by converting them to umiip_pr_debug(), just
> > to err on the cautious side without having to add a new command line
> > argument.
> 
> Yap, that's a good idea too:
> 
> ---
> From: Borislav Petkov <bp@...e.de>
> Date: Wed, 15 Sep 2021 16:39:18 +0200
> Subject: [PATCH] x86/umip: Downgrade warning messages to debug loglevel
> MIME-Version: 1.0
> Content-Type: text/plain; charset=UTF-8
> Content-Transfer-Encoding: 8bit
> 
> After four years in the wild, those have not fullfilled their
> initial purpose of pushing people to fix their software to not use
> UMIP-emulated instructions, and to warn users about the degraded
> emulation performance.
> 
> Yet, the only thing that "degrades" performance is overflowing dmesg
> with those:
> 
>   [Di Sep  7 00:24:05 2021] umip_printk: 1345 callbacks suppressed
>   [Di Sep  7 00:24:05 2021] umip: someapp.exe[29231] ip:14064cdba sp:11b7c0: SIDT instruction cannot be used by applications.
>   [Di Sep  7 00:24:05 2021] umip: someapp.exe[29231] ip:14064cdba sp:11b7c0: For now, expensive software emulation returns the result.
>   ...
>   [Di Sep  7 00:26:06 2021] umip_printk: 2227 callbacks suppressed
>   [Di Sep  7 00:26:06 2021] umip: someapp.exe[29231] ip:14064cdba sp:11b940: SIDT instruction cannot be used by applications.
> 
> and users don't really care about that - they just want to play their
> games in wine.
> 
> So convert those to debug loglevel - in case someone is still interested
> in them, someone can boot with "debug" on the kernel cmdline.
> 
> Reported-by: Marcus Rückert <mrueckert@...e.com>
> Signed-off-by: Borislav Petkov <bp@...e.de>
> Link: https://lkml.kernel.org/r/20210907200454.30458-1-bp@alien8.de
> ---
>  arch/x86/kernel/umip.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/x86/kernel/umip.c b/arch/x86/kernel/umip.c
> index 576b47e7523d..5a4b21389b1d 100644
> --- a/arch/x86/kernel/umip.c
> +++ b/arch/x86/kernel/umip.c
> @@ -92,8 +92,8 @@ static const char * const umip_insns[5] = {
>  
>  #define umip_pr_err(regs, fmt, ...) \
>  	umip_printk(regs, KERN_ERR, fmt, ##__VA_ARGS__)
> -#define umip_pr_warn(regs, fmt, ...) \
> -	umip_printk(regs, KERN_WARNING, fmt,  ##__VA_ARGS__)
> +#define umip_pr_debug(regs, fmt, ...) \
> +	umip_printk(regs, KERN_DEBUG, fmt,  ##__VA_ARGS__)
>  
>  /**
>   * umip_printk() - Print a rate-limited message
> @@ -361,10 +361,10 @@ bool fixup_umip_exception(struct pt_regs *regs)
>  	if (umip_inst < 0)
>  		return false;
>  
> -	umip_pr_warn(regs, "%s instruction cannot be used by applications.\n",
> +	umip_pr_debug(regs, "%s instruction cannot be used by applications.\n",
>  			umip_insns[umip_inst]);
>  
> -	umip_pr_warn(regs, "For now, expensive software emulation returns the result.\n");
> +	umip_pr_debug(regs, "For now, expensive software emulation returns the result.\n");
>  
>  	if (emulate_umip_insn(&insn, umip_inst, dummy_data, &dummy_data_size,
>  			      user_64bit_mode(regs)))

FWIW, Reviewed-by: Ricardo Neri <ricardo.neri-calderon@...ux.intel.com>

Thanks and BR,
Ricardo