public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* Behaviour of compat_msgsnd/compat_msgrcv calls
@ 2006-10-25 18:59 Suzuki K P
  2006-10-25 20:49 ` David Miller
  0 siblings, 1 reply; 5+ messages in thread
From: Suzuki K P @ 2006-10-25 18:59 UTC (permalink / raw)
  To: lkml

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.

         if (!version) {
            [...]
                 if (copy_from_user (&ipck, uptr, sizeof(ipck)))
                         goto out;
                 uptr = compat_ptr(ipck.msgp);
                 msgtyp = ipck.msgtyp;

         }

         p = compat_alloc_user_space(second + sizeof(struct msgbuf));

Do we really need this allocation ?

         err = sys_msgrcv(first, p, second, msgtyp, third);

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);

Thanks,

-Suzuki

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: Behaviour of compat_msgsnd/compat_msgrcv calls
  2006-10-25 18:59 Behaviour of compat_msgsnd/compat_msgrcv calls Suzuki K P
@ 2006-10-25 20:49 ` David Miller
  2006-11-08 17:36   ` Suzuki K P
  0 siblings, 1 reply; 5+ messages in thread
From: David Miller @ 2006-10-25 20:49 UTC (permalink / raw)
  To: suzuki; +Cc: linux-kernel

From: Suzuki K P <suzuki@in.ibm.com>
Date: Wed, 25 Oct 2006 11:59:05 -0700

> 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);

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.

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: Behaviour of compat_msgsnd/compat_msgrcv calls
  2006-10-25 20:49 ` David Miller
@ 2006-11-08 17:36   ` Suzuki K P
  2006-11-08 18:27     ` Suzuki K P
  0 siblings, 1 reply; 5+ messages in thread
From: Suzuki K P @ 2006-11-08 17:36 UTC (permalink / raw)
  To: David Miller; +Cc: linux-kernel, Andrew Morton

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


-- 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







[-- Attachment #2: fix-compat-ipc-msg-size-limit.diff --]
[-- Type: text/x-patch, Size: 4818 bytes --]

* 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@in.ibm.com>

Index: linux-2.6.18.i386/include/linux/msg.h
===================================================================
--- linux-2.6.18.i386.orig/include/linux/msg.h	2006-09-19 20:42:06.000000000 -0700
+++ linux-2.6.18.i386/include/linux/msg.h	2006-11-02 10:42:13.000000000 -0800
@@ -92,6 +92,12 @@
 	struct list_head q_senders;
 };
 
+/* Helper routines for sys_msgxxx */
+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.18.i386/ipc/compat.c
===================================================================
--- linux-2.6.18.i386.orig/ipc/compat.c	2006-09-19 20:42:06.000000000 -0700
+++ linux-2.6.18.i386/ipc/compat.c	2006-11-03 14:23:02.000000000 -0800
@@ -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.18.i386/ipc/msg.c
===================================================================
--- linux-2.6.18.i386.orig/ipc/msg.c	2006-09-19 20:42:06.000000000 -0700
+++ linux-2.6.18.i386/ipc/msg.c	2006-11-02 10:43:10.000000000 -0800
@@ -575,8 +575,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;
@@ -590,7 +590,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);
 
@@ -669,6 +669,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)
 {
 	/*
@@ -688,8 +694,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;
@@ -834,7 +840,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);
@@ -842,6 +848,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)
 {

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: Behaviour of compat_msgsnd/compat_msgrcv calls
  2006-11-08 17:36   ` Suzuki K P
@ 2006-11-08 18:27     ` Suzuki K P
  2006-11-11  1:24       ` [RFC] [PATCH] Fix compat space msg size limit for msgsnd/msgrcv ( Was Re: Behaviour of compat_msgsnd/compat_msgrcv calls ) suzuki
  0 siblings, 1 reply; 5+ messages in thread
From: Suzuki K P @ 2006-11-08 18:27 UTC (permalink / raw)
  To: David Miller; +Cc: linux-kernel, Andrew Morton

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

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
>>
>>
>>
>>
>>
>>
>>

[-- Attachment #2: fix-compat-msg-size.diff --]
[-- Type: text/x-patch, Size: 4828 bytes --]


* 	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@in.ibm.com>

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)
 {

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [RFC] [PATCH] Fix compat space msg size limit for msgsnd/msgrcv ( Was Re: Behaviour of compat_msgsnd/compat_msgrcv calls )
  2006-11-08 18:27     ` Suzuki K P
@ 2006-11-11  1:24       ` suzuki
  0 siblings, 0 replies; 5+ messages in thread
From: suzuki @ 2006-11-11  1:24 UTC (permalink / raw)
  To: David Miller, Andrew Morton; +Cc: linux-kernel

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

[Resending]

hi,

The compat_sys_{msgsnd,msgrcv} calls cannot handle a message of size > 
(64k - sizeof(struct msgbuf)). These calls would fail with -EINVAL for 
such invocations.

This is mainly due to the difference in the layout of the msgbuf 
structure in compat space and the native space. In compat_sys_msgrcv() 
we allocate space (at most 64k, and hence the limit) on the user stack 
using compat_alloc_user_space() and use this as the msgbuf for 
sys_msgrcv(). The results are later copied IN user space using 
copy_in_user(), which is an overhead.

The attached patch introduces (as per Dave's suggestion) helper routines 
   which accepts an additional pointer to the buf for the msgbuf. Thus 
this avoids the message size limit as well as the overhead of a 
copy_in_user().

Comments ?

Thanks

Suzuki
Linux Technology Centre
IBM Systems & Technology Labs.




Suzuki K P wrote:
> 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
>>>
>>>
>>>
>>>
>>>
>>>
>>>
> 
> ------------------------------------------------------------------------

[-- Attachment #2: fix-compat-msg-size.diff --]
[-- Type: text/x-patch, Size: 4860 bytes --]


* 	Fix the size limit of compat space msgsize. 

Currently we allocate 64k space on the user stack and use it the msgbuf for sys_{msgrcv,msgsnd}
 for compat and the results are later copied in user [ by copy_in_user]. This patch introduces 
helper routines for sys_{msgrcv,msgsnd} 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@in.ibm.com>

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)
 {

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2006-11-11  1:26 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2006-10-25 18:59 Behaviour of compat_msgsnd/compat_msgrcv calls Suzuki K P
2006-10-25 20:49 ` David Miller
2006-11-08 17:36   ` Suzuki K P
2006-11-08 18:27     ` Suzuki K P
2006-11-11  1:24       ` [RFC] [PATCH] Fix compat space msg size limit for msgsnd/msgrcv ( Was Re: Behaviour of compat_msgsnd/compat_msgrcv calls ) suzuki

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox