Date: Tue, 07 Aug 2001 19:29:47 +0400 From: vova@express.ru To: FreeBSD-gnats-submit@freebsd.org Subject: kern/29499: problem with sending creditionals and descriptors in one message through AF_UNIX socket Message-ID: <E15U8nj-0001a9-00@vbook.express.ru>
next in thread | raw e-mail | index | archive | help
>Number: 29499 >Category: kern >Synopsis: it is not possible to send creditionals and descriptors in one message through AF_UNIX socket >Confidential: no >Severity: non-critical >Priority: low >Responsible: freebsd-bugs >State: open >Quarter: >Keywords: >Date-Required: >Class: sw-bug >Submitter-Id: current-users >Arrival-Date: Tue Aug 07 08:30:05 PDT 2001 >Closed-Date: >Last-Modified: >Originator: Vladimir B. Grebenschikov >Release: FreeBSD 4.3-RELEASE i386 >Organization: TSB "Russian Express" >Environment: FreeBSD idx0.express.ru 4.3-RELEASE FreeBSD 4.3-RELEASE #1: Wed Jul 11 14:23:23 MSD 2001 root@idx0.express.ru:/usr/src/sys/compile/IDX0 i386 >Description: When special message is send with regular data through unix domain socket it is possible to transfer onle one special message a time (creditionals or file descriptors), but not possible to send both, there is CMSG_NXTHDR macro to get next special message. >How-To-Repeat: Simple code that allows don't use setuids at all located here http://pm.kmost.express.ru/~pm/nosuid.tgz without patch it will not work under freebsd, but will work under linux if use macro CMSG_NXTHDR from freebsd. >Fix: There is very ugly patch (it can lead to fd-leak) patch assumes sizeof(int) == sizeof(struct file *), that in general not right. --- /usr/src/sys/kern/uipc_usrreq.c.orig Thu May 10 11:56:25 2001 +++ /usr/src/sys/kern/uipc_usrreq.c Sun Aug 5 17:37:32 2001 @@ -897,6 +901,8 @@ } return (EMSGSIZE); } + +#if 0 /* * now change each pointer to an fd in the global table to * an integer that is the index to the local fd table entry @@ -909,6 +915,7 @@ * do it in reverse order. */ if (sizeof (struct file *) >= sizeof (int)) { +#endif fdp = (int *)(cm + 1); rp = (struct file **)CMSG_DATA(cm); for (i = 0; i < newfds; i++) { @@ -920,6 +927,7 @@ unp_rights--; *fdp++ = f; } +#if 0 } else { fdp = (int *)(cm + 1) + newfds - 1; rp = (struct file **)CMSG_DATA(cm) + newfds - 1; @@ -940,6 +948,7 @@ */ cm->cmsg_len = CMSG_LEN(newfds * sizeof(int)); rights->m_len = cm->cmsg_len; +#endif return (0); } @@ -969,40 +978,56 @@ register int i, fd, *fdp; register struct cmsgcred *cmcred; int oldfds; - u_int newlen; +/* u_int newlen; */ + struct cmsghdr *cmend; +#if 0 if ((cm->cmsg_type != SCM_RIGHTS && cm->cmsg_type != SCM_CREDS) || - cm->cmsg_level != SOL_SOCKET || cm->cmsg_len != control->m_len) + cm->cmsg_level != SOL_SOCKET || cm->cmsg_len != control->m_len) { +uprintf("cmsg_type %d, cmsg_level %d, cmsg_len %d, m_len %d\n", cm->cmsg_type, cm->cmsg_level, cm->cmsg_len, control->m_len); + return (EINVAL); + } +#else + if (cm->cmsg_level != SOL_SOCKET) { return (EINVAL); + } +#endif + cmend = (struct cmsghdr *)(((char*)cm) + control->m_len); + for(; cm < cmend; + cm = (struct cmsghdr *)(((char*)cm)+cm->cmsg_len)) { /* * Fill in credential information. */ - if (cm->cmsg_type == SCM_CREDS) { - cmcred = (struct cmsgcred *)(cm + 1); - cmcred->cmcred_pid = p->p_pid; - cmcred->cmcred_uid = p->p_cred->p_ruid; - cmcred->cmcred_gid = p->p_cred->p_rgid; - cmcred->cmcred_euid = p->p_ucred->cr_uid; - cmcred->cmcred_ngroups = MIN(p->p_ucred->cr_ngroups, +/*uprintf("cm = %p, cmsg_type = %d\n", cm, cm->cmsg_type);*/ + switch(cm->cmsg_type) { + case SCM_CREDS: + cmcred = (struct cmsgcred *)(cm + 1); + cmcred->cmcred_pid = p->p_pid; + cmcred->cmcred_uid = p->p_cred->p_ruid; + cmcred->cmcred_gid = p->p_cred->p_rgid; + cmcred->cmcred_euid = p->p_ucred->cr_uid; + cmcred->cmcred_ngroups = MIN(p->p_ucred->cr_ngroups, CMGROUP_MAX); - for (i = 0; i < cmcred->cmcred_ngroups; i++) - cmcred->cmcred_groups[i] = p->p_ucred->cr_groups[i]; - return(0); - } + for (i = 0; i < cmcred->cmcred_ngroups; i++) + cmcred->cmcred_groups[i] = p->p_ucred->cr_groups[i]; + break; + case SCM_RIGHTS: - oldfds = (cm->cmsg_len - sizeof (*cm)) / sizeof (int); - /* - * check that all the FDs passed in refer to legal OPEN files - * If not, reject the entire operation. - */ - fdp = (int *)(cm + 1); - for (i = 0; i < oldfds; i++) { - fd = *fdp++; - if ((unsigned)fd >= fdescp->fd_nfiles || - fdescp->fd_ofiles[fd] == NULL) - return (EBADF); - } + oldfds = (cm->cmsg_len - sizeof (*cm)) / sizeof (int); + /* + * check that all the FDs passed in refer to legal OPEN files + * If not, reject the entire operation. + */ + fdp = (int *)(cm + 1); + for (i = 0; i < oldfds; i++) { + fd = *fdp++; + if ((unsigned)fd >= fdescp->fd_nfiles || + fdescp->fd_ofiles[fd] == NULL) + return (EBADF); + } + +#if 0 /* * Now replace the integer FDs with pointers to * the associated global file table entry.. @@ -1010,6 +1035,7 @@ * enough, return E2BIG. */ newlen = CMSG_LEN(oldfds * sizeof(struct file *)); +/*uprintf("newlen %d, m_len %d\n", newlen, control->m_len);*/ if (newlen > MCLBYTES) return (E2BIG); if (newlen - control->m_len > M_TRAILINGSPACE(control)) { @@ -1029,34 +1055,42 @@ * differs. */ control->m_len = cm->cmsg_len = newlen; +#endif - /* - * Transform the file descriptors into struct file pointers. - * If sizeof (struct file *) is bigger than or equal to sizeof int, - * then do it in reverse order so that the int won't get until - * we're done. - * If sizeof (struct file *) is smaller than sizeof int, then - * do it in forward order. - */ - if (sizeof (struct file *) >= sizeof (int)) { - fdp = (int *)(cm + 1) + oldfds - 1; - rp = (struct file **)CMSG_DATA(cm) + oldfds - 1; - for (i = 0; i < oldfds; i++) { - fp = fdescp->fd_ofiles[*fdp--]; - *rp-- = fp; - fp->f_count++; - fp->f_msgcount++; - unp_rights++; - } - } else { - fdp = (int *)(cm + 1); - rp = (struct file **)CMSG_DATA(cm); - for (i = 0; i < oldfds; i++) { - fp = fdescp->fd_ofiles[*fdp++]; - *rp++ = fp; - fp->f_count++; - fp->f_msgcount++; - unp_rights++; +#if 0 + /* + * Transform the file descriptors into struct file pointers. + * If sizeof (struct file *) is bigger than or equal to sizeof int, + * then do it in reverse order so that the int won't get until + * we're done. + * If sizeof (struct file *) is smaller than sizeof int, then + * do it in forward order. + */ + if (sizeof (struct file *) >= sizeof (int)) { +#endif + fdp = (int *)(cm + 1) + oldfds - 1; + rp = (struct file **)CMSG_DATA(cm) + oldfds - 1; + for (i = 0; i < oldfds; i++) { + fp = fdescp->fd_ofiles[*fdp--]; + *rp-- = fp; + fp->f_count++; + fp->f_msgcount++; + unp_rights++; + } +#if 0 + } else { + fdp = (int *)(cm + 1); + rp = (struct file **)CMSG_DATA(cm); + for (i = 0; i < oldfds; i++) { + fp = fdescp->fd_ofiles[*fdp++]; + *rp++ = fp; + fp->f_count++; + fp->f_msgcount++; + unp_rights++; + } + } +#endif + break; } } return (0); >Release-Note: >Audit-Trail: >Unformatted: To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-bugs" in the body of the message
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?E15U8nj-0001a9-00>