POSIX message queues

POSIX message queues

[Krzysztof Benedyczak]

 Hello

After getting some response from lkml we are ready for
work on new version of POSIX message queues.

Main difference (as Christoph Hellwig suggested) would be
implementing it as a virtual filesystem (based on tmpfs and
parts of Jakub Jelinek code).
I think that we can agree that idea of moving whole stuff to
user space isn't good. There is still a problem with SIGEV_THREAD
version of notification but after (brief) looking into NPTL it
should be possible to implement (in difference to NGPT)


So our question is:
Is above version acceptable for Linux kernel?

Main advantages of such approach are: no need for new
system call and no mess in fork/exit.


 Krzysztof Benedyczak

POSIX message queues

[Krzysztof Benedyczak]

Hello

For quite a long time there are two implementations of posix mqueues
around. I think it is time to decide at least if both of them have
chances of beeing applied to official kernel. So I would
like to know if Peter Waechtler's implementations is considered superior
or there is possible some discussion and further work on our
implementation is worthwhile.

There are a lot of differencies but if the most important one is use of
ioctl vs syscalls it can be changed (in fact our implementation loong time
ago used syscalls).

In another words: is our implementation in the position
of NGPT or better? ;-)

Regards
Krzysiek

Re: POSIX message queues

[Manfred Spraul]

Krzysztof Benedyczak wrote:

>Hello
>
>For quite a long time there are two implementations of posix mqueues
>around. I think it is time to decide at least if both of them have
>chances of beeing applied to official kernel. So I would
>like to know if Peter Waechtler's implementations is considered superior
>or there is possible some discussion and further work on our
>implementation is worthwhile.
>  
>
Could you try to merge your work? Or at least: look at each others work. 
For example Krzysiek/Michal's implementation has wake-one semantics, 
which is IMHO a requirement.

Krzysiek: What is MQ_IOC_CLOSE? It looks like a stale ioctl. Please 
remove such code from the patch.

The last time I looked at your patch I noticed a race between creation and setting queue attributes. Did you fix that?


>There are a lot of differencies but if the most important one is use of
>ioctl vs syscalls it can be changed (in fact our implementation loong time
>ago used syscalls).
>  
>
I personally prefer syscalls, but that's just my personal preference. 
For example the notification info is a structure, and printing it to a 
text stream and then parsing it back again is just odd. And I don't see 
how you can fix the O_CREAT+unusual mq_maxmsg races.
Why do you check against MQ_MAXMSG in user space? That's wrong. The 
kernel will reject too large limits, probably depending on 
/proc/sys/kern/ configuration. Checking in user space doesn't gain 
anything, except that you loose the ability for runtime changes.
Please reuse the load_msg/store_msg functions instead of a 
kmalloc(arg.msg_len, GFP_KERNEL) + copy_from_user. kmalloc(16384) is not 
reliable - it needs a continuous block of 16 kB, and after a long 
runtime, the memory is so fragmented that such memory may not exist. 
This is a known problem for x86_64: They would prefer to have 16 kB 
blocks for the stack, but this results in errors during stress testing.
proc_write_max_queues: off-by-one error. tmp[16] ='\0' overwrites the stack.
Is is necessary that the filesystem is visible to user space? What about 
chroot environments, or environments with per-user mount points. I don't 
like the dependence on /proc/mounts.

>In another words: is our implementation in the position
>of NGPT or better? ;-)
>  
>
Do you know if Ulrich Drepper has looked at your user space libraries? 
Your code must end up in glibc, and he's the maintainer.

--
    Manfred

Re: POSIX message queues

[Ulrich Drepper]

Krzysztof Benedyczak wrote:

> There are a lot of differencies but if the most important one is use of
> ioctl vs syscalls it can be changed (in fact our implementation loong time
> ago used syscalls).

Syscalls are always better.  At least from my perspective.  Just imagine
how the runtime should determine that the kernel doesn't support msqs?
With syscalls I get -ENOSYS back.  With ioctls I get EINVAL.  But what
this mean?  Functionality not available?  Invalid parameters to the
existing implementation?

ALso think about strace which is an important part in many peoples life.
 Hiding the functionality in some ioctls doesn't make it easy to follow
the program even if strace gets even more code added to the ioctl decoder.

Basically, demultiplexers are bad.  Syscalls are cheap.


> In another words: is our implementation in the position
> of NGPT or better? ;-)

I don't understand.  Why NGPT and what about "position"?  If you mean
including a solution in the runtime (librt), sure, this will happen.
But not before I see a solution in the official kernel.

-- 
--------------.                        ,-.            444 Castro Street
Ulrich Drepper \    ,-----------------'   \ Mountain View, CA 94041 USA
Red Hat         `--' drepper at redhat.com `---------------------------

Re: POSIX message queues

[Jamie Lokier]

Ulrich Drepper wrote:
> > In another words: is our implementation in the position
> > of NGPT or better? ;-)
> 
> I don't understand.  Why NGPT and what about "position"?

He is asking if the work will be wasted effort that is dismissed or
superceded, like NGPT was.

> If you mean
> including a solution in the runtime (librt), sure, this will happen.
> But not before I see a solution in the official kernel.

Speaking of librt - I should not have to link in pthreads and the
run-time overhead associated with it (locking stdio etc.) just so I
can use shm_open().  Any chance of fixing this?

-- Jamie

Re: POSIX message queues

[Jakub Jelinek]

On Sun, Oct 05, 2003 at 07:16:30PM +0100, Jamie Lokier wrote:
> Ulrich Drepper wrote:
> > > In another words: is our implementation in the position
> > > of NGPT or better? ;-)
> > 
> > I don't understand.  Why NGPT and what about "position"?
> 
> He is asking if the work will be wasted effort that is dismissed or
> superceded, like NGPT was.
> 
> > If you mean
> > including a solution in the runtime (librt), sure, this will happen.
> > But not before I see a solution in the official kernel.
> 
> Speaking of librt - I should not have to link in pthreads and the
> run-time overhead associated with it (locking stdio etc.) just so I
> can use shm_open().  Any chance of fixing this?

That overhead is mostly gone in current glibcs (when using NPTL):
a) e.g. locking is done unconditionally even when libpthread is not present
   (it is just lock cmpxchgl, inlined)
b) things like cancellation aware syscall wrappers for cancellable syscalls
   and various other things are only done after first pthread_create has
   been called, it doesn't matter whether libpthread is loaded or not

	Jakub

Re: POSIX message queues

[Jamie Lokier]

Jakub Jelinek wrote:
> > Speaking of librt - I should not have to link in pthreads and the
> > run-time overhead associated with it (locking stdio etc.) just so I
> > can use shm_open().  Any chance of fixing this?
> 
> That overhead is mostly gone in current glibcs (when using NPTL):
> a) e.g. locking is done unconditionally even when libpthread is not present
>    (it is just lock cmpxchgl, inlined)
> b) things like cancellation aware syscall wrappers for cancellable syscalls
>    and various other things are only done after first pthread_create has
>    been called, it doesn't matter whether libpthread is loaded or not

That's good.  I still don't like linking in pthreads when I'm not
using threads or any thread-using services, so I'll continue to use a
non-libc version of shm_open() in my own programs, particularly the
ones which use clone() directly.

Why isn't shm_open() simply part of libc?

-- Jamie

Re: POSIX message queues

[Ulrich Drepper]

Jamie Lokier wrote:

> Why isn't shm_open() simply part of libc?

Because it's defined to be part of librt by POSIX.

-- 
--------------.                        ,-.            444 Castro Street
Ulrich Drepper \    ,-----------------'   \ Mountain View, CA 94041 USA
Red Hat         `--' drepper at redhat.com `---------------------------

Re: POSIX message queues

[Krzysztof Benedyczak]

On Sun, 5 Oct 2003, Manfred Spraul wrote:

> Krzysiek: What is MQ_IOC_CLOSE? It looks like a stale ioctl. Please
> remove such code from the patch.
It is used. If this ioctl will succeed we know that it was done on mqueue
fs file. And thanks to it we get rid of the possibility of closing
ordinary file descriptor with mq_close().

 >
> The last time I looked at your patch I noticed a race between creation
> and setting queue attributes. Did you fix that?
Yes - as Alan Cox suggested. But see below.


> I personally prefer syscalls, but that's just my personal preference.
In our opinion also - and that was the reason why we initially had done
this with syscall. But this was criticized. Mostly but Christopher Hellwig
AFAIR. So we changed it ;-) ... Anyway:

