From owner-freebsd-emulation@FreeBSD.ORG Thu Mar 10 11:41:09 2011 Return-Path: Delivered-To: freebsd-emulation@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id D85401065676 for ; Thu, 10 Mar 2011 11:41:09 +0000 (UTC) (envelope-from alexander@leidinger.net) Received: from mail.ebusiness-leidinger.de (mail.ebusiness-leidinger.de [217.11.53.44]) by mx1.freebsd.org (Postfix) with ESMTP id 8327A8FC15 for ; Thu, 10 Mar 2011 11:41:09 +0000 (UTC) Received: from outgoing.leidinger.net (p5B1554E8.dip.t-dialin.net [91.21.84.232]) by mail.ebusiness-leidinger.de (Postfix) with ESMTPSA id A3D4584400E; Thu, 10 Mar 2011 12:41:04 +0100 (CET) Received: from webmail.leidinger.net (unknown [IPv6:fd73:10c7:2053:1::2:102]) by outgoing.leidinger.net (Postfix) with ESMTP id 5091327B5; Thu, 10 Mar 2011 12:41:01 +0100 (CET) Received: (from www@localhost) by webmail.leidinger.net (8.14.4/8.13.8/Submit) id p2ABeobe001914; Thu, 10 Mar 2011 12:40:50 +0100 (CET) (envelope-from Alexander@Leidinger.net) Received: from pslux.ec.europa.eu (pslux.ec.europa.eu [158.169.9.14]) by webmail.leidinger.net (Horde Framework) with HTTP; Thu, 10 Mar 2011 12:40:50 +0100 Message-ID: <20110310124050.12625ar9uhfio7c4@webmail.leidinger.net> Date: Thu, 10 Mar 2011 12:40:50 +0100 From: Alexander Leidinger To: John Wehle References: <201103080630.p286UGKU008835@freefall.freebsd.org> In-Reply-To: <201103080630.p286UGKU008835@freefall.freebsd.org> MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8; DelSp="Yes"; format="flowed" Content-Disposition: inline Content-Transfer-Encoding: 7bit User-Agent: Dynamic Internet Messaging Program (DIMP) H3 (1.1.4) X-EBL-MailScanner-Information: Please contact the ISP for more information X-EBL-MailScanner-ID: A3D4584400E.A75CE X-EBL-MailScanner: Found to be clean X-EBL-MailScanner-SpamCheck: not spam, spamhaus-ZEN, SpamAssassin (not cached, score=1.951, required 6, autolearn=disabled, J_CHICKENPOX_47 0.60, RDNS_NONE 1.27, TW_CV 0.08) X-EBL-MailScanner-SpamScore: s X-EBL-MailScanner-From: alexander@leidinger.net X-EBL-MailScanner-Watermark: 1300362065.66046@nGiF8dxcpBW8GuqBJwVRGw X-EBL-Spam-Status: No Cc: freebsd-emulation@FreeBSD.org Subject: Re: kern/149168: [linux] [patch] Linux sendmsg / recvmsg / etc fixes for pulseaudio X-BeenThere: freebsd-emulation@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Development of Emulators of other operating systems List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 10 Mar 2011 11:41:09 -0000 Quoting John Wehle (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