Various cleanups and a couple fixes to ipc/mqueue

Various cleanups and a couple fixes to ipc/mqueue

[Doug Ledford]

We had a customer come up with a problem while trying to upgrade from
our 2.6.18 kernel to our 2.6.32 kernel.  In diagnosing their problem,
it was determined that when commit b231cca4 changed the msg size max
from INT_MAX to 8192*128, that's what broke their setup.  While fixing
this problem, testing showed that if you increase the max values of a
msg queue, then attempt to create one without an attr struct passed in
to the open call, it could fail because it sets the queue size to the
max of both the msg size and queue size.  If these are large enough,
they over run the default RLIMIT_MSGQUEUE.  This change was also
introduced in the b231cca4 commit.  We then found that the msg queue
limits were not all being enforced on CAP_SYS_RESOURCE apps.  Finally,
we found that commit 9cf18e1d fiddled with HARD_MSGMAX without
realizing that the reason it was set to what it was, was to avoid
trying to kmalloc a chunk larger than 128K.  So, this series of
patches cleans up the various defines, takes us back to having a
larger HARD_MSGSIZEMAX, goes back to using a separate define for the
case where a user doesn't pass in an attr struct in case the maxes
have been raised too large for RLIMIT_MSGQUEUE, enforces the maximums
on CAP_SYS_RESOURCE apps, uses vmalloc instead of kmalloc when the
msg pointer array is too large, and documents all of this so it
shouldn't happen again.

Authors of the two original commits have been Cc:ed in case they want
to have any input.

Patches are also available via git at:

git://github.com/dledford/linux.git upstream/mqueue

[PATCH 1/4] ipc/mqueue: cleanup definition names and locations
[PATCH 2/4] ipc/mqueue: switch back to using non-max values on
[PATCH 3/4] ipc/mqueue: enforce hard limits
[PATCH 4/4] ipc/mqueue: update maximums for the mqueue subsystem

 include/linux/ipc_namespace.h |   40 +++++++++++++++++++++++++++++++++++-----
 ipc/mq_sysctl.c               |   31 ++++++++-----------------------
 ipc/mqueue.c                  |   23 ++++++++++++++++-------
 3 files changed, 59 insertions(+), 35 deletions(-)

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

[PATCH 1/4] 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.6

[PATCH 2/4] 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.

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 ed049ea..c6597fc 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.6

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

[KOSAKI Motohiro]

> 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.
>
> Signed-off-by: Doug Ledford <dledford@redhat.com>

NAK.

Commit b231cca is 3 years old commit. It's too late to revert.
Moreover Commit b231cca changed hardcoded limit to kernel knob,
and this patch do reverse. Of course, latter direction changes almost
always bring us compatibility issue.

   commit b231cca4381ee15ec99afbfb244fbc0324869927
   Author: Joe Korty <joe.korty@ccur.com>
   Date:   Sat Oct 18 20:28:32 2008 -0700

       message queues: increase range limits

The right way is to create new knob likes mqueue/msgsize_default
and to separate default and maximum value.

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

[KOSAKI Motohiro]

> NAK.
> 
> Commit b231cca is 3 years old commit. It's too late to revert.
> Moreover Commit b231cca changed hardcoded limit to kernel knob,
> and this patch do reverse. Of course, latter direction changes almost
> always bring us compatibility issue.
> 
>    commit b231cca4381ee15ec99afbfb244fbc0324869927
>    Author: Joe Korty <joe.korty@ccur.com>
>    Date:   Sat Oct 18 20:28:32 2008 -0700
> 
>        message queues: increase range limits
> 
> The right way is to create new knob likes mqueue/msgsize_default
> and to separate default and maximum value.

I discussed with Doug a little. OK, I probably understand his motivation.
Now, I ack this series with a few reservation.

I'll post a few incremental patches for resolving compatibility issue.

[PATCH 5/4] ipc/mqueue: revert bump up DFLT_*MAX

[KOSAKI Motohiro]

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@gmail.com>
Cc: Doug Ledford <dledford@redhat.com>
Cc: Amerigo Wang <amwang@redhat.com>
Cc: Serge E. Hallyn <serue@us.ibm.com>
Cc: Jiri Slaby <jslaby@suse.cz>
Cc: Joe Korty <joe.korty@ccur.com>
---
 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 e2bac00..2d7c5e0 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.5.2

Re: [PATCH 5/4] ipc/mqueue: revert bump up DFLT_*MAX

[Doug Ledford]

----- Original Message -----
> 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.

The RLIMIT_MSGQUEUE will kick in at 819200 bytes.  The purpose of
changing the default maximums was simply to allow applications
a little more flexibility in how they use their 819200 byte limit,
not to allow them an unbounded limit.  And even if you raise the
RLIMIT value, there is still the fact that msgqueue bytes are
accounted on a per user id basis and there is a hard limit of
2GB (INT_MAX) on any given user id.  So the situation isn't as
dire as you painted ;-)

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

Given that there are knobs to adjust things, I'm fine putting the
defaults back where they were.

Acked-by: Doug Ledford <dledford@redhat.com>

