linux-kernel - Re: [PATCH] module: add debugging auto-load duplicate module support

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] [thread-next>] [day] [month] [year] [list]

Date:   Wed, 19 Apr 2023 16:32:30 -0700
From:   Luis Chamberlain <mcgrof@...nel.org>
To:     Greg KH <gregkh@...uxfoundation.org>
Cc:     david@...hat.com, patches@...ts.linux.dev,
        linux-modules@...r.kernel.org, linux-mm@...ck.org,
        linux-kernel@...r.kernel.org, pmladek@...e.com,
        petr.pavlu@...e.com, prarit@...hat.com,
        torvalds@...ux-foundation.org, rafael@...nel.org,
        christophe.leroy@...roup.eu, tglx@...utronix.de,
        peterz@...radead.org, song@...nel.org, rppt@...nel.org,
        dave@...olabs.net, willy@...radead.org, vbabka@...e.cz,
        mhocko@...e.com, dave.hansen@...ux.intel.com,
        colin.i.king@...il.com, jim.cromie@...il.com,
        catalin.marinas@....com, jbaron@...mai.com,
        rick.p.edgecombe@...el.com
Subject: Re: [PATCH] module: add debugging auto-load duplicate module support

On Wed, Apr 19, 2023 at 09:15:11AM +0200, Greg KH wrote:
> On Tue, Apr 18, 2023 at 01:46:36PM -0700, Luis Chamberlain wrote:
> You get 72 columns for changelog text, so you can use it :)

Sure! I forget what the limit is, but now I won't forget, new vimrc
settings:

set textwidth=100
autocmd FileType gitcommit set textwidth=72 
set colorcolumn=+1

> > +config MODULE_AUTOLOAD_SUPRESS_DUPS
> 
> MODULE_DEBUG_DUPLICATE perhaps?  It has nothing to do with autoloading
> (other than that is what userspace is doing)

I refer to module auto-loading as the kernel's use of the *request_module()
APIs. This code is used by the module auto-loading request_module() API
callers, prior to us even dealing with userspace.

> and you aren't suppressing anything except throwing up warnings, right?

Actually the code does converge duplicate auto-loading requests into one, but'll
just rename to MODULE_DEBUG_AUTOLOAD_DUPS.

> > +	bool "Debug duplicate modules with auto-loading"
> > +	help
> > +	  Module autoloading allows in-kernel code to request modules through
> > +	  the *request_module*() API calls. This in turn just calls userspace
> > +	  modprobe. Although modprobe checks to see if a module is already
> > +	  loaded before trying to load a module there is a small time window in
> > +	  which multiple duplicate requests can end up in userspace and multiple
> > +	  modprobe calls race calling finit_module() around the same time for
> > +	  duplicate modules. The finit_module() system call can consume in the
> > +	  worst case more than twice the respective module size in virtual
> > +	  memory for each duplicate module requests. Although duplicate module
> > +	  requests are non-fatal virtual memory is a limited resource and each
> > +	  duplicate module request ends up just wasting virtual memory.
> 
> It's not "wasted", as it is returned when the module is determined to be
> a duplicate.  Otherwise everyone will want this enabled as they think it
> will actually save memory.

I'll change the language to be clear the issue is memory pressure early
on boot. I'll also add a bit of language to help at least guide people
to realize that the real value-add for this, ie, I'll have to mention we
suspect issue is udev and not module auto-loading and that this however
may still help find a few cases we can optimize for.

  Luis