linux-kernel - Re: [PATCH] video: fbdev: uvesafb: use CONFIG_X86_PAE surround _PAGE

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: <53184655.2090106@ti.com>
Date:	Thu, 6 Mar 2014 11:56:37 +0200
From:	Tomi Valkeinen <tomi.valkeinen@...com>
To:	Wang YanQing <udknight@...il.com>
CC:	<plagnioj@...osoft.com>, <fengguang.wu@...el.com>,
	<linux-fbdev@...r.kernel.org>, <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] video: fbdev: uvesafb: use CONFIG_X86_PAE surround _PAGE_NX
 check code

On 05/03/14 18:16, Wang YanQing wrote:
> Because _PAGE_NX check will always false when we don't define
> CONFIG_X86_PAE for CONFIG_X86_32, so use CONFIG_X86_PAE surround
> the check code.
> 
> Although I believe "smart" compile will optimize out and generate
> the same code, but use CONFIG_X86_PAE surround check code will
> clear it and prohibit warning from static source code analyze tool.
> 
> [ This patch fix warning report by fengguang.wu@...el.com
>   "drivers/video/fbdev/uvesafb.c:816 uvesafb_vbe_init()
>    warn: bitwise AND condition is false here" ]
> 
> Signed-off-by: Wang YanQing <udknight@...il.com>
> ---
>  drivers/video/fbdev/uvesafb.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/video/fbdev/uvesafb.c b/drivers/video/fbdev/uvesafb.c
> index 509d452..102858c 100644
> --- a/drivers/video/fbdev/uvesafb.c
> +++ b/drivers/video/fbdev/uvesafb.c
> @@ -813,6 +813,7 @@ static int uvesafb_vbe_init(struct fb_info *info)
>  	par->ypan = ypan;
>  
>  	if (par->pmi_setpal || par->ypan) {
> +#ifdef CONFIG_X86_PAE
>  		if (__supported_pte_mask & _PAGE_NX) {
>  			par->pmi_setpal = par->ypan = 0;
>  			printk(KERN_WARNING "uvesafb: NX protection is active, "
> @@ -820,6 +821,9 @@ static int uvesafb_vbe_init(struct fb_info *info)
>  		} else {
>  			uvesafb_vbe_getpmi(task, par);
>  		}
> +#else
> +		uvesafb_vbe_getpmi(task, par);
> +#endif
>  	}

I don't like this, I think this makes the code more messy, just to avoid
that warning.

And it might even be buggy. For your patch to work correctly, you need
to know the internals of _PAGE_NX, i.e. that when CONFIG_X86_PAE is not
defined, _PAGE_NX is 0. But the driver should not depend on things like
that. If _PAGE_NX is changed later, the driver will not work correctly.

 Tomi



Download attachment "signature.asc" of type "application/pgp-signature" (902 bytes)