[Devel] [PATCH RHEL7 COMMIT] ms/ipc/mqueue.c: only perform resource calculation if user valid

Konstantin Khorenko khorenko at virtuozzo.com
Thu Nov 2 20:57:55 MSK 2023


The commit is pushed to "branch-rh7-3.10.0-1160.99.1.vz7.211.x-ovz" and will appear at https://src.openvz.org/scm/ovz/vzkernel.git
after rh7-3.10.0-1160.99.1.vz7.211.2
------>
commit 487f55cb8ec8a4ab0afe07e80f6eb2ef960eb30e
Author: Kees Cook <keescook at chromium.org>
Date:   Thu Sep 28 21:19:52 2023 +0300

    ms/ipc/mqueue.c: only perform resource calculation if user valid
    
    Andreas Christoforou reported:
    
      UBSAN: Undefined behaviour in ipc/mqueue.c:414:49 signed integer overflow:
      9 * 2305843009213693951 cannot be represented in type 'long int'
      ...
      Call Trace:
        mqueue_evict_inode+0x8e7/0xa10 ipc/mqueue.c:414
        evict+0x472/0x8c0 fs/inode.c:558
        iput_final fs/inode.c:1547 [inline]
        iput+0x51d/0x8c0 fs/inode.c:1573
        mqueue_get_inode+0x8eb/0x1070 ipc/mqueue.c:320
        mqueue_create_attr+0x198/0x440 ipc/mqueue.c:459
        vfs_mkobj+0x39e/0x580 fs/namei.c:2892
        prepare_open ipc/mqueue.c:731 [inline]
        do_mq_open+0x6da/0x8e0 ipc/mqueue.c:771
    
    Which could be triggered by:
    
            struct mq_attr attr = {
                    .mq_flags = 0,
                    .mq_maxmsg = 9,
                    .mq_msgsize = 0x1fffffffffffffff,
                    .mq_curmsgs = 0,
            };
    
            if (mq_open("/testing", 0x40, 3, &attr) == (mqd_t) -1)
                    perror("mq_open");
    
    mqueue_get_inode() was correctly rejecting the giant mq_msgsize, and
    preparing to return -EINVAL.  During the cleanup, it calls
    mqueue_evict_inode() which performed resource usage tracking math for
    updating "user", before checking if there was a valid "user" at all
    (which would indicate that the calculations would be sane).  Instead,
    delay this check to after seeing a valid "user".
    
    The overflow was real, but the results went unused, so while the flaw is
    harmless, it's noisy for kernel fuzzers, so just fix it by moving the
    calculation under the non-NULL "user" where it actually gets used.
    
    Link: http://lkml.kernel.org/r/201906072207.ECB65450@keescook
    Signed-off-by: Kees Cook <keescook at chromium.org>
    Reported-by: Andreas Christoforou <andreaschristofo at gmail.com>
    Acked-by: "Eric W. Biederman" <ebiederm at xmission.com>
    Cc: Al Viro <viro at zeniv.linux.org.uk>
    Cc: Arnd Bergmann <arnd at arndb.de>
    Cc: Davidlohr Bueso <dave at stgolabs.net>
    Cc: Manfred Spraul <manfred at colorfullife.com>
    Signed-off-by: Andrew Morton <akpm at linux-foundation.org>
    Signed-off-by: Linus Torvalds <torvalds at linux-foundation.org>
    
    (cherry picked from ms commit a318f12ed8843cfac53198390c74a565c632f417)
    https://jira.vzint.dev/browse/PSBM-150648
    Signed-off-by: Alexander Atanasov <alexander.atanasov at virtuozzo.com>
---
 ipc/mqueue.c | 19 ++++++++++---------
 1 file changed, 10 insertions(+), 9 deletions(-)

diff --git a/ipc/mqueue.c b/ipc/mqueue.c
index 31c55ff0c746..b86a6f35d22c 100644
--- a/ipc/mqueue.c
+++ b/ipc/mqueue.c
@@ -516,7 +516,6 @@ static void mqueue_evict_inode(struct inode *inode)
 {
 	struct mqueue_inode_info *info;
 	struct user_struct *user;
-	unsigned long mq_bytes, mq_treesize;
 	struct ipc_namespace *ipc_ns;
 	struct msg_msg *msg, *nmsg;
 	LIST_HEAD(tmp_msg);
@@ -541,16 +540,18 @@ static void mqueue_evict_inode(struct inode *inode)
 		free_msg(msg);
 	}
 
-	/* Total amount of bytes accounted for the mqueue */
-	mq_treesize = info->attr.mq_maxmsg * sizeof(struct msg_msg) +
-		min_t(unsigned int, info->attr.mq_maxmsg, MQ_PRIO_MAX) *
-		sizeof(struct posix_msg_tree_node);
-
-	mq_bytes = mq_treesize + (info->attr.mq_maxmsg *
-				  info->attr.mq_msgsize);
-
 	user = info->user;
 	if (user) {
+		unsigned long mq_bytes, mq_treesize;
+
+		/* Total amount of bytes accounted for the mqueue */
+		mq_treesize = info->attr.mq_maxmsg * sizeof(struct msg_msg) +
+			min_t(unsigned int, info->attr.mq_maxmsg, MQ_PRIO_MAX) *
+			sizeof(struct posix_msg_tree_node);
+
+		mq_bytes = mq_treesize + (info->attr.mq_maxmsg *
+					  info->attr.mq_msgsize);
+
 		spin_lock(&mq_lock);
 		user->mq_bytes -= mq_bytes;
 		/*


More information about the Devel mailing list