linux-kernel - Re: [PATCH v2] x86/kernel: use pr_<level>() and dev

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: <20160214140808.GB32622@pd.tnic>
Date:	Sun, 14 Feb 2016 15:08:08 +0100
From:	Borislav Petkov <bp@...en8.de>
To:	Chen Yucong <slaoub@...il.com>
Cc:	mingo@...nel.org, tglx@...utronix.de, hpa@...or.com,
	x86@...nel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2] x86/kernel: use pr_<level>() and dev_<level>

On Sun, Feb 14, 2016 at 12:10:47PM +0800, Chen Yucong wrote:
> arch/x86/kernel/* use a mixture of printk(KERN_<level> ) and pr_<level>().
> This patch converts the bulk of printk(KERN_<level> ) to pr_<level>() and
> uses dev_dbg() instead of the dev_printk(KERN_DEBUG,). All pr_warning()
> calls have been replaced with pr_warn().
> 
> Not sure what to do about the printk(KERN_DEFAULT) and printk() without a
> log level.

...

> diff --git a/arch/x86/kernel/alternative.c b/arch/x86/kernel/alternative.c
> index 25f9093..0ecb579 100644
> --- a/arch/x86/kernel/alternative.c
> +++ b/arch/x86/kernel/alternative.c
> @@ -59,7 +59,7 @@ __setup("noreplace-paravirt", setup_noreplace_paravirt);
>  #define DPRINTK(fmt, args...)						\
>  do {									\
>  	if (debug_alternative)						\
> -		printk(KERN_DEBUG "%s: " fmt "\n", __func__, ##args);	\
> +		pr_debug("%s: " fmt "\n", __func__, ##args);	\
>  } while (0)
>  
>  #define DUMP_BYTES(buf, len, fmt, args...)				\
> @@ -70,10 +70,10 @@ do {									\
>  		if (!(len))						\
>  			break;						\
>  									\
> -		printk(KERN_DEBUG fmt, ##args);				\
> +		pr_debug(fmt, ##args);				\
>  		for (j = 0; j < (len) - 1; j++)				\
> -			printk(KERN_CONT "%02hhx ", buf[j]);		\
> -		printk(KERN_CONT "%02hhx\n", buf[j]);			\
> +			pr_cont("%02hhx ", buf[j]);		\
> +		pr_cont("%02hhx\n", buf[j]);			\
>  	}								\
>  } while (0)
>  

NAK the hell out of that hunk!

Did you actually look at how pr_debug() is defined?

Yeah, I don't think so. With your change, when I boot with
"debug-alternative" I get:

...
[    0.064005] Last level dTLB entries: 4KB 512, 2MB 255, 4MB 127, 1GB 0
[    0.068005] e9 d5 3e d3 00
[    0.072004] e9 e8 92 21 ff
[    0.075003] eb 11 0f 1f 00
[    0.077906] e8 c5 b6 30 00
[    0.084009] f3 48 0f b8 c7
[    0.091241] f3 48 0f b8 c7
[    0.094259] e8 d9 b5 30 00
[    0.097611] f3 48 0f b8 c7
[    0.100004] f3 48 0f b8 c7
[    0.103067] 90 90 90
[    0.105575] 0f ae f0
[    0.108004] 0f ae f0
[    0.112007] 90 90 90
[    0.114331] 0f ae f0
[    0.116004] 0f ae f0
[    0.118365] e8 07 21 62 ff

How is that useful?!

Please stop for a second with those senseless conversions and think
first. Try the change you're doing in kvm, take a look at what it
affects and think hard whether it makes any sense at all. Only if it
does, *then* send out the patch.

I'm willing to bet that *all* pr_debug* conversions below are wrong too.

Geez :(

-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.