Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.debian.security > #5163 > unrolled thread

/init/main.c issue

Started bySticky Chocolate <fumeriangaming@gmail.com>
First post2017-01-02 17:40 +0100
Last post2017-01-02 18:30 +0100
Articles 2 — 2 participants

Back to article view | Back to linux.debian.security


Contents

  /init/main.c issue Sticky Chocolate <fumeriangaming@gmail.com> - 2017-01-02 17:40 +0100
    Re: /init/main.c issue Povl Ole Haarlev Olsen <debian-security@stderr.dk> - 2017-01-02 18:30 +0100

#5163 — /init/main.c issue

FromSticky Chocolate <fumeriangaming@gmail.com>
Date2017-01-02 17:40 +0100
Subject/init/main.c issue
Message-ID<sVdOp-19V-11@gated-at.bofh.it>

[Multipart message — attachments visible in raw view] — view raw

I found a issue with some functions in /init/main.c of the linux kernel.
They all involve the problem of the use of strcpy(I found that 3 functions
use strcpy) . I thought maby this could lead to buffer overflow. Im not
completely sure

__________________________________________________________________________________________________

main.c:ln 839,col 59
static void __init do_initcall_level(int level)
{
    initcall_t *fn;

    strcpy(initcall_command_line, saved_command_line);
    parse_args(initcall_level_names[level],
           initcall_command_line, __start___param,
           __stop___param - __start___param,
           level, level,
           NULL, &repair_env_string);

    for (fn = initcall_levels[level]; fn < initcall_levels[level+1]; fn++)
        do_one_initcall(*fn);
}
__________________________________________________________________________________________________

line:ln 699,col 55

static int __init initcall_blacklist(char *str)
{
    char *str_entry;
    struct blacklist_entry *entry;

    /* str argument is a comma-separated list of functions */
    do {
        str_entry = strsep(&str, ",");
        if (str_entry) {
            pr_debug("blacklisting initcall %s\n", str_entry);
            entry = alloc_bootmem(sizeof(*entry));
            entry->buf = alloc_bootmem(strlen(str_entry) + 1);
            strcpy(entry->buf, str_entry);
            list_add(&entry->next, &blacklisted_initcalls);
        }
    } while (str_entry);

    return 0;
}
____________________________________________________________________________________________________________________________________________________

line:ln 368,ln 369,col 55, col 51

static void __init setup_command_line(char *command_line)
{
    saved_command_line =
        memblock_virt_alloc(strlen(boot_command_line) + 1, 0);
    initcall_command_line =
        memblock_virt_alloc(strlen(boot_command_line) + 1, 0);
    static_command_line = memblock_virt_alloc(strlen(command_line) + 1, 0);
    strcpy(saved_command_line, boot_command_line);
    strcpy(static_command_line, command_line);
}

[toc] | [next] | [standalone]


#5164

FromPovl Ole Haarlev Olsen <debian-security@stderr.dk>
Date2017-01-02 18:30 +0100
Message-ID<sVeAN-1Nr-5@gated-at.bofh.it>
In reply to#5163
On Mon, 2 Jan 2017, Sticky Chocolate wrote:
> I found a issue with some functions in /init/main.c of the linux kernel.
> They all involve the problem of the use of strcpy(I found that 3 functions
> use strcpy) . I thought maby this could lead to buffer overflow. Im not
> completely sure

Well, let's do a quick analysis...

> __________________________________________________________________________________________________
>
> main.c:ln 839,col 59
> static void __init do_initcall_level(int level)
> {
>    initcall_t *fn;
>
>    strcpy(initcall_command_line, saved_command_line);

initcall_command_line is defined as a static char * and saved_command_line 
is defined as a char *. At this point we don't know how much space is 
allocated for either of them, but I'll get back to this one line in a bit.

>    parse_args(initcall_level_names[level],
>           initcall_command_line, __start___param,
>           __stop___param - __start___param,
>           level, level,
>           NULL, &repair_env_string);
>
>    for (fn = initcall_levels[level]; fn < initcall_levels[level+1]; fn++)
>        do_one_initcall(*fn);
> }
> __________________________________________________________________________________________________
>
> line:ln 699,col 55
>
> static int __init initcall_blacklist(char *str)
> {
>    char *str_entry;
>    struct blacklist_entry *entry;
>
>    /* str argument is a comma-separated list of functions */
>    do {
>        str_entry = strsep(&str, ",");
>        if (str_entry) {
>            pr_debug("blacklisting initcall %s\n", str_entry);
>            entry = alloc_bootmem(sizeof(*entry));
>            entry->buf = alloc_bootmem(strlen(str_entry) + 1);
>            strcpy(entry->buf, str_entry);

First strlen(str_entry) + 1 bytes are allocated for entry->buf (in other 
words enough bytes for str_entry and the following '\0') and then 
str_entry is copied to entry->buf, which is obviously large enough since 
we just allocated enough bytes for str_entry and the '\0'. No problem 
here...

>            list_add(&entry->next, &blacklisted_initcalls);
>        }
>    } while (str_entry);
>
>    return 0;
> }
> ____________________________________________________________________________________________________________________________________________________
>
> line:ln 368,ln 369,col 55, col 51
>
> static void __init setup_command_line(char *command_line)
> {
>    saved_command_line =
>        memblock_virt_alloc(strlen(boot_command_line) + 1, 0);
>    initcall_command_line =
>        memblock_virt_alloc(strlen(boot_command_line) + 1, 0);
>    static_command_line = memblock_virt_alloc(strlen(command_line) + 1, 0);
>    strcpy(saved_command_line, boot_command_line);
>    strcpy(static_command_line, command_line);

strlen(boot_command_line) + 1 bytes are allocated for saved_command_line 
and initcall_command_line. In other words, they are the same size. That 
will be important in a bit.

strlen(command_line) + 1 bytes are allocated for static_command_line.

boot_coomand_line is copied to saved_command_line, which we just allocated 
enough space for. No problem here...

command_line is copied to static_command_line. Again, we just allocated 
enough space for this. No problem here either...

Ok, back to saved_command_line, initcall_command_line and the first 
strcpy() you mentioned:

>    strcpy(initcall_command_line, saved_command_line);

Since saved_command_line and initcall_command_line are the same size, a 
string that fits in saved_command_line will also fit in 
initcall_command_line, so no problem there either. (Unless one of them are 
reallocated at another point in the code. I haven't checked that.)

> }
>

TL;DR: No problems as far as I can tell...

-- 
Povl Ole

[toc] | [prev] | [standalone]


Back to top | Article view | linux.debian.security


csiph-web