Hello Suren Baghdasaryan,
Commit e88dfe467aa7 ("alloc_tag: load module tags into separate
contiguous memory") from Oct 23, 2024 (linux-next), leads to the
following Smatch static checker warning:
kernel/module/main.c:2594 move_module()
warn: 'dest' isn't an ERR_PTR
kernel/module/main.c
2554 static int move_module(struct module *mod, struct load_info *info)
2555 {
2556 int i;
2557 enum mod_mem_type t = 0;
2558 int ret = -ENOMEM;
2559 bool codetag_section_found = false;
2560
2561 for_each_mod_mem_type(type) {
2562 if (!mod->mem[type].size) {
2563 mod->mem[type].base = NULL;
2564 mod->mem[type].rw_copy = NULL;
2565 continue;
2566 }
2567
2568 ret = module_memory_alloc(mod, type);
2569 if (ret) {
2570 t = type;
2571 goto out_err;
2572 }
2573 }
2574
2575 /* Transfer each section which specifies SHF_ALLOC */
2576 pr_debug("Final section addresses for %s:\n", mod->name);
2577 for (i = 0; i < info->hdr->e_shnum; i++) {
2578 void *dest;
2579 Elf_Shdr *shdr = &info->sechdrs[i];
2580 const char *sname;
2581 unsigned long addr;
2582
2583 if (!(shdr->sh_flags & SHF_ALLOC))
2584 continue;
2585
2586 sname = info->secstrings + shdr->sh_name;
2587 /*
2588 * Load codetag sections separately as they might
still be used
2589 * after module unload.
2590 */
2591 if (codetag_needs_module_section(mod, sname,
shdr->sh_size)) {
2592 dest = codetag_alloc_module_section(mod,
sname, shdr->sh_size,
2593 arch_mod_section_prepend(mod,
i), shdr->sh_addralign);
--> 2594 if (IS_ERR(dest)) {
2595 ret = PTR_ERR(dest);
2596 goto out_err;
2597 }
This is a false positive caused my specific kernel config. The
codetag_alloc_module_section() function returns both error pointers and NULL.
It can return NULL because of the .config, because the section isn't found,
because the size is too small or because the desc.alloc_section_mem() function
isn't implemented (which is a bug).
2598 addr = (unsigned long)dest;
2599 codetag_section_found = true;
2600 } else {
2601 enum mod_mem_type type = shdr->sh_entsize >>
SH_ENTSIZE_TYPE_SHIFT;
2602 unsigned long offset = shdr->sh_entsize &
SH_ENTSIZE_OFFSET_MASK;
2603
2604 addr = (unsigned long)mod->mem[type].base +
offset;
2605 dest = mod->mem[type].rw_copy + offset;
2606 }
2607
2608 if (shdr->sh_type != SHT_NOBITS) {
2609 /*
2610 * Our ELF checker already validated this, but
let's
2611 * be pedantic and make the goal clearer. We
actually
2612 * end up copying over all modifications made
to the
2613 * userspace copy of the entire struct module.
2614 */
2615 if (i == info->index.mod &&
2616 (WARN_ON_ONCE(shdr->sh_size !=
sizeof(struct module)))) {
This handles the case where the size is too small. Why not return an error
pointer if the section is not found or if there is a bug. We could also add a
NULL check here.
2617 ret = -ENOEXEC;
2618 goto out_err;
2619 }
2620 memcpy(dest, (void *)shdr->sh_addr,
shdr->sh_size);
^^^^^
Dereferenced without testing.
2621 }
2622 /*
2623 * Update the userspace copy's ELF section address to
point to
2624 * our newly allocated memory as a pure convenience so
that
2625 * users of info can keep taking advantage and using
the newly
2626 * minted official memory area.
2627 */
2628 shdr->sh_addr = addr;
2629 pr_debug("\t0x%lx 0x%.8lx %s\n", (long)shdr->sh_addr,
2630 (long)shdr->sh_size, info->secstrings +
shdr->sh_name);
2631 }
2632
2633 return 0;
2634 out_err:
2635 for (t--; t >= 0; t--)
2636 module_memory_free(mod, t);
2637 if (codetag_section_found)
2638 codetag_free_module_sections(mod);
2639
2640 return ret;
2641 }
regards,
dan carpenter
