All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Michael Kerrisk (man-pages)" <mtk.manpages-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
To: Doug Ledford <dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
Cc: mtk.manpages-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org,
	Davidlohr Bueso <dave-h16yJtLeMjHk1uMJSBkQmQ@public.gmane.org>,
	"linux-man-u79uwXL29TY76Z2rM5mHXA@public.gmane.org"
	<linux-man-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>,
	lkml <linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>,
	Madars Vitolins <m@silodev.com>
Subject: Re: Document POSIX MQ /proc/sys/fs/mqueue files
Date: Wed, 01 Oct 2014 12:02:26 +0200	[thread overview]
Message-ID: <542BD132.7000105@gmail.com> (raw)
In-Reply-To: <1412011687.15492.39.camel-v+aXH1h/sVwpzh8Nc7Vzg+562jBIR2Zt@public.gmane.org>

On 09/29/2014 07:28 PM, Doug Ledford wrote:
> On Mon, 2014-09-29 at 11:10 +0200, Michael Kerrisk (man-pages) wrote:
>> Hello Doug, David,
>>
>> I think you two were the last ones to make significant 
>> changes to the semantics of the files in /proc/sys/fs/mqueue,
>> so I wonder if you (or anyone else who is willing) might
>> take a look at the man page text below that I've written
>> (for the mq_overview(7) page) to describe past and current
>> reality, and let me know of improvements of corrections.
>>
>> By the way, Doug, your commit ce2d52cc1364 appears to have
>> changed/broken the semantics of the files in the /dev/mqueue 
>> filesystem. Formerly, the QSIZE field in these files showed
>> the number of bytes of real user data in all of the queued
>> messages. After that commit, QSIZE now includes kernel 
>> overhead bytes, which does not seem very useful for user 
>> space. Was that change intentional? I see no mention of the
>> change in the commit message, so it sounds like it was not 
>> intended.
> 
> That change didn't come in that commit.  That commit modified it, but
> didn't introduce it.
> 
> Now, was it intentional? Yes.  Is it valuable, useful?  That depends on
> your perspective.
> 
> One of the problems I ran into with that code relates to the rlimit
> checks that happen at queue creation time.  We used to check to see if
> 
>  msg_num * (msg_size + sizeof struct msg_msg *)
> 
> would fit within the user's currently available rlimit for
> RLIMIT_MSGQUEUE.  This was not an accurate check though.  It accounted
> for the msg number, and the payload size, and the array of pointers we
> used to point to the msg_msg structs that held each message, but ignored
> the msg_msg structs themselves.  Given that we accept the creation of
> message queues with a msg_size of 1, this could be used to create a
> minor DoS because of the fact that there was such a large size
> difference between the sizeof struct msg_msg and the size of our
> messages.  In this scenario, a msg_size of 1 would result in us
> accounting 9/5 bytes per message on 64bit/32bit OSes respecitively, but
> actually using 49bytes/19bytes respectively.  That's a 4:1 ratio at the
> worst case for the different between actual memory used and memory usage
> accounted against the RLIMIT_MSGQUEUE limit. So before I ever got around
> to doing the rbtree update, I fixed this to at least be more accurate
> and it became
> 
>  msg_num * (msg_size + sizeof struct msg_msg * + sizeof struct msg_msg)
> 
> Even this wasn't totally accurate though, as large messages could result
> in the allocation of additional msg_msgseg segments.  However, I ignored
> that inaccuracy because once the message size is large enough to need
> additional SG segments, we are no longer in danger of any sort of minor
> DoS because our own overhead will become nothing more than noise to the
> calculation.

So, for what it's worth, I applied the following patch in getrlimit.2
to describe the post 3.5 behavior. Look okay?

Cheers,

Michael



diff --git a/man2/getrlimit.2 b/man2/getrlimit.2
index 91fed13..a3e4285 100644
--- a/man2/getrlimit.2
+++ b/man2/getrlimit.2
@@ -250,8 +250,19 @@ Each message queue that the user creates counts (until it i
s removed)
 against this limit according to the formula:
 .nf
 
