arch/riscv/kernel/module.c:639 process_accumulated_relocations() error: uninitialized symbol 'curr_type'.

Dan Carpenter
2023-12-13T13:05:25Z

tree: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master
head: cf52eed70e555e864120cfaf280e979e2a035c66
commit: 8fd6c5142395a106b63c8668e9f4a7106b6a0772 riscv: Add remaining module relocations
config: riscv-randconfig-r071-20231211 (https://download.01.org/0day-ci/archive/20231213/202312130859.wnkuzVWY-lkp@intel.com/config)
compiler: riscv64-linux-gcc (GCC) 13.2.0
reproduce: (https://download.01.org/0day-ci/archive/20231213/202312130859.wnkuzVWY-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@intel.com>
| Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
| Closes: https://lore.kernel.org/r/202312130859.wnkuzVWY-lkp@intel.com/

New smatch warnings:
arch/riscv/kernel/module.c:639 process_accumulated_relocations() error: uninitialized symbol 'curr_type'.

Old smatch warnings:
arch/riscv/kernel/module.c:632 process_accumulated_relocations() error: dereferencing freed memory 'rel_entry_iter'
arch/riscv/kernel/module.c:629 process_accumulated_relocations() error: dereferencing freed memory 'rel_head_iter'
arch/riscv/kernel/module.c:628 process_accumulated_relocations() error: dereferencing freed memory 'bucket_iter'

vim +/curr_type +639 arch/riscv/kernel/module.c

8fd6c5142395a1 Charlie Jenkins 2023-11-01 602 void process_accumulated_relocations(struct module *me)
8fd6c5142395a1 Charlie Jenkins 2023-11-01 603 {
8fd6c5142395a1 Charlie Jenkins 2023-11-01 604 /*
8fd6c5142395a1 Charlie Jenkins 2023-11-01 605 * Only ADD/SUB/SET/ULEB128 should end up here.
8fd6c5142395a1 Charlie Jenkins 2023-11-01 606 *
8fd6c5142395a1 Charlie Jenkins 2023-11-01 607 * Each bucket may have more than one relocation location. All
8fd6c5142395a1 Charlie Jenkins 2023-11-01 608 * relocations for a location are stored in a list in a bucket.
8fd6c5142395a1 Charlie Jenkins 2023-11-01 609 *
8fd6c5142395a1 Charlie Jenkins 2023-11-01 610 * Relocations are applied to a temp variable before being stored to the
8fd6c5142395a1 Charlie Jenkins 2023-11-01 611 * provided location to check for overflow. This also allows ULEB128 to
8fd6c5142395a1 Charlie Jenkins 2023-11-01 612 * properly decide how many entries are needed before storing to
8fd6c5142395a1 Charlie Jenkins 2023-11-01 613 * location. The final value is stored into location using the handler
8fd6c5142395a1 Charlie Jenkins 2023-11-01 614 * for the last relocation to an address.
8fd6c5142395a1 Charlie Jenkins 2023-11-01 615 *
8fd6c5142395a1 Charlie Jenkins 2023-11-01 616 * Three layers of indexing:
8fd6c5142395a1 Charlie Jenkins 2023-11-01 617 * - Each of the buckets in use
8fd6c5142395a1 Charlie Jenkins 2023-11-01 618 * - Groups of relocations in each bucket by location address
8fd6c5142395a1 Charlie Jenkins 2023-11-01 619 * - Each relocation entry for a location address
8fd6c5142395a1 Charlie Jenkins 2023-11-01 620 */
8fd6c5142395a1 Charlie Jenkins 2023-11-01 621 struct used_bucket *bucket_iter;
8fd6c5142395a1 Charlie Jenkins 2023-11-01 622 struct relocation_head *rel_head_iter;
8fd6c5142395a1 Charlie Jenkins 2023-11-01 623 struct relocation_entry *rel_entry_iter;
8fd6c5142395a1 Charlie Jenkins 2023-11-01 624 int curr_type;
8fd6c5142395a1 Charlie Jenkins 2023-11-01 625 void *location;
8fd6c5142395a1 Charlie Jenkins 2023-11-01 626 long buffer;
8fd6c5142395a1 Charlie Jenkins 2023-11-01 627
8fd6c5142395a1 Charlie Jenkins 2023-11-01 628 list_for_each_entry(bucket_iter, &used_buckets_list, head) {
8fd6c5142395a1 Charlie Jenkins 2023-11-01 629 hlist_for_each_entry(rel_head_iter, bucket_iter->bucket, node) {
8fd6c5142395a1 Charlie Jenkins 2023-11-01 630 buffer = 0;
8fd6c5142395a1 Charlie Jenkins 2023-11-01 631 location = rel_head_iter->location;
8fd6c5142395a1 Charlie Jenkins 2023-11-01 632 list_for_each_entry(rel_entry_iter,
8fd6c5142395a1 Charlie Jenkins 2023-11-01 633 rel_head_iter->rel_entry, head) {
8fd6c5142395a1 Charlie Jenkins 2023-11-01 634 curr_type = rel_entry_iter->type;
8fd6c5142395a1 Charlie Jenkins 2023-11-01 635 reloc_handlers[curr_type].reloc_handler(
8fd6c5142395a1 Charlie Jenkins 2023-11-01 636 me, &buffer, rel_entry_iter->value);
8fd6c5142395a1 Charlie Jenkins 2023-11-01 637 kfree(rel_entry_iter);

This kfree() will lead to a NULL dereference on the next iteration
through the loop. You need to use list_for_each_entry_safe().

8fd6c5142395a1 Charlie Jenkins 2023-11-01 638 }
8fd6c5142395a1 Charlie Jenkins 2023-11-01 @639 reloc_handlers[curr_type].accumulate_handler(
^^^^^^^^^
Can the list be empty? Uninitialized in that case.

8fd6c5142395a1 Charlie Jenkins 2023-11-01 640 me, location, buffer);
8fd6c5142395a1 Charlie Jenkins 2023-11-01 641 kfree(rel_head_iter);
8fd6c5142395a1 Charlie Jenkins 2023-11-01 642 }
8fd6c5142395a1 Charlie Jenkins 2023-11-01 643 kfree(bucket_iter);
8fd6c5142395a1 Charlie Jenkins 2023-11-01 644 }
8fd6c5142395a1 Charlie Jenkins 2023-11-01 645
8fd6c5142395a1 Charlie Jenkins 2023-11-01 646 kfree(relocation_hashtable);
8fd6c5142395a1 Charlie Jenkins 2023-11-01 647 }

--
0-DAY CI Kernel Test Service