[<prev] [next>] [<thread-prev] [thread-next>] [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
Powered by blists - more mailing lists