-    bytes = attr.mq_maxmsg * sizeof(struct msg_msg *) +
-            attr.mq_maxmsg * attr.mq_msgsize
+    Since Linux 3.5:
+        bytes = attr.mq_maxmsg * sizeof(struct msg_msg) +
+                min(attr.mq_maxmsg, MQ_PRIO_MAX) *
+                      sizeof(struct posix_msg_tree_node)+ 
+                                /* For overhead */
+                attr.mq_maxmsg * attr.mq_msgsize;
+                                /* For message data */
+
+    Linux 3.4 and earlier:
+        bytes = attr.mq_maxmsg * sizeof(struct msg_msg *) +
+                                /* For overhead */
+                attr.mq_maxmsg * attr.mq_msgsize;
+                                /* For message data */
 
 .fi
 where
@@ -259,11 +270,16 @@ where
 is the
 .I mq_attr
 structure specified as the fourth argument to
-.BR mq_open (3).
+.BR mq_open (3),
+and the
+.I msg_msg
+and
+.I posix_msg_tree_node
+structures are kernel-internal structures.
 
-The first addend in the formula, which includes
-.I "sizeof(struct msg_msg\ *)"
-(4 bytes on Linux/i386), ensures that the user cannot
+The "overhead" addend in the formula accounts for overhead
+bytes required by the implementation
+and ensures that the user cannot
 create an unlimited number of zero-length messages (such messages
 nevertheless each consume some system memory for bookkeeping overhead).
 .TP

-- 
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/
--
To unsubscribe from this list: send the line "unsubscribe linux-man" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

WARNING: multiple messages have this Message-ID (diff)
From: "Michael Kerrisk (man-pages)" <mtk.manpages@gmail.com>
To: Doug Ledford <dledford@redhat.com>
Cc: mtk.manpages@gmail.com, Davidlohr Bueso <dave@stgolabs.net>,
	"linux-man@vger.kernel.org" <linux-man@vger.kernel.org>,
	lkml <linux-kernel@vger.kernel.org>,
	Madars Vitolins <m@silodev.com>
Subject: Re: Document POSIX MQ /proc/sys/fs/mqueue files
Date: Wed, 01 Oct 2014 12:02:26 +0200	[thread overview]
Message-ID: <542BD132.7000105@gmail.com> (raw)
In-Reply-To: <1412011687.15492.39.camel@firewall.xsintricity.com>

On 09/29/2014 07:28 PM, Doug Ledford wrote:
> On Mon, 2014-09-29 at 11:10 +0200, Michael Kerrisk (man-pages) wrote:
>> Hello Doug, David,
>>
>> I think you two were the last ones to make significant 
>> changes to the semantics of the files in /proc/sys/fs/mqueue,
>> so I wonder if you (or anyone else who is willing) might
>> take a look at the man page text below that I've written
>> (for the mq_overview(7) page) to describe past and current
>> reality, and let me know of improvements of corrections.
>>
>> By the way, Doug, your commit ce2d52cc1364 appears to have
>> changed/broken the semantics of the files in the /dev/mqueue 
>> filesystem. Formerly, the QSIZE field in these files showed
>> the number of bytes of real user data in all of the queued
>> messages. After that commit, QSIZE now includes kernel 
>> overhead bytes, which does not seem very useful for user 
>> space. Was that change intentional? I see no mention of the
>> change in the commit message, so it sounds like it was not 
>> intended.
> 
> That change didn't come in that commit.  That commit modified it, but
> didn't introduce it.
> 
> Now, was it intentional? Yes.  Is it valuable, useful?  That depends on
> your perspective.
> 
> One of the problems I ran into with that code relates to the rlimit
> checks that happen at queue creation time.  We used to check to see if
> 
>  msg_num * (msg_size + sizeof struct msg_msg *)
> 
> would fit within the user's currently available rlimit for
> RLIMIT_MSGQUEUE.  This was not an accurate check though.  It accounted
> for the msg number, and the payload size, and the array of pointers we
> used to point to the msg_msg structs that held each message, but ignored
> the msg_msg structs themselves.  Given that we accept the creation of
> message queues with a msg_size of 1, this could be used to create a
> minor DoS because of the fact that there was such a large size
> difference between the sizeof struct msg_msg and the size of our
> messages.  In this scenario, a msg_size of 1 would result in us
> accounting 9/5 bytes per message on 64bit/32bit OSes respecitively, but
> actually using 49bytes/19bytes respectively.  That's a 4:1 ratio at the
> worst case for the different between actual memory used and memory usage
> accounted against the RLIMIT_MSGQUEUE limit. So before I ever got around
> to doing the rbtree update, I fixed this to at least be more accurate
> and it became
> 
>  msg_num * (msg_size + sizeof struct msg_msg * + sizeof struct msg_msg)
> 
> Even this wasn't totally accurate though, as large messages could result
> in the allocation of additional msg_msgseg segments.  However, I ignored
> that inaccuracy because once the message size is large enough to need
> additional SG segments, we are no longer in danger of any sort of minor
> DoS because our own overhead will become nothing more than noise to the
> calculation.

