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
