In message <202110192004.19jk4jn3069...@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 <k...@freebsd.org>
> AuthorDate: 2021-10-15 18:57:17 +0000
> Commit:     Konstantin Belousov <k...@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.schub...@cschubert.com>
FreeBSD UNIX:  <c...@freebsd.org>   Web:  https://FreeBSD.org
NTP:           <c...@nwtime.org>    Web:  https://nwtime.org

        The need of the many outweighs the greed of the few.




Reply via email to