linux-kernel - Re: [PATCH v6 02/15] module: Introduce ksys_finit

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: <9c68676cd810d6f6f9bb2bce0939ba2a04a4f8d2.camel@huaweicloud.com>
Date: Wed, 20 Nov 2024 10:18:01 +0100
From: Roberto Sassu <roberto.sassu@...weicloud.com>
To: Luis Chamberlain <mcgrof@...nel.org>, Christoph Hellwig <hch@....de>
Cc: zohar@...ux.ibm.com, dmitry.kasatkin@...il.com,
 eric.snowberg@...cle.com,  corbet@....net, petr.pavlu@...e.com,
 samitolvanen@...gle.com, da.gomez@...sung.com,  akpm@...ux-foundation.org,
 paul@...l-moore.com, jmorris@...ei.org,  serge@...lyn.com,
 shuah@...nel.org, mcoquelin.stm32@...il.com,  alexandre.torgue@...s.st.com,
 linux-integrity@...r.kernel.org,  linux-doc@...r.kernel.org,
 linux-kernel@...r.kernel.org,  linux-api@...r.kernel.org,
 linux-modules@...r.kernel.org,  linux-security-module@...r.kernel.org,
 linux-kselftest@...r.kernel.org,  wufan@...ux.microsoft.com,
 pbrobinson@...il.com, zbyszek@...waw.pl,  mjg59@...f.ucam.org,
 pmatilai@...hat.com, jannh@...gle.com, dhowells@...hat.com, 
 jikos@...nel.org, mkoutny@...e.com, ppavlu@...e.com, petr.vorel@...il.com, 
 mzerqung@...inter.de, kgold@...ux.ibm.com, Roberto Sassu
 <roberto.sassu@...wei.com>
Subject: Re: [PATCH v6 02/15] module: Introduce ksys_finit_module()

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@...wei.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