Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
From:   ebiederm@xmission.com (Eric W. Biederman)
To:     Davidlohr Bueso <dave@stgolabs.net>
Cc:     akpm@linux-foundation.org, mhocko@kernel.org,
        mtk.manpages@gmail.com, keescook@chromium.org,
        linux-api@vger.kernel.org, linux-kernel@vger.kernel.org
References: <20180213174136.6346-1-dave@stgolabs.net>
Date:   Tue, 13 Feb 2018 14:21:31 -0600
In-Reply-To: <20180213174136.6346-1-dave@stgolabs.net> (Davidlohr Bueso's
        message of "Tue, 13 Feb 2018 09:41:33 -0800")
Message-ID: <878tbw3ak4.fsf@xmission.com>
User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/25.1 (gnu/linux)
MIME-Version: 1.0
Content-Type: text/plain
Subject: Re: [PATCH -next 0/3] sysvipc: introduce STAT_ALL commands
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

Davidlohr Bueso <dave@stgolabs.net> writes:

> 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:

I am just going to kibitz for a moment.

Could we name this _STAT_ANY or _STAT_NOPERM instead of _STAT_ALL.

I keep thinking a name with _ALL in it should affect all ipc opbjects of
a given type, not simply work any ipc object regardless of permissions.

Eric

> diff --git a/man2/shmctl.2 b/man2/shmctl.2
> index 7bb503999941..bb00bbe21a57 100644
> --- a/man2/shmctl.2
> +++ b/man2/shmctl.2
> @@ -41,6 +41,7 @@
>  .\" 2005-04-25, mtk -- noted aberrant Linux behavior w.r.t. new
>  .\"	attaches to a segment that has already been marked for deletion.
>  .\" 2005-08-02, mtk: Added IPC_INFO, SHM_INFO, SHM_STAT descriptions.
> +.\" 2018-02-13, dbueso: Added SHM_STAT_ALL description.
>  .\"
>  .TH SHMCTL 2 2017-09-15 "Linux" "Linux Programmer's Manual"
>  .SH NAME
> @@ -242,6 +243,18 @@ However, the
>  argument is not a segment identifier, but instead an index into
>  the kernel's internal array that maintains information about
>  all shared memory segments on the system.
> +.TP
> +.BR SHM_STAT_ALL " (Linux-specific)"
> +Return a
> +.I shmid_ds
> +structure as for
> +.BR SHM_STAT .
> +However, the
> +.I shm_perm.mode
> +is not checked for read access for
> +.IR shmid ,
> +resembing the behaviour of
> +/proc/sysvipc/shm.
>  .PP
>  The caller can prevent or allow swapping of a shared
>  memory segment with the following \fIcmd\fP values:
> @@ -287,7 +300,7 @@ operation returns the index of the highest used entry in the
>  kernel's internal array recording information about all
>  shared memory segments.
>  (This information can be used with repeated
> -.B SHM_STAT
> +.B SHM_STAT/SHM_STAT_ALL
>  operations to obtain information about all shared memory segments
>  on the system.)
>  A successful
> @@ -328,7 +341,7 @@ isn't accessible.
>  \fIshmid\fP is not a valid identifier, or \fIcmd\fP
>  is not a valid command.
>  Or: for a
> -.B SHM_STAT
> +.B SHM_STAT/SHM_STAT_ALL
>  operation, the index value specified in
>  .I shmid
>  referred to an array slot that is currently unused.
>
>
> Thanks!
>
> [1] https://lkml.org/lkml/2017/12/19/220
>
> Davidlohr Bueso (3):
>   ipc/shm: introduce shmctl(SHM_STAT_ALL)
>   ipc/sem: introduce shmctl(SEM_STAT_ALL)
>   ipc/msg: introduce shmctl(MSG_STAT_ALL)
>
>  include/uapi/linux/msg.h |  1 +
>  include/uapi/linux/sem.h |  1 +
>  include/uapi/linux/shm.h |  5 +++--
>  ipc/msg.c                | 17 ++++++++++++-----
>  ipc/sem.c                | 17 ++++++++++++-----
>  ipc/shm.c                | 23 ++++++++++++++++++-----
>  security/selinux/hooks.c |  3 +++
>  7 files changed, 50 insertions(+), 17 deletions(-)