linux-kernel - Re: arch/riscv/kernel/module.c:639 process_accumulated_relocations() error: uninitialized symbol 'curr

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date: Wed, 3 Jan 2024 12:27:15 -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:639 process_accumulated_relocations()
 error: uninitialized symbol 'curr_type'.

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
>