Received-SPF: pass (google.com: domain of linux-kernel+bounces-12332-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) client-ip=2604:1380:45e3:2400::1;
Date: Wed, 27 Dec 2023 16:59:25 -0800
From: Charlie Jenkins <charlie@rivosinc.com>
To: Dan Carpenter <dan.carpenter@linaro.org>
Cc: oe-kbuild@lists.linux.dev, lkp@intel.com, oe-kbuild-all@lists.linux.dev,
	linux-kernel@vger.kernel.org, Palmer Dabbelt <palmer@rivosinc.com>
Subject: Re: arch/riscv/kernel/module.c:639 process_accumulated_relocations()
 error: uninitialized symbol 'curr_type'.
Message-ID: <ZYzIbVfge6I8Ptw0@ghost>
References: <d0897fb3-1af8-430b-aa8b-9aa829bad1d7@suswa.mountain>
 <ZXoFhu2TPXgrsInY@ghost>
 <a46f0c36-8fd2-4a85-880c-eb462d4a837b@suswa.mountain>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <a46f0c36-8fd2-4a85-880c-eb462d4a837b@suswa.mountain>

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/