Re: svn commit: r298656 - head/sys/kern
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_pri
Re: svn commit: r298656 - head/sys/kern
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 > > 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.cTue Apr 26 18:11:45 2016(r298655) > +++ head/sys/kern/sysv_msg.cTue 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.cTue Apr 26 18:11:45 2016(r298655) > +++ head/sys/kern/sysv_sem.cTue 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(empt
svn commit: r298656 - head/sys/kern
Author: jamie Date: Tue Apr 26 18:17:44 2016 New Revision: 298656 URL: https://svnweb.freebsd.org/changeset/base/298656 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.cTue Apr 26 18:11:45 2016(r298655) +++ head/sys/kern/sysv_msg.cTue 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.cTue Apr 26 18:11:45 2016(r298655) +++ head/sys/kern/sysv_sem.cTue 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 != -