Re: git: 4a5fa1086184 - main - procfs require PRIV_PROC_MEM_WRITE to write mem

On Thu, Sep 19, 2024 at 08:11:12PM +0000, Simon J. Gerraty wrote:
> The branch main has been updated by sjg:
> 
> URL: https://cgit.FreeBSD.org/src/commit/?id=4a5fa1086184f7450f63d4a8e403b16f40a78fce
> 
> commit 4a5fa1086184f7450f63d4a8e403b16f40a78fce
> Author:     Simon J. Gerraty <sjg@FreeBSD.org>
> AuthorDate: 2024-09-19 20:10:27 +0000
> Commit:     Simon J. Gerraty <sjg@FreeBSD.org>
> CommitDate: 2024-09-19 20:10:27 +0000
> 
>     procfs require PRIV_PROC_MEM_WRITE to write mem
>     
>     Add a priv_check for PRIV_PROC_MEM_WRITE which will be blocked
>     by mac_veriexec if being enforced, unless the process has a maclabel
>     to grant priv.
>     
>     Reviewed by:    stevek
>     Sponsored by:   Juniper Networks, Inc.
>     Differential Revision:  https://reviews.freebsd.org/D46692
> ---
>  sys/fs/procfs/procfs_mem.c                       | 3 +++
>  sys/kern/kern_priv.c                             | 4 +++-
>  sys/security/mac_grantbylabel/mac_grantbylabel.c | 2 ++
>  sys/security/mac_veriexec/mac_veriexec.c         | 1 +
>  sys/sys/priv.h                                   | 1 +
>  5 files changed, 10 insertions(+), 1 deletion(-)
> 
> diff --git a/sys/fs/procfs/procfs_mem.c b/sys/fs/procfs/procfs_mem.c
> index 6ef725ee0ee7..159b40785172 100644
> --- a/sys/fs/procfs/procfs_mem.c
> +++ b/sys/fs/procfs/procfs_mem.c
> @@ -41,6 +41,7 @@
>  #include <sys/ptrace.h>
>  #include <sys/systm.h>
>  #include <sys/uio.h>
> +#include <sys/priv.h>
>  
>  #include <fs/pseudofs/pseudofs.h>
>  #include <fs/procfs/procfs.h>
> @@ -61,6 +62,8 @@ procfs_doprocmem(PFS_FILL_ARGS)
>  
>  	PROC_LOCK(p);
>  	error = p_candebug(td, p);
> +	if (error == 0 && uio->uio_rw == UIO_WRITE)
> +		error = priv_check(td, PRIV_PROC_MEM_WRITE);

Why is this check here and not in proc_rwmem()?  procfs isn't the only
interface to this kind of functionality, and it isn't even the main one.

>  	PROC_UNLOCK(p);
>  	if (error == 0)
>  		error = proc_rwmem(p, uio);
> diff --git a/sys/kern/kern_priv.c b/sys/kern/kern_priv.c
> index c146f9e6f8d5..83fd246eef9b 100644
> --- a/sys/kern/kern_priv.c
> +++ b/sys/kern/kern_priv.c
> @@ -242,7 +242,9 @@ priv_check_cred(struct ucred *cred, int priv)
>  	 * but non-root users are expected to be able to read it (provided they
>  	 * have permission to access /dev/[k]mem).
>  	 */
> -	if (priv == PRIV_KMEM_READ) {
> +	switch (priv) {
> +	case PRIV_KMEM_READ:
> +	case PRIV_PROC_MEM_WRITE:	/* we already checked candebug */
>  		error = 0;
>  		goto out;
>  	}
> diff --git a/sys/security/mac_grantbylabel/mac_grantbylabel.c b/sys/security/mac_grantbylabel/mac_grantbylabel.c
> index 848131e54590..4d14577820eb 100644
> --- a/sys/security/mac_grantbylabel/mac_grantbylabel.c
> +++ b/sys/security/mac_grantbylabel/mac_grantbylabel.c
> @@ -218,6 +218,7 @@ mac_grantbylabel_priv_grant(struct ucred *cred, int priv)
>  		return rc;		/* not interested */
>  
>  	switch (priv) {
> +	case PRIV_PROC_MEM_WRITE:
>  	case PRIV_KMEM_READ:
>  	case PRIV_KMEM_WRITE:
>  		break;
> @@ -244,6 +245,7 @@ mac_grantbylabel_priv_grant(struct ucred *cred, int priv)
>  		if (label & GBL_IPC)
>  			rc = 0;
>  		break;
> +	case PRIV_PROC_MEM_WRITE:
>  	case PRIV_KMEM_READ:
>  	case PRIV_KMEM_WRITE:
>  		if (label & GBL_KMEM)
> diff --git a/sys/security/mac_veriexec/mac_veriexec.c b/sys/security/mac_veriexec/mac_veriexec.c
> index 7ac09e2acf0f..490601863197 100644
> --- a/sys/security/mac_veriexec/mac_veriexec.c
> +++ b/sys/security/mac_veriexec/mac_veriexec.c
> @@ -435,6 +435,7 @@ mac_veriexec_priv_check(struct ucred *cred, int priv)
>  	error = 0;
>  	switch (priv) {
>  	case PRIV_KMEM_WRITE:
> +	case PRIV_PROC_MEM_WRITE:
>  	case PRIV_VERIEXEC_CONTROL:
>  		/*
>  		 * Do not allow writing to memory or manipulating veriexec,
> diff --git a/sys/sys/priv.h b/sys/sys/priv.h
> index a61de8d32fe0..7a5773da220f 100644
> --- a/sys/sys/priv.h
> +++ b/sys/sys/priv.h
> @@ -513,6 +513,7 @@
>   */
>  #define	PRIV_KMEM_READ		680	/* Open mem/kmem for reading. */
>  #define	PRIV_KMEM_WRITE		681	/* Open mem/kmem for writing. */
> +#define	PRIV_PROC_MEM_WRITE	682	/* Open /proc/<pid>/mem for writing. */
>  
>  /*
>   * Kernel debugger privileges.