linux-kernel - Re: [PATCH] ipc: optimize semget/shmget/msgget for lots of keys

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, 9 Aug 2017 18:15:37 +0200
From:   Guillaume Knispel <guillaume.knispel@...ersonicimagine.com>
To:     Davidlohr Bueso <dave@...olabs.net>
Cc:     Andrew Morton <akpm@...ux-foundation.org>,
        Manfred Spraul <manfred@...orfullife.com>,
        Kees Cook <keescook@...omium.org>,
        Alexey Dobriyan <adobriyan@...il.com>,
        "Eric W. Biederman" <ebiederm@...ssion.com>,
        "Peter Zijlstra (Intel)" <peterz@...radead.org>,
        Ingo Molnar <mingo@...nel.org>,
        Sebastian Andrzej Siewior <bigeasy@...utronix.de>,
        Serge Hallyn <serge@...lyn.com>,
        Andrey Vagin <avagin@...nvz.org>,
        Marc Pardo <marc.pardo@...ersonicimagine.com>,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH] ipc: optimize semget/shmget/msgget for lots of keys

On Mon, Aug 07, 2017 at 11:21:03AM -0700, Davidlohr Bueso wrote:
> On Mon, 31 Jul 2017, Guillaume Knispel wrote:
> > struct ipc_ids {
> > 	int in_use;
> > 	unsigned short seq;
> > +	bool tables_initialized;
> 
> So this is really ugly to have, but I understand why you added it. I
> wonder what folks would think if we just panic() in the rhashtable_init()
> ENOMEM case, and convert the EINVALs to WARNs. This way the function
> would always be called successfully. This is similar to what futex_init
> does, with the underlying hash table allocator panicing. sems and msg
> would probably have to be converted to pure_initcall, but hey, we could
> at least get the symmetry back.

I think we could only afford to panic() on ENOMEM during boot, but
ipc_init_ids() is also called through create_ipc_ns() on namespace
creation. Besides, I would not be very comfortable with only warning on
EINVAL but continuing execution using potentially uninitialized data.
Granted, this will probably never happen in production, but the intent
was to leave the system usable (except that it would not be possible to
create sysv ipc objects) with no risk of additionnal crash for cases
like people hacking rhashtable and testing their modifications, if they
merely introduce a correctly reported error.

Cheers!
Guillaume