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:   Sun, 24 Oct 2021 11:57:02 +0100
From:   Matthew Wilcox <willy@...radead.org>
To:     Mike Rapoport <rppt@...nel.org>
Cc:     Kees Cook <keescook@...omium.org>,
        Dmitry Vyukov <dvyukov@...gle.com>,
        syzbot <syzbot+b904a1de3ec43711eba5@...kaller.appspotmail.com>,
        Jordy Zomer <jordy@...ing.systems>, akpm@...ux-foundation.org,
        linux-kernel@...r.kernel.org, linux-mm@...ck.org,
        syzkaller-bugs@...glegroups.com
Subject: Re: [syzbot] WARNING: refcount bug in sys_memfd_secret

On Sun, Oct 24, 2021 at 08:37:59AM +0300, Mike Rapoport wrote:
> On Sat, Oct 23, 2021 at 11:46:18PM +0100, Matthew Wilcox wrote:
> > On Sat, Oct 23, 2021 at 10:03:11AM -0700, Kees Cook wrote:
> > > On October 23, 2021 8:27:28 AM PDT, Mike Rapoport <rppt@...nel.org> wrote:
> > > >and my first reaction was to send a revert the untested commit 110860541f44
> > > >("mm/secretmem: use refcount_t instead of atomic_t"). 
> > 
> > I think you should.  This isn't a real problem. 
> 
> Do you mean that creation of 4 billion of file descriptors is not feasible?

On a sufficiently large machine, it is.  But then we have the same
problem with other atomic_t.  If you really care, just check whether
secretmem_users has gone negative, and return -ENFILE.  It doesn't
even have to be all that exact; you've got 2 billion values of slop
to use before you hit the wrap from negative to 0 which is the actual
problem.

ie this:

+++ b/mm/secretmem.c
@@ -203,6 +203,8 @@ SYSCALL_DEFINE1(memfd_secret, unsigned int, flags)

        if (flags & ~(SECRETMEM_FLAGS_MASK | O_CLOEXEC))
                return -EINVAL;
+       if (atomic_read(&secretmem_users) < 0)
+               return -ENFILE;

        fd = get_unused_fd_flags(flags & O_CLOEXEC);
        if (fd < 0)


Also, why does secretmem depend on !EMBEDDED?

config EMBEDDED
        bool "Embedded system"
        select EXPERT
        help
          This option should be enabled if compiling the kernel for
          an embedded system so certain expert options are available
          for configuration.

This is the only Kconfig option that depends on !EMBEDDED.  It's usually
used to avoid showing questions.  It means that my allmodconfig build
*doesn't* build secretmem, which is surely not what you wanted.

+++ b/mm/Kconfig
@@ -892,7 +892,7 @@ config IO_MAPPING
        bool

 config SECRETMEM
-       def_bool ARCH_HAS_SET_DIRECT_MAP && !EMBEDDED
+       def_bool ARCH_HAS_SET_DIRECT_MAP

 source "mm/damon/Kconfig"


Powered by blists - more mailing lists