> Signed-off-by: KOSAKI Motohiro <kosaki.motohiro@gmail.com>
> Cc: Doug Ledford <dledford@redhat.com>
> Cc: Amerigo Wang <amwang@redhat.com>
> Cc: Serge E. Hallyn <serue@us.ibm.com>
> Cc: Jiri Slaby <jslaby@suse.cz>
> Cc: Joe Korty <joe.korty@ccur.com>
> ---
>  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 e2bac00..2d7c5e0 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.5.2
> 
> 
> --
> 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/
> 

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

Re: [PATCH 5/4] ipc/mqueue: revert bump up DFLT_*MAX

[Joe Korty]

On Wed, Oct 26, 2011 at 11:37:10AM -0400, KOSAKI Motohiro wrote:
> 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@gmail.com>
> Cc: Doug Ledford <dledford@redhat.com>
> Cc: Amerigo Wang <amwang@redhat.com>
> Cc: Serge E. Hallyn <serue@us.ibm.com>
> Cc: Jiri Slaby <jslaby@suse.cz>
> Cc: Joe Korty <joe.korty@ccur.com>
> ---
>  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 e2bac00..2d7c5e0 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.5.2

Acked-by: Joe Korty <joe.korty@ccur.com>

[PATCH 6/4] ipc/mqueue: don't use kmalloc(KMALLOC_MAX_SIZE)

[KOSAKI Motohiro]

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 <kosaki.motohiro@gmail.com>
Cc: Doug Ledford <dledford@redhat.com>
Cc: Amerigo Wang <amwang@redhat.com>
Cc: Serge E. Hallyn <serue@us.ibm.com>
Cc: Jiri Slaby <jslaby@suse.cz>
Cc: Joe Korty <joe.korty@ccur.com>
---
 ipc/mqueue.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/ipc/mqueue.c b/ipc/mqueue.c
index 229a5fb..91ca145 100644
--- a/ipc/mqueue.c
+++ b/ipc/mqueue.c
@@ -151,7 +151,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);
@@ -275,7 +275,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.5.2

Re: [PATCH 6/4] ipc/mqueue: don't use kmalloc(KMALLOC_MAX_SIZE)

[Doug Ledford]

----- Original Message -----
> 
> 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 <kosaki.motohiro@gmail.com>
> Cc: Doug Ledford <dledford@redhat.com>
> Cc: Amerigo Wang <amwang@redhat.com>
> Cc: Serge E. Hallyn <serue@us.ibm.com>
> Cc: Jiri Slaby <jslaby@suse.cz>
> Cc: Joe Korty <joe.korty@ccur.com>

Acked-by: Doug Ledford <dledford@redhat.com>

> ---
>  ipc/mqueue.c |    4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/ipc/mqueue.c b/ipc/mqueue.c
> index 229a5fb..91ca145 100644
> --- a/ipc/mqueue.c
> +++ b/ipc/mqueue.c
> @@ -151,7 +151,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);
> @@ -275,7 +275,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.5.2
> 
> 
> --
> 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/
> 

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

Re: [PATCH 6/4] ipc/mqueue: don't use kmalloc(KMALLOC_MAX_SIZE)

[Joe Korty]

On Wed, Oct 26, 2011 at 11:37:48AM -0400, KOSAKI Motohiro wrote:
> 
> 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 <kosaki.motohiro@gmail.com>
> Cc: Doug Ledford <dledford@redhat.com>
> Cc: Amerigo Wang <amwang@redhat.com>
> Cc: Serge E. Hallyn <serue@us.ibm.com>
> Cc: Jiri Slaby <jslaby@suse.cz>
> Cc: Joe Korty <joe.korty@ccur.com>
> ---
>  ipc/mqueue.c |    4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/ipc/mqueue.c b/ipc/mqueue.c
> index 229a5fb..91ca145 100644
> --- a/ipc/mqueue.c
> +++ b/ipc/mqueue.c
> @@ -151,7 +151,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);
> @@ -275,7 +275,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.5.2

Acked-by: Joe Korty <joe.korty@ccur.com>

[PATCH 7/4] ipc/mqueue: separate mqueue default value from maximum value

[KOSAKI Motohiro]

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@gmail.com>
Cc: Doug Ledford <dledford@redhat.com>
Cc: Amerigo Wang <amwang@redhat.com>
Cc: Serge E. Hallyn <serue@us.ibm.com>
Cc: Jiri Slaby <jslaby@suse.cz>
Cc: Joe Korty <joe.korty@ccur.com>
---
 Documentation/sysctl/fs.txt   |    7 +++++++
 include/linux/ipc_namespace.h |    3 +++
 ipc/mq_sysctl.c               |   18 ++++++++++++++++++
 ipc/mqueue.c                  |    9 ++++++---
 ipc/msgutil.c                 |    2 ++
 5 files changed, 36 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 2d7c5e0..9e25a33 100644
