linux-kernel - Re: [PATCH v2 1/2] ARC: U-boot: check arguments paranoidly

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: <f27a52c2-3f1a-51c2-6b4f-dfbdc94b8aff@synopsys.com>
Date:   Fri, 15 Feb 2019 15:55:08 -0800
From:   Vineet Gupta <vineet.gupta1@...opsys.com>
To:     Eugeniy Paltsev <eugeniy.paltsev@...opsys.com>,
        <linux-snps-arc@...ts.infradead.org>
CC:     <khilman@...libre.com>,
        Alexey Brodkin <alexey.brodkin@...opsys.com>,
        <linux-kernel@...r.kernel.org>,
        Corentin Labbe <clabbe@...libre.com>
Subject: Re: [PATCH v2 1/2] ARC: U-boot: check arguments paranoidly


[...]

> -char __initdata *uboot_arg;
> +unsigned int __initdata uboot_arg;

Why ?

In both places it is actually used, it is intended as a pointer. The cast for
range check is needed but lets cast there. See below for real reason.


> -static inline int is_kernel(unsigned long addr)
> +static inline bool uboot_arg_invalid(unsigned int addr)
>  {
> -	if (addr >= (unsigned long)_stext && addr <= (unsigned long)_end)
> -		return 1;
> +
> +	/* Check that address doesn't clobber resident kernel image */
> +	return addr >= (unsigned int)_stext && addr <= (unsigned int)_end;

...

> +
> +	/* see if U-boot passed an external Device Tree blob */
> +	if (uboot_tag == UBOOT_TAG_DTB) {
> +		machine_desc = setup_machine_fdt((void *)uboot_arg);

On a 64-bit paradigm, with LP64 ABI, this will break since int will be 4b, while
pointer 8b.

I'll fix it up locally and push !

-Vineet