2006-10-25 18:59:10

by suzuki

[permalink] [raw]
Subject: Behaviour of compat_msgsnd/compat_msgrcv calls

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


2006-10-25 20:49:41

by David Miller

[permalink] [raw]
Subject: Re: Behaviour of compat_msgsnd/compat_msgrcv calls

From: Suzuki K P <[email protected]>
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.

2006-11-08 17:36:06

by suzuki

[permalink] [raw]
Subject: Re: Behaviour of compat_msgsnd/compat_msgrcv calls


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







Attachments:
fix-compat-ipc-msg-size-limit.diff (4.71 kB)

2006-11-08 18:27:13

by suzuki

[permalink] [raw]
Subject: Re: Behaviour of compat_msgsnd/compat_msgrcv calls

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


Attachments:
fix-compat-msg-size.diff (4.71 kB)

2006-11-11 01:26:06

by Suzuki

[permalink] [raw]
Subject: [RFC] [PATCH] Fix compat space msg size limit for msgsnd/msgrcv ( Was Re: Behaviour of compat_msgsnd/compat_msgrcv calls )

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


Attachments:
fix-compat-msg-size.diff (4.75 kB)