[Patch 0/8] Fix POSIX mqueue open issue

[Patch 0/8] Fix POSIX mqueue open issue

[Doug Ledford]

On 03/14/2012 05:38 PM, Andrew Morton wrote:
> On Wed, 14 Mar 2012 17:28:33 -0400
> Doug Ledford <dledford@redhat.com> wrote:
> 
>> This has obviously fallen through the cracks.
> 
> It sure has.  Please dig out whatever is the currently favored
> patchset, refresh, retest and resend, with changelogging which fully
> covers the reasoning and decision process?

Done.

>> so that if there are any apps out there that have been coded to expect
>> this behavior, then there is a workaround path for them until they get
>> coded
> 
> We'll be especially interested in the implications of this part.

The implications are this.  Since commit b231cca4381 on Oct 18, 2008,
calls to mq_open() that did not pass in an attribute struct and expected
to get default values for the size of the queue and the max message
size now get the system wide maximums instead of hardwired defaults
like they used to get.

This was uncovered when one of the earlier patches in this patch set
increased the default system wide maximums at the same time it increased
the hard ceiling on the system wide maximums (a customer specifically
needed the hard ceiling brought back up, the new ceiling that commit
b231cca4381 introduced was too low for their production systems).  By
increasing the default maximums and not realising they were tied to
any attempt to create a message queue without an attribute struct, I
had inadvertently made it such that all message queue creation attempts
without an attribute struct were failing because the new default
maximums would create a queue that exceeded the default rlimit for
message queue bytes.

As a result, the system wide defaults were brought back down to their
previous levels, and the system wide ceilings on the maximums were
raised to meet the customer's needs.  However, the fact that the
no attribute struct behavior of mq_open() could be broken by changing
the system wide maximums for message queues was seen as fundamentally
broken itself.  So we hardwired the no attribute case back like it
used to be.  But, then we realized that on the very off chance that
some piece of software in the wild depended on that behavior, we
could work around that issue by adding two new knobs to /proc that
allowed setting the defaults for message queues created without an
attr struct separately from the system wide maximums.

What is not an option IMO is to leave the current behavior in place.
No piece of software should ever rely on setting the system wide
maximums in order to get a desired message queue.  Such a reliance
would be so fundamentally multitasking OS unfriendly as to not really
be tolerable.  Fortunately, we don't know of any software in the
wild that uses this except for a regression test program that caught
the issue in the first place.  If there is though, we have made
accommodations with the two new /proc knobs (and that's all the
accommodations such fundamentally broken software can be allowed)..

Doug Ledford (5):
  ipc/mqueue: cleanup definition names and locations
  ipc/mqueue: switch back to using non-max values on create
  ipc/mqueue: enforce hard limits
  ipc/mqueue: update maximums for the mqueue subsystem
  selftests: add mq_open_tests

KOSAKI Motohiro (3):
  mqueue: revert bump up DFLT_*MAX
  mqueue: don't use kmalloc with KMALLOC_MAX_SIZE
  mqueue: separate mqueue default value from maximum value v2

 Documentation/sysctl/fs.txt                    |    7 +
 include/linux/ipc_namespace.h                  |   42 ++-
 ipc/mq_sysctl.c                                |   49 ++--
 ipc/mqueue.c                                   |   26 +-
 tools/testing/selftests/Makefile               |    2 +-
 tools/testing/selftests/mqueue/Makefile        |    8 +
 tools/testing/selftests/mqueue/mq_open_tests.c |  492 ++++++++++++++++++++++++
 7 files changed, 590 insertions(+), 36 deletions(-)
 create mode 100644 tools/testing/selftests/mqueue/Makefile
 create mode 100644 tools/testing/selftests/mqueue/mq_open_tests.c

-- 
1.7.7.6

[Patch 1/8] ipc/mqueue: cleanup definition names and locations

[Doug Ledford]

The various defines for minimums and maximums of the sysctl controllable
mqueue values are scattered amongst different files and named
inconsistently.  Move them all into ipc_namespace.h and make them have
consistent names.  Additionally, make the number of queues per namespace
also have a minimum and maximum and use the same sysctl function as the
other two settable variables.

Signed-off-by: Doug Ledford <dledford@redhat.com>
---
 include/linux/ipc_namespace.h |    5 +++++
 ipc/mq_sysctl.c               |   31 ++++++++-----------------------
 2 files changed, 13 insertions(+), 23 deletions(-)

diff --git a/include/linux/ipc_namespace.h b/include/linux/ipc_namespace.h
index 8a297a5..1372b56 100644
--- a/include/linux/ipc_namespace.h
+++ b/include/linux/ipc_namespace.h
@@ -91,10 +91,15 @@ static inline void shm_destroy_orphaned(struct ipc_namespace *ns) {}
 #ifdef CONFIG_POSIX_MQUEUE
 extern int mq_init_ns(struct ipc_namespace *ns);
 /* default values */
+#define MIN_QUEUESMAX  1
 #define DFLT_QUEUESMAX 256     /* max number of message queues */
+#define HARD_QUEUESMAX 1024
+#define MIN_MSGMAX     1
 #define DFLT_MSGMAX    10      /* max number of messages in each queue */
 #define HARD_MSGMAX    (32768*sizeof(void *)/4)
+#define MIN_MSGSIZEMAX  128
 #define DFLT_MSGSIZEMAX 8192   /* max message size */
+#define HARD_MSGSIZEMAX (8192*128)
 #else
 static inline int mq_init_ns(struct ipc_namespace *ns) { return 0; }
 #endif
diff --git a/ipc/mq_sysctl.c b/ipc/mq_sysctl.c
index 0c09366..e22336a 100644
--- a/ipc/mq_sysctl.c
+++ b/ipc/mq_sysctl.c
@@ -13,15 +13,6 @@
 #include <linux/ipc_namespace.h>
 #include <linux/sysctl.h>
 
-/*
- * Define the ranges various user-specified maximum values can
- * be set to.
- */
-#define MIN_MSGMAX	1		/* min value for msg_max */
-#define MAX_MSGMAX	HARD_MSGMAX	/* max value for msg_max */
-#define MIN_MSGSIZEMAX	128		/* min value for msgsize_max */
-#define MAX_MSGSIZEMAX	(8192*128)	/* max value for msgsize_max */
-
 #ifdef CONFIG_PROC_SYSCTL
 static void *get_mq(ctl_table *table)
 {
@@ -31,16 +22,6 @@ static void *get_mq(ctl_table *table)
 	return which;
 }
 
-static int proc_mq_dointvec(ctl_table *table, int write,
-	void __user *buffer, size_t *lenp, loff_t *ppos)
-{
-	struct ctl_table mq_table;
-	memcpy(&mq_table, table, sizeof(mq_table));
-	mq_table.data = get_mq(table);
-
-	return proc_dointvec(&mq_table, write, buffer, lenp, ppos);
-}
-
 static int proc_mq_dointvec_minmax(ctl_table *table, int write,
 	void __user *buffer, size_t *lenp, loff_t *ppos)
 {
@@ -52,15 +33,17 @@ static int proc_mq_dointvec_minmax(ctl_table *table, int write,
 					lenp, ppos);
 }
 #else
-#define proc_mq_dointvec NULL
 #define proc_mq_dointvec_minmax NULL
 #endif
 
+static int msg_queues_limit_min = MIN_QUEUESMAX;
+static int msg_queues_limit_max = HARD_QUEUESMAX;
+
 static int msg_max_limit_min = MIN_MSGMAX;
-static int msg_max_limit_max = MAX_MSGMAX;
+static int msg_max_limit_max = HARD_MSGMAX;
 
 static int msg_maxsize_limit_min = MIN_MSGSIZEMAX;
