linux-kernel - Re: [patch] mtrr: fix issues with large addresses

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: <m13b5kw2qj.fsf@ebiederm.dsl.xmission.com>
Date:	Mon, 05 Feb 2007 17:26:12 -0700
From:	ebiederm@...ssion.com (Eric W. Biederman)
To:	"Andreas Herrmann" <andreas.herrmann3@....com>
Cc:	"Andi Kleen" <ak@...e.de>, linux-kernel@...r.kernel.org,
	discuss@...-64.org, "Richard Gooch" <rgooch@...e-mbox.com>
Subject: Re: [patch] mtrr: fix issues with large addresses

"Andreas Herrmann" <andreas.herrmann3@....com> writes:

> Hi,
>
> This is a repost of a mail sent to Richard Gooch and lkml some time
> ago. Meanwhile I noticed that Richard has a new email address. And it
> seems that he does not maintain the mtrr code anymore. (So how about
> updating the MAINTAINERS file?)
>
> Here we go again -- with new recipient and a slightly modified
> version of the patch.
>
>
> Regards,
>
> Andreas
>
>
> mtrr: fix issues with large addresses
>
> Fixes some issues with /proc/mtrr interface:
> o If physical address size crosses the 44 bit boundary
>   size_or_mask is evaluated wrong
> o size_and_mask limits physical base
>   address for an MTRR to be less than 44 bit
> o added check to restrict base address to 36 bit on i386

The limit is per cpu not per architecture.  So if you run a
cpu that can run in 64bit mode in 32bit mode the limit
is not 36 bits.  Even PAE in 32bit mode doesn't have that limit.

> Signed-off-by: Andreas Herrmann <andreas.herrmann3@....com>
>
> --
> diff --git a/arch/i386/kernel/cpu/mtrr/generic.c
> b/arch/i386/kernel/cpu/mtrr/generic.c
> index f77fc53..aa21d15 100644
> --- a/arch/i386/kernel/cpu/mtrr/generic.c
> +++ b/arch/i386/kernel/cpu/mtrr/generic.c
> @@ -172,7 +172,7 @@ int generic_get_free_region(unsigned long base, unsigned
> long size, int replace_
>  static void generic_get_mtrr(unsigned int reg, unsigned long *base,
>  			     unsigned long *size, mtrr_type *type)
>  {
> -	unsigned int mask_lo, mask_hi, base_lo, base_hi;
> +	unsigned long mask_lo, mask_hi, base_lo, base_hi;

Why?  Given the low and the high I am assuming these are all implicitly
32bit quantities.  unsigned int is fine.
  
>  	rdmsr(MTRRphysMask_MSR(reg), mask_lo, mask_hi);
>  	if ((mask_lo & 0x800) == 0) {
> diff --git a/arch/i386/kernel/cpu/mtrr/if.c b/arch/i386/kernel/cpu/mtrr/if.c
> index 5ae1705..3abc3f1 100644
> --- a/arch/i386/kernel/cpu/mtrr/if.c
> +++ b/arch/i386/kernel/cpu/mtrr/if.c
> @@ -137,6 +137,10 @@ mtrr_write(struct file *file, const char __user *buf,
> size_t len, loff_t * ppos)
>  	for (i = 0; i < MTRR_NUM_TYPES; ++i) {
>  		if (strcmp(ptr, mtrr_strings[i]))
>  			continue;
> +#ifndef CONFIG_X86_64
> +		if (base > 0xfffffffffULL)
> +			return -EINVAL;
> +#endif

That is just silly.  If the cpu is running in long mode or should
not affect this capability.

>  		base >>= PAGE_SHIFT;
>  		size >>= PAGE_SHIFT;
>  		err =
> diff --git a/arch/i386/kernel/cpu/mtrr/main.c b/arch/i386/kernel/cpu/mtrr/main.c
> index 16bb7ea..0acfb6a 100644
> --- a/arch/i386/kernel/cpu/mtrr/main.c
> +++ b/arch/i386/kernel/cpu/mtrr/main.c
> @@ -50,7 +50,7 @@ u32 num_var_ranges = 0;
>  unsigned int *usage_table;
>  static DEFINE_MUTEX(mtrr_mutex);
>  
> -u32 size_or_mask, size_and_mask;
> +u64 size_or_mask, size_and_mask;
>  
>  static struct mtrr_ops * mtrr_ops[X86_VENDOR_NUM] = {};
>  
> @@ -662,8 +662,8 @@ void __init mtrr_bp_init(void)
>  			     boot_cpu_data.x86_mask == 0x4))
>  				phys_addr = 36;
>  
> -			size_or_mask = ~((1 << (phys_addr - PAGE_SHIFT)) - 1);
> -			size_and_mask = ~size_or_mask & 0xfff00000;
> + size_or_mask = ~((1ULL << (phys_addr - PAGE_SHIFT)) - 1);
> +			size_and_mask = ~size_or_mask & 0xfffff00000ULL;

Don't you want to make this hard coded mask 0xfffffffffff00000ULL?

Eric
-
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/