Re: [PATCH v6 02/15] module: Introduce ksys_finit_module()

[Roberto Sassu <roberto.sassu@huaweicloud.com>]

On Wed, 2024-11-20 at 10:16 +0100, Roberto Sassu wrote:
> On Tue, 2024-11-19 at 12:10 -0800, Luis Chamberlain wrote:
> > On Tue, Nov 19, 2024 at 01:14:02PM +0100, Christoph Hellwig wrote:
> > > On Tue, Nov 19, 2024 at 11:49:09AM +0100, Roberto Sassu wrote:
> > > > From: Roberto Sassu <roberto.sassu@huawei.com>
> > > > 
> > > > Introduce ksys_finit_module() to let kernel components request a kernel
> > > > module without requiring running modprobe.
> > > 
> > > That does sound more than sketchy, even more so because the commit log
> > > completely fails to explain why you'd need to do that.
> > 
> > I also don't think the commit log is correct, I don't see how the
> > code is preventing calling modprobe, the indepotent check is intended
> > to prevent duplicate module init calls which may allocate extra vmalloc
> > space only to release it. You can test to see if your patch has any
> > improvments by enabling MODULE_STATS and MODULE_DEBUG_AUTOLOAD_DUPS
> > and check before / after results of /sys/kernel/debug/modules/stats  ,
> > right now this patch and commit log is not telling me anything useful.
> 
> Maybe I misunderstood the code, but what causes modprobe to be executed
> in user space is a call to request_module().
> 
> In my patch, I simply ported the code of the finit_module() system call
> to _ksys_finit_module(), net the conversion from struct fd to struct
> file, which is kept in the system call code.
> 
> Also, from the kernel side, I'm providing a valid address for module
> arguments, and duplicating the string either with kmemdup() or
> strndup_user() in load_module(), depending on where the memory belongs
> to.
> 
> Again, maybe I misunderstood, but I'm not introducing any functional
> change to the current behavior, the kernel side also provides a file
> descriptor and module arguments as user space would do (e.g. by
> executing insmod).
> 
> As for the motivation, please have a look at my response to Christian:

Christoph, of course.

Roberto


> https://lore.kernel.org/linux-integrity/ZzzvAPetAn7CUEvx@bombadil.infradead.org/T/#ma8656b921bb5bfb60e7f10331061d462a87ce9f4
> 
> 
> In addition, you could also see how ksys_finit_module() is used here:
> 
> https://lore.kernel.org/linux-integrity/20241119104922.2772571-8-roberto.sassu@huaweicloud.com/
> 
> Thanks
> 
> Roberto