--- a/include/linux/ipc_namespace.h
+++ b/include/linux/ipc_namespace.h
@@ -63,6 +63,9 @@ struct ipc_namespace {
 	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 91ca145..f762fe6 100644
--- a/ipc/mqueue.c
+++ b/ipc/mqueue.c
@@ -143,9 +143,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;
@@ -1262,6 +1263,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)) {
diff --git a/ipc/msgutil.c b/ipc/msgutil.c
index 8b5ce5d3..a344216 100644
--- a/ipc/msgutil.c
+++ b/ipc/msgutil.c
@@ -31,6 +31,8 @@ struct ipc_namespace init_ipc_ns = {
 	.mq_queues_max   = DFLT_QUEUESMAX,
 	.mq_msg_max      = DFLT_MSGMAX,
 	.mq_msgsize_max  = DFLT_MSGSIZEMAX,
+	.mq_msg_default	    = DFLT_MSG,
+	.mq_msgsize_default = DFLT_MSGSIZE,
 #endif
 	.user_ns = &init_user_ns,
 };
-- 
1.7.5.2

Re: [PATCH 7/4] ipc/mqueue: separate mqueue default value from maximum value

[Doug Ledford]

----- Original Message -----
> 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@gmail.com>
> Cc: Doug Ledford <dledford@redhat.com>
> Cc: Amerigo Wang <amwang@redhat.com>
> Cc: Serge E. Hallyn <serue@us.ibm.com>
> Cc: Jiri Slaby <jslaby@suse.cz>
> Cc: Joe Korty <joe.korty@ccur.com>

Acked-by: Doug Ledford <dledford@redhat.com>

> ---
>  Documentation/sysctl/fs.txt   |    7 +++++++
>  include/linux/ipc_namespace.h |    3 +++
>  ipc/mq_sysctl.c               |   18 ++++++++++++++++++
>  ipc/mqueue.c                  |    9 ++++++---
>  ipc/msgutil.c                 |    2 ++
>  5 files changed, 36 insertions(+), 3 deletions(-)

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

Re: [PATCH 7/4] ipc/mqueue: separate mqueue default value from maximum value

[Joe Korty]

On Wed, Oct 26, 2011 at 11:38:35AM -0400, KOSAKI Motohiro wrote:
> 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@gmail.com>
> Cc: Doug Ledford <dledford@redhat.com>
> Cc: Amerigo Wang <amwang@redhat.com>
> Cc: Serge E. Hallyn <serue@us.ibm.com>
> Cc: Jiri Slaby <jslaby@suse.cz>
> Cc: Joe Korty <joe.korty@ccur.com>
> ---
>  Documentation/sysctl/fs.txt   |    7 +++++++
>  include/linux/ipc_namespace.h |    3 +++
>  ipc/mq_sysctl.c               |   18 ++++++++++++++++++
>  ipc/mqueue.c                  |    9 ++++++---
>  ipc/msgutil.c                 |    2 ++
>  5 files changed, 36 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
>  --------------------------------------------------------


Very much needed; maximums applied as defaults was a
bad idea.

Acked-by: Joe Korty <joe.korty@ccur.com>



> diff --git a/include/linux/ipc_namespace.h b/include/linux/ipc_namespace.h
> index 2d7c5e0..9e25a33 100644
> --- a/include/linux/ipc_namespace.h
> +++ b/include/linux/ipc_namespace.h
> @@ -63,6 +63,9 @@ struct ipc_namespace {
>  	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 91ca145..f762fe6 100644
> --- a/ipc/mqueue.c
> +++ b/ipc/mqueue.c
> @@ -143,9 +143,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;
> @@ -1262,6 +1263,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)) {
> diff --git a/ipc/msgutil.c b/ipc/msgutil.c
> index 8b5ce5d3..a344216 100644
> --- a/ipc/msgutil.c
> +++ b/ipc/msgutil.c
> @@ -31,6 +31,8 @@ struct ipc_namespace init_ipc_ns = {
>  	.mq_queues_max   = DFLT_QUEUESMAX,
>  	.mq_msg_max      = DFLT_MSGMAX,
>  	.mq_msgsize_max  = DFLT_MSGSIZEMAX,
> +	.mq_msg_default	    = DFLT_MSG,
> +	.mq_msgsize_default = DFLT_MSGSIZE,
>  #endif
>  	.user_ns = &init_user_ns,
>  };
> -- 
> 1.7.5.2

[PATCH 3/4] 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 c6597fc..7cda828 100644
--- a/ipc/mqueue.c
+++ b/ipc/mqueue.c
@@ -310,8 +310,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;
 	}
@@ -591,7 +592,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.6

[PATCH 4/4] 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..ceeef68 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 7cda828..0474ddb 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;
 
@@ -271,7 +274,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.6

Re: [PATCH 4/4] ipc/mqueue: update maximums for the mqueue subsystem

[KOSAKI Motohiro]

2011/9/27 Doug Ledford <dledford@redhat.com>:
> 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..ceeef68 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

NAK.
To change hard coded limit is safe and we should restore
pre b231cca438 value.
However, to increase DFLT_*MAX is wrong idea. mqueue data can't
be swapped out. Thus, this patch increase a chance fo DoS attack
by unprivileged user.

You have to change only HARD_*MAX.