From owner-svn-src-head@freebsd.org Tue Apr 26 18:49:25 2016 Return-Path: Delivered-To: svn-src-head@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id F0E33B1C939; Tue, 26 Apr 2016 18:49:25 +0000 (UTC) (envelope-from jamie@freebsd.org) Received: from gritton.org (gritton.org [162.220.209.3]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client CN "www.gritton.org", Issuer "StartCom Class 1 Primary Intermediate Server CA" (not verified)) by mx1.freebsd.org (Postfix) with ESMTPS id D64141E4A; Tue, 26 Apr 2016 18:49:25 +0000 (UTC) (envelope-from jamie@freebsd.org) Received: from gritton.org (gritton.org [162.220.209.3]) by gritton.org (8.15.2/8.15.2) with ESMTPS id u3QInNCN044745 (version=TLSv1.2 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=NO); Tue, 26 Apr 2016 12:49:24 -0600 (MDT) (envelope-from jamie@freebsd.org) Received: (from www@localhost) by gritton.org (8.15.2/8.15.2/Submit) id u3QInNdn044744; Tue, 26 Apr 2016 12:49:23 -0600 (MDT) (envelope-from jamie@freebsd.org) X-Authentication-Warning: gritton.org: www set sender to jamie@freebsd.org using -f To: cem@freebsd.org Subject: Re: svn commit: r298656 - head/sys/kern X-PHP-Originating-Script: 0:rcube.php MIME-Version: 1.0 Date: Tue, 26 Apr 2016 12:49:23 -0600 From: James Gritton Cc: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org In-Reply-To: References: <201604261817.u3QIHi2e094653@repo.freebsd.org> Message-ID: <156985b5d93d652823cfa5b8943f3568@gritton.org> X-Sender: jamie@freebsd.org User-Agent: Roundcube Webmail/1.1.2 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit X-Content-Filtered-By: Mailman/MimeDel 2.1.21 X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 26 Apr 2016 18:49:26 -0000 It turns out I was just about to commit that anyway, so the only difference is I didn't need to put "(mis)" in the comment :-). ipcs(1) was prone to allocation errors that I could bypass by rewriting the sysctls without sbufs. So false positive, but for the best anyway. - Jamie On 2016-04-26 12:46, Conrad Meyer wrote: > The sbuf use here was fine before. > > Best, Conrad > > On Tue, Apr 26, 2016 at 11:17 AM, Jamie Gritton wrote: > >> Author: jamie >> Date: Tue Apr 26 18:17:44 2016 >> New Revision: 298656 >> URL: https://svnweb.freebsd.org/changeset/base/298656 [1] >> >> Log: >> Redo the changes to the SYSV IPC sysctl functions from r298585, so they >> don't (mis)use sbufs. >> >> PR: 48471 >> >> Modified: >> head/sys/kern/sysv_msg.c >> head/sys/kern/sysv_sem.c >> head/sys/kern/sysv_shm.c >> >> Modified: head/sys/kern/sysv_msg.c >> ============================================================================== >> --- head/sys/kern/sysv_msg.c Tue Apr 26 18:11:45 2016 (r298655) >> +++ head/sys/kern/sysv_msg.c Tue Apr 26 18:17:44 2016 (r298656) >> @@ -65,7 +65,6 @@ __FBSDID("$FreeBSD$"); >> #include >> #include >> #include >> -#include >> #include >> #include >> #include >> @@ -1423,38 +1422,28 @@ sys_msgrcv(td, uap) >> static int >> sysctl_msqids(SYSCTL_HANDLER_ARGS) >> { >> - struct sbuf sb; >> - struct msqid_kernel tmp, empty; >> - struct msqid_kernel *msqkptr; >> - struct prison *rpr; >> + struct msqid_kernel tmsqk; >> + struct prison *pr, *rpr; >> int error, i; >> >> - error = sysctl_wire_old_buffer(req, 0); >> - if (error != 0) >> - goto done; >> + pr = req->td->td_ucred->cr_prison; >> rpr = msg_find_prison(req->td->td_ucred); >> - sbuf_new_for_sysctl(&sb, NULL, sizeof(struct msqid_kernel) * >> - msginfo.msgmni, req); >> - >> - bzero(&empty, sizeof(empty)); >> + error = 0; >> for (i = 0; i < msginfo.msgmni; i++) { >> - msqkptr = &msqids[i]; >> - if (msqkptr->u.msg_qbytes == 0 || rpr == NULL || >> - msq_prison_cansee(rpr, msqkptr) != 0) { >> - msqkptr = ∅ >> - } else if (req->td->td_ucred->cr_prison != >> - msqkptr->cred->cr_prison) { >> - bcopy(msqkptr, &tmp, sizeof(tmp)); >> - msqkptr = &tmp; >> - msqkptr->u.msg_perm.key = IPC_PRIVATE; >> + mtx_lock(&msq_mtx); >> + if (msqids[i].u.msg_qbytes == 0 || rpr == NULL || >> + msq_prison_cansee(rpr, &msqids[i]) != 0) >> + bzero(&tmsqk, sizeof(tmsqk)); >> + else { >> + tmsqk = msqids[i]; >> + if (tmsqk.cred->cr_prison != pr) >> + tmsqk.u.msg_perm.key = IPC_PRIVATE; >> } >> - >> - sbuf_bcat(&sb, msqkptr, sizeof(*msqkptr)); >> + mtx_unlock(&msq_mtx); >> + error = SYSCTL_OUT(req, &tmsqk, sizeof(tmsqk)); >> + if (error != 0) >> + break; >> } >> - error = sbuf_finish(&sb); >> - sbuf_delete(&sb); >> - >> -done: >> return (error); >> } >> >> @@ -1470,7 +1459,8 @@ SYSCTL_INT(_kern_ipc, OID_AUTO, msgssz, >> "Size of a message segment"); >> SYSCTL_INT(_kern_ipc, OID_AUTO, msgseg, CTLFLAG_RDTUN, &msginfo.msgseg, 0, >> "Number of message segments"); >> -SYSCTL_PROC(_kern_ipc, OID_AUTO, msqids, CTLTYPE_OPAQUE | CTLFLAG_RD, >> +SYSCTL_PROC(_kern_ipc, OID_AUTO, msqids, >> + CTLTYPE_OPAQUE | CTLFLAG_RD | CTLFLAG_MPSAFE, >> NULL, 0, sysctl_msqids, "", "Message queue IDs"); >> >> static int >> >> Modified: head/sys/kern/sysv_sem.c >> ============================================================================== >> --- head/sys/kern/sysv_sem.c Tue Apr 26 18:11:45 2016 (r298655) >> +++ head/sys/kern/sysv_sem.c Tue Apr 26 18:17:44 2016 (r298656) >> @@ -52,7 +52,6 @@ __FBSDID("$FreeBSD$"); >> #include >> #include >> #include >> -#include >> #include >> #include >> #include >> @@ -220,7 +219,8 @@ SYSCTL_INT(_kern_ipc, OID_AUTO, semvmx, >> "Semaphore maximum value"); >> SYSCTL_INT(_kern_ipc, OID_AUTO, semaem, CTLFLAG_RWTUN, &seminfo.semaem, 0, >> "Adjust on exit max value"); >> -SYSCTL_PROC(_kern_ipc, OID_AUTO, sema, CTLTYPE_OPAQUE | CTLFLAG_RD, >> +SYSCTL_PROC(_kern_ipc, OID_AUTO, sema, >> + CTLTYPE_OPAQUE | CTLFLAG_RD | CTLFLAG_MPSAFE, >> NULL, 0, sysctl_sema, "", "Semaphore id pool"); >> >> static struct syscall_helper_data sem_syscalls[] = { >> @@ -1465,38 +1465,28 @@ semexit_myhook(void *arg, struct proc *p >> static int >> sysctl_sema(SYSCTL_HANDLER_ARGS) >> { >> - struct prison *rpr; >> - struct sbuf sb; >> - struct semid_kernel tmp, empty; >> - struct semid_kernel *semakptr; >> + struct prison *pr, *rpr; >> + struct semid_kernel tsemak; >> int error, i; >> >> - error = sysctl_wire_old_buffer(req, 0); >> - if (error != 0) >> - goto done; >> + pr = req->td->td_ucred->cr_prison; >> rpr = sem_find_prison(req->td->td_ucred); >> - sbuf_new_for_sysctl(&sb, NULL, sizeof(struct semid_kernel) * >> - seminfo.semmni, req); >> - >> - bzero(&empty, sizeof(empty)); >> + error = 0; >> for (i = 0; i < seminfo.semmni; i++) { >> - semakptr = &sema[i]; >> - if ((semakptr->u.sem_perm.mode & SEM_ALLOC) == 0 || >> - rpr == NULL || sem_prison_cansee(rpr, semakptr) != 0) { >> - semakptr = ∅ >> - } else if (req->td->td_ucred->cr_prison != >> - semakptr->cred->cr_prison) { >> - bcopy(semakptr, &tmp, sizeof(tmp)); >> - semakptr = &tmp; >> - semakptr->u.sem_perm.key = IPC_PRIVATE; >> + mtx_lock(&sema_mtx[i]); >> + if ((sema[i].u.sem_perm.mode & SEM_ALLOC) == 0 || >> + rpr == NULL || sem_prison_cansee(rpr, &sema[i]) != 0) >> + bzero(&tsemak, sizeof(tsemak)); >> + else { >> + tsemak = sema[i]; >> + if (tsemak.cred->cr_prison != pr) >> + tsemak.u.sem_perm.key = IPC_PRIVATE; >> } >> - >> - sbuf_bcat(&sb, semakptr, sizeof(*semakptr)); >> + mtx_unlock(&sema_mtx[i]); >> + error = SYSCTL_OUT(req, &tsemak, sizeof(tsemak)); >> + if (error != 0) >> + break; >> } >> - error = sbuf_finish(&sb); >> - sbuf_delete(&sb); >> - >> -done: >> return (error); >> } >> >> Modified: head/sys/kern/sysv_shm.c >> ============================================================================== >> --- head/sys/kern/sysv_shm.c Tue Apr 26 18:11:45 2016 (r298655) >> +++ head/sys/kern/sysv_shm.c Tue Apr 26 18:17:44 2016 (r298656) >> @@ -80,7 +80,6 @@ __FBSDID("$FreeBSD$"); >> #include >> #include >> #include >> -#include >> #include >> #include >> #include >> @@ -1009,40 +1008,28 @@ shmunload(void) >> static int >> sysctl_shmsegs(SYSCTL_HANDLER_ARGS) >> { >> - struct prison *rpr; >> - struct sbuf sb; >> - struct shmid_kernel tmp, empty; >> - struct shmid_kernel *shmseg; >> + struct shmid_kernel tshmseg; >> + struct prison *pr, *rpr; >> int error, i; >> >> SYSVSHM_LOCK(); >> - >> - error = sysctl_wire_old_buffer(req, 0); >> - if (error != 0) >> - goto done; >> + pr = req->td->td_ucred->cr_prison; >> rpr = shm_find_prison(req->td->td_ucred); >> - sbuf_new_for_sysctl(&sb, NULL, shmalloced * sizeof(shmsegs[0]), req); >> - >> - bzero(&empty, sizeof(empty)); >> - empty.u.shm_perm.mode = SHMSEG_FREE; >> + error = 0; >> for (i = 0; i < shmalloced; i++) { >> - shmseg = &shmsegs[i]; >> - if ((shmseg->u.shm_perm.mode & SHMSEG_ALLOCATED) == 0 || >> + if ((shmsegs[i].u.shm_perm.mode & SHMSEG_ALLOCATED) == 0 || >> rpr == NULL || shm_prison_cansee(rpr, &shmsegs[i]) != 0) { >> - shmseg = ∅ >> - } else if (req->td->td_ucred->cr_prison != >> - shmseg->cred->cr_prison) { >> - bcopy(shmseg, &tmp, sizeof(tmp)); >> - shmseg = &tmp; >> - shmseg->u.shm_perm.key = IPC_PRIVATE; >> + bzero(&tshmseg, sizeof(tshmseg)); >> + tshmseg.u.shm_perm.mode = SHMSEG_FREE; >> + } else { >> + tshmseg = shmsegs[i]; >> + if (tshmseg.cred->cr_prison != pr) >> + tshmseg.u.shm_perm.key = IPC_PRIVATE; >> } >> - >> - sbuf_bcat(&sb, shmseg, sizeof(*shmseg)); >> + error = SYSCTL_OUT(req, &tshmseg, sizeof(tshmseg)); >> + if (error != 0) >> + break; >> } >> - error = sbuf_finish(&sb); >> - sbuf_delete(&sb); >> - >> -done: >> SYSVSHM_UNLOCK(); >> return (error); >> } Links: ------ [1] https://svnweb.freebsd.org/changeset/base/298656