Re: Behaviour of compat_msgsnd/compat_msgrcv calls

From: Suzuki K P
Date: Wed Nov 08 2006 - 13:27:44 EST

Next message: Francois Romieu: "Re: New laptop - problems with linux"
Previous message: Michael Holzheu: "Re: How to document dimension units for virtual files?"
In reply to: Suzuki K P: "Re: Behaviour of compat_msgsnd/compat_msgrcv calls"
Next in thread: suzuki: "[RFC] [PATCH] Fix compat space msg size limit for msgsnd/msgrcv (Was Re: Behaviour of compat_msgsnd/compat_msgrcv calls )"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

Corrected patch for some coding style errs..

Comments ?

Thanks

Suzuki

Suzuki K P wrote:

Suzuki K P wrote:

-- Original Post ---

Hi,

I have a question regarding the behaviour of the comapt_msgsnd/ compat_msgrcv ()s. Don't know if this has been discussed already or if as I could not find any threads in the archives. Please bear with me if this is really a stupid question.

The maximum length of the message that can be sent or received in any of those functions above is MAXBUF-(sizeof (struct msgbuf)), where MAXBUF is 64k.

ipc/compat.c : compat_msgrcv()
if (second < 0 || (second >= MAXBUF - sizeof(struct msgbuf)))
^^^^^^
return -EINVAL;

Is this limit due to the buffer allocation in user space as below ?

And the way we are doing this is by allocating a buffer of msgsize on the userspace stack using compat_alloc_user_space() instead of using the buffer provided by the user and later copying the result back to the user buffer.

Is there any specific reason behind this ? Can't we just use the user buffer directly instead of doing an additional copy_in_user ?
ie,
err = sys_msgrcv(first, uptr, second, msgtyp, third);

-- Dave suggested --

It's the cleanest way to deal with the difference in
"struct msgbuf" layout between native and compat userspace.

I guess we could make some common do_sys_msgrcv() function
that passed in a pointer to the "msgbuf" and the buffer
seperately.

Dave,

I have attached a patch which does the same here. It introduces
do_msgrcv()/ do_msgsnd() routines to service the sys_msgxxx variants.
The do_msgxxx variants accepts an additional "mtext" parameter along
with the sys_msgxxx paramters.
i.e,

asmlinkage long sys_msgrcv(int msqid, struct msgbuf __user *msgp,
size_t msgsz, long msgtyp, int msgflg)
{
return do_msgrcv(msqid, msgp, msgp->mtext, msgsz, msgtyp,
msgflg);
}

Comments ?

Thanks,

-Suzuki

* Fix the size limit of compat space msgsize.

Currently we allocate 64k space on the user stack and use it the msgbuf for sys_msgxxx, and the results are later copied in user [ by copy_in_user]. This patch introduces helper routines for sys_msgxxx which would accept the pointer to msgbuf along with the msgp->mtext. This avoids the need to allocate the msgsize on the userspace ( thus removing the size limt ) and the overhead of an extra copy_in_user().

Signed-off-by: Suzuki K P <suzuki@xxxxxxxxxx>

Index: linux-2.6.19-rc2/include/linux/msg.h
===================================================================
--- linux-2.6.19-rc2.orig/include/linux/msg.h 2006-09-20 09:12:06.000000000 +0530
+++ linux-2.6.19-rc2/include/linux/msg.h 2006-11-08 23:45:57.000000000 +0530
@@ -92,6 +92,12 @@
struct list_head q_senders;
};

+/* Helper routines for sys_msgsnd and sys_msgrcv */
+extern long do_msgsnd(int msqid, struct msgbuf __user *msgp, void __user *mtext,
+ size_t msgsz, int msgflg);
+extern long do_msgrcv(int msqid, struct msgbuf __user *msgp, void __user *mtext,
+ size_t msgsz, long msgtyp, int msgflg);
+
#endif /* __KERNEL__ */

#endif /* _LINUX_MSG_H */
Index: linux-2.6.19-rc2/ipc/compat.c
===================================================================
--- linux-2.6.19-rc2.orig/ipc/compat.c 2006-09-20 09:12:06.000000000 +0530
+++ linux-2.6.19-rc2/ipc/compat.c 2006-11-08 23:43:51.000000000 +0530
@@ -115,7 +115,6 @@

extern int sem_ctls[];
#define sc_semopm (sem_ctls[2])
-#define MAXBUF (64*1024)