Removing ioctls has mostly advantages (maybe except checking for
permissions) and it's simply. Reusing code of msg_load/free/store - also
no problem. Third issue is filesystem. IMHO removing it from userspace is
unnecessary. It gives a lot of valuable informations (about
notifications which can't be gather with POSIX calls). It is also
convenient to rm queue to poll it.
The things I think should be changed:
mqueues should be be accessible from the module init time
touch can create queue with system limits (?)
multiple mounts of mqueue fs should show the same content.

With this functionality we will have some more convenient queues.
Is it sensible?

(
Implementing with syscalls will also solve proc/mounts dependency (which
BTW can be turned of in library configure).
)


Regards
Krzysiek

Re: POSIX message queues

[Peter Waechtler]

 
On Sunday, October 05, 2003, at 08:32PM, Jakub Jelinek <jakub@redhat.com> wrote:

>> Speaking of librt - I should not have to link in pthreads and the
>> run-time overhead associated with it (locking stdio etc.) just so I
>> can use shm_open().  Any chance of fixing this?
>
>That overhead is mostly gone in current glibcs (when using NPTL):
>a) e.g. locking is done unconditionally even when libpthread is not present
>   (it is just lock cmpxchgl, inlined)


a "lock cmpxchg" is > 100 cycles (according to a recent Linux Journal article
from Paul McKenney: 107ns on 700MHz PentiumIII)

But I think you will have benchmarked the alternatives?
BTW, what are they?

you suggested naming the syscall number symbols NR_mq_open instead of
NR_sys_mq_open. In the stub I want to overload some syscalls (e.g. mq_open)
but others not (e.g. mq_timedsend).

How to deal with that?

Re: POSIX message queues

[Jakub Jelinek]

On Tue, Oct 07, 2003 at 09:50:16AM +0200, Peter Waechtler wrote:
>  
> On Sunday, October 05, 2003, at 08:32PM, Jakub Jelinek <jakub@redhat.com> wrote:
> 
> >> Speaking of librt - I should not have to link in pthreads and the
> >> run-time overhead associated with it (locking stdio etc.) just so I
> >> can use shm_open().  Any chance of fixing this?
> >
> >That overhead is mostly gone in current glibcs (when using NPTL):
> >a) e.g. locking is done unconditionally even when libpthread is not present
> >   (it is just lock cmpxchgl, inlined)
> 
> 
> a "lock cmpxchg" is > 100 cycles (according to a recent Linux Journal article
> from Paul McKenney: 107ns on 700MHz PentiumIII)

Here is exactly what it does on IA-32/i686+:

# define lll_lock(futex) \
  (void) ({ int ignore1, ignore2;                                             \
            __asm __volatile ("cmpl $0, %%gs:%P6\n\t"                         \
                              "je,pt 0f\n\t"                                  \
                              "lock\n"                                        \
                              "0:\tcmpxchgl %1, %2\n\t"                       \
                              "jnz _L_mutex_lock_%=\n\t"                      \
                              ".subsection 1\n\t"                             \
                              ".type _L_mutex_lock_%=,@function\n"            \
                              "_L_mutex_lock_%=:\n\t"                         \
                              "leal %2, %%ecx\n\t"                            \
                              "call __lll_mutex_lock_wait\n\t"                \
                              "jmp 1f\n\t"                                    \
                              ".size _L_mutex_lock_%=,.-_L_mutex_lock_%=\n"   \
                              ".previous\n"                                   \
                              "1:"                                            \
                              : "=a" (ignore1), "=c" (ignore2), "=m" (futex)  \
                              : "0" (0), "1" (1), "m" (futex),                \
                                "i" (offsetof (tcbhead_t, multiple_threads))  \
                              : "memory"); })

> you suggested naming the syscall number symbols NR_mq_open instead of
> NR_sys_mq_open. In the stub I want to overload some syscalls (e.g. mq_open)
> but others not (e.g. mq_timedsend).
> 
> How to deal with that?

The syscall is still mq_open, isn't it? So it should be __NR_mq_open.
You simply put the ones where you want to implement them directly in the kernel
into syscalls.list (and add sysdeps/generic/mq_*.c stubs; also, mq_{timed,}{receive,send}
are cancellation points according to POSIX 2003, so they need to be marked as such).
Where you need to do some handling before/after the syscall in userland, you simply write
mq_open.c etc. and use INLINE_SYSCALL (or INTERNAL_SYSCALL) in it.

	Jakub

posix message queues

[Andrew Morton]

Manfred, if/when this gets merged up the long-suffering arch maintainers
might appreciate a little test app to exercise the syscalls which must be
added.  Do you have something suitable at hand?


Also, before we go too far the 64-bit guys may like to comment on the
syscall interface.   For example, this:

struct mq_attr {
	long	mq_flags;	/* message queue flags			*/
	long	mq_maxmsg;	/* maximum number of messages		*/
	long	mq_msgsize;	/* maximum message size			*/
	long	mq_curmsgs;	/* number of messages currently queued	*/
	long	__reserved[4];	/* ignored for input, zeroed for output */
};

looks like it will require emulation for 32-bit apps.  But if we were to
make these __u32 perhaps that could be avoided?

The patches are at

ftp://ftp.kernel.org/pub/linux/kernel/people/akpm/patches/2.6/2.6.5/2.6.5-mc2/broken-out/mq-01-codemove.patch
ftp://ftp.kernel.org/pub/linux/kernel/people/akpm/patches/2.6/2.6.5/2.6.5-mc2/broken-out/mq-02-syscalls.patch
ftp://ftp.kernel.org/pub/linux/kernel/people/akpm/patches/2.6/2.6.5/2.6.5-mc2/broken-out/mq-03-core-update.patch
ftp://ftp.kernel.org/pub/linux/kernel/people/akpm/patches/2.6/2.6.5/2.6.5-mc2/broken-out/mq-03-core.patch
ftp://ftp.kernel.org/pub/linux/kernel/people/akpm/patches/2.6/2.6.5/2.6.5-mc2/broken-out/mq-04-linuxext-poll.patch
ftp://ftp.kernel.org/pub/linux/kernel/people/akpm/patches/2.6/2.6.5/2.6.5-mc2/broken-out/mq-05-linuxext-mount.patch
ftp://ftp.kernel.org/pub/linux/kernel/people/akpm/patches/2.6/2.6.5/2.6.5-mc2/broken-out/mq-security-fix.patch
ftp://ftp.kernel.org/pub/linux/kernel/people/akpm/patches/2.6/2.6.5/2.6.5-mc2/broken-out/mq-update-01.patch

Re: posix message queues

[Manfred Spraul]

Andrew Morton wrote:

>Manfred, if/when this gets merged up the long-suffering arch maintainers
>might appreciate a little test app to exercise the syscalls which must be
>added.  Do you have something suitable at hand?
>  
>
I have a few separate apps, I'll merge them into one app and post it to 
lkml. Probably tomorrow or Friday.

>
>Also, before we go too far the 64-bit guys may like to comment on the
>syscall interface.   For example, this:
>
>struct mq_attr {
>	long	mq_flags;	/* message queue flags			*/
>	long	mq_maxmsg;	/* maximum number of messages		*/
>	long	mq_msgsize;	/* maximum message size			*/
>	long	mq_curmsgs;	/* number of messages currently queued	*/
>	long	__reserved[4];	/* ignored for input, zeroed for output */
>};
>
>looks like it will require emulation for 32-bit apps.  But if we were to
>make these __u32 perhaps that could be avoided?
>  
>
No. The unix spec mandates 'long' for the final C-library api. Using u32 
would mean that glibc would have to add an emulation layer in user space 
for 64-bit archs. In the long run, 64-bit kernels running 64-bit apps 
will be the common case. I don't want to add an emulation layer to the 
common case.

--
    Manfred

Re: posix message queues

[Arnd Bergmann]

On Wednesday 07 April 2004 21:15, you wrote:
> >looks like it will require emulation for 32-bit apps.  But if we were to
> >make these __u32 perhaps that could be avoided?
> >  
> >
> No. The unix spec mandates 'long' for the final C-library api. Using u32 
> would mean that glibc would have to add an emulation layer in user space 
> for 64-bit archs. In the long run, 64-bit kernels running 64-bit apps 
> will be the common case. I don't want to add an emulation layer to the 
> common case.

Coincidentally, I have just finished writing handlers for them. 
I have tested the code with the open posix test suite and found the
same four failures for both 64-bit and compat mode, most tests
pass. The patch is against -mc1, but I guess it also applies to the
other trees around.

