From 31cdfdc5848de7fd9524f240347c7b9f6f3e12aa Mon Sep 17 00:00:00 2001i
From: Davidlohr Bueso <dave@stgolabs.net>
Date: Thu, 1 Mar 2018 07:28:25 -0800
Subject: [PATCH 1/3] ipc/shm: introduce shmctl(SHM_STAT_ANY)
Patch-mainline: v4.17-rc1
Git-commit: c21a6970ae727839a2f300cd8dd957de0d0238c3
References: bsc#1072689
There is a permission discrepancy when consulting shm ipc object metadata
between /proc/sysvipc/shm (0444) and the SHM_STAT shmctl command. The
later does permission checks for the object vs S_IRUGO. As such there can
be cases where EACCESS is returned via syscall but the info is displayed
anyways in the procfs files.
While this might have security implications via info leaking (albeit no
writing to the shm metadata), this behavior goes way back and showing all
the objects regardless of the permissions was most likely an overlook - so
we are stuck with it. Furthermore, modifying either the syscall or the
procfs file can cause userspace programs to break (ie ipcs). Some
applications require getting the procfs info (without root privileges) and
can be rather slow in comparison with a syscall -- up to 500x in some
reported cases.
This patch introduces a new SHM_STAT_ANY command such that the shm ipc
object permissions are ignored, and only audited instead. In addition,
I've left the lsm security hook checks in place, as if some policy can
block the call, then the user has no other choice than just parsing the
procfs file.
Link: http://lkml.kernel.org/r/20180215162458.10059-2-dave@stgolabs.net
Signed-off-by: Davidlohr Bueso <dbueso@suse.de>
Acked-by: Michal Hocko <mhocko@suse.com>
Cc: Michael Kerrisk <mtk.manpages@gmail.com>
Cc: Manfred Spraul <manfred@colorfullife.com>
Cc: Eric W. Biederman <ebiederm@xmission.com>
Cc: Kees Cook <keescook@chromium.org>
Cc: Robert Kettler <robert.kettler@outlook.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
---
include/uapi/linux/shm.h | 1 +
ipc/compat.c | 1 +
ipc/shm.c | 23 ++++++++++++++++++-----
security/selinux/hooks.c | 1 +
security/smack/smack_lsm.c | 1 +
5 files changed, 22 insertions(+), 5 deletions(-)
diff --git a/include/uapi/linux/shm.h b/include/uapi/linux/shm.h
index 1fbf24ea37fd..2be7f7f8c14e 100644
--- a/include/uapi/linux/shm.h
+++ b/include/uapi/linux/shm.h
@@ -57,6 +57,7 @@ struct shmid_ds {
/* ipcs ctl commands */
#define SHM_STAT 13
#define SHM_INFO 14
+#define SHM_STAT_ANY 15
/* Obsolete, used only for backwards compatibility */
struct shminfo {
diff --git a/ipc/compat.c b/ipc/compat.c
index 9b3c85f8a538..60fcce058dd8 100644
--- a/ipc/compat.c
+++ b/ipc/compat.c
@@ -715,6 +715,7 @@ COMPAT_SYSCALL_DEFINE3(shmctl, int, first, int, second, void __user *, uptr)
case IPC_STAT:
case SHM_STAT:
+ case SHM_STAT_ANY:
p = compat_alloc_user_space(sizeof(sem64));
err = sys_shmctl(first, second, p);
if (err < 0)
diff --git a/ipc/shm.c b/ipc/shm.c
index 8828b4c3a190..66749aa094c2 100644
--- a/ipc/shm.c
+++ b/ipc/shm.c
@@ -925,20 +925,21 @@ static int shmctl_nolock(struct ipc_namespace *ns, int shmid,
goto out;
}
case SHM_STAT:
+ case SHM_STAT_ANY:
case IPC_STAT:
{
struct shmid64_ds tbuf;
int result;
rcu_read_lock();
- if (cmd == SHM_STAT) {
+ if (cmd == SHM_STAT || cmd == SHM_STAT_ANY) {
shp = shm_obtain_object(ns, shmid);
if (IS_ERR(shp)) {
err = PTR_ERR(shp);
goto out_unlock;
}
result = shp->shm_perm.id;
- } else {
+ } else { /* IPC_STAT */
shp = shm_obtain_object_check(ns, shmid);
if (IS_ERR(shp)) {
err = PTR_ERR(shp);
@@ -947,9 +948,20 @@ static int shmctl_nolock(struct ipc_namespace *ns, int shmid,
result = 0;
}
- err = -EACCES;
- if (ipcperms(ns, &shp->shm_perm, S_IRUGO))
- goto out_unlock;
+ /*
+ * Semantically SHM_STAT_ANY ought to be identical to
+ * that functionality provided by the /proc/sysvipc/
+ * interface. As such, only audit these calls and
+ * do not do traditional S_IRUGO permission checks on
+ * the ipc object.
+ */
+ if (cmd == SHM_STAT_ANY)
+ audit_ipc_obj(&shp->shm_perm);
+ else {
+ err = -EACCES;
+ if (ipcperms(ns, &shp->shm_perm, S_IRUGO))
+ goto out_unlock;
+ }
err = security_shm_shmctl(shp, cmd);
if (err)
@@ -998,6 +1010,7 @@ SYSCALL_DEFINE3(shmctl, int, shmid, int, cmd, struct shmid_ds __user *, buf)
case IPC_INFO:
case SHM_INFO:
case SHM_STAT:
+ case SHM_STAT_ANY:
case IPC_STAT:
return shmctl_nolock(ns, shmid, cmd, version, buf);
case IPC_RMID:
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index f059bdfadc43..45f7b4dfecb4 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -5674,6 +5674,7 @@ static int selinux_shm_shmctl(struct shmid_kernel *shp, int cmd)
SECCLASS_SYSTEM, SYSTEM__IPC_INFO, NULL);
case IPC_STAT:
case SHM_STAT:
+ case SHM_STAT_ANY:
perms = SHM__GETATTR | SHM__ASSOCIATE;
break;
case IPC_SET:
diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
index 658f5d8c7e76..e9933199213d 100644
--- a/security/smack/smack_lsm.c
+++ b/security/smack/smack_lsm.c
@@ -3061,6 +3061,7 @@ static int smack_shm_shmctl(struct shmid_kernel *shp, int cmd)
switch (cmd) {
case IPC_STAT:
case SHM_STAT:
+ case SHM_STAT_ANY:
may = MAY_READ;
break;
case IPC_SET:
--
2.13.6