| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Wed, 13 Dec 2023 11:29:29 -0800
From: Charlie Jenkins <charlie@...osinc.com>
To: Dan Carpenter <dan.carpenter@...aro.org>
Cc: oe-kbuild@...ts.linux.dev, lkp@...el.com,
oe-kbuild-all@...ts.linux.dev, linux-kernel@...r.kernel.org,
Palmer Dabbelt <palmer@...osinc.com>
Subject: Re: arch/riscv/kernel/module.c:727 add_relocation_to_accumulate()
error: dereferencing freed memory 'rel_head'
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/202312132019.iYGTwW0L-lkp@intel.com/config)
> compiler: riscv64-linux-gcc (GCC) 13.2.0
> reproduce: (https://download.01.org/0day-ci/archive/20231213/202312132019.iYGTwW0L-lkp@intel.com/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 <lkp@...el.com>
> | Reported-by: Dan Carpenter <dan.carpenter@...aro.org>
> | Closes: https://lore.kernel.org/r/202312132019.iYGTwW0L-lkp@intel.com/
>
> 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
>
Powered by blists - more mailing lists