* arch/riscv/kernel/module.c:639 process_accumulated_relocations() error: uninitialized symbol 'curr_type'.
@ 2023-12-13 13:05 Dan Carpenter
2023-12-13 19:27 ` Charlie Jenkins
0 siblings, 1 reply; 10+ messages in thread
From: Dan Carpenter @ 2023-12-13 13:05 UTC (permalink / raw)
To: oe-kbuild, Charlie Jenkins
Cc: lkp, oe-kbuild-all, linux-kernel, Palmer Dabbelt
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
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: arch/riscv/kernel/module.c:639 process_accumulated_relocations() error: uninitialized symbol 'curr_type'.
2023-12-13 13:05 arch/riscv/kernel/module.c:639 process_accumulated_relocations() error: uninitialized symbol 'curr_type' Dan Carpenter
@ 2023-12-13 19:27 ` Charlie Jenkins
2023-12-14 8:00 ` Dan Carpenter
0 siblings, 1 reply; 10+ messages in thread
From: Charlie Jenkins @ 2023-12-13 19:27 UTC (permalink / raw)
To: Dan Carpenter; +Cc: oe-kbuild, lkp, oe-kbuild-all, linux-kernel, Palmer Dabbelt
On Wed, Dec 13, 2023 at 04:05:06PM +0300, Dan Carpenter wrote:
> 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().
>
This has been fixed in 6.7-rc5.
> 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.
That's a tricky one, the list cannot be empty. Each bucket in the
bucket_iter is guarunteed to have at least one rel_entry. I can probably
resolve this by extracting this for loop into a do-while loop.
- Charlie
>
> 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
> https://github.com/intel/lkp-tests/wiki
>
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: arch/riscv/kernel/module.c:639 process_accumulated_relocations() error: uninitialized symbol 'curr_type'.
2023-12-13 19:27 ` Charlie Jenkins
@ 2023-12-14 8:00 ` Dan Carpenter
2023-12-14 19:26 ` Charlie Jenkins
2023-12-28 0:59 ` Charlie Jenkins
0 siblings, 2 replies; 10+ messages in thread
From: Dan Carpenter @ 2023-12-14 8:00 UTC (permalink / raw)
To: Charlie Jenkins
Cc: oe-kbuild, lkp, oe-kbuild-all, linux-kernel, Palmer Dabbelt
On Wed, Dec 13, 2023 at 11:27:02AM -0800, Charlie Jenkins wrote:
> > 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.
>
> That's a tricky one, the list cannot be empty. Each bucket in the
> bucket_iter is guarunteed to have at least one rel_entry. I can probably
> resolve this by extracting this for loop into a do-while loop.
You can just ignore false positives. It's not really a fix to change it
to a do-while loop. I reviewed the do while code before reading this
email and I still wondered about empty lists, but just to hear that it's
not going to be empty is enough. Just the email was sufficient.
regards,
dan carpenter
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: arch/riscv/kernel/module.c:639 process_accumulated_relocations() error: uninitialized symbol 'curr_type'.
2023-12-14 8:00 ` Dan Carpenter
@ 2023-12-14 19:26 ` Charlie Jenkins
2023-12-28 0:59 ` Charlie Jenkins
1 sibling, 0 replies; 10+ messages in thread
From: Charlie Jenkins @ 2023-12-14 19:26 UTC (permalink / raw)
To: Dan Carpenter; +Cc: oe-kbuild, lkp, oe-kbuild-all, linux-kernel, Palmer Dabbelt
On Thu, Dec 14, 2023 at 11:00:46AM +0300, Dan Carpenter wrote:
> On Wed, Dec 13, 2023 at 11:27:02AM -0800, Charlie Jenkins wrote:
> > > 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.
> >
> > That's a tricky one, the list cannot be empty. Each bucket in the
> > bucket_iter is guarunteed to have at least one rel_entry. I can probably
> > resolve this by extracting this for loop into a do-while loop.
>
> You can just ignore false positives. It's not really a fix to change it
> to a do-while loop. I reviewed the do while code before reading this
> email and I still wondered about empty lists, but just to hear that it's
> not going to be empty is enough. Just the email was sufficient.
>
> regards,
> dan carpenter
>
The freeing was actually broken so that needed to be fixed and I figured
that it was worthwhile to include the do-while in the same patch to get
rid of the warning.
- Charlie
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: arch/riscv/kernel/module.c:639 process_accumulated_relocations() error: uninitialized symbol 'curr_type'.
2023-12-14 8:00 ` Dan Carpenter
2023-12-14 19:26 ` Charlie Jenkins
@ 2023-12-28 0:59 ` Charlie Jenkins
2024-01-02 12:37 ` Dan Carpenter
1 sibling, 1 reply; 10+ messages in thread
From: Charlie Jenkins @ 2023-12-28 0:59 UTC (permalink / raw)
To: Dan Carpenter; +Cc: oe-kbuild, lkp, oe-kbuild-all, linux-kernel, Palmer Dabbelt
On Thu, Dec 14, 2023 at 11:00:46AM +0300, Dan Carpenter wrote:
> On Wed, Dec 13, 2023 at 11:27:02AM -0800, Charlie Jenkins wrote:
> > > 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.
> >
> > That's a tricky one, the list cannot be empty. Each bucket in the
> > bucket_iter is guarunteed to have at least one rel_entry. I can probably
> > resolve this by extracting this for loop into a do-while loop.
>
> You can just ignore false positives. It's not really a fix to change it
> to a do-while loop. I reviewed the do while code before reading this
> email and I still wondered about empty lists, but just to hear that it's
> not going to be empty is enough. Just the email was sufficient.
>
> regards,
> dan carpenter
>
The fix isn't the do-while loop but rather the use after free, the
incorrect sizeof, and incorrect error handling when
initialize_relocation_hashtable fails. I decided to include the do-while
code because I was already touching the surrounding code. Can you review
[1]? If you would prefer that the do-while is reverted, I can do that,
but it is important that the rest of the fixes are merged before 6.7 is
released.
[1] https://lore.kernel.org/all/20231213-module_loading_fix-v1-1-da9b7c92ade5@rivosinc.com/
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: arch/riscv/kernel/module.c:639 process_accumulated_relocations() error: uninitialized symbol 'curr_type'.
2023-12-28 0:59 ` Charlie Jenkins
@ 2024-01-02 12:37 ` Dan Carpenter
2024-01-03 20:27 ` Charlie Jenkins
0 siblings, 1 reply; 10+ messages in thread
From: Dan Carpenter @ 2024-01-02 12:37 UTC (permalink / raw)
To: Charlie Jenkins
Cc: oe-kbuild, lkp, oe-kbuild-all, linux-kernel, Palmer Dabbelt
On Wed, Dec 27, 2023 at 04:59:25PM -0800, Charlie Jenkins wrote:
> On Thu, Dec 14, 2023 at 11:00:46AM +0300, Dan Carpenter wrote:
> > On Wed, Dec 13, 2023 at 11:27:02AM -0800, Charlie Jenkins wrote:
> > > > 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.
> > >
> > > That's a tricky one, the list cannot be empty. Each bucket in the
> > > bucket_iter is guarunteed to have at least one rel_entry. I can probably
> > > resolve this by extracting this for loop into a do-while loop.
> >
> > You can just ignore false positives. It's not really a fix to change it
> > to a do-while loop. I reviewed the do while code before reading this
> > email and I still wondered about empty lists, but just to hear that it's
> > not going to be empty is enough. Just the email was sufficient.
> >
> > regards,
> > dan carpenter
> >
>
> The fix isn't the do-while loop but rather the use after free, the
> incorrect sizeof, and incorrect error handling when
> initialize_relocation_hashtable fails. I decided to include the do-while
> code because I was already touching the surrounding code. Can you review
> [1]? If you would prefer that the do-while is reverted, I can do that,
> but it is important that the rest of the fixes are merged before 6.7 is
> released.
>
> [1] https://lore.kernel.org/all/20231213-module_loading_fix-v1-1-da9b7c92ade5@rivosinc.com/
Most subsystems wouldn't merge a patch like this which does so many
things at the same time...
regards,
dan carpenter
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: arch/riscv/kernel/module.c:639 process_accumulated_relocations() error: uninitialized symbol 'curr_type'.
2024-01-02 12:37 ` Dan Carpenter
@ 2024-01-03 20:27 ` Charlie Jenkins
0 siblings, 0 replies; 10+ messages in thread
From: Charlie Jenkins @ 2024-01-03 20:27 UTC (permalink / raw)
To: Dan Carpenter; +Cc: oe-kbuild, lkp, oe-kbuild-all, linux-kernel, Palmer Dabbelt
On Tue, Jan 02, 2024 at 03:37:34PM +0300, Dan Carpenter wrote:
> On Wed, Dec 27, 2023 at 04:59:25PM -0800, Charlie Jenkins wrote:
> > On Thu, Dec 14, 2023 at 11:00:46AM +0300, Dan Carpenter wrote:
> > > On Wed, Dec 13, 2023 at 11:27:02AM -0800, Charlie Jenkins wrote:
> > > > > 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.
> > > >
> > > > That's a tricky one, the list cannot be empty. Each bucket in the
> > > > bucket_iter is guarunteed to have at least one rel_entry. I can probably
> > > > resolve this by extracting this for loop into a do-while loop.
> > >
> > > You can just ignore false positives. It's not really a fix to change it
> > > to a do-while loop. I reviewed the do while code before reading this
> > > email and I still wondered about empty lists, but just to hear that it's
> > > not going to be empty is enough. Just the email was sufficient.
> > >
> > > regards,
> > > dan carpenter
> > >
> >
> > The fix isn't the do-while loop but rather the use after free, the
> > incorrect sizeof, and incorrect error handling when
> > initialize_relocation_hashtable fails. I decided to include the do-while
> > code because I was already touching the surrounding code. Can you review
> > [1]? If you would prefer that the do-while is reverted, I can do that,
> > but it is important that the rest of the fixes are merged before 6.7 is
> > released.
> >
> > [1] https://lore.kernel.org/all/20231213-module_loading_fix-v1-1-da9b7c92ade5@rivosinc.com/
>
>
> Most subsystems wouldn't merge a patch like this which does so many
> things at the same time...
That is a good point, I split it across 4 different patches.
https://lore.kernel.org/linux-riscv/20240103-module_loading_fix-v2-0-292b160552c9@rivosinc.com/
>
> regards,
> dan carpenter
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* arch/riscv/kernel/module.c:639 process_accumulated_relocations() error: uninitialized symbol 'curr_type'.
@ 2023-12-13 9:54 kernel test robot
0 siblings, 0 replies; 10+ messages in thread
From: kernel test robot @ 2023-12-13 9:54 UTC (permalink / raw)
To: oe-kbuild; +Cc: lkp, Dan Carpenter
BCC: lkp@intel.com
CC: oe-kbuild-all@lists.linux.dev
CC: linux-kernel@vger.kernel.org
TO: Charlie Jenkins <charlie@rivosinc.com>
CC: Palmer Dabbelt <palmer@rivosinc.com>
tree: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master
head: 88035e5694a86a7167d490bb95e9df97a9bb162b
commit: 8fd6c5142395a106b63c8668e9f4a7106b6a0772 riscv: Add remaining module relocations
date: 5 weeks ago
:::::: branch date: 9 hours ago
:::::: commit date: 5 weeks ago
config: riscv-randconfig-r071-20231211 (https://download.01.org/0day-ci/archive/20231213/202312131734.tvtEi4Aq-lkp@intel.com/config)
compiler: riscv64-linux-gcc (GCC) 13.2.0
reproduce: (https://download.01.org/0day-ci/archive/20231213/202312131734.tvtEi4Aq-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 <error27@gmail.com>
| Closes: https://lore.kernel.org/r/202312131734.tvtEi4Aq-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
e2c0cdfba7f699 Palmer Dabbelt 2017-07-10 601
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);
8fd6c5142395a1 Charlie Jenkins 2023-11-01 638 }
8fd6c5142395a1 Charlie Jenkins 2023-11-01 @639 reloc_handlers[curr_type].accumulate_handler(
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 }
8fd6c5142395a1 Charlie Jenkins 2023-11-01 648
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 10+ messages in thread* arch/riscv/kernel/module.c:639 process_accumulated_relocations() error: uninitialized symbol 'curr_type'.
@ 2023-12-13 0:27 kernel test robot
0 siblings, 0 replies; 10+ messages in thread
From: kernel test robot @ 2023-12-13 0:27 UTC (permalink / raw)
To: oe-kbuild; +Cc: lkp, Dan Carpenter
BCC: lkp@intel.com
CC: oe-kbuild-all@lists.linux.dev
CC: linux-kernel@vger.kernel.org
TO: Charlie Jenkins <charlie@rivosinc.com>
CC: Palmer Dabbelt <palmer@rivosinc.com>
tree: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master
head: cf52eed70e555e864120cfaf280e979e2a035c66
commit: 8fd6c5142395a106b63c8668e9f4a7106b6a0772 riscv: Add remaining module relocations
date: 5 weeks ago
:::::: branch date: 5 hours ago
:::::: commit date: 5 weeks ago
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 <error27@gmail.com>
| 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
e2c0cdfba7f699 Palmer Dabbelt 2017-07-10 601
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);
8fd6c5142395a1 Charlie Jenkins 2023-11-01 638 }
8fd6c5142395a1 Charlie Jenkins 2023-11-01 @639 reloc_handlers[curr_type].accumulate_handler(
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 }
8fd6c5142395a1 Charlie Jenkins 2023-11-01 648
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 10+ messages in thread* arch/riscv/kernel/module.c:639 process_accumulated_relocations() error: uninitialized symbol 'curr_type'.
@ 2023-12-11 22:03 kernel test robot
0 siblings, 0 replies; 10+ messages in thread
From: kernel test robot @ 2023-12-11 22:03 UTC (permalink / raw)
To: oe-kbuild; +Cc: lkp, Dan Carpenter
BCC: lkp@intel.com
CC: oe-kbuild-all@lists.linux.dev
CC: linux-kernel@vger.kernel.org
TO: Charlie Jenkins <charlie@rivosinc.com>
CC: Palmer Dabbelt <palmer@rivosinc.com>
tree: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master
head: a39b6ac3781d46ba18193c9dbb2110f31e9bffe9
commit: 8fd6c5142395a106b63c8668e9f4a7106b6a0772 riscv: Add remaining module relocations
date: 5 weeks ago
:::::: branch date: 23 hours ago
:::::: commit date: 5 weeks ago
config: riscv-randconfig-r071-20231211 (https://download.01.org/0day-ci/archive/20231212/202312120522.AqRyfHzJ-lkp@intel.com/config)
compiler: riscv64-linux-gcc (GCC) 13.2.0
reproduce: (https://download.01.org/0day-ci/archive/20231212/202312120522.AqRyfHzJ-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 <error27@gmail.com>
| Closes: https://lore.kernel.org/r/202312120522.AqRyfHzJ-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
e2c0cdfba7f699 Palmer Dabbelt 2017-07-10 601
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);
8fd6c5142395a1 Charlie Jenkins 2023-11-01 638 }
8fd6c5142395a1 Charlie Jenkins 2023-11-01 @639 reloc_handlers[curr_type].accumulate_handler(
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 }
8fd6c5142395a1 Charlie Jenkins 2023-11-01 648
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2024-01-03 20:27 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-12-13 13:05 arch/riscv/kernel/module.c:639 process_accumulated_relocations() error: uninitialized symbol 'curr_type' Dan Carpenter
2023-12-13 19:27 ` Charlie Jenkins
2023-12-14 8:00 ` Dan Carpenter
2023-12-14 19:26 ` Charlie Jenkins
2023-12-28 0:59 ` Charlie Jenkins
2024-01-02 12:37 ` Dan Carpenter
2024-01-03 20:27 ` Charlie Jenkins
-- strict thread matches above, loose matches on Subject: below --
2023-12-13 9:54 kernel test robot
2023-12-13 0:27 kernel test robot
2023-12-11 22:03 kernel test robot
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.