* [Patch 1/8] ipc/mqueue: cleanup definition names and locations
2012-04-17 15:46 [Patch 0/8] Fix POSIX mqueue open issue Doug Ledford
@ 2012-04-17 15:46 ` Doug Ledford
2012-04-17 17:03 ` KOSAKI Motohiro
2012-04-18 3:14 ` Serge E. Hallyn
2012-04-17 15:46 ` [Patch 2/8] ipc/mqueue: switch back to using non-max values on create Doug Ledford
` (6 subsequent siblings)
7 siblings, 2 replies; 21+ messages in thread
From: Doug Ledford @ 2012-04-17 15:46 UTC (permalink / raw)
To: linux-kernel; +Cc: akpm, kosaki.motohiro, 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
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [Patch 1/8] ipc/mqueue: cleanup definition names and locations
2012-04-17 15:46 ` [Patch 1/8] ipc/mqueue: cleanup definition names and locations Doug Ledford
@ 2012-04-17 17:03 ` KOSAKI Motohiro
2012-04-18 3:14 ` Serge E. Hallyn
1 sibling, 0 replies; 21+ messages in thread
From: KOSAKI Motohiro @ 2012-04-17 17:03 UTC (permalink / raw)
To: Doug Ledford; +Cc: linux-kernel, akpm, 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.
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [Patch 1/8] ipc/mqueue: cleanup definition names and locations
2012-04-17 15:46 ` [Patch 1/8] ipc/mqueue: cleanup definition names and locations Doug Ledford
2012-04-17 17:03 ` KOSAKI Motohiro
@ 2012-04-18 3:14 ` Serge E. Hallyn
1 sibling, 0 replies; 21+ messages in thread
From: Serge E. Hallyn @ 2012-04-18 3:14 UTC (permalink / raw)
To: Doug Ledford; +Cc: linux-kernel, akpm, kosaki.motohiro
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/
^ permalink raw reply [flat|nested] 21+ messages in thread
* [Patch 2/8] ipc/mqueue: switch back to using non-max values on create
2012-04-17 15:46 [Patch 0/8] Fix POSIX mqueue open issue Doug Ledford
2012-04-17 15:46 ` [Patch 1/8] ipc/mqueue: cleanup definition names and locations Doug Ledford
@ 2012-04-17 15:46 ` Doug Ledford
2012-04-17 22:17 ` Andrew Morton
2012-04-17 15:46 ` [Patch 3/8] ipc/mqueue: enforce hard limits Doug Ledford
` (5 subsequent siblings)
7 siblings, 1 reply; 21+ messages in thread
From: Doug Ledford @ 2012-04-17 15:46 UTC (permalink / raw)
To: linux-kernel; +Cc: akpm, kosaki.motohiro, 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
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [Patch 2/8] ipc/mqueue: switch back to using non-max values on create
2012-04-17 15:46 ` [Patch 2/8] ipc/mqueue: switch back to using non-max values on create Doug Ledford
@ 2012-04-17 22:17 ` Andrew Morton
2012-04-17 22:32 ` KOSAKI Motohiro
0 siblings, 1 reply; 21+ messages in thread
From: Andrew Morton @ 2012-04-17 22:17 UTC (permalink / raw)
To: Doug Ledford; +Cc: linux-kernel, kosaki.motohiro
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?
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [Patch 2/8] ipc/mqueue: switch back to using non-max values on create
2012-04-17 22:17 ` Andrew Morton
@ 2012-04-17 22:32 ` KOSAKI Motohiro
2012-04-17 23:00 ` Andrew Morton
0 siblings, 1 reply; 21+ messages in thread
From: KOSAKI Motohiro @ 2012-04-17 22:32 UTC (permalink / raw)
To: Andrew Morton; +Cc: Doug Ledford, linux-kernel, 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.
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [Patch 2/8] ipc/mqueue: switch back to using non-max values on create
2012-04-17 22:32 ` KOSAKI Motohiro
@ 2012-04-17 23:00 ` Andrew Morton
2012-04-18 14:22 ` Doug Ledford
0 siblings, 1 reply; 21+ messages in thread
From: Andrew Morton @ 2012-04-17 23:00 UTC (permalink / raw)
To: KOSAKI Motohiro; +Cc: Doug Ledford, linux-kernel
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?
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [Patch 2/8] ipc/mqueue: switch back to using non-max values on create
2012-04-17 23:00 ` Andrew Morton
@ 2012-04-18 14:22 ` Doug Ledford
0 siblings, 0 replies; 21+ messages in thread
From: Doug Ledford @ 2012-04-18 14:22 UTC (permalink / raw)
To: Andrew Morton; +Cc: KOSAKI Motohiro, linux-kernel
[-- 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 --]
^ permalink raw reply [flat|nested] 21+ messages in thread
* [Patch 3/8] ipc/mqueue: enforce hard limits
2012-04-17 15:46 [Patch 0/8] Fix POSIX mqueue open issue Doug Ledford
2012-04-17 15:46 ` [Patch 1/8] ipc/mqueue: cleanup definition names and locations Doug Ledford
2012-04-17 15:46 ` [Patch 2/8] ipc/mqueue: switch back to using non-max values on create Doug Ledford
@ 2012-04-17 15:46 ` Doug Ledford
2012-04-17 15:46 ` [Patch 4/8] ipc/mqueue: update maximums for the mqueue subsystem Doug Ledford
` (4 subsequent siblings)
7 siblings, 0 replies; 21+ messages in thread
From: Doug Ledford @ 2012-04-17 15:46 UTC (permalink / raw)
To: linux-kernel; +Cc: akpm, kosaki.motohiro, 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
^ permalink raw reply related [flat|nested] 21+ messages in thread* [Patch 4/8] ipc/mqueue: update maximums for the mqueue subsystem
2012-04-17 15:46 [Patch 0/8] Fix POSIX mqueue open issue Doug Ledford
` (2 preceding siblings ...)
2012-04-17 15:46 ` [Patch 3/8] ipc/mqueue: enforce hard limits Doug Ledford
@ 2012-04-17 15:46 ` Doug Ledford
2012-04-17 15:46 ` [Patch 5/8] mqueue: revert bump up DFLT_*MAX Doug Ledford
` (3 subsequent siblings)
7 siblings, 0 replies; 21+ messages in thread
From: Doug Ledford @ 2012-04-17 15:46 UTC (permalink / raw)
To: linux-kernel; +Cc: akpm, kosaki.motohiro, 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
^ permalink raw reply related [flat|nested] 21+ messages in thread* [Patch 5/8] mqueue: revert bump up DFLT_*MAX
2012-04-17 15:46 [Patch 0/8] Fix POSIX mqueue open issue Doug Ledford
` (3 preceding siblings ...)
2012-04-17 15:46 ` [Patch 4/8] ipc/mqueue: update maximums for the mqueue subsystem Doug Ledford
@ 2012-04-17 15:46 ` Doug Ledford
2012-04-18 3:22 ` Serge E. Hallyn
2012-04-17 15:46 ` [Patch 6/8] mqueue: don't use kmalloc with KMALLOC_MAX_SIZE Doug Ledford
` (2 subsequent siblings)
7 siblings, 1 reply; 21+ messages in thread
From: Doug Ledford @ 2012-04-17 15:46 UTC (permalink / raw)
To: linux-kernel
Cc: akpm, kosaki.motohiro, KOSAKI Motohiro, Amerigo Wang,
Serge E. Hallyn, Jiri Slaby
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
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [Patch 5/8] mqueue: revert bump up DFLT_*MAX
2012-04-17 15:46 ` [Patch 5/8] mqueue: revert bump up DFLT_*MAX Doug Ledford
@ 2012-04-18 3:22 ` Serge E. Hallyn
2012-04-18 3:37 ` KOSAKI Motohiro
2012-04-18 14:25 ` Doug Ledford
0 siblings, 2 replies; 21+ messages in thread
From: Serge E. Hallyn @ 2012-04-18 3:22 UTC (permalink / raw)
To: Doug Ledford
Cc: linux-kernel, akpm, kosaki.motohiro, KOSAKI Motohiro,
Amerigo Wang, Serge E. Hallyn, Jiri Slaby
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/
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [Patch 5/8] mqueue: revert bump up DFLT_*MAX
2012-04-18 3:22 ` Serge E. Hallyn
@ 2012-04-18 3:37 ` KOSAKI Motohiro
2012-04-18 14:25 ` Doug Ledford
1 sibling, 0 replies; 21+ messages in thread
From: KOSAKI Motohiro @ 2012-04-18 3:37 UTC (permalink / raw)
To: Serge E. Hallyn
Cc: Doug Ledford, linux-kernel, akpm, kosaki.motohiro,
KOSAKI Motohiro, Amerigo Wang, Serge E. Hallyn, Jiri Slaby
(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...
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [Patch 5/8] mqueue: revert bump up DFLT_*MAX
2012-04-18 3:22 ` Serge E. Hallyn
2012-04-18 3:37 ` KOSAKI Motohiro
@ 2012-04-18 14:25 ` Doug Ledford
2012-04-18 15:33 ` Serge E. Hallyn
1 sibling, 1 reply; 21+ messages in thread
From: Doug Ledford @ 2012-04-18 14:25 UTC (permalink / raw)
To: Serge E. Hallyn
Cc: linux-kernel, akpm, kosaki.motohiro, KOSAKI Motohiro,
Amerigo Wang, Serge E. Hallyn, Jiri Slaby
[-- 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 --]
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [Patch 5/8] mqueue: revert bump up DFLT_*MAX
2012-04-18 14:25 ` Doug Ledford
@ 2012-04-18 15:33 ` Serge E. Hallyn
0 siblings, 0 replies; 21+ messages in thread
From: Serge E. Hallyn @ 2012-04-18 15:33 UTC (permalink / raw)
To: Doug Ledford
Cc: Serge E. Hallyn, linux-kernel, akpm, kosaki.motohiro,
KOSAKI Motohiro, Amerigo Wang, Serge E. Hallyn, Jiri Slaby
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
^ permalink raw reply [flat|nested] 21+ messages in thread
* [Patch 6/8] mqueue: don't use kmalloc with KMALLOC_MAX_SIZE
2012-04-17 15:46 [Patch 0/8] Fix POSIX mqueue open issue Doug Ledford
` (4 preceding siblings ...)
2012-04-17 15:46 ` [Patch 5/8] mqueue: revert bump up DFLT_*MAX Doug Ledford
@ 2012-04-17 15:46 ` Doug Ledford
2012-04-18 3:24 ` Serge E. Hallyn
2012-04-17 15:46 ` [Patch 7/8] mqueue: separate mqueue default value from maximum value v2 Doug Ledford
2012-04-17 15:46 ` [Patch 8/8] selftests: add mq_open_tests Doug Ledford
7 siblings, 1 reply; 21+ messages in thread
From: Doug Ledford @ 2012-04-17 15:46 UTC (permalink / raw)
To: linux-kernel
Cc: akpm, kosaki.motohiro, KOSAKI Motohiro, KOSAKI Motohiro,
Amerigo Wang, Serge E. Hallyn, Jiri Slaby
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
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [Patch 6/8] mqueue: don't use kmalloc with KMALLOC_MAX_SIZE
2012-04-17 15:46 ` [Patch 6/8] mqueue: don't use kmalloc with KMALLOC_MAX_SIZE Doug Ledford
@ 2012-04-18 3:24 ` Serge E. Hallyn
0 siblings, 0 replies; 21+ messages in thread
From: Serge E. Hallyn @ 2012-04-18 3:24 UTC (permalink / raw)
To: Doug Ledford
Cc: linux-kernel, akpm, kosaki.motohiro, KOSAKI Motohiro,
KOSAKI Motohiro, Amerigo Wang, Serge E. Hallyn, Jiri Slaby,
Dave Hansen
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/
^ permalink raw reply [flat|nested] 21+ messages in thread
* [Patch 7/8] mqueue: separate mqueue default value from maximum value v2
2012-04-17 15:46 [Patch 0/8] Fix POSIX mqueue open issue Doug Ledford
` (5 preceding siblings ...)
2012-04-17 15:46 ` [Patch 6/8] mqueue: don't use kmalloc with KMALLOC_MAX_SIZE Doug Ledford
@ 2012-04-17 15:46 ` Doug Ledford
2012-04-18 3:30 ` Serge E. Hallyn
2012-04-17 15:46 ` [Patch 8/8] selftests: add mq_open_tests Doug Ledford
7 siblings, 1 reply; 21+ messages in thread
From: Doug Ledford @ 2012-04-17 15:46 UTC (permalink / raw)
To: linux-kernel
Cc: akpm, kosaki.motohiro, KOSAKI Motohiro, Amerigo Wang,
Serge E. Hallyn, Jiri Slaby
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
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [Patch 7/8] mqueue: separate mqueue default value from maximum value v2
2012-04-17 15:46 ` [Patch 7/8] mqueue: separate mqueue default value from maximum value v2 Doug Ledford
@ 2012-04-18 3:30 ` Serge E. Hallyn
0 siblings, 0 replies; 21+ messages in thread
From: Serge E. Hallyn @ 2012-04-18 3:30 UTC (permalink / raw)
To: Doug Ledford
Cc: linux-kernel, akpm, kosaki.motohiro, KOSAKI Motohiro,
Amerigo Wang, Serge E. Hallyn, Jiri Slaby
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/
^ permalink raw reply [flat|nested] 21+ messages in thread
* [Patch 8/8] selftests: add mq_open_tests
2012-04-17 15:46 [Patch 0/8] Fix POSIX mqueue open issue Doug Ledford
` (6 preceding siblings ...)
2012-04-17 15:46 ` [Patch 7/8] mqueue: separate mqueue default value from maximum value v2 Doug Ledford
@ 2012-04-17 15:46 ` Doug Ledford
7 siblings, 0 replies; 21+ messages in thread
From: Doug Ledford @ 2012-04-17 15:46 UTC (permalink / raw)
To: linux-kernel; +Cc: akpm, kosaki.motohiro, 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
^ permalink raw reply related [flat|nested] 21+ messages in thread