Date: Tue, 19 Oct 2021 13:35:13 -0700 From: Cy Schubert <Cy.Schubert@cschubert.com> To: Konstantin Belousov <kib@FreeBSD.org> Cc: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org Subject: Re: git: 7ae879b14a20 - main - kern_procctl(): convert the function to be table-driven Message-ID: <202110192035.19JKZDqM026085@slippy.cwsent.com> In-Reply-To: <202110192004.19JK4jN3069844@gitrepo.freebsd.org> References: <202110192004.19JK4jN3069844@gitrepo.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
In message <202110192004.19JK4jN3069844@gitrepo.freebsd.org>, Konstantin Belous ov writes: > The branch main has been updated by kib: > > URL: https://cgit.FreeBSD.org/src/commit/?id=7ae879b14a2086df521c59c4a379d3a0 > 72e08bc6 > > commit 7ae879b14a2086df521c59c4a379d3a072e08bc6 > Author: Konstantin Belousov <kib@FreeBSD.org> > AuthorDate: 2021-10-15 18:57:17 +0000 > Commit: Konstantin Belousov <kib@FreeBSD.org> > CommitDate: 2021-10-19 20:04:34 +0000 > > kern_procctl(): convert the function to be table-driven > > Reviewed by: emaste, markj > Sponsored by: The FreeBSD Foundation > MFC after: 1 week > Differential revision: https://reviews.freebsd.org/D32513 > --- > sys/kern/kern_procctl.c | 123 +++++++++++++++++++++++++++------------------- > -- > 1 file changed, 69 insertions(+), 54 deletions(-) > > diff --git a/sys/kern/kern_procctl.c b/sys/kern/kern_procctl.c > index eb36f0822938..90c5e63c7219 100644 > --- a/sys/kern/kern_procctl.c > +++ b/sys/kern/kern_procctl.c > @@ -656,6 +656,57 @@ wxmap_status(struct thread *td, struct proc *p, int *dat > a) > return (0); > } > > +struct procctl_cmd_info { > + int lock_tree; > + bool one_proc : 1; > +}; > +static const struct procctl_cmd_info procctl_cmds_info[] = { > + [PROC_SPROTECT] = > + { .lock_tree = SA_SLOCKED, .one_proc = false, }, > + [PROC_REAP_ACQUIRE] = > + { .lock_tree = SA_XLOCKED, .one_proc = true, }, > + [PROC_REAP_RELEASE] = > + { .lock_tree = SA_XLOCKED, .one_proc = true, }, > + [PROC_REAP_STATUS] = > + { .lock_tree = SA_SLOCKED, .one_proc = true, }, > + [PROC_REAP_GETPIDS] = > + { .lock_tree = SA_SLOCKED, .one_proc = true, }, > + [PROC_REAP_KILL] = > + { .lock_tree = SA_SLOCKED, .one_proc = true, }, > + [PROC_TRACE_CTL] = > + { .lock_tree = SA_SLOCKED, .one_proc = false, }, > + [PROC_TRACE_STATUS] = > + { .lock_tree = SA_UNLOCKED, .one_proc = true, }, > + [PROC_TRAPCAP_CTL] = > + { .lock_tree = SA_SLOCKED, .one_proc = false, }, > + [PROC_TRAPCAP_STATUS] = > + { .lock_tree = SA_UNLOCKED, .one_proc = true, }, > + [PROC_PDEATHSIG_CTL] = > + { .lock_tree = SA_UNLOCKED, .one_proc = true, }, > + [PROC_PDEATHSIG_STATUS] = > + { .lock_tree = SA_UNLOCKED, .one_proc = true, }, > + [PROC_ASLR_CTL] = > + { .lock_tree = SA_UNLOCKED, .one_proc = true, }, > + [PROC_ASLR_STATUS] = > + { .lock_tree = SA_UNLOCKED, .one_proc = true, }, > + [PROC_PROTMAX_CTL] = > + { .lock_tree = SA_UNLOCKED, .one_proc = true, }, > + [PROC_PROTMAX_STATUS] = > + { .lock_tree = SA_UNLOCKED, .one_proc = true, }, > + [PROC_STACKGAP_CTL] = > + { .lock_tree = SA_UNLOCKED, .one_proc = true, }, > + [PROC_STACKGAP_STATUS] = > + { .lock_tree = SA_UNLOCKED, .one_proc = true, }, > + [PROC_NO_NEW_PRIVS_CTL] = > + { .lock_tree = SA_SLOCKED, .one_proc = true, }, > + [PROC_NO_NEW_PRIVS_STATUS] = > + { .lock_tree = SA_UNLOCKED, .one_proc = true, }, > + [PROC_WXMAP_CTL] = > + { .lock_tree = SA_UNLOCKED, .one_proc = true, }, > + [PROC_WXMAP_STATUS] = > + { .lock_tree = SA_UNLOCKED, .one_proc = true, }, > +}; > + > int > sys_procctl(struct thread *td, struct procctl_args *uap) > { > @@ -812,33 +863,14 @@ kern_procctl(struct thread *td, idtype_t idtype, id_t i > d, int com, void *data) > { > struct pgrp *pg; > struct proc *p; > + const struct procctl_cmd_info *cmd_info; > int error, first_error, ok; > int signum; > - bool tree_locked; > > - switch (com) { > - case PROC_ASLR_CTL: > - case PROC_ASLR_STATUS: > - case PROC_PROTMAX_CTL: > - case PROC_PROTMAX_STATUS: > - case PROC_REAP_ACQUIRE: > - case PROC_REAP_RELEASE: > - case PROC_REAP_STATUS: > - case PROC_REAP_GETPIDS: > - case PROC_REAP_KILL: > - case PROC_STACKGAP_CTL: > - case PROC_STACKGAP_STATUS: > - case PROC_TRACE_STATUS: > - case PROC_TRAPCAP_STATUS: > - case PROC_PDEATHSIG_CTL: > - case PROC_PDEATHSIG_STATUS: > - case PROC_NO_NEW_PRIVS_CTL: > - case PROC_NO_NEW_PRIVS_STATUS: > - case PROC_WXMAP_CTL: > - case PROC_WXMAP_STATUS: > - if (idtype != P_PID) > - return (EINVAL); > - } > + MPASS(com > 0 && com < nitems(procctl_cmds_info)); > + cmd_info = &procctl_cmds_info[com]; > + if (idtype != P_PID && cmd_info->one_proc) > + return (EINVAL); > > switch (com) { > case PROC_PDEATHSIG_CTL: > @@ -861,37 +893,13 @@ kern_procctl(struct thread *td, idtype_t idtype, id_t i > d, int com, void *data) > return (0); > } > > - switch (com) { > - case PROC_SPROTECT: > - case PROC_REAP_STATUS: > - case PROC_REAP_GETPIDS: > - case PROC_REAP_KILL: > - case PROC_TRACE_CTL: > - case PROC_TRAPCAP_CTL: > - case PROC_NO_NEW_PRIVS_CTL: > - sx_slock(&proctree_lock); > - tree_locked = true; > - break; > - case PROC_REAP_ACQUIRE: > - case PROC_REAP_RELEASE: > + switch (cmd_info->lock_tree) { > + case SA_XLOCKED: > sx_xlock(&proctree_lock); > - tree_locked = true; > break; > - case PROC_ASLR_CTL: > - case PROC_ASLR_STATUS: > - case PROC_PROTMAX_CTL: > - case PROC_PROTMAX_STATUS: > - case PROC_STACKGAP_CTL: > - case PROC_STACKGAP_STATUS: > - case PROC_TRACE_STATUS: > - case PROC_TRAPCAP_STATUS: > - case PROC_NO_NEW_PRIVS_STATUS: > - case PROC_WXMAP_CTL: > - case PROC_WXMAP_STATUS: > - tree_locked = false; > + case SA_SLOCKED: > + sx_slock(&proctree_lock); > break; > - default: > - return (EINVAL); > } > > switch (idtype) { > @@ -949,7 +957,14 @@ kern_procctl(struct thread *td, idtype_t idtype, id_t id > , int com, void *data) > error = EINVAL; > break; > } > - if (tree_locked) > - sx_unlock(&proctree_lock); > + > + switch (cmd_info->lock_tree) { > + case SA_XLOCKED: > + sx_xunlock(&proctree_lock); > + break; > + case SA_SLOCKED: > + sx_sunlock(&proctree_lock); > + break; > + } > return (error); > } > Should SA_* in fact be LA_*? SA_* in sys/sx.h assumes INVARIANTS whereas LA_* in sys/lock.h has no such requirement. -- Cheers, Cy Schubert <Cy.Schubert@cschubert.com> FreeBSD UNIX: <cy@FreeBSD.org> Web: https://FreeBSD.org NTP: <cy@nwtime.org> Web: https://nwtime.org The need of the many outweighs the greed of the few.
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202110192035.19JKZDqM026085>