-static int msg_maxsize_limit_max = MAX_MSGSIZEMAX;
+static int msg_maxsize_limit_max = HARD_MSGSIZEMAX;
 
 static ctl_table mq_sysctls[] = {
 	{
@@ -68,7 +51,9 @@ static ctl_table mq_sysctls[] = {
 		.data		= &init_ipc_ns.mq_queues_max,
 		.maxlen		= sizeof(int),
 		.mode		= 0644,
-		.proc_handler	= proc_mq_dointvec,
+		.proc_handler	= proc_mq_dointvec_minmax,
+		.extra1		= &msg_queues_limit_min,
+		.extra2		= &msg_queues_limit_max,
 	},
 	{
 		.procname	= "msg_max",
-- 
1.7.7.6

[Patch 2/8] ipc/mqueue: switch back to using non-max values on create

[Doug Ledford]

Commit b231cca4381ee15ec99afbfb244fbc0324869927 changed
how we create a queue that does not include an attr
struct passed to open so that it creates the queue
with whatever the maximum values are.  However, if the
admin has set the maximums to allow flexibility in
creating a queue (aka, both a large size and large queue
are allowed, but combined they create a queue too large
for the RLIMIT_MSGQUEUE of the user), then attempts to
create a queue without an attr struct will fail.  Switch
back to using acceptable defaults regardless of what
the maximums are.

Note: so far, we only know of a few applications that rely
on this behavior (specifically, set the maximums in /proc,
then run the application which calls mq_open() without
passing in an attr struct, and the application expects the
newly created message queue to have the maximum sizes that
were set in /proc used on the mq_open() call, and all of
those applications that we know of are actually part of
regression test suites that were coded to do something
like this:

for size in 4096 65536 $((1024 * 1024)) $((16 * 1024 * 1024)); do
	echo $size > /proc/sys/fs/mqueue/msgsize_max
	mq_open || echo "Error opening mq with size $size"
done

These test suites that depend on any behavior like this are
broken.  The concept that programs should rely upon the
system wide maximum in order to get their desired results
instead of simply using a attr struct to specify what they
want is fundamentally unfriendly programming practice for
any multi-tasking OS.

Fixing this will break those few apps that we know of (and
those app authors recognize the brokenness of their code
and the need to fix it).  However, a future patch will allow
a workaround in the form of new knobs for the default
msg queue creation parameters for any software out there that
we don't already know about that might rely on this behavior
at the moment.

Signed-off-by: Doug Ledford <dledford@redhat.com>
---
 include/linux/ipc_namespace.h |    2 ++
 ipc/mqueue.c                  |    5 +++--
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/include/linux/ipc_namespace.h b/include/linux/ipc_namespace.h
index 1372b56..bde094e 100644
--- a/include/linux/ipc_namespace.h
+++ b/include/linux/ipc_namespace.h
@@ -95,9 +95,11 @@ extern int mq_init_ns(struct ipc_namespace *ns);
 #define DFLT_QUEUESMAX 256     /* max number of message queues */
 #define HARD_QUEUESMAX 1024
 #define MIN_MSGMAX     1
+#define DFLT_MSG       10U
 #define DFLT_MSGMAX    10      /* max number of messages in each queue */
 #define HARD_MSGMAX    (32768*sizeof(void *)/4)
 #define MIN_MSGSIZEMAX  128
+#define DFLT_MSGSIZE    8192U
 #define DFLT_MSGSIZEMAX 8192   /* max message size */
 #define HARD_MSGSIZEMAX (8192*128)
 #else
diff --git a/ipc/mqueue.c b/ipc/mqueue.c
index 28bd64d..b178268 100644
--- a/ipc/mqueue.c
+++ b/ipc/mqueue.c
@@ -142,8 +142,9 @@ static struct inode *mqueue_get_inode(struct super_block *sb,
 		info->qsize = 0;
 		info->user = NULL;	/* set when all is ok */
 		memset(&info->attr, 0, sizeof(info->attr));
-		info->attr.mq_maxmsg = ipc_ns->mq_msg_max;
-		info->attr.mq_msgsize = ipc_ns->mq_msgsize_max;
+		info->attr.mq_maxmsg = min(ipc_ns->mq_msg_max, DFLT_MSG);
+		info->attr.mq_msgsize =
+			min(ipc_ns->mq_msgsize_max, DFLT_MSGSIZE);
 		if (attr) {
 			info->attr.mq_maxmsg = attr->mq_maxmsg;
 			info->attr.mq_msgsize = attr->mq_msgsize;
-- 
1.7.7.6

[Patch 3/8] ipc/mqueue: enforce hard limits

[Doug Ledford]

In two places we don't enforce the hard limits for
CAP_SYS_RESOURCE apps.  In preparation for making more
reasonable hard limits, start enforcing them even on
CAP_SYS_RESOURCE.

Signed-off-by: Doug Ledford <dledford@redhat.com>
---
 ipc/mqueue.c |    8 +++++---
 1 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/ipc/mqueue.c b/ipc/mqueue.c
index b178268..8f75d84 100644
--- a/ipc/mqueue.c
+++ b/ipc/mqueue.c
@@ -299,8 +299,9 @@ static int mqueue_create(struct inode *dir, struct dentry *dentry,
 		error = -EACCES;
 		goto out_unlock;
 	}
-	if (ipc_ns->mq_queues_count >= ipc_ns->mq_queues_max &&
-			!capable(CAP_SYS_RESOURCE)) {
+	if (ipc_ns->mq_queues_count >= HARD_QUEUESMAX ||
+	    (ipc_ns->mq_queues_count >= ipc_ns->mq_queues_max &&
+	     !capable(CAP_SYS_RESOURCE))) {
 		error = -ENOSPC;
 		goto out_unlock;
 	}
@@ -584,7 +585,8 @@ static int mq_attr_ok(struct ipc_namespace *ipc_ns, struct mq_attr *attr)
 	if (attr->mq_maxmsg <= 0 || attr->mq_msgsize <= 0)
 		return 0;
 	if (capable(CAP_SYS_RESOURCE)) {
-		if (attr->mq_maxmsg > HARD_MSGMAX)
+		if (attr->mq_maxmsg > HARD_MSGMAX ||
+		    attr->mq_msgsize > HARD_MSGSIZEMAX)
 			return 0;
 	} else {
 		if (attr->mq_maxmsg > ipc_ns->mq_msg_max ||
-- 
1.7.7.6

[Patch 4/8] ipc/mqueue: update maximums for the mqueue subsystem

[Doug Ledford]

Commit b231cca4381ee15ec99afbfb244fbc0324869927 changed
the maximum size of a message in a message queue from
INT_MAX to 8192*128.  Unfortunately, we had customers
that relied on a size much larger than 8192*128 on their
production systems.  After reviewing POSIX, we found that
it is silent on the maximum message size.  We did find
a couple other areas in which it was not silent.  Fix up
the mqueue maximums so that the customer's system can
continue to work, and document both the POSIX and real
world requirements in ipc_namespace.h so that we don't
have this issue crop back up.

Also, commit 9cf18e1dd74cd0061d58ac55029784ca3dd88f6a
fiddled with HARD_MSGMAX without realizing that the
number was intentionally in place to limit the msg
queue depth to one that was small enough to kmalloc
an array of pointers (hence why we divided 128k by
sizeof(long)).  If we wish to meet POSIX requirements,
we have no choice but to change our allocation to
a vmalloc instead (at least for the large queue size
case).  With that, it's possible to increase our
allowed maximum to the POSIX requirements (or more if
we choose).

Signed-off-by: Doug Ledford <dledford@redhat.com>
---
 include/linux/ipc_namespace.h |   47 ++++++++++++++++++++++++++++++----------
 ipc/mqueue.c                  |   10 +++++++-
 2 files changed, 43 insertions(+), 14 deletions(-)

diff --git a/include/linux/ipc_namespace.h b/include/linux/ipc_namespace.h
index bde094e..6e1dd08 100644
--- a/include/linux/ipc_namespace.h
+++ b/include/linux/ipc_namespace.h
@@ -90,18 +90,41 @@ static inline void shm_destroy_orphaned(struct ipc_namespace *ns) {}
 
 #ifdef CONFIG_POSIX_MQUEUE
 extern int mq_init_ns(struct ipc_namespace *ns);
-/* default values */
-#define MIN_QUEUESMAX  1
-#define DFLT_QUEUESMAX 256     /* max number of message queues */
-#define HARD_QUEUESMAX 1024
-#define MIN_MSGMAX     1
-#define DFLT_MSG       10U
-#define DFLT_MSGMAX    10      /* max number of messages in each queue */
-#define HARD_MSGMAX    (32768*sizeof(void *)/4)
-#define MIN_MSGSIZEMAX  128
-#define DFLT_MSGSIZE    8192U
-#define DFLT_MSGSIZEMAX 8192   /* max message size */
-#define HARD_MSGSIZEMAX (8192*128)
+/*
+ * POSIX Message Queue default values:
+ *
+ * MIN_*: Lowest value an admin can set the maximum unprivileged limit to
+ * DFLT_*MAX: Default values for the maximum unprivileged limits
+ * DFLT_{MSG,MSGSIZE}: Default values used when the user doesn't supply
+ *   an attribute to the open call and the queue must be created
+ * HARD_*: Highest value the maximums can be set to.  These are enforced
+ *   on CAP_SYS_RESOURCE apps as well making them inviolate (so make them
+ *   suitably high)
+ *
+ * POSIX Requirements:
+ *   Per app minimum openable message queues - 8.  This does not map well
+ *     to the fact that we limit the number of queues on a per namespace
+ *     basis instead of a per app basis.  So, make the default high enough
+ *     that no given app should have a hard time opening 8 queues.
+ *   Minimum maximum for HARD_MSGMAX - 32767.  I bumped this to 65536.
+ *   Minimum maximum for HARD_MSGSIZEMAX - POSIX is silent on this.  However,
+ *     we have run into a situation where running applications in the wild
+ *     require this to be at least 5MB, and preferably 10MB, so I set the
+ *     value to 16MB in hopes that this user is the worst of the bunch and
+ *     the new maximum will handle anyone else.  I may have to revisit this
+ *     in the future.
+ */
+#define MIN_QUEUESMAX			1
+#define DFLT_QUEUESMAX		      256
+#define HARD_QUEUESMAX		     1024
+#define MIN_MSGMAX			1
+#define DFLT_MSG		       64U
+#define DFLT_MSGMAX		     1024
+#define HARD_MSGMAX		    65536
+#define MIN_MSGSIZEMAX		      128
+#define DFLT_MSGSIZE		     8192U
+#define DFLT_MSGSIZEMAX	       (1024*1024)
+#define HARD_MSGSIZEMAX	    (16*1024*1024)
 #else
 static inline int mq_init_ns(struct ipc_namespace *ns) { return 0; }
 #endif
diff --git a/ipc/mqueue.c b/ipc/mqueue.c
index 8f75d84..3ced596 100644
--- a/ipc/mqueue.c
+++ b/ipc/mqueue.c
@@ -150,7 +150,10 @@ static struct inode *mqueue_get_inode(struct super_block *sb,
 			info->attr.mq_msgsize = attr->mq_msgsize;
 		}
 		mq_msg_tblsz = info->attr.mq_maxmsg * sizeof(struct msg_msg *);
-		info->messages = kmalloc(mq_msg_tblsz, GFP_KERNEL);
+		if (mq_msg_tblsz > KMALLOC_MAX_SIZE)
+			info->messages = vmalloc(mq_msg_tblsz);
+		else
+			info->messages = kmalloc(mq_msg_tblsz, GFP_KERNEL);
 		if (!info->messages)
 			goto out_inode;
 
@@ -260,7 +263,10 @@ static void mqueue_evict_inode(struct inode *inode)
 	spin_lock(&info->lock);
 	for (i = 0; i < info->attr.mq_curmsgs; i++)
 		free_msg(info->messages[i]);
-	kfree(info->messages);
+	if (info->attr.mq_maxmsg * sizeof(struct msg_msg *) > KMALLOC_MAX_SIZE)
+		vfree(info->messages);
+	else
+		kfree(info->messages);
 	spin_unlock(&info->lock);
 
 	/* Total amount of bytes accounted for the mqueue */
-- 
1.7.7.6

[Patch 5/8] mqueue: revert bump up DFLT_*MAX

[Doug Ledford]

From: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>

Mqueue limitation is slightly naieve parameter likes other ipcs
because unprivileged user can consume kernel memory by using ipcs.

Thus, too aggressive raise bring us security issue. Example,
current setting allow evil unprivileged user use 256GB (= 256
* 1024 * 1024*1024) and it's enough large to system will belome
unresponsive. Don't do that.

Instead, every admin should adjust the knobs for their own systems.

Signed-off-by: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>
Acked-by: Doug Ledford <dledford@redhat.com>
Acked-by: Joe Korty <joe.korty@ccur.com>
Cc: Amerigo Wang <amwang@redhat.com>
Cc: Serge E. Hallyn <serue@us.ibm.com>
Cc: Jiri Slaby <jslaby@suse.cz>
---
 include/linux/ipc_namespace.h |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/include/linux/ipc_namespace.h b/include/linux/ipc_namespace.h
index 6e1dd08..2488535 100644
--- a/include/linux/ipc_namespace.h
+++ b/include/linux/ipc_namespace.h
@@ -118,12 +118,12 @@ extern int mq_init_ns(struct ipc_namespace *ns);
 #define DFLT_QUEUESMAX		      256
 #define HARD_QUEUESMAX		     1024
 #define MIN_MSGMAX			1
-#define DFLT_MSG		       64U
-#define DFLT_MSGMAX		     1024
+#define DFLT_MSG		       10U
+#define DFLT_MSGMAX		       10
 #define HARD_MSGMAX		    65536
 #define MIN_MSGSIZEMAX		      128
 #define DFLT_MSGSIZE		     8192U
-#define DFLT_MSGSIZEMAX	       (1024*1024)
+#define DFLT_MSGSIZEMAX		     8192
 #define HARD_MSGSIZEMAX	    (16*1024*1024)
 #else
 static inline int mq_init_ns(struct ipc_namespace *ns) { return 0; }
-- 
1.7.7.6

[Patch 6/8] mqueue: don't use kmalloc with KMALLOC_MAX_SIZE

[Doug Ledford]

From: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>

KMALLOC_MAX_SIZE is no good threshold. It is extream high and
problematic. Unfortunately, some silly drivers depend on and
we can't change it. but any new code don't use such extream
ugly high order allocations. It bring us awful fragmentation
issue and system slowdown.

Signed-off-by: KOSAKI Motohiro <mkosaki@jp.fujitsu.com>
Acked-by: Doug Ledford <dledford@redhat.com>
Acked-by: Joe Korty <joe.korty@ccur.com>
Cc: Amerigo Wang <amwang@redhat.com>
Cc: Serge E. Hallyn <serue@us.ibm.com>
Cc: Jiri Slaby <jslaby@suse.cz>
---
 ipc/mqueue.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/ipc/mqueue.c b/ipc/mqueue.c
index 3ced596..f9f0782 100644
--- a/ipc/mqueue.c
+++ b/ipc/mqueue.c
@@ -150,7 +150,7 @@ static struct inode *mqueue_get_inode(struct super_block *sb,
 			info->attr.mq_msgsize = attr->mq_msgsize;
 		}
 		mq_msg_tblsz = info->attr.mq_maxmsg * sizeof(struct msg_msg *);
-		if (mq_msg_tblsz > KMALLOC_MAX_SIZE)
+		if (mq_msg_tblsz > PAGE_SIZE)
 			info->messages = vmalloc(mq_msg_tblsz);
 		else
 			info->messages = kmalloc(mq_msg_tblsz, GFP_KERNEL);
@@ -263,7 +263,7 @@ static void mqueue_evict_inode(struct inode *inode)
 	spin_lock(&info->lock);
 	for (i = 0; i < info->attr.mq_curmsgs; i++)
 		free_msg(info->messages[i]);
-	if (info->attr.mq_maxmsg * sizeof(struct msg_msg *) > KMALLOC_MAX_SIZE)
+	if (is_vmalloc_addr(info->messages))
 		vfree(info->messages);
 	else
 		kfree(info->messages);
-- 
1.7.7.6

[Patch 7/8] mqueue: separate mqueue default value from maximum value v2

[Doug Ledford]

From: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>

commit b231cca438 (message queues: increase range limits)
changed mqueue default value when attr parameter is specified NULL
from hard coded value to fs.mqueue.{msg,msgsize}_max sysctl value.

This made large side effect. When user need to use two mqueue
applications 1) using !NULL attr parameter and it require big
message size and 2) using NULL attr parameter and only need small
size message, app (1) require to raise fs.mqueue.msgsize_max and
app (2) consume large memory size even though it doesn't need.

Doug Ledford propsed to switch back it to static hard coded value.
However it also has a compatibility problem. Some applications might
started depend on the default value is tunable.

The solution is to separate default value from maximum value.

Signed-off-by: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>
Acked-by: Doug Ledford <dledford@redhat.com>
Acked-by: Joe Korty <joe.korty@ccur.com>
Cc: Amerigo Wang <amwang@redhat.com>
Cc: Serge E. Hallyn <serue@us.ibm.com>
Cc: Jiri Slaby <jslaby@suse.cz>

v1->v2: Slightly modified because msgutil.c no longer needs mq
  namespace initialization

Signed-off-by (v2): Doug Ledford <dledford@redhat.com>
---
 Documentation/sysctl/fs.txt   |    7 +++++++
 include/linux/ipc_namespace.h |    2 ++
 ipc/mq_sysctl.c               |   18 ++++++++++++++++++
 ipc/mqueue.c                  |    9 ++++++---
 4 files changed, 33 insertions(+), 3 deletions(-)

diff --git a/Documentation/sysctl/fs.txt b/Documentation/sysctl/fs.txt
index 88fd7f5..13d6166 100644
--- a/Documentation/sysctl/fs.txt
+++ b/Documentation/sysctl/fs.txt
@@ -225,6 +225,13 @@ a queue must be less or equal then msg_max.
 maximum  message size value (it is every  message queue's attribute set during
 its creation).
 
+/proc/sys/fs/mqueue/msg_default is  a read/write  file for setting/getting the
+default number of messages in a queue value if attr parameter of mq_open(2) is
+NULL. If it exceed msg_max, the default value is initialized msg_max.
+
+/proc/sys/fs/mqueue/msgsize_default is a read/write file for setting/getting
+the default message size value if attr parameter of mq_open(2) is NULL. If it
+exceed msgsize_max, the default value is initialized msgsize_max.
 
 4. /proc/sys/fs/epoll - Configuration options for the epoll interface
 --------------------------------------------------------
diff --git a/include/linux/ipc_namespace.h b/include/linux/ipc_namespace.h
index 2488535..5499c92 100644
--- a/include/linux/ipc_namespace.h
+++ b/include/linux/ipc_namespace.h
@@ -62,6 +62,8 @@ struct ipc_namespace {
 	unsigned int    mq_queues_max;   /* initialized to DFLT_QUEUESMAX */
 	unsigned int    mq_msg_max;      /* initialized to DFLT_MSGMAX */
 	unsigned int    mq_msgsize_max;  /* initialized to DFLT_MSGSIZEMAX */
+	unsigned int    mq_msg_default;
+	unsigned int    mq_msgsize_default;
 
 	/* user_ns which owns the ipc ns */
 	struct user_namespace *user_ns;
diff --git a/ipc/mq_sysctl.c b/ipc/mq_sysctl.c
index e22336a..383d638 100644
--- a/ipc/mq_sysctl.c
+++ b/ipc/mq_sysctl.c
@@ -73,6 +73,24 @@ static ctl_table mq_sysctls[] = {
 		.extra1		= &msg_maxsize_limit_min,
 		.extra2		= &msg_maxsize_limit_max,
 	},
+	{
+		.procname	= "msg_default",
+		.data		= &init_ipc_ns.mq_msg_default,
+		.maxlen		= sizeof(int),
+		.mode		= 0644,
+		.proc_handler	= proc_mq_dointvec_minmax,
+		.extra1		= &msg_max_limit_min,
+		.extra2		= &msg_max_limit_max,
+	},
+	{
+		.procname	= "msgsize_default",
+		.data		= &init_ipc_ns.mq_msgsize_default,
+		.maxlen		= sizeof(int),
+		.mode		= 0644,
+		.proc_handler	= proc_mq_dointvec_minmax,
+		.extra1		= &msg_maxsize_limit_min,
+		.extra2		= &msg_maxsize_limit_max,
+	},
 	{}
 };
 
diff --git a/ipc/mqueue.c b/ipc/mqueue.c
index f9f0782..04cc77e 100644
--- a/ipc/mqueue.c
+++ b/ipc/mqueue.c
@@ -142,9 +142,10 @@ static struct inode *mqueue_get_inode(struct super_block *sb,
 		info->qsize = 0;
 		info->user = NULL;	/* set when all is ok */
 		memset(&info->attr, 0, sizeof(info->attr));
-		info->attr.mq_maxmsg = min(ipc_ns->mq_msg_max, DFLT_MSG);
-		info->attr.mq_msgsize =
-			min(ipc_ns->mq_msgsize_max, DFLT_MSGSIZE);
+		info->attr.mq_maxmsg = min(ipc_ns->mq_msg_max,
+					   ipc_ns->mq_msg_default);
+		info->attr.mq_msgsize = min(ipc_ns->mq_msgsize_max,
+					    ipc_ns->mq_msgsize_default);
 		if (attr) {
 			info->attr.mq_maxmsg = attr->mq_maxmsg;
 			info->attr.mq_msgsize = attr->mq_msgsize;
@@ -1254,6 +1255,8 @@ int mq_init_ns(struct ipc_namespace *ns)
 	ns->mq_queues_max    = DFLT_QUEUESMAX;
 	ns->mq_msg_max       = DFLT_MSGMAX;
 	ns->mq_msgsize_max   = DFLT_MSGSIZEMAX;
+	ns->mq_msg_default   = DFLT_MSG;
+	ns->mq_msgsize_default  = DFLT_MSGSIZE;
 
 	ns->mq_mnt = kern_mount_data(&mqueue_fs_type, ns);
 	if (IS_ERR(ns->mq_mnt)) {
-- 
1.7.7.6

[Patch 8/8] selftests: add mq_open_tests

[Doug Ledford]

Add a directory to house POSIX message queue subsystem specific tests.
Add first test which checks the operation of mq_open() under various
corner conditions.

Signed-off-by: Doug Ledford <dledford@redhat.com>
---
 tools/testing/selftests/Makefile               |    2 +-
 tools/testing/selftests/mqueue/Makefile        |    8 +
 tools/testing/selftests/mqueue/mq_open_tests.c |  492 ++++++++++++++++++++++++
 3 files changed, 501 insertions(+), 1 deletions(-)
 create mode 100644 tools/testing/selftests/mqueue/Makefile
 create mode 100644 tools/testing/selftests/mqueue/mq_open_tests.c

diff --git a/tools/testing/selftests/Makefile b/tools/testing/selftests/Makefile
index 28bc57e..14972017a 100644
--- a/tools/testing/selftests/Makefile
+++ b/tools/testing/selftests/Makefile
@@ -1,4 +1,4 @@
-TARGETS = breakpoints vm
+TARGETS = breakpoints mqueue vm
 
 all:
 	for TARGET in $(TARGETS); do \
diff --git a/tools/testing/selftests/mqueue/Makefile b/tools/testing/selftests/mqueue/Makefile
new file mode 100644
index 0000000..bd74142
--- /dev/null
+++ b/tools/testing/selftests/mqueue/Makefile
@@ -0,0 +1,8 @@
+all:
+	gcc -O2 -lrt mq_open_tests.c -o mq_open_tests
+
+run_tests:
+	./mq_open_tests /test1
+
+clean:
+	rm -f mq_open_tests
diff --git a/tools/testing/selftests/mqueue/mq_open_tests.c b/tools/testing/selftests/mqueue/mq_open_tests.c
new file mode 100644
index 0000000..f790506
--- /dev/null
+++ b/tools/testing/selftests/mqueue/mq_open_tests.c
@@ -0,0 +1,492 @@
+/*
+ * This application is Copyright 2012 Red Hat, Inc.
+ *	Doug Ledford <dledford@redhat.com>
+ *
+ * mq_open_tests is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, version 3.
+ *
+ * mq_open_tests is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * For the full text of the license, see <http://www.gnu.org/licenses/>.
+ *
+ * mq_open_tests.c
+ *   Tests the various situations that should either succeed or fail to
+ *   open a posix message queue and then reports whether or not they
+ *   did as they were supposed to.
+ *
+ */
+#include <stdio.h>
+#include <stdlib.h>
+#include <unistd.h>
+#include <fcntl.h>
+#include <string.h>
+#include <limits.h>
+#include <errno.h>
+#include <sys/types.h>
+#include <sys/time.h>
+#include <sys/resource.h>
+#include <sys/stat.h>
+#include <mqueue.h>
+
+static char *usage =
+"Usage:\n"
+"  %s path\n"
+"\n"
+"	path	Path name of the message queue to create\n"
+"\n"
+"	Note: this program must be run as root in order to enable all tests\n"
+"\n";
+
+char *DEF_MSGS = "/proc/sys/fs/mqueue/msg_default";
+char *DEF_MSGSIZE = "/proc/sys/fs/mqueue/msgsize_default";
+char *MAX_MSGS = "/proc/sys/fs/mqueue/msg_max";
+char *MAX_MSGSIZE = "/proc/sys/fs/mqueue/msgsize_max";
+
+int default_settings;
+struct rlimit saved_limits, cur_limits;
+int saved_def_msgs, saved_def_msgsize, saved_max_msgs, saved_max_msgsize;
+int cur_def_msgs, cur_def_msgsize, cur_max_msgs, cur_max_msgsize;
+FILE *def_msgs, *def_msgsize, *max_msgs, *max_msgsize;
+char *queue_path;
+mqd_t queue = -1;
+
+static inline void __set(FILE *stream, int value, char *err_msg);
+void shutdown(int exit_val, char *err_cause, int line_no);
+static inline int get(FILE *stream);
+static inline void set(FILE *stream, int value);
+static inline void getr(int type, struct rlimit *rlim);
+static inline void setr(int type, struct rlimit *rlim);
+void validate_current_settings();
+static inline void test_queue(struct mq_attr *attr, struct mq_attr *result);
+static inline int test_queue_fail(struct mq_attr *attr, struct mq_attr *result);
+
+static inline void __set(FILE *stream, int value, char *err_msg)
+{
+	rewind(stream);
+	if (fprintf(stream, "%d", value) < 0)
+		perror(err_msg);
+}
+
+
+void shutdown(int exit_val, char *err_cause, int line_no)
+{
+	static int in_shutdown = 0;
+
+	/* In case we get called recursively by a set() call below */
+	if (in_shutdown++)
+		return;
+
+	seteuid(0);
+
+	if (queue != -1)
+		if (mq_close(queue))
+			perror("mq_close() during shutdown");
+	if (queue_path)
+		/*
+		 * Be silent if this fails, if we cleaned up already it's
+		 * expected to fail
+		 */
+		mq_unlink(queue_path);
+	if (default_settings) {
+		if (saved_def_msgs)
+			__set(def_msgs, saved_def_msgs,
+			      "failed to restore saved_def_msgs");
+		if (saved_def_msgsize)
+			__set(def_msgsize, saved_def_msgsize,
+			      "failed to restore saved_def_msgsize");
+	}
+	if (saved_max_msgs)
+		__set(max_msgs, saved_max_msgs,
+		      "failed to restore saved_max_msgs");
+	if (saved_max_msgsize)
+		__set(max_msgsize, saved_max_msgsize,
+		      "failed to restore saved_max_msgsize");
+	if (exit_val)
+		error(exit_val, errno, "%s at %d", err_cause, line_no);
+	exit(0);
+}
+
+static inline int get(FILE *stream)
+{
+	int value;
+	rewind(stream);
+	if (fscanf(stream, "%d", &value) != 1)
+		shutdown(4, "Error reading /proc entry", __LINE__ - 1);
+	return value;
+}
+
+static inline void set(FILE *stream, int value)
+{
+	int new_value;
+
+	rewind(stream);
+	if (fprintf(stream, "%d", value) < 0)
+		return shutdown(5, "Failed writing to /proc file",
+				__LINE__ - 1);
+	new_value = get(stream);
+	if (new_value != value)
+		return shutdown(5, "We didn't get what we wrote to /proc back",
+				__LINE__ - 1);
+}
+
+static inline void getr(int type, struct rlimit *rlim)
+{
+	if (getrlimit(type, rlim))
+		shutdown(6, "getrlimit()", __LINE__ - 1);
+}
+
+static inline void setr(int type, struct rlimit *rlim)
+{
+	if (setrlimit(type, rlim))
+		shutdown(7, "setrlimit()", __LINE__ - 1);
+}
+
+void validate_current_settings()
+{
+	int rlim_needed;
+
+	if (cur_limits.rlim_cur < 4096) {
+		printf("Current rlimit value for POSIX message queue bytes is "
+		       "unreasonably low,\nincreasing.\n\n");
+		cur_limits.rlim_cur = 8192;
+		cur_limits.rlim_max = 16384;
+		setr(RLIMIT_MSGQUEUE, &cur_limits);
+	}
+
+	if (default_settings) {
+		rlim_needed = (cur_def_msgs + 1) * (cur_def_msgsize + 1 +
+						    2 * sizeof(void *));
+		if (rlim_needed > cur_limits.rlim_cur) {
+			printf("Temporarily lowering default queue parameters "
+			       "to something that will work\n"
+			       "with the current rlimit values.\n\n");
+			set(def_msgs, 10);
+			cur_def_msgs = 10;
+			set(def_msgsize, 128);
+			cur_def_msgsize = 128;
+		}
+	} else {
+		rlim_needed = (cur_max_msgs + 1) * (cur_max_msgsize + 1 +
+						    2 * sizeof(void *));
+		if (rlim_needed > cur_limits.rlim_cur) {
+			printf("Temporarily lowering maximum queue parameters "
+			       "to something that will work\n"
+			       "with the current rlimit values in case this is "
+			       "a kernel that ties the default\n"
+			       "queue parameters to the maximum queue "
+			       "parameters.\n\n");
+			set(max_msgs, 10);
+			cur_max_msgs = 10;
+			set(max_msgsize, 128);
+			cur_max_msgsize = 128;
+		}
+	}
+}
+
+/*
+ * test_queue - Test opening a queue, shutdown if we fail.  This should
+ * only be called in situations that should never fail.  We clean up
+ * after ourselves and return the queue attributes in *result.
+ */
+static inline void test_queue(struct mq_attr *attr, struct mq_attr *result)
+{
+	int flags = O_RDWR | O_EXCL | O_CREAT;
+	int perms = DEFFILEMODE;
+
+	if ((queue = mq_open(queue_path, flags, perms, attr)) == -1)
+		shutdown(1, "mq_open()", __LINE__);
+	if (mq_getattr(queue, result))
+		shutdown(1, "mq_getattr()", __LINE__);
+	if (mq_close(queue))
+		shutdown(1, "mq_close()", __LINE__);
+	queue = -1;
+	if (mq_unlink(queue_path))
+		shutdown(1, "mq_unlink()", __LINE__);
+}
+
+/*
+ * Same as test_queue above, but failure is not fatal.
+ * Returns:
+ * 0 - Failed to create a queue
+ * 1 - Created a queue, attributes in *result
+ */
+static inline int test_queue_fail(struct mq_attr *attr, struct mq_attr *result)
+{
+	int flags = O_RDWR | O_EXCL | O_CREAT;
+	int perms = DEFFILEMODE;
+
+	if ((queue = mq_open(queue_path, flags, perms, attr)) == -1)
+		return 0;
+	if (mq_getattr(queue, result))
+		shutdown(1, "mq_getattr()", __LINE__);
+	if (mq_close(queue))
+		shutdown(1, "mq_close()", __LINE__);
+	queue = -1;
+	if (mq_unlink(queue_path))
+		shutdown(1, "mq_unlink()", __LINE__);
+	return 1;
+}
+
+int main(int argc, char *argv[])
+{
+	struct mq_attr attr, result;
+
+	if (argc != 2) {
+		fprintf(stderr, "Must pass a valid queue name\n\n");
+		fprintf(stderr, usage, argv[0]);
+		exit(1);
+	}
+
+	/*
+	 * Although we can create a msg queue with a non-absolute path name,
+	 * unlink will fail.  So, if the name doesn't start with a /, add one
+	 * when we save it.
+	 */
+	if (*argv[1] == '/')
+		queue_path = strdup(argv[1]);
+	else {
+		queue_path = malloc(strlen(argv[1]) + 2);
+		if (!queue_path) {
+			perror("malloc()");
+			exit(1);
+		}
+		queue_path[0] = '/';
+		queue_path[1] = 0;
+		strcat(queue_path, argv[1]);
+	}
+
+	if (getuid() != 0) {
+		fprintf(stderr, "Not running as root, but almost all tests "
+			"require root in order to modify\nsystem settings.  "
+			"Exiting.\n");
+		exit(1);
+	}
+
+	/* Find out what files there are for us to make tweaks in */
+	def_msgs = fopen(DEF_MSGS, "r+");
+	def_msgsize = fopen(DEF_MSGSIZE, "r+");
+	max_msgs = fopen(MAX_MSGS, "r+");
+	max_msgsize = fopen(MAX_MSGSIZE, "r+");
+
+	if (!max_msgs)
+		shutdown(2, "Failed to open msg_max", __LINE__);
+	if (!max_msgsize)
+		shutdown(2, "Failed to open msgsize_max", __LINE__);
+	if (def_msgs || def_msgsize)
+		default_settings = 1;
+
+	/* Load up the current system values for everything we can */
+	getr(RLIMIT_MSGQUEUE, &saved_limits);
+	cur_limits = saved_limits;
+	if (default_settings) {
+		saved_def_msgs = cur_def_msgs = get(def_msgs);
+		saved_def_msgsize = cur_def_msgsize = get(def_msgsize);
+	}
+	saved_max_msgs = cur_max_msgs = get(max_msgs);
+	saved_max_msgsize = cur_max_msgsize = get(max_msgsize);
+
+	/* Tell the user our initial state */
+	printf("\nInitial system state:\n");
+	printf("\tUsing queue path:\t\t%s\n", queue_path);
+	printf("\tRLIMIT_MSGQUEUE(soft):\t\t%d\n", saved_limits.rlim_cur);
+	printf("\tRLIMIT_MSGQUEUE(hard):\t\t%d\n", saved_limits.rlim_max);
+	printf("\tMaximum Message Size:\t\t%d\n", saved_max_msgsize);
+	printf("\tMaximum Queue Size:\t\t%d\n", saved_max_msgs);
+	if (default_settings) {
+		printf("\tDefault Message Size:\t\t%d\n", saved_def_msgsize);
+		printf("\tDefault Queue Size:\t\t%d\n", saved_def_msgs);
+	} else {
+		printf("\tDefault Message Size:\t\tNot Supported\n");
+		printf("\tDefault Queue Size:\t\tNot Supported\n");
+	}
+	printf("\n");
+	
+	validate_current_settings();
+
+	printf("Adjusted system state for testing:\n");
+	printf("\tRLIMIT_MSGQUEUE(soft):\t\t%d\n", cur_limits.rlim_cur);
+	printf("\tRLIMIT_MSGQUEUE(hard):\t\t%d\n", cur_limits.rlim_max);
+	printf("\tMaximum Message Size:\t\t%d\n", cur_max_msgsize);
+	printf("\tMaximum Queue Size:\t\t%d\n", cur_max_msgs);
+	if (default_settings) {
+		printf("\tDefault Message Size:\t\t%d\n", cur_def_msgsize);
+		printf("\tDefault Queue Size:\t\t%d\n", cur_def_msgs);
+	}
+
+	printf("\n\nTest series 1, behavior when no attr struct "
+	       "passed to mq_open:\n");
+	if (!default_settings) {
+		test_queue(NULL, &result);
+		printf("Given sane system settings, mq_open without an attr "
+		       "struct succeeds:\tPASS\n");
+		if (result.mq_maxmsg != cur_max_msgs ||
+		    result.mq_msgsize != cur_max_msgsize) {
+			printf("Kernel does not support setting the default "
+			       "mq attributes,\nbut also doesn't tie the "
+			       "defaults to the maximums:\t\t\tPASS\n");
+		} else {
+			set(max_msgs, ++cur_max_msgs);
+			set(max_msgsize, ++cur_max_msgsize);
+			test_queue(NULL, &result);
+			if (result.mq_maxmsg == cur_max_msgs &&
+			    result.mq_msgsize == cur_max_msgsize)
+				printf("Kernel does not support setting the "
+				       "default mq attributes and\n"
+				       "also ties system wide defaults to "
+				       "the system wide maximums:\t\t"
+				       "FAIL\n");
+			else
+				printf("Kernel does not support setting the "
+				       "default mq attributes,\n"
+				       "but also doesn't tie the defaults to "
+				       "the maximums:\t\t\tPASS\n");
+		}
+	} else {
+		printf("Kernel supports setting defaults separately from "
+		       "maximums:\t\tPASS\n");
+		/*
+		 * While we are here, go ahead and test that the kernel
+		 * properly follows the default settings
+		 */
+		test_queue(NULL, &result);
+		printf("Given sane values, mq_open without an attr struct "
+		       "succeeds:\t\tPASS\n");
+		if (result.mq_maxmsg != cur_def_msgs ||
+		    result.mq_msgsize != cur_def_msgsize)
+			printf("Kernel supports setting defaults, but does "
+			       "not actually honor them:\tFAIL\n\n");
+		else {
+			set(def_msgs, ++cur_def_msgs);
+			set(def_msgsize, ++cur_def_msgsize);
+			/* In case max was the same as the default */
+			set(max_msgs, ++cur_max_msgs);
+			set(max_msgsize, ++cur_max_msgsize);
+			test_queue(NULL, &result);
+			if (result.mq_maxmsg != cur_def_msgs ||
+			    result.mq_msgsize != cur_def_msgsize)
+				printf("Kernel supports setting defaults, but "
+				       "does not actually honor them:\t"
+				       "FAIL\n");
+			else
+				printf("Kernel properly honors default setting "
+				       "knobs:\t\t\t\tPASS\n");
+		}
+		set(def_msgs, cur_max_msgs + 1);
+		cur_def_msgs = cur_max_msgs + 1;
+		set(def_msgsize, cur_max_msgsize + 1);
+		cur_def_msgsize = cur_max_msgsize + 1;
+		if (cur_def_msgs * (cur_def_msgsize + 2 * sizeof(void *)) >=
+		    cur_limits.rlim_cur) {
+			cur_limits.rlim_cur = (cur_def_msgs + 2) *
+				(cur_def_msgsize + 2 * sizeof(void *));
+			cur_limits.rlim_max = 2 * cur_limits.rlim_cur;
+			setr(RLIMIT_MSGQUEUE, &cur_limits);
+		}
+		if (test_queue_fail(NULL, &result)) {
+			if (result.mq_maxmsg == cur_max_msgs &&
+			    result.mq_msgsize == cur_max_msgsize)
+				printf("Kernel properly limits default values "
+				       "to lesser of default/max:\t\tPASS\n");
+			else
+				printf("Kernel does not properly set default "
+				       "queue parameters when\ndefaults > "
+				       "max:\t\t\t\t\t\t\t\tFAIL\n");
+		} else
+			printf("Kernel fails to open mq because defaults are "
+			       "greater than maximums:\tFAIL\n");
+		set(def_msgs, --cur_def_msgs);
+		set(def_msgsize, --cur_def_msgsize);
+		cur_limits.rlim_cur = cur_limits.rlim_max = cur_def_msgs *
+			cur_def_msgsize;
+		setr(RLIMIT_MSGQUEUE, &cur_limits);
+		if (test_queue_fail(NULL, &result))
+			printf("Kernel creates queue even though defaults "
+			       "would exceed\nrlimit setting:"
+			       "\t\t\t\t\t\t\t\tFAIL\n");
+		else
+			printf("Kernel properly fails to create queue when "
+			       "defaults would\nexceed rlimit:"
+			       "\t\t\t\t\t\t\t\tPASS\n");
+	}
+
+	/*
+	 * Test #2 - open with an attr struct that exceeds rlimit
+	 */
+	printf("\n\nTest series 2, behavior when attr struct is "
+	       "passed to mq_open:\n");
+	cur_max_msgs = 32;
+	cur_max_msgsize = cur_limits.rlim_max >> 4;
+	set(max_msgs, cur_max_msgs);
+	set(max_msgsize, cur_max_msgsize);
+	attr.mq_maxmsg = cur_max_msgs;
+	attr.mq_msgsize = cur_max_msgsize;
+	if (test_queue_fail(&attr, &result))
+		printf("Queue open in excess of rlimit max when euid = 0 "
+		       "succeeded:\t\tFAIL\n");
+	else
+		printf("Queue open in excess of rlimit max when euid = 0 "
+		       "failed:\t\tPASS\n");
+	attr.mq_maxmsg = cur_max_msgs + 1;
+	attr.mq_msgsize = 10;
+	if (test_queue_fail(&attr, &result))
+		printf("Queue open with mq_maxmsg > limit when euid = 0 "
+		       "succeeded:\t\tPASS\n");
+	else
+		printf("Queue open with mq_maxmsg > limit when euid = 0 "
+		       "failed:\t\tFAIL\n");
+	attr.mq_maxmsg = 1;
+	attr.mq_msgsize = cur_max_msgsize + 1;
+	if (test_queue_fail(&attr, &result))
+		printf("Queue open with mq_msgsize > limit when euid = 0 "
+		       "succeeded:\t\tPASS\n");
+	else
+		printf("Queue open with mq_msgsize > limit when euid = 0 "
+		       "failed:\t\tFAIL\n");
+	attr.mq_maxmsg = 65536;
+	attr.mq_msgsize = 65536;
+	if (test_queue_fail(&attr, &result))
+		printf("Queue open with total size > 2GB when euid = 0 "
+		       "succeeded:\t\tFAIL\n");
+	else
+		printf("Queue open with total size > 2GB when euid = 0 "
+		       "failed:\t\t\tPASS\n");
+	seteuid(99);
+	attr.mq_maxmsg = cur_max_msgs;
+	attr.mq_msgsize = cur_max_msgsize;
+	if (test_queue_fail(&attr, &result))
+		printf("Queue open in excess of rlimit max when euid = 99 "
+		       "succeeded:\t\tFAIL\n");
+	else
+		printf("Queue open in excess of rlimit max when euid = 99 "
+		       "failed:\t\tPASS\n");
+	attr.mq_maxmsg = cur_max_msgs + 1;
+	attr.mq_msgsize = 10;
+	if (test_queue_fail(&attr, &result))
+		printf("Queue open with mq_maxmsg > limit when euid = 99 "
+		       "succeeded:\t\tFAIL\n");
+	else
+		printf("Queue open with mq_maxmsg > limit when euid = 99 "
+		       "failed:\t\tPASS\n");
+	attr.mq_maxmsg = 1;
+	attr.mq_msgsize = cur_max_msgsize + 1;
+	if (test_queue_fail(&attr, &result))
+		printf("Queue open with mq_msgsize > limit when euid = 99 "
+		       "succeeded:\t\tFAIL\n");
+	else
+		printf("Queue open with mq_msgsize > limit when euid = 99 "
+		       "failed:\t\tPASS\n");
+	attr.mq_maxmsg = 65536;
+	attr.mq_msgsize = 65536;
+	if (test_queue_fail(&attr, &result))
+		printf("Queue open with total size > 2GB when euid = 99 "
+		       "succeeded:\t\tFAIL\n");
+	else
+		printf("Queue open with total size > 2GB when euid = 99 "
+		       "failed:\t\t\tPASS\n");
+
+	shutdown(0,"",0);
+}
-- 
1.7.7.6

Re: [Patch 1/8] ipc/mqueue: cleanup definition names and locations

[KOSAKI Motohiro]

(4/17/12 11:46 AM), Doug Ledford wrote:
> The various defines for minimums and maximums of the sysctl controllable
> mqueue values are scattered amongst different files and named
> inconsistently.  Move them all into ipc_namespace.h and make them have
> consistent names.  Additionally, make the number of queues per namespace
> also have a minimum and maximum and use the same sysctl function as the
> other two settable variables.
> 
> Signed-off-by: Doug Ledford<dledford@redhat.com>

Ack all of this series.

thanks for patience and good effort.

Re: [Patch 2/8] ipc/mqueue: switch back to using non-max values on create

[Andrew Morton]

On Tue, 17 Apr 2012 11:46:19 -0400
Doug Ledford <dledford@redhat.com> wrote:

> Commit b231cca4381ee15ec99afbfb244fbc0324869927 changed
> how we create a queue that does not include an attr
> struct passed to open so that it creates the queue
> with whatever the maximum values are.  However, if the
> admin has set the maximums to allow flexibility in
> creating a queue (aka, both a large size and large queue
> are allowed, but combined they create a queue too large
> for the RLIMIT_MSGQUEUE of the user), then attempts to
> create a queue without an attr struct will fail.  Switch
> back to using acceptable defaults regardless of what
> the maximums are.
> 
> Note: so far, we only know of a few applications that rely
> on this behavior (specifically, set the maximums in /proc,
> then run the application which calls mq_open() without
> passing in an attr struct, and the application expects the
> newly created message queue to have the maximum sizes that
> were set in /proc used on the mq_open() call, and all of
> those applications that we know of are actually part of
> regression test suites that were coded to do something
> like this:
> 
> for size in 4096 65536 $((1024 * 1024)) $((16 * 1024 * 1024)); do
> 	echo $size > /proc/sys/fs/mqueue/msgsize_max
> 	mq_open || echo "Error opening mq with size $size"
> done
> 
> These test suites that depend on any behavior like this are
> broken.  The concept that programs should rely upon the
> system wide maximum in order to get their desired results
> instead of simply using a attr struct to specify what they
> want is fundamentally unfriendly programming practice for
> any multi-tasking OS.
> 
> Fixing this will break those few apps that we know of (and
> those app authors recognize the brokenness of their code
> and the need to fix it).  However, a future patch will allow
> a workaround in the form of new knobs for the default
> msg queue creation parameters for any software out there that
> we don't already know about that might rely on this behavior
> at the moment.

Here the "future patch" is "mqueue: separate mqueue default value from
maximum value v2" in this series, yes?

So people who have applications which are broken by this patch will
need to manually set /proc/sys/fs/mqueue/msg_default and/or
/proc/sys/fs/mqueue/msgsize_default to get those apps working again?

Re: [Patch 2/8] ipc/mqueue: switch back to using non-max values on create

[KOSAKI Motohiro]

(4/17/12 6:17 PM), Andrew Morton wrote:
> On Tue, 17 Apr 2012 11:46:19 -0400
> Doug Ledford<dledford@redhat.com>  wrote:
>
>> Commit b231cca4381ee15ec99afbfb244fbc0324869927 changed
>> how we create a queue that does not include an attr
>> struct passed to open so that it creates the queue
>> with whatever the maximum values are.  However, if the
>> admin has set the maximums to allow flexibility in
>> creating a queue (aka, both a large size and large queue
>> are allowed, but combined they create a queue too large
>> for the RLIMIT_MSGQUEUE of the user), then attempts to
>> create a queue without an attr struct will fail.  Switch
>> back to using acceptable defaults regardless of what
>> the maximums are.
>>
>> Note: so far, we only know of a few applications that rely
>> on this behavior (specifically, set the maximums in /proc,
>> then run the application which calls mq_open() without
>> passing in an attr struct, and the application expects the
>> newly created message queue to have the maximum sizes that
>> were set in /proc used on the mq_open() call, and all of
>> those applications that we know of are actually part of
>> regression test suites that were coded to do something
>> like this:
>>
>> for size in 4096 65536 $((1024 * 1024)) $((16 * 1024 * 1024)); do
>> 	echo $size>  /proc/sys/fs/mqueue/msgsize_max
>> 	mq_open || echo "Error opening mq with size $size"
>> done
>>
>> These test suites that depend on any behavior like this are
>> broken.  The concept that programs should rely upon the
>> system wide maximum in order to get their desired results
>> instead of simply using a attr struct to specify what they
>> want is fundamentally unfriendly programming practice for
>> any multi-tasking OS.
>>
>> Fixing this will break those few apps that we know of (and
>> those app authors recognize the brokenness of their code
>> and the need to fix it).  However, a future patch will allow
>> a workaround in the form of new knobs for the default
>> msg queue creation parameters for any software out there that
>> we don't already know about that might rely on this behavior
>> at the moment.
>
> Here the "future patch" is "mqueue: separate mqueue default value from
> maximum value v2" in this series, yes?
>
> So people who have applications which are broken by this patch will
> need to manually set /proc/sys/fs/mqueue/msg_default and/or
> /proc/sys/fs/mqueue/msgsize_default to get those apps working again?
>

Yes, it works.

Re: [Patch 2/8] ipc/mqueue: switch back to using non-max values on create

[Andrew Morton]

On Tue, 17 Apr 2012 18:32:25 -0400
KOSAKI Motohiro <kosaki.motohiro@gmail.com> wrote:

> > Here the "future patch" is "mqueue: separate mqueue default value from
> > maximum value v2" in this series, yes?
> >
> > So people who have applications which are broken by this patch will
> > need to manually set /proc/sys/fs/mqueue/msg_default and/or
> > /proc/sys/fs/mqueue/msgsize_default to get those apps working again?
> >
> 
> Yes, it works.
> 

OK, I updated the changelog for this patch to reflect that.

I worry a bit that some people who we don't know about will hit this
problem and will have to spend a lot of time working out why it broke. 
Is there some way in which we can make it easier for them?  A little
printk_once() in a suitable place?

Re: [Patch 1/8] ipc/mqueue: cleanup definition names and locations

[Serge E. Hallyn]

Quoting Doug Ledford (dledford@redhat.com):
> The various defines for minimums and maximums of the sysctl controllable
> mqueue values are scattered amongst different files and named
> inconsistently.  Move them all into ipc_namespace.h and make them have
> consistent names.  Additionally, make the number of queues per namespace
> also have a minimum and maximum and use the same sysctl function as the
> other two settable variables.
> 
> Signed-off-by: Doug Ledford <dledford@redhat.com>

I can't speak as to whether anyone would have a problem as a result of
the newly introduced min/max for # msg_queues, but other than that

Acked-by: Serge Hallyn <serge.hallyn@canonical.com>

> ---
>  include/linux/ipc_namespace.h |    5 +++++
>  ipc/mq_sysctl.c               |   31 ++++++++-----------------------
>  2 files changed, 13 insertions(+), 23 deletions(-)
> 
> diff --git a/include/linux/ipc_namespace.h b/include/linux/ipc_namespace.h
> index 8a297a5..1372b56 100644
> --- a/include/linux/ipc_namespace.h
> +++ b/include/linux/ipc_namespace.h
> @@ -91,10 +91,15 @@ static inline void shm_destroy_orphaned(struct ipc_namespace *ns) {}
>  #ifdef CONFIG_POSIX_MQUEUE
>  extern int mq_init_ns(struct ipc_namespace *ns);
>  /* default values */
> +#define MIN_QUEUESMAX  1
>  #define DFLT_QUEUESMAX 256     /* max number of message queues */
> +#define HARD_QUEUESMAX 1024
> +#define MIN_MSGMAX     1
>  #define DFLT_MSGMAX    10      /* max number of messages in each queue */
>  #define HARD_MSGMAX    (32768*sizeof(void *)/4)
> +#define MIN_MSGSIZEMAX  128
>  #define DFLT_MSGSIZEMAX 8192   /* max message size */
> +#define HARD_MSGSIZEMAX (8192*128)
>  #else
>  static inline int mq_init_ns(struct ipc_namespace *ns) { return 0; }
>  #endif
> diff --git a/ipc/mq_sysctl.c b/ipc/mq_sysctl.c
> index 0c09366..e22336a 100644
> --- a/ipc/mq_sysctl.c
> +++ b/ipc/mq_sysctl.c
> @@ -13,15 +13,6 @@
>  #include <linux/ipc_namespace.h>
>  #include <linux/sysctl.h>
>  
> -/*
> - * Define the ranges various user-specified maximum values can
> - * be set to.
> - */
> -#define MIN_MSGMAX	1		/* min value for msg_max */
> -#define MAX_MSGMAX	HARD_MSGMAX	/* max value for msg_max */
> -#define MIN_MSGSIZEMAX	128		/* min value for msgsize_max */
> -#define MAX_MSGSIZEMAX	(8192*128)	/* max value for msgsize_max */
> -
>  #ifdef CONFIG_PROC_SYSCTL
>  static void *get_mq(ctl_table *table)
>  {
> @@ -31,16 +22,6 @@ static void *get_mq(ctl_table *table)
>  	return which;
>  }
>  
> -static int proc_mq_dointvec(ctl_table *table, int write,
> -	void __user *buffer, size_t *lenp, loff_t *ppos)
> -{
> -	struct ctl_table mq_table;
> -	memcpy(&mq_table, table, sizeof(mq_table));
> -	mq_table.data = get_mq(table);
> -
> -	return proc_dointvec(&mq_table, write, buffer, lenp, ppos);
> -}
> -
>  static int proc_mq_dointvec_minmax(ctl_table *table, int write,
>  	void __user *buffer, size_t *lenp, loff_t *ppos)
>  {
> @@ -52,15 +33,17 @@ static int proc_mq_dointvec_minmax(ctl_table *table, int write,
>  					lenp, ppos);
>  }
>  #else
> -#define proc_mq_dointvec NULL
>  #define proc_mq_dointvec_minmax NULL
>  #endif
>  
> +static int msg_queues_limit_min = MIN_QUEUESMAX;
> +static int msg_queues_limit_max = HARD_QUEUESMAX;
> +
>  static int msg_max_limit_min = MIN_MSGMAX;
> -static int msg_max_limit_max = MAX_MSGMAX;
> +static int msg_max_limit_max = HARD_MSGMAX;
>  
>  static int msg_maxsize_limit_min = MIN_MSGSIZEMAX;
> -static int msg_maxsize_limit_max = MAX_MSGSIZEMAX;
> +static int msg_maxsize_limit_max = HARD_MSGSIZEMAX;
>  
>  static ctl_table mq_sysctls[] = {
>  	{
> @@ -68,7 +51,9 @@ static ctl_table mq_sysctls[] = {
>  		.data		= &init_ipc_ns.mq_queues_max,
>  		.maxlen		= sizeof(int),
>  		.mode		= 0644,
> -		.proc_handler	= proc_mq_dointvec,
> +		.proc_handler	= proc_mq_dointvec_minmax,
> +		.extra1		= &msg_queues_limit_min,
> +		.extra2		= &msg_queues_limit_max,
>  	},
>  	{
>  		.procname	= "msg_max",
> -- 
> 1.7.7.6
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

Re: [Patch 5/8] mqueue: revert bump up DFLT_*MAX

[Serge E. Hallyn]

Quoting Doug Ledford (dledford@redhat.com):
> From: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>
> 
> Mqueue limitation is slightly naieve parameter likes other ipcs
> because unprivileged user can consume kernel memory by using ipcs.
> 
> Thus, too aggressive raise bring us security issue. Example,
> current setting allow evil unprivileged user use 256GB (= 256
> * 1024 * 1024*1024) and it's enough large to system will belome
> unresponsive. Don't do that.
> 
> Instead, every admin should adjust the knobs for their own systems.

Would you be terribly averse to having a higher limit in init_ipc_ns,
and the lower values by default in all child namespaces?

Sorry it sounds from the intro like you've already had quite a bit of
discussion on this...

Of course I realize the values can just be raised by distro boot
scripts...

> Signed-off-by: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>
> Acked-by: Doug Ledford <dledford@redhat.com>
> Acked-by: Joe Korty <joe.korty@ccur.com>
> Cc: Amerigo Wang <amwang@redhat.com>
> Cc: Serge E. Hallyn <serue@us.ibm.com>

Acked-by: Serge E. Hallyn <serge.hallyn@canonical.com>

> Cc: Jiri Slaby <jslaby@suse.cz>
> ---
>  include/linux/ipc_namespace.h |    6 +++---
>  1 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/include/linux/ipc_namespace.h b/include/linux/ipc_namespace.h
> index 6e1dd08..2488535 100644
> --- a/include/linux/ipc_namespace.h
> +++ b/include/linux/ipc_namespace.h
> @@ -118,12 +118,12 @@ extern int mq_init_ns(struct ipc_namespace *ns);
>  #define DFLT_QUEUESMAX		      256
>  #define HARD_QUEUESMAX		     1024
>  #define MIN_MSGMAX			1
> -#define DFLT_MSG		       64U
> -#define DFLT_MSGMAX		     1024
> +#define DFLT_MSG		       10U
> +#define DFLT_MSGMAX		       10
>  #define HARD_MSGMAX		    65536
>  #define MIN_MSGSIZEMAX		      128
>  #define DFLT_MSGSIZE		     8192U
> -#define DFLT_MSGSIZEMAX	       (1024*1024)
> +#define DFLT_MSGSIZEMAX		     8192
>  #define HARD_MSGSIZEMAX	    (16*1024*1024)
>  #else
>  static inline int mq_init_ns(struct ipc_namespace *ns) { return 0; }
> -- 
> 1.7.7.6
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

Re: [Patch 6/8] mqueue: don't use kmalloc with KMALLOC_MAX_SIZE

[Serge E. Hallyn]

Quoting Doug Ledford (dledford@redhat.com):
> From: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>
> 
> KMALLOC_MAX_SIZE is no good threshold. It is extream high and
> problematic. Unfortunately, some silly drivers depend on and
> we can't change it. but any new code don't use such extream
> ugly high order allocations. It bring us awful fragmentation
> issue and system slowdown.
> 
> Signed-off-by: KOSAKI Motohiro <mkosaki@jp.fujitsu.com>
> Acked-by: Doug Ledford <dledford@redhat.com>
> Acked-by: Joe Korty <joe.korty@ccur.com>
> Cc: Amerigo Wang <amwang@redhat.com>
> Cc: Serge E. Hallyn <serue@us.ibm.com>

Looks reasonable to me, but that doesn't mean much.  Cc:d Dave Hansen
explicitly, he'd have a better idea.

> Cc: Jiri Slaby <jslaby@suse.cz>
> ---
>  ipc/mqueue.c |    4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/ipc/mqueue.c b/ipc/mqueue.c
> index 3ced596..f9f0782 100644
> --- a/ipc/mqueue.c
> +++ b/ipc/mqueue.c
> @@ -150,7 +150,7 @@ static struct inode *mqueue_get_inode(struct super_block *sb,
>  			info->attr.mq_msgsize = attr->mq_msgsize;
>  		}
>  		mq_msg_tblsz = info->attr.mq_maxmsg * sizeof(struct msg_msg *);
> -		if (mq_msg_tblsz > KMALLOC_MAX_SIZE)
> +		if (mq_msg_tblsz > PAGE_SIZE)
>  			info->messages = vmalloc(mq_msg_tblsz);
>  		else
>  			info->messages = kmalloc(mq_msg_tblsz, GFP_KERNEL);
> @@ -263,7 +263,7 @@ static void mqueue_evict_inode(struct inode *inode)
>  	spin_lock(&info->lock);
>  	for (i = 0; i < info->attr.mq_curmsgs; i++)
>  		free_msg(info->messages[i]);
> -	if (info->attr.mq_maxmsg * sizeof(struct msg_msg *) > KMALLOC_MAX_SIZE)
> +	if (is_vmalloc_addr(info->messages))
>  		vfree(info->messages);
>  	else
>  		kfree(info->messages);
> -- 
> 1.7.7.6
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

Re: [Patch 7/8] mqueue: separate mqueue default value from maximum value v2

[Serge E. Hallyn]

Quoting Doug Ledford (dledford@redhat.com):
> From: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>
> 
> commit b231cca438 (message queues: increase range limits)
> changed mqueue default value when attr parameter is specified NULL
> from hard coded value to fs.mqueue.{msg,msgsize}_max sysctl value.
> 
> This made large side effect. When user need to use two mqueue
> applications 1) using !NULL attr parameter and it require big
> message size and 2) using NULL attr parameter and only need small
> size message, app (1) require to raise fs.mqueue.msgsize_max and
> app (2) consume large memory size even though it doesn't need.
> 
> Doug Ledford propsed to switch back it to static hard coded value.
> However it also has a compatibility problem. Some applications might
> started depend on the default value is tunable.
> 
> The solution is to separate default value from maximum value.
> 
> Signed-off-by: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>
> Acked-by: Doug Ledford <dledford@redhat.com>
> Acked-by: Joe Korty <joe.korty@ccur.com>
> Cc: Amerigo Wang <amwang@redhat.com>
> Cc: Serge E. Hallyn <serue@us.ibm.com>

Seems reasonable.  Thanks.

Acked-by: Serge E. Hallyn <serge.hallyn@canonical.com>

> Cc: Jiri Slaby <jslaby@suse.cz>
> 
> v1->v2: Slightly modified because msgutil.c no longer needs mq
>   namespace initialization
> 
> Signed-off-by (v2): Doug Ledford <dledford@redhat.com>
> ---
>  Documentation/sysctl/fs.txt   |    7 +++++++
>  include/linux/ipc_namespace.h |    2 ++
>  ipc/mq_sysctl.c               |   18 ++++++++++++++++++
>  ipc/mqueue.c                  |    9 ++++++---
>  4 files changed, 33 insertions(+), 3 deletions(-)
> 
> diff --git a/Documentation/sysctl/fs.txt b/Documentation/sysctl/fs.txt
> index 88fd7f5..13d6166 100644
> --- a/Documentation/sysctl/fs.txt
> +++ b/Documentation/sysctl/fs.txt
> @@ -225,6 +225,13 @@ a queue must be less or equal then msg_max.
>  maximum  message size value (it is every  message queue's attribute set during
>  its creation).
>  
> +/proc/sys/fs/mqueue/msg_default is  a read/write  file for setting/getting the
> +default number of messages in a queue value if attr parameter of mq_open(2) is
> +NULL. If it exceed msg_max, the default value is initialized msg_max.
> +
> +/proc/sys/fs/mqueue/msgsize_default is a read/write file for setting/getting
> +the default message size value if attr parameter of mq_open(2) is NULL. If it
> +exceed msgsize_max, the default value is initialized msgsize_max.
>  
>  4. /proc/sys/fs/epoll - Configuration options for the epoll interface
>  --------------------------------------------------------
> diff --git a/include/linux/ipc_namespace.h b/include/linux/ipc_namespace.h
> index 2488535..5499c92 100644
> --- a/include/linux/ipc_namespace.h
> +++ b/include/linux/ipc_namespace.h
> @@ -62,6 +62,8 @@ struct ipc_namespace {
>  	unsigned int    mq_queues_max;   /* initialized to DFLT_QUEUESMAX */
>  	unsigned int    mq_msg_max;      /* initialized to DFLT_MSGMAX */
>  	unsigned int    mq_msgsize_max;  /* initialized to DFLT_MSGSIZEMAX */
> +	unsigned int    mq_msg_default;
> +	unsigned int    mq_msgsize_default;

Mention here what they are initialized to, for completeness?

>  
>  	/* user_ns which owns the ipc ns */
>  	struct user_namespace *user_ns;
> diff --git a/ipc/mq_sysctl.c b/ipc/mq_sysctl.c
> index e22336a..383d638 100644
> --- a/ipc/mq_sysctl.c
> +++ b/ipc/mq_sysctl.c
> @@ -73,6 +73,24 @@ static ctl_table mq_sysctls[] = {
>  		.extra1		= &msg_maxsize_limit_min,
>  		.extra2		= &msg_maxsize_limit_max,
>  	},
> +	{
> +		.procname	= "msg_default",
> +		.data		= &init_ipc_ns.mq_msg_default,
> +		.maxlen		= sizeof(int),
> +		.mode		= 0644,
> +		.proc_handler	= proc_mq_dointvec_minmax,
> +		.extra1		= &msg_max_limit_min,
> +		.extra2		= &msg_max_limit_max,
> +	},
> +	{
> +		.procname	= "msgsize_default",
> +		.data		= &init_ipc_ns.mq_msgsize_default,
> +		.maxlen		= sizeof(int),
> +		.mode		= 0644,
> +		.proc_handler	= proc_mq_dointvec_minmax,
> +		.extra1		= &msg_maxsize_limit_min,
> +		.extra2		= &msg_maxsize_limit_max,
> +	},
>  	{}
>  };
>  
> diff --git a/ipc/mqueue.c b/ipc/mqueue.c
> index f9f0782..04cc77e 100644
> --- a/ipc/mqueue.c
> +++ b/ipc/mqueue.c
> @@ -142,9 +142,10 @@ static struct inode *mqueue_get_inode(struct super_block *sb,
>  		info->qsize = 0;
>  		info->user = NULL;	/* set when all is ok */
>  		memset(&info->attr, 0, sizeof(info->attr));
> -		info->attr.mq_maxmsg = min(ipc_ns->mq_msg_max, DFLT_MSG);
> -		info->attr.mq_msgsize =
> -			min(ipc_ns->mq_msgsize_max, DFLT_MSGSIZE);
> +		info->attr.mq_maxmsg = min(ipc_ns->mq_msg_max,
> +					   ipc_ns->mq_msg_default);
> +		info->attr.mq_msgsize = min(ipc_ns->mq_msgsize_max,
> +					    ipc_ns->mq_msgsize_default);
>  		if (attr) {
>  			info->attr.mq_maxmsg = attr->mq_maxmsg;
>  			info->attr.mq_msgsize = attr->mq_msgsize;
> @@ -1254,6 +1255,8 @@ int mq_init_ns(struct ipc_namespace *ns)
>  	ns->mq_queues_max    = DFLT_QUEUESMAX;
>  	ns->mq_msg_max       = DFLT_MSGMAX;
>  	ns->mq_msgsize_max   = DFLT_MSGSIZEMAX;
> +	ns->mq_msg_default   = DFLT_MSG;
> +	ns->mq_msgsize_default  = DFLT_MSGSIZE;
>  
>  	ns->mq_mnt = kern_mount_data(&mqueue_fs_type, ns);
>  	if (IS_ERR(ns->mq_mnt)) {
> -- 
> 1.7.7.6
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

Re: [Patch 5/8] mqueue: revert bump up DFLT_*MAX

[KOSAKI Motohiro]

(4/17/12 11:22 PM), Serge E. Hallyn wrote:
> Quoting Doug Ledford (dledford@redhat.com):
>> From: KOSAKI Motohiro<kosaki.motohiro@jp.fujitsu.com>
>>
>> Mqueue limitation is slightly naieve parameter likes other ipcs
>> because unprivileged user can consume kernel memory by using ipcs.
>>
>> Thus, too aggressive raise bring us security issue. Example,
>> current setting allow evil unprivileged user use 256GB (= 256
>> * 1024 * 1024*1024) and it's enough large to system will belome
>> unresponsive. Don't do that.
>>
>> Instead, every admin should adjust the knobs for their own systems.
>
> Would you be terribly averse to having a higher limit in init_ipc_ns,
> and the lower values by default in all child namespaces?

No, I just focused to don't create any regressions. i.e. I mainly focused
no namespace use case. And, I'm sorry, I don't think I clearly understand
recent namespace update. I'm not against any namespace enhancement. Please
only think just I don't understand neither a ipc namespace requirement nor
the code.



> Sorry it sounds from the intro like you've already had quite a bit of
> discussion on this...
>
> Of course I realize the values can just be raised by distro boot
> scripts...

Re: [Patch 2/8] ipc/mqueue: switch back to using non-max values on create

[Doug Ledford]

[-- Attachment #1: Type: text/plain, Size: 2578 bytes --]

On 4/17/2012 7:00 PM, Andrew Morton wrote:
> On Tue, 17 Apr 2012 18:32:25 -0400
> KOSAKI Motohiro <kosaki.motohiro@gmail.com> wrote:
> 
>>> Here the "future patch" is "mqueue: separate mqueue default value from
>>> maximum value v2" in this series, yes?
>>>
>>> So people who have applications which are broken by this patch will
>>> need to manually set /proc/sys/fs/mqueue/msg_default and/or
>>> /proc/sys/fs/mqueue/msgsize_default to get those apps working again?
>>>
>>
>> Yes, it works.
>>
> 
> OK, I updated the changelog for this patch to reflect that.
> 
> I worry a bit that some people who we don't know about will hit this
> problem and will have to spend a lot of time working out why it broke. 
> Is there some way in which we can make it easier for them?  A little
> printk_once() in a suitable place?

It might be doable, although rather tricky.  The deal is that we won't
know on mq_open if they really wanted a default sized mq or a max sized
mq.  And we wouldn't even know on send if they wanted default or max
sized mqs.  The best we could do is flag an mq as being a
no-attr-struct-used mq, then wait and see if they ever try to send too
many or too large of a message to that mq, and then we could do a
printk_once(), but we would almost need a printk_once_per_pid() because
they could have more than one app guilty of this behavior (if any are
guilty of it, something I doubt).  But even with all of this, we
wouldn't necessarily know that the app was intending to get a max sized
mq, it could simply be that the app expected a default sized mq but
tried to send an overly large message because it is a poorly coded app,
or it could have run out of space because the receiving app is blocked.
 So, we could print something out, but personally I'm not entirely
convinced that it's worth the extra lines of code in the kernel to make
it happen properly.  This problem, if it existed in the real world,
would be such poor programming practice that I'm having a hard time
imagining that it really exists.  For a simple, whip it up in 20 minutes
test program as part of a regression suite?  Sure, I can see that.  For
anything intended to be robust and reliable?  No, I can't see this bad
behavior being done there.  But, who knows, maybe I give programmers
writing real programs too much credit...


-- 
Doug Ledford <dledford@redhat.com>
              GPG KeyID: 0E572FDD
	      http://people.redhat.com/dledford

Infiniband specific RPMs available at
	      http://people.redhat.com/dledford/Infiniband


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 898 bytes --]

Re: [Patch 5/8] mqueue: revert bump up DFLT_*MAX

[Doug Ledford]

[-- Attachment #1: Type: text/plain, Size: 1564 bytes --]

On 4/17/2012 11:22 PM, Serge E. Hallyn wrote:
> Quoting Doug Ledford (dledford@redhat.com):
>> From: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>
>>
>> Mqueue limitation is slightly naieve parameter likes other ipcs
>> because unprivileged user can consume kernel memory by using ipcs.
>>
>> Thus, too aggressive raise bring us security issue. Example,
>> current setting allow evil unprivileged user use 256GB (= 256
>> * 1024 * 1024*1024) and it's enough large to system will belome
>> unresponsive. Don't do that.
>>
>> Instead, every admin should adjust the knobs for their own systems.
> 
> Would you be terribly averse to having a higher limit in init_ipc_ns,
> and the lower values by default in all child namespaces?
> 
> Sorry it sounds from the intro like you've already had quite a bit of
> discussion on this...
> 
> Of course I realize the values can just be raised by distro boot
> scripts...

The default maximums this patch put into place were in fact in place
from 2008 until my earlier patch in this same series, so in that regard
this is merely restoring an already established default maximum.  It
*could* be raised, yes, but as Motohiro pointed out, this is pinned
memory that any user can allocate, so the smaller the default amount the
better.  The sysadmin can make changes as they see fit.

-- 
Doug Ledford <dledford@redhat.com>
              GPG KeyID: 0E572FDD
	      http://people.redhat.com/dledford

Infiniband specific RPMs available at
	      http://people.redhat.com/dledford/Infiniband


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 898 bytes --]

Re: [Patch 5/8] mqueue: revert bump up DFLT_*MAX

[Serge E. Hallyn]

Quoting Doug Ledford (dledford@redhat.com):
> On 4/17/2012 11:22 PM, Serge E. Hallyn wrote:
> > Quoting Doug Ledford (dledford@redhat.com):
> >> From: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>
> >>
> >> Mqueue limitation is slightly naieve parameter likes other ipcs
> >> because unprivileged user can consume kernel memory by using ipcs.
> >>
> >> Thus, too aggressive raise bring us security issue. Example,
> >> current setting allow evil unprivileged user use 256GB (= 256
> >> * 1024 * 1024*1024) and it's enough large to system will belome
> >> unresponsive. Don't do that.
> >>
> >> Instead, every admin should adjust the knobs for their own systems.
> > 
> > Would you be terribly averse to having a higher limit in init_ipc_ns,
> > and the lower values by default in all child namespaces?
> > 
> > Sorry it sounds from the intro like you've already had quite a bit of
> > discussion on this...
> > 
> > Of course I realize the values can just be raised by distro boot
> > scripts...
> 
> The default maximums this patch put into place were in fact in place
> from 2008 until my earlier patch in this same series, so in that regard
> this is merely restoring an already established default maximum.  It

Then never mind :)

(My ack stands)

> *could* be raised, yes, but as Motohiro pointed out, this is pinned
> memory that any user can allocate, so the smaller the default amount the
> better.  The sysadmin can make changes as they see fit.

Thanks,
-serge