So, for what it's worth, I applied the following patch in getrlimit.2
to describe the post 3.5 behavior. Look okay?

Cheers,

Michael



diff --git a/man2/getrlimit.2 b/man2/getrlimit.2
index 91fed13..a3e4285 100644
--- a/man2/getrlimit.2
+++ b/man2/getrlimit.2
@@ -250,8 +250,19 @@ Each message queue that the user creates counts (until it i
s removed)
 against this limit according to the formula:
 .nf
 
-    bytes = attr.mq_maxmsg * sizeof(struct msg_msg *) +
-            attr.mq_maxmsg * attr.mq_msgsize
+    Since Linux 3.5:
+        bytes = attr.mq_maxmsg * sizeof(struct msg_msg) +
+                min(attr.mq_maxmsg, MQ_PRIO_MAX) *
+                      sizeof(struct posix_msg_tree_node)+ 
+                                /* For overhead */
+                attr.mq_maxmsg * attr.mq_msgsize;
+                                /* For message data */
+
+    Linux 3.4 and earlier:
+        bytes = attr.mq_maxmsg * sizeof(struct msg_msg *) +
+                                /* For overhead */
+                attr.mq_maxmsg * attr.mq_msgsize;
+                                /* For message data */
 
 .fi
 where
@@ -259,11 +270,16 @@ where
 is the
 .I mq_attr
 structure specified as the fourth argument to
-.BR mq_open (3).
+.BR mq_open (3),
+and the
+.I msg_msg
+and
+.I posix_msg_tree_node
+structures are kernel-internal structures.
 
-The first addend in the formula, which includes
-.I "sizeof(struct msg_msg\ *)"
-(4 bytes on Linux/i386), ensures that the user cannot
+The "overhead" addend in the formula accounts for overhead
+bytes required by the implementation
+and ensures that the user cannot
 create an unlimited number of zero-length messages (such messages
 nevertheless each consume some system memory for bookkeeping overhead).
 .TP

-- 
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/

  parent reply	other threads:[~2014-10-01 10:02 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-09-29  9:10 Document POSIX MQ /proc/sys/fs/mqueue files Michael Kerrisk (man-pages)
2014-09-29  9:10 ` Michael Kerrisk (man-pages)
2014-09-29 17:28 ` Doug Ledford
     [not found]   ` <1412011687.15492.39.camel-v+aXH1h/sVwpzh8Nc7Vzg+562jBIR2Zt@public.gmane.org>
2014-09-30 10:12     ` Michael Kerrisk (man-pages)
2014-09-30 10:12       ` Michael Kerrisk (man-pages)
2014-09-30 17:30       ` Davidlohr Bueso
2014-09-30 17:42         ` Davidlohr Bueso
     [not found]       ` <CAKgNAkj9+Z1yO-zrQ_qWFut7BqOkzNtNCruSSRyTKMpXZOcBaw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2014-09-30 19:57         ` Doug Ledford
2014-09-30 19:57           ` Doug Ledford
     [not found]           ` <1412107074.4930.9.camel-v+aXH1h/sVwpzh8Nc7Vzg+562jBIR2Zt@public.gmane.org>
2014-10-01  8:19             ` Michael Kerrisk (man-pages)
2014-10-01  8:19               ` Michael Kerrisk (man-pages)
2014-10-01 10:02     ` Michael Kerrisk (man-pages) [this message]
2014-10-01 10:02       ` Michael Kerrisk (man-pages)
2014-09-29 20:23 ` Davidlohr Bueso
     [not found]   ` <1412022198.23497.23.camel-dxKd5G12XOI1EaDjlw0dpg@public.gmane.org>
2014-09-30 10:49     ` Michael Kerrisk (man-pages)
2014-09-30 10:49       ` Michael Kerrisk (man-pages)

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=542BD132.7000105@gmail.com \
    --to=mtk.manpages-re5jqeeqqe8avxtiumwx3w@public.gmane.org \
    --cc=dave-h16yJtLeMjHk1uMJSBkQmQ@public.gmane.org \
    --cc=dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org \
    --cc=linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=linux-man-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=m@silodev.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.