On Tuesday 14 November 2006 00:58, [email protected] wrote:
> Subject: Fix compat space msg size limit for msgsnd/msgrcv
> From: suzuki <[email protected]>
>
> 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
> limit) and the overhead of an extra copy_in_user().
>
> Signed-off-by: Suzuki K P <[email protected]>
> Cc: Arnd Bergmann <[email protected]>
> Cc: "David S. Miller" <[email protected]>
> Signed-off-by: Andrew Morton <[email protected]>
This patch is definitely a big step in the right direction here, but why
not go all the way and pass msgp->mtype to do_msgsnd/do_msgrcv as kernel
data instead of a user space pointer? This way you can get rid of the
compat_alloc_userspace entirely and save avoid doing an extra
put_user/get_user pair in the compat_ function.
Arnd <><
Arnd Bergmann wrote:
> On Tuesday 14 November 2006 00:58, [email protected] wrote:
>
>
>>Subject: Fix compat space msg size limit for msgsnd/msgrcv
>>From: suzuki <[email protected]>
>>
>>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
>>limit) and the overhead of an extra copy_in_user().
>>
>>Signed-off-by: Suzuki K P <[email protected]>
>>Cc: Arnd Bergmann <[email protected]>
>>Cc: "David S. Miller" <[email protected]>
>>Signed-off-by: Andrew Morton <[email protected]>
>
>
> This patch is definitely a big step in the right direction here, but why
> not go all the way and pass msgp->mtype to do_msgsnd/do_msgrcv as kernel
> data instead of a user space pointer? This way you can get rid of the
> compat_alloc_userspace entirely and save avoid doing an extra
> put_user/get_user pair in the compat_ function.
>
I left it as such, inorder to avoid the future changes that may come in
the struct msgbuf -if at all-, which would make us to pass every single
field as a parameter to do_msgrcv/do_msgsnd.
thanks,
Suzuki
> Arnd <><
On Tuesday 14 November 2006 02:28, suzuki wrote:
>
> I left it as such, inorder to avoid the future changes that may come in
> the struct msgbuf -if at all-, which would make us to pass every single
> field as a parameter to do_msgrcv/do_msgsnd.
struct msgbuf is part of the kernel ABI and will never change, so that's
no problem at all.
Arnd <><
Hello Arnd,
Arnd Bergmann wrote:
> On Tuesday 14 November 2006 02:28, suzuki wrote:
>
>>I left it as such, inorder to avoid the future changes that may come in
>>the struct msgbuf -if at all-, which would make us to pass every single
>>field as a parameter to do_msgrcv/do_msgsnd.
>
>
> struct msgbuf is part of the kernel ABI and will never change, so that's
> no problem at all.
Ok.
Does the following change look fine ?
do_msgsnd() - Accepting the mtype and user space ptr to the mtext. i.e.,
long do_msgsnd(int msqid, long mtype, void __user *mtext,
size_t msgsz, int msgflg);
and,
do_msgrcv() - accepting the kernel space data ptr to pmtype and user
space ptr to mtext. The caller has to copy the *pmtype back to the user
space.
i.e.,
long do_msgrcv(int msqid, long *pmtype, void __user *mtext,
size_t msgsz, long msgtyp, int msgflg);
or
Can we use the kernel space "struct msgbuf" instead of the mtype being
passed explicitly.
Thanks,
Suzuki
>
> Arnd <><
On Tuesday 14 November 2006 22:22, suzuki wrote:
> Does the following change look fine ?
>
> do_msgsnd() - Accepting the mtype and user space ptr to the mtext. i.e.,
>
> long do_msgsnd(int msqid, long mtype, void __user *mtext,
> ????????????????size_t msgsz, int msgflg);
> and,
>
> do_msgrcv() - accepting the kernel space data ptr to pmtype and user
> space ptr to mtext. The caller has to copy the *pmtype back to the user
> space.
>
> i.e.,
>
> long do_msgrcv(int msqid, long *pmtype, void __user *mtext,
> ? ? ? ? ? ? ? ? ? ? ? ? size_t msgsz, long msgtyp, int msgflg);
Yes, that looks fine.
> Can we use the kernel space "struct msgbuf" instead of the mtype being
> passed explicitly.
That works as well, although it may be a little confusing to have
the extra mtext byte of that structure included there, so I'd prefer
the first solution.
Arnd <><
Arnd Bergmann wrote:
> On Tuesday 14 November 2006 22:22, suzuki wrote:
>
>>Does the following change look fine ?
>>
>>do_msgsnd() - Accepting the mtype and user space ptr to the mtext. i.e.,
>>
>>long do_msgsnd(int msqid, long mtype, void __user *mtext,
>> size_t msgsz, int msgflg);
>>and,
>>
>>do_msgrcv() - accepting the kernel space data ptr to pmtype and user
>>space ptr to mtext. The caller has to copy the *pmtype back to the user
>>space.
>>
>>i.e.,
>>
>>long do_msgrcv(int msqid, long *pmtype, void __user *mtext,
>> size_t msgsz, long msgtyp, int msgflg);
>
>
> Yes, that looks fine.
>
>
>>Can we use the kernel space "struct msgbuf" instead of the mtype being
>>passed explicitly.
>
>
> That works as well, although it may be a little confusing to have
> the extra mtext byte of that structure included there, so I'd prefer
> the first solution.
Arnd,
Thanks for the input !
Here we go ! As per the suggestions, I have modified the patch.
Thanks,
Suzuki
>
> Arnd <><