Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

ipc/msg: introduce msgctl(MSG_STAT_ANY)

There is a permission discrepancy when consulting msq ipc object
metadata between /proc/sysvipc/msg (0444) and the MSG_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 msq 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 for shm.

This patch introduces a new MSG_STAT_ANY command such that the msq 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-4-dave@stgolabs.net
Signed-off-by: Davidlohr Bueso <dbueso@suse.de>
Reported-by: Robert Kettler <robert.kettler@outlook.com>
Cc: Eric W. Biederman <ebiederm@xmission.com>
Cc: Kees Cook <keescook@chromium.org>
Cc: Manfred Spraul <manfred@colorfullife.com>
Cc: Michael Kerrisk <mtk.manpages@gmail.com>
Cc: Michal Hocko <mhocko@kernel.org>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>

authored by

Davidlohr Bueso and committed by
Linus Torvalds
23c8cec8 a280d6dc

+15 -5
+1
include/uapi/linux/msg.h
··· 7 7 /* ipcs ctl commands */ 8 8 #define MSG_STAT 11 9 9 #define MSG_INFO 12 10 + #define MSG_STAT_ANY 13 10 11 11 12 /* msgrcv options */ 12 13 #define MSG_NOERROR 010000 /* no error if message is too big */
+12 -5
ipc/msg.c
··· 497 497 memset(p, 0, sizeof(*p)); 498 498 499 499 rcu_read_lock(); 500 - if (cmd == MSG_STAT) { 500 + if (cmd == MSG_STAT || cmd == MSG_STAT_ANY) { 501 501 msq = msq_obtain_object(ns, msqid); 502 502 if (IS_ERR(msq)) { 503 503 err = PTR_ERR(msq); 504 504 goto out_unlock; 505 505 } 506 506 id = msq->q_perm.id; 507 - } else { 507 + } else { /* IPC_STAT */ 508 508 msq = msq_obtain_object_check(ns, msqid); 509 509 if (IS_ERR(msq)) { 510 510 err = PTR_ERR(msq); ··· 512 512 } 513 513 } 514 514 515 - err = -EACCES; 516 - if (ipcperms(ns, &msq->q_perm, S_IRUGO)) 517 - goto out_unlock; 515 + /* see comment for SHM_STAT_ANY */ 516 + if (cmd == MSG_STAT_ANY) 517 + audit_ipc_obj(&msq->q_perm); 518 + else { 519 + err = -EACCES; 520 + if (ipcperms(ns, &msq->q_perm, S_IRUGO)) 521 + goto out_unlock; 522 + } 518 523 519 524 err = security_msg_queue_msgctl(&msq->q_perm, cmd); 520 525 if (err) ··· 577 572 return err; 578 573 } 579 574 case MSG_STAT: /* msqid is an index rather than a msg queue id */ 575 + case MSG_STAT_ANY: 580 576 case IPC_STAT: 581 577 err = msgctl_stat(ns, msqid, cmd, &msqid64); 582 578 if (err < 0) ··· 696 690 } 697 691 case IPC_STAT: 698 692 case MSG_STAT: 693 + case MSG_STAT_ANY: 699 694 err = msgctl_stat(ns, msqid, cmd, &msqid64); 700 695 if (err < 0) 701 696 return err;
+1
security/selinux/hooks.c
··· 6006 6006 SECCLASS_SYSTEM, SYSTEM__IPC_INFO, NULL); 6007 6007 case IPC_STAT: 6008 6008 case MSG_STAT: 6009 + case MSG_STAT_ANY: 6009 6010 perms = MSGQ__GETATTR | MSGQ__ASSOCIATE; 6010 6011 break; 6011 6012 case IPC_SET:
+1
security/smack/smack_lsm.c
··· 3230 3230 switch (cmd) { 3231 3231 case IPC_STAT: 3232 3232 case MSG_STAT: 3233 + case MSG_STAT_ANY: 3233 3234 may = MAY_READ; 3234 3235 break; 3235 3236 case IPC_SET: