git: afc10f8bba3d - main - sys_procctl(): Make it clear that negative commands are invalid

The branch main has been updated by olce:

URL: https://cgit.FreeBSD.org/src/commit/?id=afc10f8bba3dd293a66461aaca41237c986b6ca7

commit afc10f8bba3dd293a66461aaca41237c986b6ca7
Author:     Olivier Certner <olce@FreeBSD.org>
AuthorDate: 2024-04-10 14:32:32 +0000
Commit:     Olivier Certner <olce@FreeBSD.org>
CommitDate: 2024-04-10 15:15:25 +0000

    sys_procctl(): Make it clear that negative commands are invalid
    
    An initial reading of the preamble of sys_procctl() gives the impression
    that no test prevents a malicious user from passing a negative commands
    index (in 'uap->com'), which is soon used as an index into the static
    array procctl_cmds_info[].
    
    However, a closer examination leads to the conclusion that the existing
    code is technically correct.  Indeed, the comparison of 'uap->com' to
    the nitems() expression, which expands to a ratio of sizeof(), leads to
    a conversion of 'uap->com' to an 'unsigned int' as per Usual Arithmetic
    Conversions/Integer Promotions applied by '<=', because sizeof() returns
    'size_t' values, and we define 'size_t' as an equivalent of 'unsigned
    int' (which is not mandated by the standard, the latter allowing, e.g.,
    integers of lower ranks).
    
    With this conversion, negative values of 'uap->com' are automatically
    ruled-out since they are converted to very big unsigned integers which
    are caught by the test.  An analysis of assembly code produced by LLVM
    16 on amd64 and practical tests confirm that no exploitation is possible.
    
    However, the guard code as written is misleading to readers and might
    trip up static analysis tools.  Make sure that negative values are
    explicitly excluded so that it is immediately clear that EINVAL will be
    returned in this case.
    
    Build tested with clang 16 and GCC 12.
    
    Approved by:    markj (mentor)
    MFC after:      1 week
    Sponsored by:   The FreeBSD Foundation
---
 sys/kern/kern_procctl.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sys/kern/kern_procctl.c b/sys/kern/kern_procctl.c
index 46ddfaf709bd..150a8612c2f8 100644
--- a/sys/kern/kern_procctl.c
+++ b/sys/kern/kern_procctl.c
@@ -1126,7 +1126,7 @@ sys_procctl(struct thread *td, struct procctl_args *uap)
 	if (uap->com >= PROC_PROCCTL_MD_MIN)
 		return (cpu_procctl(td, uap->idtype, uap->id,
 		    uap->com, uap->data));
-	if (uap->com == 0 || uap->com >= nitems(procctl_cmds_info))
+	if (uap->com <= 0 || uap->com >= nitems(procctl_cmds_info))
 		return (EINVAL);
 	cmd_info = &procctl_cmds_info[uap->com];
 	bzero(&x, sizeof(x));