static inline int compat_ipc_parse_version(int *cmd)
{
@@ -313,16 +312,15 @@

if (first < 0)
return -EINVAL;
- if (second < 0 || (second >= MAXBUF - sizeof(struct msgbuf)))
+ if (second < 0)
return -EINVAL;

- p = compat_alloc_user_space(second + sizeof(struct msgbuf));
+ p = compat_alloc_user_space(sizeof(struct msgbuf));
if (get_user(type, &up->mtype) ||
- put_user(type, &p->mtype) ||
- copy_in_user(p->mtext, up->mtext, second))
+ put_user(type, &p->mtype))
return -EFAULT;

- return sys_msgsnd(first, p, second, third);
+ return do_msgsnd(first, p, up->mtext, second, third);
}

long compat_sys_msgrcv(int first, int second, int msgtyp, int third,
@@ -335,7 +333,7 @@

if (first < 0)
return -EINVAL;
- if (second < 0 || (second >= MAXBUF - sizeof(struct msgbuf)))
+ if (second < 0)
return -EINVAL;

if (!version) {
@@ -349,14 +347,13 @@
uptr = compat_ptr(ipck.msgp);
msgtyp = ipck.msgtyp;
}
- p = compat_alloc_user_space(second + sizeof(struct msgbuf));
- err = sys_msgrcv(first, p, second, msgtyp, third);
+ up = uptr;
+ p = compat_alloc_user_space(sizeof(struct msgbuf));
+ err = do_msgrcv(first, p, up->mtext, second, msgtyp, third);
if (err < 0)
goto out;
- up = uptr;
if (get_user(type, &p->mtype) ||
- put_user(type, &up->mtype) ||
- copy_in_user(up->mtext, p->mtext, err))
+ put_user(type, &up->mtype))
err = -EFAULT;
out:
return err;
Index: linux-2.6.19-rc2/ipc/msg.c
===================================================================
--- linux-2.6.19-rc2.orig/ipc/msg.c 2006-10-17 10:50:56.000000000 +0530
+++ linux-2.6.19-rc2/ipc/msg.c 2006-11-08 23:43:51.000000000 +0530
@@ -625,8 +625,8 @@
return 0;
}

-asmlinkage long
-sys_msgsnd(int msqid, struct msgbuf __user *msgp, size_t msgsz, int msgflg)
+long do_msgsnd(int msqid, struct msgbuf __user *msgp, void __user *mtext,
+ size_t msgsz, int msgflg)
{
struct msg_queue *msq;
struct msg_msg *msg;
@@ -643,7 +643,7 @@
if (mtype < 1)
return -EINVAL;

- msg = load_msg(msgp->mtext, msgsz);
+ msg = load_msg(mtext, msgsz);
if (IS_ERR(msg))
return PTR_ERR(msg);

@@ -722,6 +722,12 @@
return err;
}

+asmlinkage long
+sys_msgsnd(int msqid, struct msgbuf __user *msgp, size_t msgsz, int msgflg)
+{
+ return do_msgsnd(msqid, msgp, msgp->mtext, msgsz, msgflg);
+}
+
static inline int convert_mode(long *msgtyp, int msgflg)
{
/*
@@ -741,8 +747,8 @@
return SEARCH_EQUAL;
}

-asmlinkage long sys_msgrcv(int msqid, struct msgbuf __user *msgp, size_t msgsz,
- long msgtyp, int msgflg)
+long do_msgrcv(int msqid, struct msgbuf __user *msgp, void __user *mtext,
+ size_t msgsz, long msgtyp, int msgflg)
{
struct msg_queue *msq;
struct msg_msg *msg;
@@ -889,7 +895,7 @@

msgsz = (msgsz > msg->m_ts) ? msg->m_ts : msgsz;
if (put_user (msg->m_type, &msgp->mtype) ||
- store_msg(msgp->mtext, msg, msgsz)) {
+ store_msg(mtext, msg, msgsz)) {
msgsz = -EFAULT;
}
free_msg(msg);
@@ -897,6 +903,12 @@
return msgsz;
}

+asmlinkage long sys_msgrcv(int msqid, struct msgbuf __user *msgp, size_t msgsz,
+ long msgtyp, int msgflg)
+{
+ return do_msgrcv(msqid, msgp, msgp->mtext, msgsz, msgtyp, msgflg);
+}
+
#ifdef CONFIG_PROC_FS
static int sysvipc_msg_proc_show(struct seq_file *s, void *it)
{

Next message: Francois Romieu: "Re: New laptop - problems with linux"
Previous message: Michael Holzheu: "Re: How to document dimension units for virtual files?"
In reply to: Suzuki K P: "Re: Behaviour of compat_msgsnd/compat_msgrcv calls"
Next in thread: suzuki: "[RFC] [PATCH] Fix compat space msg size limit for msgsnd/msgrcv (Was Re: Behaviour of compat_msgsnd/compat_msgrcv calls )"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]