lkml.org 
[lkml]   [2017]   [Jan]   [11]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH v6] x86: fix kaslr and memmap collision
On Tue, 10 Jan 2017, Dave Jiang wrote:
> +unsigned long simple_strtoul(const char *cp, char **endp, unsigned int base);
> +long simple_strtol(const char *cp, char **endp, unsigned int base);

What are those functions for? They are not used in that patch at all.

> +static void mem_avoid_memmap(void)
> +{
> + char arg[128];
> + int rc = 0;

What's the point of defining this variable here and zero initializing it?

> + /* see if we have any memmap areas */

Sentences start with an upper case letter. Everywhere!

> + if (cmdline_find_option("memmap", arg, sizeof(arg)) > 0) {

You can spare an indentation level by just returning when the search fails.

> + int i = 0;
> + char *str = arg;
> +
> + while (str && (i < MAX_MEMMAP_REGIONS)) {

for (i = 0; str && (i < MAX_MEMMAP_REGIONS); i++) {

Please.

> + unsigned long long start, size;
> + char *k = strchr(str, ',');
> +
> + if (k)
> + *k++ = 0;
> +
> + rc = parse_memmap(str, &start, &size);
> + if (rc < 0)
> + break;
> + str = k;
> + /* a usable region that should not be skipped */
> + if (size == 0)
> + continue;
> +
> + mem_avoid[MEM_AVOID_MEMMAP_BEGIN + i].start = start;
> + mem_avoid[MEM_AVOID_MEMMAP_BEGIN + i].size = size;

So this makes no sense. You parse start/size as unsigned long long and
then store it in an unsigned long. Works on 64bit, but on 32bit this is
just wrong:

Assume a memmap above 4G, which then will create a avoid region below 4G
due to truncation to unsigned long.

Thanks,

tglx

\
 
 \ /
  Last update: 2017-01-11 13:01    [W:0.086 / U:1.900 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site