mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Hao Ge <hao.ge@linux.dev>
To: Daniel Gomez <da.gomez@kernel.org>
Cc: Luis Chamberlain <mcgrof@kernel.org>,
	Petr Pavlu <petr.pavlu@suse.com>,
	Sami Tolvanen <samitolvanen@google.com>,
	Aaron Tomlin <atomlin@atomlin.com>,
	Suren Baghdasaryan <surenb@google.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	linux-modules@vger.kernel.org, linux-kernel@vger.kernel.org,
	Sashiko <sashiko-bot@kernel.org>,
	Bradley Morgan <brads@mainlining.org>,
	stable@vger.kernel.org
Subject: Re: [PATCH v2] module: fix lost error code from codetag_load_module()
Date: Thu, 10 Sep 2026 11:28:09 +0800	[thread overview]
Message-ID: <675259f9-c093-439c-a411-1937b23ddaa2@linux.dev> (raw)
In-Reply-To: <20260909234838.3-da.gomez@kernel.org>

Hi Daniel

Thanks for you comment.

On 2026/9/10 06:00, Daniel Gomez wrote:
> On 2026-09-08T17:50:33+08:00, Hao Ge <hao.ge@linux.dev> wrote:
>> On 2026/9/8 03:42, Daniel Gomez wrote:
> 
>> I skipped it because alloc_tag is the only codetag type and it
>> already cleans up after itself.
>>
>> With a second codetag type the problem Sashiko reported could indeed
>> be triggered.
>>
>> My thinking at the time was to do it in codetag itself, as a separate patch,
>> since it feels odd to unload something whose load just failed.
> 
> Then, why returning an error at all? I don't see why not doing it inside

codetag is a generic framework with a dedicated section in modules.
We keep this error path to account for possible future data stored
in this section, allowing us to perform security and validity
checks for codetag within codetag_load_module.

> codetag itself wouldn't work but it's a "bigger" change. IMO, the error
> is supposed to be handled at load_module() then, the unwinding as the
> rest.
> 
> 

I'm worried about this as well. Invoking codetag_unload_module() here
brings kvfree_rcu_barrier() into the error path and triggers two runs of
->free_section_mem() (used=true from the unload, used=false later from
module_deallocate).

My idea is to let codetag handle its own cleanup while keeping the logic of
our current patch. Maybe we can put this patch on hold temporarily until I
work out a proper solution. How does this sound?

Thanks
Best Regards
Hao

      parent reply	other threads:[~2026-09-10  3:27 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-27  3:05 Hao Ge
2026-09-07 19:42 ` Daniel Gomez
2026-09-08  9:50   ` Hao Ge
2026-09-09 22:00     ` Daniel Gomez
2026-09-10  3:17       ` Hao Ge
2026-09-10  3:28       ` Hao Ge [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=675259f9-c093-439c-a411-1937b23ddaa2@linux.dev \
    --to=hao.ge@linux.dev \
    --cc=akpm@linux-foundation.org \
    --cc=atomlin@atomlin.com \
    --cc=brads@mainlining.org \
    --cc=da.gomez@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-modules@vger.kernel.org \
    --cc=mcgrof@kernel.org \
    --cc=petr.pavlu@suse.com \
    --cc=samitolvanen@google.com \
    --cc=sashiko-bot@kernel.org \
    --cc=stable@vger.kernel.org \
    --cc=surenb@google.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

all inboxes | Powered by JetHome®