linux-kernel - Re: [PATCH 2/2] module: add support to avoid duplicates early on load

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]

Message-ID: <ZG/a+nrt4/AAUi5z@bombadil.infradead.org>
Date:   Thu, 25 May 2023 15:02:34 -0700
From:   Luis Chamberlain <mcgrof@...nel.org>
To:     Linus Torvalds <torvalds@...ux-foundation.org>
Cc:     Lucas De Marchi <lucas.demarchi@...el.com>,
        Petr Pavlu <petr.pavlu@...e.com>, gregkh@...uxfoundation.org,
        rafael@...nel.org, song@...nel.org, lucas.de.marchi@...il.com,
        christophe.leroy@...roup.eu, peterz@...radead.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, yujie.liu@...el.com, david@...hat.com,
        tglx@...utronix.de, hch@....de, patches@...ts.linux.dev,
        linux-modules@...r.kernel.org, linux-mm@...ck.org,
        linux-kernel@...r.kernel.org, pmladek@...e.com, prarit@...hat.com,
        lennart@...ttering.net
Subject: Re: [PATCH 2/2] module: add support to avoid duplicates early on load

On Thu, May 25, 2023 at 02:12:49PM -0700, Linus Torvalds wrote:
> On Thu, May 25, 2023 at 11:45 AM Lucas De Marchi
> <lucas.demarchi@...el.com> wrote:
> >
> > Are you willig to merge (a possibly improved version of) your patch
> > or the userspace change is still something that would be desired?
> 
> I think a user space change should still be something that people
> should look at, particularly as the kernel side patch I'm willing to
> accept doesn't catch the "completely serial" cases, only the "trying
> to load at the same time that the same module is literally busy being
> loaded".
> 
> But I've cleaned up my patch a bit, and while the cleaned-up version
> is rather larger as a patch (mainly because of just also re-organizing
> the finit_module() code to do all the 'struct file' prep), I'm
> actually pretty happy with this attached patch conceptually.
> 
> In this form, it actually "makes sense" to me, rather than being just
> clearly a workaround.  Also, unlike the previous patch, this doesn't
> actually make any changes to the basic kernel_read_file() set of
> functions, it's all done by the module loading code itself.
> 
> Luis, would you mind testing this version on your load? It still won't
> actually handle the purely serial case, so there *will* be those
> spurious double module reads from different CPU's just doing the
> things serially, but the exclusive file access region has been
> extended to not just cover the actual file content reading, but to
> cover the whole "turn it into a a real module" part too.
> 
> Also, this does *not* update some of the comments in the module
> loading. I changed finit_module to use "kernel_read_file()" instead of
> "kernel_read_file_from_fd()", since it actually now has to look up the
> file descriptor anyway. But the comments still talk about that
> "from_fd" thing.
> 
> Anyway, this is back to "ENTIRELY UNTESTED" territory, in that I've
> compiled this, but haven't booted it. The changes look obvious, but
> hey, mistakes happen.
> 
> And the commit message is just a place-holder. Obviously. I won't sign
> off on this or write more of a commit message until it has had some
> real testing.

With 255 vcpus:

Before:

vagrant@...d ~ $ sudo systemd-analyze                                            
Startup finished in 41.653s (kernel) + 44.305s (userspace) = 1min 25.958s        
graphical.target reached after 44.178s in userspace.  

root@...d ~ # grep "Virtual mem wasted bytes"
/sys/kernel/debug/modules/stats    
 Virtual mem wasted bytes       1949006968                                       

So ~1.8 GiB.

After:

root@...d ~ # systemd-analyze 
Startup finished in 35.872s (kernel) + 41.715s (userspace) = 1min 17.588s 
graphical.target reached after 41.594s in userspace.

root@...d ~ # cat /sys/kernel/debug/modules/stats
         Mods ever loaded       66
     Mods failed on kread       0
Mods failed on decompress       0
  Mods failed on becoming       1
      Mods failed on load       0
        Total module size       11268096
      Total mod text size       4149248
       Failed kread bytes       0
  Failed decompress bytes       0
    Failed becoming bytes       474688
        Failed kmod bytes       0
 Virtual mem wasted bytes       474688
         Average mod size       170729
    Average mod text size       62868
  Avg fail becoming bytes       474688
Duplicate failed modules:
              Module-name        How-many-times                    Reason
                   cryptd                     1                  Becoming

root@...d ~ # du -b /lib/modules/6.3.0-next-20230505+/kernel/crypto/cryptd.ko
475409 /lib/modules/6.3.0-next-20230505+/kernel/crypto/cryptd.ko

So yeah definitely a pretty good improvement. Sometimes the system boots
without any duplicates at all, for some reason Vs the previous attempt.

Tested-by: Luis Chamberlain <mcgrof@...nel.org>

  Luis