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
| ||
|
Message-ID: <YXV8Uq17fjBVvQNn@kernel.org> Date: Sun, 24 Oct 2021 18:31:30 +0300 From: Mike Rapoport <rppt@...nel.org> To: Matthew Wilcox <willy@...radead.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 11:57:02AM +0100, Matthew Wilcox wrote: > 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; So you suggest to prevent creation of the file descriptor to ensure there is no overflow of secretmem_users. I don't feel it's a clean and elegant solution. > > fd = get_unused_fd_flags(flags & O_CLOEXEC); > if (fd < 0) > > > Also, why does secretmem depend on !EMBEDDED? There was a request from tiny-config maintainers to keep this code outside tiny-config and the best option I could find to make secretmem depend on !EMBEDDED. -- Sincerely yours, Mike.
Powered by blists - more mailing lists