linux-kernel - Re: [PATCH -next 0/3] sysvipc: introduce STAT

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

Date:   Tue, 13 Feb 2018 16:42:10 -0800
From:   Davidlohr Bueso <dave@...olabs.net>
To:     Andrew Morton <akpm@...ux-foundation.org>
Cc:     mhocko@...nel.org, mtk.manpages@...il.com, keescook@...omium.org,
        linux-api@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH -next 0/3] sysvipc: introduce STAT_ALL commands

On Tue, 13 Feb 2018, Andrew Morton wrote:

>On Tue, 13 Feb 2018 09:41:33 -0800 Davidlohr Bueso <dave@...olabs.net> wrote:
>
>> Hi,
>>
>> The following patches adds the discussed[1] new command for shm
>> as well as for sems and msq as they are subject to the same discrepancies
>> for ipc object permission checks between the syscall and via procfs.
>> These new commands are justified in that (1) we are stuck with this
>> semantics as changing syscall and procfs can break userland; and (2) some
>> users can benefit from performance (for large amounts of shm segments,
>> for example) from not having to parse the procfs interface.
>>
>> Once (if) merged, I will submit the necesary manpage updates. But I'm
>> thinking something like:
>>
>> ...
>>
>> [1] https://lkml.org/lkml/2017/12/19/220
>
>It would be nice to summarize the above discussion right here, rather
>than merely linking to an email thread.

I would have thought (1) and (2) above sumarize the situation. Patches
themselves also have a thorough changelog.

>
>A reported-by:mhocko would be appropriate.

This was really reported by a customer, but sure.

>
>Really, the only reason for this patchset is to speed up the userspace
>interface, yes?  But the changelog is awfull skimpy on the details
>here.  Who cares about it and why and which apps can be changed (ipcs?)
>and why do we care and how much better does it get, etc.

Yes, better performance is the goal here. The customer application is
a large and well known database vendor. When consulted as to why this
functionality was needed (and why the process couldn't be pimped to
CAP_IPC_OWNER):

""
The databaselayer saves its rowstore for some reason inside shared memory
(so large amounts of shared memory). The DB instances communicate with each
other about how much memory(ANON, but not shared) each instance is using.

Due to the nature of shared memory one instance might remove a segment,
but it will never know when it is realy gone unless we ask the kernel
to give us the correct information. When this happens we would be fine
to tell others, but to speed things up we currenty allow each instance
to calculate the shared memory size on their own.
""

As such these very specific workloads that deal with large (+10k) number
of ipc objects (shared memory segments in this case).

>
>
>Dumb question: an admin can do `chmod 0400 /proc/sysvipc/shm' and then
>this data is hidden from unprivileged users.  So doesn't this change
>represent a security hole for such users?

Sure, but you'd even hide data that the user can legitimately have
available (the same data that SHM_STAT would return). This deals with
permissions at an ipc object granularity, not file.

In any case, doing a 0400 on the file would cause 'ipcs -m' would not
show anything for other users.

Thanks,
Davidlohr