linux-kernel - Re: [PATCH 4/6] rv: Remove rv

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]

Message-ID: <eabfae8b0f29c88437cc51af21797e869d99aef6.camel@redhat.com>
Date: Mon, 21 Jul 2025 17:49:02 +0200
From: Gabriele Monaco <gmonaco@...hat.com>
To: Nam Cao <namcao@...utronix.de>
Cc: Steven Rostedt <rostedt@...dmis.org>, Masami Hiramatsu
 <mhiramat@...nel.org>,  Mathieu Desnoyers <mathieu.desnoyers@...icios.com>,
 linux-trace-kernel@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 4/6] rv: Remove rv_reactor's reference counter

On Mon, 2025-07-21 at 16:04 +0200, Nam Cao wrote:
> On Mon, Jul 21, 2025 at 03:20:44PM +0200, Gabriele Monaco wrote:
> > Mmh, I'm not understanding how, let's assume I create a custom
> > reactor
> > as a kernel module and I want to use it on existing models (built
> > in or
> > modules themselves), I'd do.
> > 
> >  insmod myreactor
> >  echo myreactor > mymodel/reactors
> >  rmmod myreactor
> >  ## I want this one to fail because the reactor is in use
> > 
> >  echo nop > mymodel/reactors
> >  rmmod myreactor
> >  # now it can succeed
> > 
> > How is MODULE_SOFTDEP helping in this scenario?
> > Am I missing something here?
> 
> You are right, MODULE_SOFTDEP does not help this use case.
> 
> I did a quick search, it seems try_module_get() and module_put() are
> what we need for this. Let me amend the commit message.
> 

I wasn't aware of it, then sure we should be using that.

> But my essential point in this patch is that, the current ref count
> implementation does not work. Furthermore, we should use the
> centralized kernel module's facilities, not implementing our own
> custom logic.
> 

Yeah I get your point. If I understand you correctly, what's broken is
that we just return EBUSY and ignore that on __exit instead of doing
something about it (set nop to all monitors using this reactor).

I wonder if we shouldn't also fix this (using the module refcount).
But that can be done in the future, I'm not even sure reactors as
modules currently work.

Also, I'd need to verify this but depending on the order of exit
functions, we might be seeing the same problems with built-in reactors
when active on shutdown. I'm going to play a bit with this and see if
this workaround of not deleting the reactor was introduced for that (I
doubt though).

Thanks,
Gabriele