What worries me more than mq_attr compatibility is the conversion
of struct sigevent, which might turn out really hard when more
fields in there are used. AFAICS, the only other part in the kernel
ABI is sys_timer_create(), so maybe it's not too late to deprecate
the current structure and create a structure that can be used
properly for compat syscalls.

	Arnd <><

http://www.arndb.de/patches/linux/2.6.5-mc1/compat_mq.diff

include/linux/compat.h      |   16 +++
include/linux/mqueue.h      |    4 
include/linux/posix_types.h |    1 
include/linux/syscalls.h    |    1 
include/linux/types.h       |    1 
ipc/Makefile                |    3 
ipc/compat_mq.c             |  185 ++++++++++++++++++++++++++++++++++++++++++++
kernel/sys.c                |    5 +
  8 files changed, 212 insertions(+), 4 deletions(-)

diff -Nru a/include/linux/compat.h b/include/linux/compat.h
--- a/include/linux/compat.h	Thu Apr  8 01:50:22 2004
+++ b/include/linux/compat.h	Thu Apr  8 01:50:22 2004
@@ -90,6 +90,22 @@
 	compat_uptr_t	sival_ptr;
 } compat_sigval_t;
 
+typedef struct compat_sigevent {
+	compat_sigval_t sigev_value;
+	compat_int_t sigev_signo;
+	compat_int_t sigev_notify;
+	union {
+		compat_int_t _pad[SIGEV_PAD_SIZE];
+		compat_int_t _tid;
+
+		struct {
+			compat_uptr_t _function;
+			compat_uptr_t _attribute;
+		} _sigev_thread;
+	} _sigev_un;
+} compat_sigevent_t;
+
+
 long compat_sys_semctl(int first, int second, int third, void __user *uptr);
 long compat_sys_msgsnd(int first, int second, int third, void __user *uptr);
 long compat_sys_msgrcv(int first, int second, int msgtyp, int third,
diff -Nru a/include/linux/mqueue.h b/include/linux/mqueue.h
--- a/include/linux/mqueue.h	Thu Apr  8 01:50:22 2004
+++ b/include/linux/mqueue.h	Thu Apr  8 01:50:22 2004
@@ -18,9 +18,9 @@
 #ifndef _LINUX_MQUEUE_H
 #define _LINUX_MQUEUE_H
 
-#define MQ_PRIO_MAX 	32768
+#include <linux/types.h>
 
-typedef int mqd_t;
+#define MQ_PRIO_MAX 	32768
 
 struct mq_attr {
 	long	mq_flags;	/* message queue flags			*/
diff -Nru a/include/linux/posix_types.h b/include/linux/posix_types.h
--- a/include/linux/posix_types.h	Thu Apr  8 01:50:22 2004
+++ b/include/linux/posix_types.h	Thu Apr  8 01:50:22 2004
@@ -42,6 +42,7 @@
 
 /* Type of a SYSV IPC key.  */
 typedef int __kernel_key_t;
+typedef int __kernel_mqd_t;
 
 #include <asm/posix_types.h>
 
diff -Nru a/include/linux/syscalls.h b/include/linux/syscalls.h
--- a/include/linux/syscalls.h	Thu Apr  8 01:50:22 2004
+++ b/include/linux/syscalls.h	Thu Apr  8 01:50:22 2004
@@ -48,7 +48,6 @@
 struct timezone;
 struct tms;
 struct utimbuf;
-typedef int mqd_t;
 struct mq_attr;
 
 #include <linux/config.h>
diff -Nru a/include/linux/types.h b/include/linux/types.h
--- a/include/linux/types.h	Thu Apr  8 01:50:22 2004
+++ b/include/linux/types.h	Thu Apr  8 01:50:22 2004
@@ -31,6 +31,7 @@
 typedef __kernel_suseconds_t	suseconds_t;
 typedef __kernel_timer_t	timer_t;
 typedef __kernel_clockid_t	clockid_t;
+typedef __kernel_mqd_t		mqd_t;
 
 #ifdef __KERNEL__
 typedef __kernel_uid32_t	uid_t;
diff -Nru a/ipc/Makefile b/ipc/Makefile
--- a/ipc/Makefile	Thu Apr  8 01:50:22 2004
+++ b/ipc/Makefile	Thu Apr  8 01:50:22 2004
@@ -4,5 +4,6 @@
 
 obj-$(CONFIG_SYSVIPC_COMPAT) += compat.o
 obj-$(CONFIG_SYSVIPC) += util.o msgutil.o msg.o sem.o shm.o
-obj-$(CONFIG_POSIX_MQUEUE) += mqueue.o msgutil.o
+obj_mq-$(CONFIG_COMPAT) += compat_mq.o
+obj-$(CONFIG_POSIX_MQUEUE) += mqueue.o msgutil.o $(obj_mq-y)
 
diff -Nru a/ipc/compat_mq.c b/ipc/compat_mq.c
--- /dev/null	Wed Dec 31 16:00:00 1969
+++ b/ipc/compat_mq.c	Thu Apr  8 01:50:22 2004
@@ -0,0 +1,185 @@
+/*
+ *  ipc/compat_mq.c
+ *    32 bit emulation for POSIX message queue system calls
+ *
+ *    Copyright (C) 2004 IBM Deutschland Entwicklung GmbH, IBM Corporation
+ *    Author: Arnd Bergmann <arnd@arndb.de>
+ */
+
+#include <linux/compat.h>
+#include <linux/fs.h>
+#include <linux/kernel.h>
+#include <linux/mqueue.h>
+#include <linux/syscalls.h>
+
+#include <asm/uaccess.h>
+
+struct compat_mq_attr {
+	compat_long_t mq_flags;      /* message queue flags		     */
+	compat_long_t mq_maxmsg;     /* maximum number of messages	     */
+	compat_long_t mq_msgsize;    /* maximum message size		     */
+	compat_long_t mq_curmsgs;    /* number of messages currently queued  */
+	compat_long_t __reserved[4]; /* ignored for input, zeroed for output */
+};
+
+static inline int get_compat_mq_attr(struct mq_attr *attr,
+			const struct compat_mq_attr __user *uattr)
+{
+	if (verify_area(VERIFY_READ, uattr, sizeof *uattr))
+		return -EFAULT;
+
+	return __get_user(attr->mq_flags, &uattr->mq_flags)
+		| __get_user(attr->mq_maxmsg, &uattr->mq_maxmsg)
+		| __get_user(attr->mq_msgsize, &uattr->mq_msgsize)
+		| __get_user(attr->mq_curmsgs, &uattr->mq_curmsgs);
+}
+
+static inline int put_compat_mq_attr(const struct mq_attr *attr,
+			struct compat_mq_attr __user *uattr)
+{
+	if (clear_user(uattr, sizeof *uattr))
+		return -EFAULT;
+
+	return __put_user(attr->mq_flags, &uattr->mq_flags)
+		| __put_user(attr->mq_maxmsg, &uattr->mq_maxmsg)
+		| __put_user(attr->mq_msgsize, &uattr->mq_msgsize)
+		| __put_user(attr->mq_curmsgs, &uattr->mq_curmsgs);
+}
+
+asmlinkage long compat_sys_mq_open(const char __user *u_name,
+			int oflag, compat_mode_t mode,
+			struct compat_mq_attr __user *u_attr)
+{
+	struct mq_attr attr;
+	mm_segment_t oldfs;
+	char *name;
+	long ret;
+
+	if (!u_attr)
+		return sys_mq_open(u_name, oflag, mode, 0);
+
+	if (get_compat_mq_attr(&attr, u_attr))
+		return -EFAULT;
+
+	name = getname(u_name);
+	if (IS_ERR(name))
+		return PTR_ERR(name);
+
+	oldfs = get_fs();
+	set_fs(KERNEL_DS);
+	ret = sys_mq_open(name, oflag, mode, &attr);
+	set_fs(oldfs);
+
+	putname(name);
+	return ret;
+}
+
+static struct timespec __user *compat_prepare_timeout(
+			const struct compat_timespec __user *u_abs_timeout)
+{
+	struct timespec ts, __user *u_ts;
+
+	if (!u_abs_timeout)
+		return 0;
+
+	u_ts = compat_alloc_user_space(sizeof(*u_ts));
+	if (get_compat_timespec(&ts, u_abs_timeout)
+		|| copy_to_user(u_ts, &ts, sizeof(*u_ts)))
+		return ERR_PTR(-EFAULT);
+
+	return u_ts;
+}
+
+asmlinkage long compat_sys_mq_timedsend(mqd_t mqdes,
+			const char __user *u_msg_ptr,
+			size_t msg_len, unsigned int msg_prio,
+			const struct compat_timespec __user *u_abs_timeout)
+{
+	struct timespec __user *u_ts;
+
+	u_ts = compat_prepare_timeout(u_abs_timeout);
+	if (IS_ERR(u_ts))
+		return -EFAULT;
+
+	return sys_mq_timedsend(mqdes, u_msg_ptr, msg_len,
+			msg_prio, u_ts);
+}
+
+asmlinkage ssize_t compat_sys_mq_timedreceive(mqd_t mqdes,
+			char __user *u_msg_ptr,
+			size_t msg_len, unsigned int __user *u_msg_prio,
+			const struct compat_timespec __user *u_abs_timeout)
+{
+	struct timespec *u_ts;
+
+	u_ts = compat_prepare_timeout(u_abs_timeout);
+	if (IS_ERR(u_ts))
+		return -EFAULT;
+
+	return sys_mq_timedreceive(mqdes, u_msg_ptr, msg_len,
+			u_msg_prio, u_ts);
+}
+
+static int get_compat_sigevent(struct sigevent *event,
+		const struct compat_sigevent __user *u_event)
+{
+	if (verify_area(VERIFY_READ, u_event, sizeof(*u_event)))
+		return -EFAULT;
+
+	return __get_user(event->sigev_value.sival_int,
+			  &u_event->sigev_value.sival_int)
+	     | __get_user(event->sigev_signo, &u_event->sigev_signo)
+	     | __get_user(event->sigev_notify, &u_event->sigev_notify)
+	     | __get_user(event->sigev_notify_thread_id,
+			  &u_event->sigev_notify_thread_id);
+}
+
+asmlinkage long compat_sys_mq_notify(mqd_t mqdes,
+			const struct compat_sigevent __user *u_notification)
+{
+	mm_segment_t oldfs;
+	struct sigevent notification;
+	long ret;
+
+	if (!u_notification)
+		return sys_mq_notify(mqdes, 0);
+
+	if (get_compat_sigevent(&notification, u_notification))
+		return -EFAULT;
+
+	oldfs = get_fs();
+	set_fs(KERNEL_DS);
+	ret = sys_mq_notify(mqdes, &notification);
+	set_fs(oldfs);
+
+	return ret;
+}
+
+asmlinkage long compat_sys_mq_getsetattr(mqd_t mqdes,
+			const struct compat_mq_attr __user *u_mqstat,
+			struct compat_mq_attr __user *u_omqstat)
+{
+	struct mq_attr mqstat, omqstat;
+	struct mq_attr *p_mqstat = 0, *p_omqstat = 0;
+	mm_segment_t oldfs;
+	long ret;
+
+	if (u_mqstat) {
+		p_mqstat = &mqstat;
+		if (get_compat_mq_attr(p_mqstat, u_mqstat))
+			return -EFAULT;
+	}
+
+	if (u_omqstat)
+		p_omqstat = &omqstat;
+	
+	oldfs = get_fs();
+	set_fs(KERNEL_DS);
+	ret = sys_mq_getsetattr(mqdes, p_mqstat, p_omqstat);
+	set_fs(oldfs);
+
+	if (ret)
+		return ret;
+
+	return (u_omqstat) ? put_compat_mq_attr(&omqstat, u_omqstat) : 0;
+}
diff -Nru a/kernel/sys.c b/kernel/sys.c
--- a/kernel/sys.c	Thu Apr  8 01:50:22 2004
+++ b/kernel/sys.c	Thu Apr  8 01:50:22 2004
@@ -266,6 +266,11 @@
 cond_syscall(sys_mq_timedreceive)
 cond_syscall(sys_mq_notify)
 cond_syscall(sys_mq_getsetattr)
+cond_syscall(compat_sys_mq_open)
+cond_syscall(compat_sys_mq_timedsend)
+cond_syscall(compat_sys_mq_timedreceive)
+cond_syscall(compat_sys_mq_notify)
+cond_syscall(compat_sys_mq_getsetattr)
 
 /* arch-specific weak syscall entries */
 cond_syscall(sys_pciconfig_read)

Re: posix message queues

[Andrew Morton]

Arnd Bergmann <arnd@arndb.de> wrote:
>
> > No. The unix spec mandates 'long' for the final C-library api. Using u32 
>  > would mean that glibc would have to add an emulation layer in user space 
>  > for 64-bit archs. In the long run, 64-bit kernels running 64-bit apps 
>  > will be the common case. I don't want to add an emulation layer to the 
>  > common case.
> 
>  Coincidentally, I have just finished writing handlers for them. 




In file included from kernel/sys.c:8:
include/linux/compat.h:98: error: `SIGEV_PAD_SIZE' undeclared here (not in a function)


---

 25-akpm/include/linux/compat.h |    1 +
 1 files changed, 1 insertion(+)

diff -puN include/linux/compat.h~compat_mq-fix include/linux/compat.h
--- 25/include/linux/compat.h~compat_mq-fix	2004-04-08 01:49:19.539180328 -0700
+++ 25-akpm/include/linux/compat.h	2004-04-08 01:49:19.541180024 -0700
@@ -13,6 +13,7 @@
 #include <linux/sem.h>
 
 #include <asm/compat.h>
+#include <asm/siginfo.h>
 
 #define compat_jiffies_to_clock_t(x)	\
 		(((unsigned long)(x) * COMPAT_USER_HZ) / HZ)

_

Re: posix message queues

[Manfred Spraul]

Arnd Bergmann wrote:

>What worries me more than mq_attr compatibility is the conversion
>of struct sigevent, which might turn out really hard when more
>fields in there are used.
>
What is the problem for the wrappers with sigevent?

>+
>+static int get_compat_sigevent(struct sigevent *event,
>+		const struct compat_sigevent __user *u_event)
>+{
>+	if (verify_area(VERIFY_READ, u_event, sizeof(*u_event)))
>+		return -EFAULT;
>+
>+	return __get_user(event->sigev_value.sival_int,
>+			  &u_event->sigev_value.sival_int)
>+	     | __get_user(event->sigev_signo, &u_event->sigev_signo)
>+	     | __get_user(event->sigev_notify, &u_event->sigev_notify)
>+	     | __get_user(event->sigev_notify_thread_id,
>+			  &u_event->sigev_notify_thread_id);
>+}
>+
>
The dangerous case for the wrappers is mq_notify for SIGEV_THREAD: The 
kernel will read (with copy_from_user()) from the pointer 
u_event->sigev_value.sival_ptr. Fixed 32-byte buffer. The data is opaque 
- the kernel copies it back to the same process without any 
interpretation. Would that work on all archs?

--
    Manfred

Re: posix message queues

[Andrew Morton]

Arnd Bergmann <arnd@arndb.de> wrote:
>
> > No. The unix spec mandates 'long' for the final C-library api. Using u32 
>  > would mean that glibc would have to add an emulation layer in user space 
>  > for 64-bit archs. In the long run, 64-bit kernels running 64-bit apps 
>  > will be the common case. I don't want to add an emulation layer to the 
>  > common case.
> 
>  Coincidentally, I have just finished writing handlers for them.

It broke ppc64.   Please check other architectures for the same pattern.



In file included from arch/ppc64/kernel/signal.c:29:
include/asm/ppc32.h:144: redefinition of `struct compat_sigevent'
include/asm/ppc32.h:156: redefinition of `compat_sigevent_t'
include/linux/compat.h:107: `compat_sigevent_t' previously declared here


---

 25-power4-akpm/include/asm-ppc64/ppc32.h |   14 --------------
 1 files changed, 14 deletions(-)

diff -puN include/asm-ppc64/ppc32.h~compat_mq-ppc-fix include/asm-ppc64/ppc32.h
--- 25-power4/include/asm-ppc64/ppc32.h~compat_mq-ppc-fix	2004-04-08 13:21:48.262814248 -0700
+++ 25-power4-akpm/include/asm-ppc64/ppc32.h	2004-04-08 13:22:35.262669184 -0700
@@ -141,20 +141,6 @@ struct ucontext32 { 
 	struct mcontext32	uc_mcontext;
 };
 
-typedef struct compat_sigevent {
-	compat_sigval_t sigev_value;
-	int sigev_signo;
-	int sigev_notify;
-	union {
-		int _pad[SIGEV_PAD_SIZE];
-		int _tid;
-		struct {
-			compat_uptr_t _function;
-			compat_uptr_t _attribute;
-		} _sigev_thread;
-	} _sigev_un;
-} compat_sigevent_t;
-
 struct ipc_kludge_32 {
 	unsigned int msgp;
 	int msgtyp;

_

Re: posix message queues

[Arnd Bergmann]

Andrew Morton <akpm@osdl.org> schrieb am 08.04.2004, 22:24:28:
> 
> It broke ppc64.   Please check other architectures for the same pattern.
> 
> In file included from arch/ppc64/kernel/signal.c:29:
> include/asm/ppc32.h:144: redefinition of `struct compat_sigevent'
> include/asm/ppc32.h:156: redefinition of `compat_sigevent_t'
> include/linux/compat.h:107: `compat_sigevent_t' previously declared here

D'oh! I remember thinking that would break but forgot to fix it. On the 
other architectures, the existing structure is called sigevent_t32 or
similar, so it does not conflict but should still be changed to use
the common definition.

      Arnd <><

Re: posix message queues

[David S. Miller]

On Wed, 07 Apr 2004 21:15:44 +0200
Manfred Spraul <manfred@colorfullife.com> wrote:

> No. The unix spec mandates 'long' for the final C-library api. Using u32 
> would mean that glibc would have to add an emulation layer in user space 
> for 64-bit archs. In the long run, 64-bit kernels running 64-bit apps 
> will be the common case. I don't want to add an emulation layer to the 
> common case.

We need to support this stuff when running 32-bit apps, you cannot
avoid this Manfred.  So we'll need kernel compat layer code to
translate these structures containing 'long'.  Please code up the
necessary compat.c code for this posix message queues feature.

Thanks.

Re: posix message queues

[Manfred Spraul]

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

David S. Miller wrote:

>On Wed, 07 Apr 2004 21:15:44 +0200
>Manfred Spraul <manfred@colorfullife.com> wrote:
>
>  
>
>>No. The unix spec mandates 'long' for the final C-library api. Using u32 
>>would mean that glibc would have to add an emulation layer in user space 
>>for 64-bit archs. In the long run, 64-bit kernels running 64-bit apps 
>>will be the common case. I don't want to add an emulation layer to the 
>>common case.
>>    
>>
>
>We need to support this stuff when running 32-bit apps, you cannot
>avoid this Manfred.
>
Of course. But the compat stuff is only used if a 32-bit app runs on a 
64-bit kernel. If I'd use u32 for the kernel interface, then a compat 
wrappers would be necessary if a 64-bit app runs on a 64-bit kernel, and 
that would be the wrong thing.

>  So we'll need kernel compat layer code to
>translate these structures containing 'long'.  Please code up the
>necessary compat.c code for this posix message queues feature.
>  
>
Arndt did 90% of the coding, it's already in 2.6.5-mm3. The only missing 
part was the SIGEV_THREAD handling, I've attached a patch that adds 
this, together a small user space test app.

--
    Manfred

[-- Attachment #2: mq_test.c --]
[-- Type: text/x-csrc, Size: 9849 bytes --]

/*
 * Test app for Linux Kernel posix message queues.
 *
 * Copyright (C) 2004 Manfred Spraul
 *
 * Redistribution of this file is permitted under the terms of the GNU 
 * Public License (GPL)
 * $Header: /home/manfred/cvs-tree/mq/mq_test.c,v 1.3 2004/04/10 11:17:31 manfred Exp $
 */
#define _GNU_SOURCE

#include <stdio.h>
#include <stdlib.h>
#include <unistd.h>
#include <errno.h>
#include <sys/types.h>
#include <sys/stat.h>
#include <sys/socket.h>
#include <fcntl.h>
#include <string.h>
#include <limits.h>


/*
 * Kernel-interface based on the Mqueue library:
 *
 * Copyright (C) 2003 Krzysztof Benedyczak & Michal Wronski

   This program is free software; you can redistribute it and/or
   modify it under the terms of the GNU Lesser General Public
   License as published by the Free Software Foundation; either
   version 2.1 of the License, or (at your option) any later version.

   It 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
   Lesser General Public License for more details.

   You should have received a copy of the GNU Lesser General Public
   License along with this software; if not, write to the Free
   Software Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA
   02111-1307 USA.
 */

/****************** implementation *****************/

#define __NR_mq_open		277

#define __NR_mq_unlink		(__NR_mq_open+1)
#define __NR_mq_timedsend	(__NR_mq_open+2)
#define __NR_mq_timedreceive	(__NR_mq_open+3)
#define __NR_mq_notify		(__NR_mq_open+4)
#define __NR_mq_getsetattr	(__NR_mq_open+5)


#define MQ_PRIO_MAX 	32768

typedef int mqd_t;

struct mq_attr {
	long	mq_flags;	/* message queue flags			*/
	long	mq_maxmsg;	/* maximum number of messages		*/
	long	mq_msgsize;	/* maximum message size			*/
	long	mq_curmsgs;	/* number of messages currently queued	*/
	long	__reserved[4];	/* unused */
};

#include <signal.h>
#include <fcntl.h>
#include <time.h>
#include <stdarg.h>

mqd_t mq_open(const char *name, int oflag, /* mode_t mode, struct mq_attr *attr */ ...);
int mq_close(mqd_t mqdes);
int mq_unlink(const char *name);
int mq_send(mqd_t mqdes, const char *msg_ptr, size_t msg_len, unsigned int msg_prio);
int mq_timedsend(mqd_t mqdes, const char *msg_ptr, size_t msg_len, unsigned int msg_prio, const struct timespec *abs_timeout);
ssize_t mq_receive(mqd_t mqdes, char *msg_ptr, size_t msg_len, unsigned int *msg_prio);
ssize_t mq_timedreceive(mqd_t mqdes, char *__restrict msg_ptr, size_t msg_len, unsigned int *__restrict msg_prio, const struct timespec *__restrict abs_timeout);
int mq_notify(mqd_t mqdes, const struct sigevent *notification);
int mq_getattr(mqd_t mqdes, struct mq_attr *mqstat);
int mq_setattr(mqd_t mqdes, const struct mq_attr *__restrict mqstat, struct mq_attr *__restrict omqstat);

/*
 * kernel interfaces.  We use glibc's syscall(3) instead of the macros
 * _syscall1, _syscall2, etc, as the macros generate compilation errors
 * when mqueue.c is built as a dynamic shared library.
 */
static inline mqd_t __mq_open(const char  *name,
				int oflag, mode_t mode, struct mq_attr* attr)
{
	return syscall(__NR_mq_open, name, oflag, mode, attr);
}

static inline int __mq_unlink(const char *name)
{
	return syscall(__NR_mq_unlink, name);
}

static inline int __mq_notify(mqd_t mqdes, const struct sigevent *notification)
{
	return syscall(__NR_mq_notify, mqdes, notification);
}

static inline int __mq_getsetattr(mqd_t mqdes, const struct mq_attr *mqstat, struct mq_attr *omqstat)
{
	return syscall(__NR_mq_getsetattr, mqdes, mqstat, omqstat);
}

static inline int __mq_timedsend(mqd_t mqdes, const char *msg_ptr,
				size_t msg_len, unsigned int msg_prio,
				const struct timespec *abs_timeout)
{
	return syscall(__NR_mq_timedsend, mqdes, msg_ptr, msg_len,
		msg_prio, abs_timeout);
}

static inline ssize_t __mq_timedreceive(mqd_t mqdes, char *msg_ptr,
				size_t msg_len, unsigned int *msg_prio,
				const struct timespec *abs_timeout)
{
	return syscall(__NR_mq_timedreceive, mqdes, msg_ptr, msg_len,
		msg_prio, abs_timeout);
}

static inline int is_valid_path(const char *name)
{
	if (name[0] != '/') {
		errno = EINVAL;
		return -1;
	}
	return 0;
}
/*
 * application-visible wrappers around the kernel interfaces
 */

mqd_t mq_open(const char *name, int oflag, ...)
{
	unsigned long 	mode;
	struct mq_attr 	*attr;
	va_list 	ap;

	va_start(ap, oflag);
	mode = va_arg(ap, unsigned long);
	attr = va_arg(ap, struct mq_attr *);
	va_end(ap);

	if (is_valid_path(name) < 0)
		return (mqd_t)-1;

	/* omit leading slash */
	return __mq_open(name + 1, oflag, mode, attr);
}

int mq_close(mqd_t mqdes)
{
	return close(mqdes);
}

int mq_unlink(const char *name)
{
	if (is_valid_path(name) < 0)
		return -1;
	
	return __mq_unlink(name+1);
}

int mq_send(mqd_t mqdes, const char *msg_ptr, size_t msg_len,
					unsigned int msg_prio)
{
	return __mq_timedsend(mqdes, msg_ptr, msg_len, msg_prio, NULL);
}

ssize_t mq_receive(mqd_t mqdes, char *msg_ptr, size_t msg_len,
					unsigned int *msg_prio)
{
	return __mq_timedreceive(mqdes, msg_ptr, msg_len, msg_prio, NULL);
}

int mq_timedsend(mqd_t mqdes, const char *msg_ptr, size_t msg_len,
					unsigned int msg_prio,
					const struct timespec *abs_timeout)
{
	return __mq_timedsend(mqdes, msg_ptr, msg_len, msg_prio, abs_timeout);
}

ssize_t mq_timedreceive(mqd_t mqdes, char *msg_ptr, size_t msg_len,
					unsigned int *msg_prio,
					const struct timespec *abs_timeout)
{
	return __mq_timedreceive(mqdes, msg_ptr, msg_len, msg_prio,
		abs_timeout);
}

int mq_getattr(mqd_t mqdes, struct mq_attr *mqstat)
{
	return __mq_getsetattr(mqdes, NULL, mqstat);
}

int mq_setattr( mqd_t mqdes, const struct mq_attr *mqstat,
					struct mq_attr *omqstat)
{
	return __mq_getsetattr(mqdes, mqstat, omqstat);
}

int mq_notify(mqd_t mqdes, const struct sigevent *notification)
{
	return __mq_notify(mqdes, notification);
}

int got_signal = 0;
void usr1_handler(int unused)
{
	printf("usr1 signal caught.\n");
	got_signal = 1;
}

char buffer[131072];

int main(int argc, char **argv)
{
static const char qname[] = "/test_123";
        int err;
        mqd_t m, m2;
	struct mq_attr ma;

	printf("Test 1: mq_open()\n");
	m = mq_open(qname, O_CREAT|O_RDWR, S_IRUSR | S_IWUSR, NULL);
	if (m == -1) {
	        printf("mq_open() failed,  errno %d.\n", errno);
		return 1;
	}

	printf("Test 2: check exclusive open()\n");
	m2 = mq_open(qname, O_EXCL|O_CREAT|O_RDWR, S_IRUSR | S_IWUSR, NULL);
	if (m2 != -1 || errno != EEXIST) {
	        printf("mq_open() succeeded or wrong errno (%d).\n", errno);
		return 1;
	}

	printf("Test 3: close handle\n");
	err = mq_close(m);

	if (err) {
		printf("mq_close() failed.\n");
	}

	printf("Test 4: mq_unlink\n");
	err = mq_unlink(qname);
	if (err) {
	        printf("mq_unlink() failed,  errno %d.\n", errno);
		return 1;
	}

	printf("Test 4: mq_unlink on nonexisting file\n");
	err = mq_unlink(qname);
	if (err != -1 || errno != ENOENT) {
	        printf("mq_unlink() failed,  errno %d.\n", errno);
		return 1;
	}

	printf("Test 5: mq_open()\n");
	m = mq_open(qname, O_CREAT|O_EXCL|O_RDWR, S_IRUSR | S_IWUSR, NULL);
	if (m == -1) {
	        printf("mq_open() failed,  errno %d.\n", errno);
		return 1;
	}

	printf("Test 6: mq_getattr()\n");
	err = mq_getattr(m, &ma);
	if (err || ma.mq_curmsgs != 0) {
	        printf("mq_getattr() failed,  errno %d, curmsg %ld.\n", errno, ma.mq_curmsgs);
		return 1;
	}

	printf("Test 7: setup signal based notify().\n");

	signal (SIGUSR1,usr1_handler);

	struct sigevent event;
	event.sigev_notify = SIGEV_SIGNAL;
	event.sigev_signo = SIGUSR1;
	err = mq_notify(m, &event);
	if (err) {
	        printf("mq_notify() failed,  errno %d.\n", errno);
		return 1;
	}

	printf("Test 8: mq_send()\n");
	err = mq_send(m, qname, strlen(qname), 1);
	if (err) {
	        printf("mq_send() failed,  errno %d.\n", errno);
		return 1;
	}
	if (!got_signal) {
		printf("No signal received.\n");
		return 1;
	}

	printf("Test 9: mq_getattr() with one pending message\n");
	err = mq_getattr(m, &ma);
	if (err || ma.mq_curmsgs != 1) {
	        printf("mq_getattr() failed,  errno %d, curmsg %ld.\n", errno, ma.mq_curmsgs);
		return 1;
	}

	printf("Test 10: mq_receive()\n");
	err = mq_receive(m, buffer, sizeof(buffer), NULL);
	if (err < 0) {
	        printf("mq_receive() failed,  errno %d.\n", errno);
		return 1;
	}

	printf("Test 11: data comparison\n");
	if (strcmp(qname, buffer)) {
		printf("Data mismatch!!!.\n");
		return 1;
	}

	printf("Test 12: request notification cookie.\n");
	{
		int fd, i;
		char inc[32], outc[64];


		fd = socket(PF_NETLINK, SOCK_RAW, 0);
		if (fd == -1) {
			printf("opening netlink socket failed, errno %d.\n", errno);
			return 1;
		}

		for (i=0;i<32;i++)
			inc[i] = 64+2*i;

		event.sigev_notify = SIGEV_THREAD;
		event.sigev_signo = fd;
		event.sigev_value.sival_ptr = inc;
		err = mq_notify(m, &event);
		if (err < 0) {
			 printf("mq_notify() failed,  errno %d.\n", errno);
			return 1;
		}
		err = mq_send(m, qname, strlen(qname), 1);
		if (err) {
		        printf("mq_send() failed,  errno %d.\n", errno);
			return 1;
		}
		err = recv(fd, outc, sizeof(outc), MSG_NOSIGNAL|MSG_DONTWAIT);
		if (err != 32) {
			printf("recv unexpected result %d, errno %d.\n", err, errno);
			return 1;
		}
		for (i=0;i<31;i++) {
			if (outc[i] != 64+2*i) {
				printf("Data mismatch in cookie at offset %d: %d.\n",
						i, outc[i]);
				return 1;
			}
		}
		if (outc[31] != 1) {
			printf("state mismatch in cookie: got %d.\n", outc[31]);
		}
	}

	printf("Test 13: another mq_unlink\n");
	err = mq_unlink(qname);
	if (err) {
	        printf("mq_unlink() failed,  errno %d.\n", errno);
		return 1;
	}

	printf("Test 14: mq_close()\n");
	err = mq_close(m);
	if (err) {
		printf("mq_close() failed, errno %d.\n", errno);
		return 1;
	}
	return 0;
}

[-- Attachment #3: patch-mqueue-compat --]
[-- Type: text/plain, Size: 793 bytes --]

// $Header$
// Kernel Version:
//  VERSION = 2
//  PATCHLEVEL = 6
//  SUBLEVEL = 5
//  EXTRAVERSION = -mm3
--- 2.6/ipc/compat_mq.c	2004-04-10 09:59:40.000000000 +0200
+++ build-2.6/ipc/compat_mq.c	2004-04-10 13:15:40.414231437 +0200
@@ -139,6 +139,7 @@
 {
 	mm_segment_t oldfs;
 	struct sigevent notification;
+	char cookie[NOTIFY_COOKIE_LEN];
 	long ret;
 
 	if (!u_notification)
@@ -147,6 +148,14 @@
 	if (get_compat_sigevent(&notification, u_notification))
 		return -EFAULT;
 
+	if (notification.sigev_notify == SIGEV_THREAD) {
+		if (copy_from_user(cookie, u_notification.sigev_value.sival_ptr,
+					NOTIFY_COOKIE_LEN)) {
+			return -EFAULT;
+		}
+		notification.sigev_value.sival_ptr = cookie;
+	}
+
 	oldfs = get_fs();
 	set_fs(KERNEL_DS);
 	ret = sys_mq_notify(mqdes, &notification);

Re: posix message queues

[Manfred Spraul]

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

Manfred Spraul wrote:

> 
>+if (notification.sigev_notify == SIGEV_THREAD) {
>+	if (copy_from_user(cookie, u_notification.sigev_value.sival_ptr,
>
notification, not u_notification: sival_ptr is a union on sival_int, and 
sival_int was copied to kernel space by get_compat_sigevent.
Updated patch attached, sorry.

--
    Manfred

[-- Attachment #2: patch-mqueue-compat --]
[-- Type: text/plain, Size: 791 bytes --]

// $Header$
// Kernel Version:
//  VERSION = 2
//  PATCHLEVEL = 6
//  SUBLEVEL = 5
//  EXTRAVERSION = -mm3
--- 2.6/ipc/compat_mq.c	2004-04-10 09:59:40.000000000 +0200
+++ build-2.6/ipc/compat_mq.c	2004-04-10 13:46:26.713887203 +0200
@@ -139,6 +139,7 @@
 {
 	mm_segment_t oldfs;
 	struct sigevent notification;
+	char cookie[NOTIFY_COOKIE_LEN];
 	long ret;
 
 	if (!u_notification)
@@ -147,6 +148,14 @@
 	if (get_compat_sigevent(&notification, u_notification))
 		return -EFAULT;
 
+	if (notification.sigev_notify == SIGEV_THREAD) {
+		if (copy_from_user(cookie, notification.sigev_value.sival_ptr,
+					NOTIFY_COOKIE_LEN)) {
+			return -EFAULT;
+		}
+		notification.sigev_value.sival_ptr = cookie;
+	}
+
 	oldfs = get_fs();
 	set_fs(KERNEL_DS);
 	ret = sys_mq_notify(mqdes, &notification);

Re: posix message queues

[Arnd Bergmann]

On Saturday 10 April 2004 13:53, Manfred Spraul wrote:
> Manfred Spraul wrote:
> 
> > 
> >+if (notification.sigev_notify == SIGEV_THREAD) {
> >+      if (copy_from_user(cookie, u_notification.sigev_value.sival_ptr,
> >
> notification, not u_notification: sival_ptr is a union on sival_int, and 
> sival_int was copied to kernel space by get_compat_sigevent.
> Updated patch attached, sorry.

I'm afraid that is still incorrect,  at least on big-endian machines:

> @@ -147,6 +148,14 @@
>         if (get_compat_sigevent(&notification, u_notification))
>                 return -EFAULT;
>  
> +       if (notification.sigev_notify == SIGEV_THREAD) {
> +               if (copy_from_user(cookie, notification.sigev_value.sival_ptr,
> +                                       NOTIFY_COOKIE_LEN)) {
> +                       return -EFAULT;
> +               }
> +               notification.sigev_value.sival_ptr = cookie;
> +       }

sival_ptr is not valid when passed through get_compat_sigevent, because
the 32 bit pointer might be copied to the upper half of the kernel pointer.
Even worse, the infamous s390 31 bit pointer conversion is missing.
This patch fixes that problem and two others reported by Jakub.

> > Arndt did 90% of the coding, it's already in 2.6.5-mm3.
BTW, my name is 'Arnd', not 'Arndt'. I hope we can stop this now before
this becomes some 'Russel' vs. 'Russell' problem ;-)

	Arnd <><

--
[PATCH] More fixups for compat_mq
- handle SIGEV_THREAD
- don't try to convert u_attr in sys_mq_open if !O_CREAT 
- handle __SI_MESGQ in copy_siginfo_to_user32  (still missing for
  ppc64 and parisc)
===== ipc/compat_mq.c 1.1 vs edited =====
--- 1.1/ipc/compat_mq.c	Sat Apr 10 16:46:12 2004
+++ edited/ipc/compat_mq.c	Sat Apr 10 20:48:02 2004
@@ -55,7 +55,7 @@
 	char *name;
 	long ret;
 
-	if (!u_attr)
+	if ((oflag & O_CREAT) == 0 || !u_attr)
 		return sys_mq_open(u_name, oflag, mode, 0);
 
 	if (get_compat_mq_attr(&attr, u_attr))
@@ -139,6 +139,8 @@
 {
 	mm_segment_t oldfs;
 	struct sigevent notification;
+	char cookie[NOTIFY_COOKIE_LEN];
+	compat_uptr_t u_cookie;
 	long ret;
 
 	if (!u_notification)
@@ -146,6 +148,15 @@
 
 	if (get_compat_sigevent(&notification, u_notification))
 		return -EFAULT;
+
+	if (notification.sigev_notify == SIGEV_THREAD) {
+		u_cookie = (compat_uptr_t)notification.sigev_value.sival_int;
+		if (copy_from_user(cookie, compat_ptr(u_cookie),
+						NOTIFY_COOKIE_LEN)) {
+			return -EFAULT;
+		}
+		notification.sigev_value.sival_ptr = cookie;
+	}
 
 	oldfs = get_fs();
 	set_fs(KERNEL_DS);
===== arch/ia64/ia32/ia32_signal.c 1.24 vs edited =====
--- 1.24/arch/ia64/ia32/ia32_signal.c	Wed Feb 25 11:31:13 2004
+++ edited/arch/ia64/ia32/ia32_signal.c	Sat Apr 10 21:06:07 2004
@@ -114,7 +114,12 @@
 			err |= __get_user(to->si_band, &from->si_band);
 			err |= __get_user(to->si_fd, &from->si_fd);
 			break;
-			/* case __SI_RT: This is not generated by the kernel as of now.  */
+		      case __SI_RT: /* This is not generated by the kernel as of now.  */
+		      case __SI_MESGQ:
+			err |= __get_user(to->si_pid, &from->si_pid);
+			err |= __get_user(to->si_uid, &from->si_uid);
+			err |= __get_user(to->si_int, &from->si_int);
+			break;
 		}
 	}
 	return err;
===== arch/mips/kernel/signal32.c 1.13 vs edited =====
--- 1.13/arch/mips/kernel/signal32.c	Wed Feb 25 11:31:13 2004
+++ edited/arch/mips/kernel/signal32.c	Sat Apr 10 21:08:53 2004
@@ -358,7 +358,12 @@
 			err |= __put_user(from->si_band, &to->si_band);
 			err |= __put_user(from->si_fd, &to->si_fd);
 			break;
-		/* case __SI_RT: This is not generated by the kernel as of now.  */
+		case __SI_RT: /* This is not generated by the kernel as of now.  */
+		case __SI_MESGQ:
+			err |= __put_user(from->si_pid, &to->si_pid);
+			err |= __put_user(from->si_uid, &to->si_uid);
+			err |= __put_user(from->si_int, &to->si_int);
+			break;
 		}
 	}
 	return err;
===== arch/s390/kernel/compat_signal.c 1.7 vs edited =====
--- 1.7/arch/s390/kernel/compat_signal.c	Sat Mar 27 12:40:46 2004
+++ edited/arch/s390/kernel/compat_signal.c	Sat Apr 10 21:05:01 2004
@@ -74,6 +74,10 @@
 		err |= __copy_to_user(&to->_sifields._pad, &from->_sifields._pad, SI_PAD_SIZE);
 	else {
 		switch (from->si_code >> 16) {
+		case __SI_RT: /* This is not generated by the kernel as of now.  */
+		case __SI_MESGQ:
+			err |= __put_user(from->si_int, &to->si_int);
+			/* fallthrough */
 		case __SI_KILL >> 16:
 			err |= __put_user(from->si_pid, &to->si_pid);
 			err |= __put_user(from->si_uid, &to->si_uid);
@@ -96,7 +100,6 @@
 			break;
 		default:
 			break;
-		/* case __SI_RT: This is not generated by the kernel as of now.  */
 		}
 	}
 	return err;
===== arch/sparc64/kernel/signal32.c 1.32 vs edited =====
--- 1.32/arch/sparc64/kernel/signal32.c	Fri Mar 26 23:16:00 2004
+++ edited/arch/sparc64/kernel/signal32.c	Sat Apr 10 21:13:56 2004
@@ -129,7 +129,12 @@
 			err |= __put_user(from->si_trapno, &to->si_trapno);
 			err |= __put_user((long)from->si_addr, &to->si_addr);
 			break;
-		/* case __SI_RT: This is not generated by the kernel as of now.  */
+		case __SI_RT: /* This is not generated by the kernel as of now.  */
+		case __SI_MESGQ:
+			err |= __put_user(from->si_pid, &to->si_pid);
+			err |= __put_user(from->si_uid, &to->si_uid);
+			err |= __put_user(from->si_int, &to->si_int);
+			break;
 		}
 	}
 	return err;
===== arch/x86_64/ia32/ia32_signal.c 1.19 vs edited =====
--- 1.19/arch/x86_64/ia32/ia32_signal.c	Mon Mar  8 15:23:47 2004
+++ edited/arch/x86_64/ia32/ia32_signal.c	Sat Apr 10 21:03:13 2004
@@ -85,7 +85,11 @@
 			err |= __put_user(from->si_overrun, &to->si_overrun); 
 			err |= __put_user((u32)(u64)from->si_ptr, &to->si_ptr);
 			break;
-		/* case __SI_RT: This is not generated by the kernel as of now.  */
+		case __SI_RT: /* This is not generated by the kernel as of now.  */
+		case __SI_MESGQ:
+			err |= __put_user(from->si_uid, &to->si_uid);
+			err |= __put_user(from->si_int, &to->si_int);
+			break;
 		}
 	}
 	return err;

POSIX Message Queues

[Ardhan Madras]

    I'm hacking a program that need to do several *privileged* tasks, such as getting ethernet's link status (ioctl's SIOCETHTOOL command), setting and saving system time, and many more, this program must run in regular user mode (unprivileged), let's call this program as 'A'. I created another program (program 'B') as service than run in privileged mode to do privileged tasks above. I'm using POSIX message queue to exchange data as follow:

struct data {
    unsigned char cmd;
    long value;
};

Creating a queue in program B:

struct mq_attr attr = { .mq_maxmsg = 1, .mq_msgsize = sizeof(struct data) };
mqd_t mqd = open("/somename", O_RDWR | O_CREAT, 0777, &attr);

Open that queue in program A:

mqd_t mqd = open("/somename", O_RDWR); /* O_RDONLY should works here */

With given queue, i can't open this queue in program A with O_RDWR or O_WRONLY flag (O_RDONLY should works), no matter how i set the flag mode in program B's mq_open() i got EACCESS, i need both program can send and receive queues.

Did i miss something here?

                            Ardhan,


_____________________________________________________________
Listen to KNAC, Hit the Home page and Tune In Live! ---> http://www.knac.com

Re: POSIX Message Queues

[Glynn Clements]

Ardhan Madras wrote:

> I'm using POSIX message queue to exchange data as follow:
> 
> struct data {
>     unsigned char cmd;
>     long value;
> };
> 
> Creating a queue in program B:
> 
> struct mq_attr attr = { .mq_maxmsg = 1, .mq_msgsize = sizeof(struct data) };
> mqd_t mqd = open("/somename", O_RDWR | O_CREAT, 0777, &attr);
> 
> Open that queue in program A:
> 
> mqd_t mqd = open("/somename", O_RDWR); /* O_RDONLY should works here */

I presume that you mean mq_open() rather than open(), right?

> With given queue, i can't open this queue in program A with O_RDWR or
> O_WRONLY flag (O_RDONLY should works), no matter how i set the flag
> mode in program B's mq_open() i got EACCESS, i need both program can
> send and receive queues.
> 
> Did i miss something here?

The requested permissions have the process' umask subtracted from
them. Unless you've changed it, the umask probably removes at least
world-write permission, and possibly others. Either clear the umask
before calling mq_open(), or set the permissions afterwards with
fchmod() (although I don't know whether the latter is portable).

Also, if the queue already exists, mq_open(O_CREAT) will open the
existing queue; it won't replace it or change the permissions. Add
O_EXCL if you want it to fail if a queue with the same name already
exists.

-- 
Glynn Clements <glynn@gclements.plus.com>

Re: POSIX Message Queues

[Ardhan Madras]

It was mq_open(), sorry my bad ;p

I did umask(0001) before creating the queue, it's works now. 'Program A' that requested O_RDWR flag to the queue can send and receive message as 'Program B' did. I also try to demonstrating this problem with another machine (Linux of course), as result i can O_RDWR'ed program A queue flag without have to call umask() first. Is this behavior can be set through system configuration?

fchmod() also works here, sorry, but what do you mean with 'not portable'?, do you mean fchmod() intended to be used only for file, in other words "not every system support fchmod() with message queue descriptor" right?

               Ardhan,


--- glynn@gclements.plus.com wrote:

From:	Glynn Clements <glynn@gclements.plus.com>
To:	<ajhwb@knac.com>
Cc:	<linux-c-programming@vger.kernel.org>
Subject: Re: POSIX Message Queues
Date:	Sat, 12 Jun 2010 14:40:07 +0100


Ardhan Madras wrote:

> I'm using POSIX message queue to exchange data as follow:
> 
> struct data {
>     unsigned char cmd;
>     long value;
> };
> 
> Creating a queue in program B:
> 
> struct mq_attr attr = { .mq_maxmsg = 1, .mq_msgsize = sizeof(struct data) };
> mqd_t mqd = open("/somename", O_RDWR | O_CREAT, 0777, &attr);
> 
> Open that queue in program A:
> 
> mqd_t mqd = open("/somename", O_RDWR); /* O_RDONLY should works here */

I presume that you mean mq_open() rather than open(), right?

> With given queue, i can't open this queue in program A with O_RDWR or
> O_WRONLY flag (O_RDONLY should works), no matter how i set the flag
> mode in program B's mq_open() i got EACCESS, i need both program can
> send and receive queues.
> 
> Did i miss something here?

The requested permissions have the process' umask subtracted from
them. Unless you've changed it, the umask probably removes at least
world-write permission, and possibly others. Either clear the umask
before calling mq_open(), or set the permissions afterwards with
fchmod() (although I don't know whether the latter is portable).

Also, if the queue already exists, mq_open(O_CREAT) will open the
existing queue; it won't replace it or change the permissions. Add
O_EXCL if you want it to fail if a queue with the same name already
exists.

-- 
Glynn Clements <glynn@gclements.plus.com>
--
To unsubscribe from this list: send the line "unsubscribe linux-c-programming" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html




_____________________________________________________________
Listen to KNAC, Hit the Home page and Tune In Live! ---> http://www.knac.com

Re: POSIX Message Queues

[Glynn Clements]

Ardhan Madras wrote:

> It was mq_open(), sorry my bad ;p
> 
> I did umask(0001) before creating the queue, it's works now. 'Program
> A' that requested O_RDWR flag to the queue can send and receive
> message as 'Program B' did. I also try to demonstrating this problem
> with another machine (Linux of course), as result i can O_RDWR'ed
> program A queue flag without have to call umask() first. Is this
> behavior can be set through system configuration?

A process' umask is inherited from its parent. For interactive
sessions, you can set it in your ~/.profile or similar. For daemons
started by init, you can set it in the rc script which starts the
daemon.

> fchmod() also works here, sorry, but what do you mean with 'not
> portable'?, do you mean fchmod() intended to be used only for file, in
> other words "not every system support fchmod() with message queue
> descriptor" right?

Yes.

-- 
Glynn Clements <glynn@gclements.plus.com>

Re: POSIX Message Queues

[Ardhan Madras]

Hi all,

        I use message queue to exchange data between 2 unrelated process, as shown below

Program A:
while (1) {
    mq_receive(); /* wait and receive request */
    /* process request */
    mq_send(); /* reply the request result */
}

Program B:
mq_send(); /* send request */
mq_receive(); /* process result */

        How it's possible to process A that call mq_send() receive it's *own* send data using mq_receive(), so process B that intended to read the message (mq_receive()) could not read it?, the problem is *sometime* happen, at most program A reply what program B requested. Did I miss something here?

        Ardhan



--- glynn@gclements.plus.com wrote:

From: Glynn Clements <glynn@gclements.plus.com>
To: <ajhwb@knac.com>
Cc: <linux-c-programming@vger.kernel.org>
Subject: Re: POSIX Message Queues
Date: Tue, 15 Jun 2010 16:33:25 +0100


Ardhan Madras wrote:

> It was mq_open(), sorry my bad ;p
> 
> I did umask(0001) before creating the queue, it's works now. 'Program
> A' that requested O_RDWR flag to the queue can send and receive
> message as 'Program B' did. I also try to demonstrating this problem
> with another machine (Linux of course), as result i can O_RDWR'ed
> program A queue flag without have to call umask() first. Is this
> behavior can be set through system configuration?

A process' umask is inherited from its parent. For interactive
sessions, you can set it in your ~/.profile or similar. For daemons
started by init, you can set it in the rc script which starts the
daemon.

> fchmod() also works here, sorry, but what do you mean with 'not
> portable'?, do you mean fchmod() intended to be used only for file, in
> other words "not every system support fchmod() with message queue
> descriptor" right?

Yes.

-- 
Glynn Clements <glynn@gclements.plus.com>




_____________________________________________________________
Listen to KNAC, Hit the Home page and Tune In Live! ---> http://www.knac.com