kern/149168: [linux] [patch] Linux sendmsg / recvmsg / etc
fixes for pulseaudio
Alexander Leidinger
Alexander at Leidinger.net
Thu Mar 10 11:41:09 UTC 2011
Quoting John Wehle <john at feith.com> (from Tue, 8 Mar 2011 06:30:16 GMT):
> --- ./compat/linux/linux_misc.c.ORIGINAL 2010-12-21 12:09:25.000000000 -0500
> +++ ./compat/linux/linux_misc.c 2011-02-26 22:41:49.000000000 -0500
> @@ -1733,6 +1733,87 @@ linux_exit_group(struct thread *td, stru
> return (0);
> }
>
> +#define _LINUX_CAPABILITY_VERSION 0x19980330
> +
> +struct l_user_cap_header {
> + l_int version;
> + l_int pid;
> +};
> +
> +struct l_user_cap_data {
> + l_int effective;
> + l_int permitted;
> + l_int inheritable;
> +};
You zero the data part in capget. Does this mean that the programs
gets told that we are not able to do the right thing, or gets the
program tricked into thinking we are doing the right thing (= security
hole)?
Please add comments to document what they are supposed to do if set.
> +int
> +linux_capget(struct thread *td, struct linux_capget_args *args)
> +{
> + struct l_user_cap_header luch;
> + struct l_user_cap_data lucd;
> + int error;
> +
> + if (! args->hdrp)
> + return (EFAULT);
> +
> + error = copyin(args->hdrp, &luch, sizeof(luch));
> + if (error != 0)
> + return (error);
> +
> + if (luch.version != _LINUX_CAPABILITY_VERSION) {
> + luch.version = _LINUX_CAPABILITY_VERSION;
> + error = copyout(&luch, args->hdrp, sizeof(luch));
Does linux do the same? I'm a little bit surprised that the kernel
does not answer in the same format as the userland requests (but if
the API is defined this way...).
> + if (error)
> + return (error);
> + return (EINVAL);
> + }
> +
> + if (luch.pid)
> + return (EPERM);
> +
> + if (args->datap) {
> + bzero (&lucd, sizeof(lucd));
> + error = copyout(&lucd, args->datap, sizeof(lucd));
> + }
> +
> + return (error);
> +}
> +
> +int
> +linux_capset(struct thread *td, struct linux_capset_args *args)
> +{
> + struct l_user_cap_header luch;
> + struct l_user_cap_data lucd;
> + int error;
> +
> + if (! args->hdrp || ! args->datap)
Style: please make explicit tests instead of negations.
> + return (EFAULT);
> +
> + error = copyin(args->hdrp, &luch, sizeof(luch));
> + if (error != 0)
> + return (error);
> +
> + if (luch.version != _LINUX_CAPABILITY_VERSION) {
> + luch.version = _LINUX_CAPABILITY_VERSION;
> + error = copyout(&luch, args->hdrp, sizeof(luch));
> + if (error)
> + return (error);
> + return (EINVAL);
> + }
> +
> + if (luch.pid)
> + return (EPERM);
> +
> + error = copyin(args->datap, &lucd, sizeof(lucd));
> + if (error != 0)
> + return (error);
> +
> + if (lucd.effective || lucd.permitted || lucd.inheritable)
> + return (EPERM);
How does the userland interprete EPERM?
> + return (0);
> +}
> +
> int
> linux_prctl(struct thread *td, struct linux_prctl_args *args)
> {
> @@ -459,7 +463,7 @@ linux_to_bsd_msghdr(struct msghdr *bhdr,
> bhdr->msg_iov = PTRIN(lhdr->msg_iov);
> bhdr->msg_iovlen = lhdr->msg_iovlen;
> bhdr->msg_control = PTRIN(lhdr->msg_control);
> - bhdr->msg_controllen = lhdr->msg_controllen;
> + /* msg_controllen skipped */
Please extend the comment to explain why.
> bhdr->msg_flags = linux_to_bsd_msg_flags(lhdr->msg_flags);
> return (0);
> }
> @@ -472,7 +476,7 @@ bsd_to_linux_msghdr(const struct msghdr
> lhdr->msg_iov = PTROUT(bhdr->msg_iov);
> lhdr->msg_iovlen = bhdr->msg_iovlen;
> lhdr->msg_control = PTROUT(bhdr->msg_control);
> - lhdr->msg_controllen = bhdr->msg_controllen;
> + /* msg_controllen skipped */
Same as above.
> /* msg_flags skipped */
And in case you know why they are skipped, it would be nice if you
could extend this comment too while you are here. :)
> return (0);
> }
> @@ -1295,11 +1339,40 @@ linux_recvmsg(struct thread *td, struct
> }
> }
> break;
> +
> + case SCM_CREDS:
> + /*
> + * Currently LOCAL_CREDS is never in
> + * effect for Linux so no need to worry
> + * about sockcred
> + */
Is there a way to check this here and fail (either with an appropriate
error return or a kernel panic if this is more appropriate... I'm not
sure I have the big picture here)?
> + if (datalen != sizeof (*cmcred)) {
> + error = EMSGSIZE;
> + goto bad;
> + }
> --- ./i386/linux/syscalls.master.ORIGINAL 2010-12-21
> 12:09:25.000000000 -0500
> +++ ./i386/linux/syscalls.master 2011-02-26 22:41:49.000000000 -0500
> @@ -329,8 +329,8 @@
> l_uid16_t uid, l_gid16_t gid); }
> 183 AUE_GETCWD STD { int linux_getcwd(char *buf, \
> l_ulong bufsize); }
> -184 AUE_CAPGET STD { int linux_capget(void); }
> -185 AUE_CAPSET STD { int linux_capset(void); }
> +184 AUE_CAPGET STD { int linux_capget(void *hdrp, void *datap); }
> +185 AUE_CAPSET STD { int linux_capset(void *hdrp, const void *datap); }
A quick search tells that the API is defined as cap_user_header_t and
cap_user_data_t and not "void *". It may be the case that in the end
those are defined to "void *" (I didn't check), but it would be nice
if you could use apropriate l_cap_user_header_t and l_cap_user_data_t.
> 186 AUE_NULL STD { int linux_sigaltstack(l_stack_t *uss, \
> l_stack_t *uoss); }
> 187 AUE_SENDFILE STD { int linux_sendfile(void); }
> --- ./amd64/linux32/syscalls.master.ORIGINAL 2010-12-21
> 12:09:25.000000000 -0500
> +++ ./amd64/linux32/syscalls.master 2011-02-26 22:41:49.000000000 -0500
> @@ -327,8 +327,8 @@
> l_uid16_t uid, l_gid16_t gid); }
> 183 AUE_GETCWD STD { int linux_getcwd(char *buf, \
> l_ulong bufsize); }
> -184 AUE_CAPGET STD { int linux_capget(void); }
> -185 AUE_CAPSET STD { int linux_capset(void); }
> +184 AUE_CAPGET STD { int linux_capget(void *hdrp, void *datap); }
> +185 AUE_CAPSET STD { int linux_capset(void *hdrp, const void *datap); }
The same here.
> 186 AUE_NULL STD { int linux_sigaltstack(l_stack_t *uss, \
> l_stack_t *uoss); }
> 187 AUE_SENDFILE STD { int linux_sendfile(void); }
Thanks a lot for working on this,
Alexander.
--
All his life he has looked away... to the horizon, to the sky,
to the future. Never his mind on where he was, on what he was doing.
-- Yoda
http://www.Leidinger.net Alexander @ Leidinger.net: PGP ID = B0063FE7
http://www.FreeBSD.org netchild @ FreeBSD.org : PGP ID = 72077137
More information about the freebsd-emulation
mailing list