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'.
Date: Wed, 3 Jan 2024 12:27:15 -0800 [thread overview]
Message-ID: <ZZXDI78QsaXzbfR+@ghost> (raw)
In-Reply-To: <c11513b4-6e60-44b5-ba12-83fbb9b7e2c3@suswa.mountain>
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
>
prev parent reply other threads:[~2024-01-03 20:27 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
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 [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=ZZXDI78QsaXzbfR+@ghost \
--to=charlie@rivosinc.com \
--cc=dan.carpenter@linaro.org \
--cc=linux-kernel@vger.kernel.org \
--cc=lkp@intel.com \
--cc=oe-kbuild-all@lists.linux.dev \
--cc=oe-kbuild@lists.linux.dev \
--cc=palmer@rivosinc.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox