linux-kernel - Re: [PATCH 1/1] x86/microcode: vsnprintf() might be unavailable

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: <1433263589.26331.40.camel@linux.intel.com>
Date:	Tue, 02 Jun 2015 19:46:29 +0300
From:	Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
To:	Borislav Petkov <bp@...e.de>
Cc:	Ingo Molnar <mingo@...nel.org>, linux-kernel@...r.kernel.org,
	Mika Westerberg <mika.westerberg@...ux.intel.com>
Subject: Re: [PATCH 1/1] x86/microcode: vsnprintf() might be unavailable


[+ Mika, who also somehow involved in the investigation]

On Tue, 2015-06-02 at 17:56 +0300, Andy Shevchenko wrote:
> On Intel CPUs with x32 kernels we call load_builtin_intel_microcode() from
> head_32.S on quite earlier stage. At that point sprintf() might be out of scope
> to be called. As a result the 32-bit kernel does not boot on Intel CPUs. It has
> been tested on Braswell and Merrifield.
> 

Small update below.

> The patch changes sprintf() call to plain code which does the same in this
> particular case.
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
> ---
>  arch/x86/kernel/cpu/microcode/intel_early.c | 13 +++++++++----
>  1 file changed, 9 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/x86/kernel/cpu/microcode/intel_early.c b/arch/x86/kernel/cpu/microcode/intel_early.c
> index 10dff3f..a250f41 100644
> --- a/arch/x86/kernel/cpu/microcode/intel_early.c
> +++ b/arch/x86/kernel/cpu/microcode/intel_early.c
> @@ -523,9 +523,9 @@ EXPORT_SYMBOL_GPL(save_mc_for_early);
>  
>  static bool __init load_builtin_intel_microcode(struct cpio_data *cp)
>  {
> -	u32 eax = 0x00000001, ebx, ecx = 0, edx;
> -	int family, model, stepping;
> -	char name[30];
> +	unsigned int eax = 0x00000001, ebx, ecx = 0, edx;
> +	unsigned int family, model, stepping;

Change types to be in align with function prototypes.

> +	char name[30] = "intel-ucode", *p = name + 11;
>  
>  	native_cpuid(&eax, &ebx, &ecx, &edx);
>  
> @@ -533,7 +533,12 @@ static bool __init load_builtin_intel_microcode(struct cpio_data *cp)
>  	model    = x86_model(eax);
>  	stepping = eax & 0xf;
>  
> -	sprintf(name, "intel-ucode/%02x-%02x-%02x", family, model, stepping);
> +	*p++ = '/';
> +	p = hex_byte_pack(p, family);
> +	*p++ = '-';
> +	p = hex_byte_pack(p, model);
> +	*p++ = '-';
> +	p = hex_byte_pack(p, stepping);
>  

Forgot to add *p = '\0'; here, but it doesn't really mater for the idea.

What I would like to tell that if I move 'call load_ucode_bsp' after
paging is done, it seems sprintf() starts working nicely. So, I don't
know which fix is better (or neither), and wondering if we need to fix
AMD part.

Please, share your opinions, suggestions.

P.S. It would be really nice to get an expert / detailed explanation
what is going on there.

>  	return get_builtin_firmware(cp, name);
>  }

-- 
Andy Shevchenko <andriy.shevchenko@...el.com>
Intel Finland Oy

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/