tree: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master
head: 88035e5694a86a7167d490bb95e9df97a9bb162b
commit: d8792a5734b0f3e58b898c2e2f910bfac48e9ee3 riscv: Safely remove entries from relocation list
config: riscv-randconfig-r071-20231211 (https://download.01.org/0day-ci/archive/20231213/[email protected]/config)
compiler: riscv64-linux-gcc (GCC) 13.2.0
reproduce: (https://download.01.org/0day-ci/archive/20231213/[email protected]/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <[email protected]>
| Reported-by: Dan Carpenter <[email protected]>
| Closes: https://lore.kernel.org/r/[email protected]/
New smatch warnings:
arch/riscv/kernel/module.c:727 add_relocation_to_accumulate() error: dereferencing freed memory 'rel_head'
arch/riscv/kernel/module.c:792 apply_relocate_add() warn: unsigned 'hashtable_bits' is never less than zero.
vim +/rel_head +727 arch/riscv/kernel/module.c
8fd6c5142395a1 Charlie Jenkins 2023-11-01 717 INIT_LIST_HEAD(rel_head->rel_entry);
8fd6c5142395a1 Charlie Jenkins 2023-11-01 718 rel_head->location = location;
8fd6c5142395a1 Charlie Jenkins 2023-11-01 719 INIT_HLIST_NODE(&rel_head->node);
8fd6c5142395a1 Charlie Jenkins 2023-11-01 720 if (!current_head->first) {
8fd6c5142395a1 Charlie Jenkins 2023-11-01 721 bucket =
8fd6c5142395a1 Charlie Jenkins 2023-11-01 722 kmalloc(sizeof(struct used_bucket), GFP_KERNEL);
d8792a5734b0f3 Charlie Jenkins 2023-11-27 723
d8792a5734b0f3 Charlie Jenkins 2023-11-27 724 if (!bucket) {
d8792a5734b0f3 Charlie Jenkins 2023-11-27 725 kfree(entry);
d8792a5734b0f3 Charlie Jenkins 2023-11-27 726 kfree(rel_head);
d8792a5734b0f3 Charlie Jenkins 2023-11-27 @727 kfree(rel_head->rel_entry);
Swap these two frees to avoid a use after free.
d8792a5734b0f3 Charlie Jenkins 2023-11-27 728 return -ENOMEM;
d8792a5734b0f3 Charlie Jenkins 2023-11-27 729 }
d8792a5734b0f3 Charlie Jenkins 2023-11-27 730
8fd6c5142395a1 Charlie Jenkins 2023-11-01 731 INIT_LIST_HEAD(&bucket->head);
8fd6c5142395a1 Charlie Jenkins 2023-11-01 732 bucket->bucket = current_head;
d8792a5734b0f3 Charlie Jenkins 2023-11-27 733 list_add(&bucket->head, used_buckets_list);
8fd6c5142395a1 Charlie Jenkins 2023-11-01 734 }
8fd6c5142395a1 Charlie Jenkins 2023-11-01 735 hlist_add_head(&rel_head->node, current_head);
8fd6c5142395a1 Charlie Jenkins 2023-11-01 736 }
[ snip ]
e2c0cdfba7f699 Palmer Dabbelt 2017-07-10 773 int apply_relocate_add(Elf_Shdr *sechdrs, const char *strtab,
e2c0cdfba7f699 Palmer Dabbelt 2017-07-10 774 unsigned int symindex, unsigned int relsec,
e2c0cdfba7f699 Palmer Dabbelt 2017-07-10 775 struct module *me)
e2c0cdfba7f699 Palmer Dabbelt 2017-07-10 776 {
e2c0cdfba7f699 Palmer Dabbelt 2017-07-10 777 Elf_Rela *rel = (void *) sechdrs[relsec].sh_addr;
8cbe0accc4a6ba Emil Renner Berthing 2023-11-01 778 int (*handler)(struct module *me, void *location, Elf_Addr v);
e2c0cdfba7f699 Palmer Dabbelt 2017-07-10 779 Elf_Sym *sym;
8cbe0accc4a6ba Emil Renner Berthing 2023-11-01 780 void *location;
e2c0cdfba7f699 Palmer Dabbelt 2017-07-10 781 unsigned int i, type;
e2c0cdfba7f699 Palmer Dabbelt 2017-07-10 782 Elf_Addr v;
e2c0cdfba7f699 Palmer Dabbelt 2017-07-10 783 int res;
8fd6c5142395a1 Charlie Jenkins 2023-11-01 784 unsigned int num_relocations = sechdrs[relsec].sh_size / sizeof(*rel);
d8792a5734b0f3 Charlie Jenkins 2023-11-27 785 struct hlist_head *relocation_hashtable;
d8792a5734b0f3 Charlie Jenkins 2023-11-27 786 struct list_head used_buckets_list;
d8792a5734b0f3 Charlie Jenkins 2023-11-27 787 unsigned int hashtable_bits;
^^^^^^^^
d8792a5734b0f3 Charlie Jenkins 2023-11-27 788
d8792a5734b0f3 Charlie Jenkins 2023-11-27 789 hashtable_bits = initialize_relocation_hashtable(num_relocations,
d8792a5734b0f3 Charlie Jenkins 2023-11-27 790 &relocation_hashtable);
d8792a5734b0f3 Charlie Jenkins 2023-11-27 791
d8792a5734b0f3 Charlie Jenkins 2023-11-27 @792 if (hashtable_bits < 0)
^^^^^^^^^^^^^^^^^^
Can't be less than zero.
d8792a5734b0f3 Charlie Jenkins 2023-11-27 793 return hashtable_bits;
d8792a5734b0f3 Charlie Jenkins 2023-11-27 794
d8792a5734b0f3 Charlie Jenkins 2023-11-27 795 INIT_LIST_HEAD(&used_buckets_list);
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
On Wed, Dec 13, 2023 at 04:19:26PM +0300, Dan Carpenter wrote:
> tree: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master
> head: 88035e5694a86a7167d490bb95e9df97a9bb162b
> commit: d8792a5734b0f3e58b898c2e2f910bfac48e9ee3 riscv: Safely remove entries from relocation list
> config: riscv-randconfig-r071-20231211 (https://download.01.org/0day-ci/archive/20231213/[email protected]/config)
> compiler: riscv64-linux-gcc (GCC) 13.2.0
> reproduce: (https://download.01.org/0day-ci/archive/20231213/[email protected]/reproduce)
>
> If you fix the issue in a separate patch/commit (i.e. not just a new version of
> the same patch/commit), kindly add following tags
> | Reported-by: kernel test robot <[email protected]>
> | Reported-by: Dan Carpenter <[email protected]>
> | Closes: https://lore.kernel.org/r/[email protected]/
>
> New smatch warnings:
> arch/riscv/kernel/module.c:727 add_relocation_to_accumulate() error: dereferencing freed memory 'rel_head'
> arch/riscv/kernel/module.c:792 apply_relocate_add() warn: unsigned 'hashtable_bits' is never less than zero.
>
> vim +/rel_head +727 arch/riscv/kernel/module.c
>
> 8fd6c5142395a1 Charlie Jenkins 2023-11-01 717 INIT_LIST_HEAD(rel_head->rel_entry);
> 8fd6c5142395a1 Charlie Jenkins 2023-11-01 718 rel_head->location = location;
> 8fd6c5142395a1 Charlie Jenkins 2023-11-01 719 INIT_HLIST_NODE(&rel_head->node);
> 8fd6c5142395a1 Charlie Jenkins 2023-11-01 720 if (!current_head->first) {
> 8fd6c5142395a1 Charlie Jenkins 2023-11-01 721 bucket =
> 8fd6c5142395a1 Charlie Jenkins 2023-11-01 722 kmalloc(sizeof(struct used_bucket), GFP_KERNEL);
> d8792a5734b0f3 Charlie Jenkins 2023-11-27 723
> d8792a5734b0f3 Charlie Jenkins 2023-11-27 724 if (!bucket) {
> d8792a5734b0f3 Charlie Jenkins 2023-11-27 725 kfree(entry);
> d8792a5734b0f3 Charlie Jenkins 2023-11-27 726 kfree(rel_head);
> d8792a5734b0f3 Charlie Jenkins 2023-11-27 @727 kfree(rel_head->rel_entry);
>
> Swap these two frees to avoid a use after free.
Thanks for pointing this out.
>
> d8792a5734b0f3 Charlie Jenkins 2023-11-27 728 return -ENOMEM;
> d8792a5734b0f3 Charlie Jenkins 2023-11-27 729 }
> d8792a5734b0f3 Charlie Jenkins 2023-11-27 730
> 8fd6c5142395a1 Charlie Jenkins 2023-11-01 731 INIT_LIST_HEAD(&bucket->head);
> 8fd6c5142395a1 Charlie Jenkins 2023-11-01 732 bucket->bucket = current_head;
> d8792a5734b0f3 Charlie Jenkins 2023-11-27 733 list_add(&bucket->head, used_buckets_list);
> 8fd6c5142395a1 Charlie Jenkins 2023-11-01 734 }
> 8fd6c5142395a1 Charlie Jenkins 2023-11-01 735 hlist_add_head(&rel_head->node, current_head);
> 8fd6c5142395a1 Charlie Jenkins 2023-11-01 736 }
>
> [ snip ]
>
> e2c0cdfba7f699 Palmer Dabbelt 2017-07-10 773 int apply_relocate_add(Elf_Shdr *sechdrs, const char *strtab,
> e2c0cdfba7f699 Palmer Dabbelt 2017-07-10 774 unsigned int symindex, unsigned int relsec,
> e2c0cdfba7f699 Palmer Dabbelt 2017-07-10 775 struct module *me)
> e2c0cdfba7f699 Palmer Dabbelt 2017-07-10 776 {
> e2c0cdfba7f699 Palmer Dabbelt 2017-07-10 777 Elf_Rela *rel = (void *) sechdrs[relsec].sh_addr;
> 8cbe0accc4a6ba Emil Renner Berthing 2023-11-01 778 int (*handler)(struct module *me, void *location, Elf_Addr v);
> e2c0cdfba7f699 Palmer Dabbelt 2017-07-10 779 Elf_Sym *sym;
> 8cbe0accc4a6ba Emil Renner Berthing 2023-11-01 780 void *location;
> e2c0cdfba7f699 Palmer Dabbelt 2017-07-10 781 unsigned int i, type;
> e2c0cdfba7f699 Palmer Dabbelt 2017-07-10 782 Elf_Addr v;
> e2c0cdfba7f699 Palmer Dabbelt 2017-07-10 783 int res;
> 8fd6c5142395a1 Charlie Jenkins 2023-11-01 784 unsigned int num_relocations = sechdrs[relsec].sh_size / sizeof(*rel);
> d8792a5734b0f3 Charlie Jenkins 2023-11-27 785 struct hlist_head *relocation_hashtable;
> d8792a5734b0f3 Charlie Jenkins 2023-11-27 786 struct list_head used_buckets_list;
> d8792a5734b0f3 Charlie Jenkins 2023-11-27 787 unsigned int hashtable_bits;
> ^^^^^^^^
>
> d8792a5734b0f3 Charlie Jenkins 2023-11-27 788
> d8792a5734b0f3 Charlie Jenkins 2023-11-27 789 hashtable_bits = initialize_relocation_hashtable(num_relocations,
> d8792a5734b0f3 Charlie Jenkins 2023-11-27 790 &relocation_hashtable);
> d8792a5734b0f3 Charlie Jenkins 2023-11-27 791
> d8792a5734b0f3 Charlie Jenkins 2023-11-27 @792 if (hashtable_bits < 0)
> ^^^^^^^^^^^^^^^^^^
> Can't be less than zero.
I am returning a negative number in an unsigned function, oops. Since
the only possible failure in the function is an out-of-memory error and
0 is not a valid return, I will return 0 in the out-of-memory case.
- Charlie
>
> d8792a5734b0f3 Charlie Jenkins 2023-11-27 793 return hashtable_bits;
> d8792a5734b0f3 Charlie Jenkins 2023-11-27 794
> d8792a5734b0f3 Charlie Jenkins 2023-11-27 795 INIT_LIST_HEAD(&used_buckets_list);
>
> --
> 0-DAY CI Kernel Test Service
> https://github.com/intel/lkp-tests/wiki
>