From: "Theo de Raadt" <deraadt@openbsd.org>
Subject: Re: [EXT] AMD SEV 1/5: ccp(4): pledge for ioctl(2
To: =?iso-8859-1?Q?Hans-J=F6rg_H=F6xer?= <Hans-Joerg_Hoexer@genua.de>
Cc: tech@openbsd.org, mlarkin@nested.page, dv@sisu.io, alexander.bluhm@gmx.net
Date: Wed, 28 Aug 2024 09:48:36 -0600

Yeah that looks better.

Hans-Jörg Höxer <Hans-Joerg_Hoexer@genua.de> wrote:

> Hi,
> 
> On Wed, Aug 28, 2024 at 09:03:05AM -0600, Theo de Raadt wrote:
> > I think those ioctl's should pledge_fail, rather than returning EPERM.
> > Meaning, crash the program that requested an unpermitted operation.
> 
> something like this?
> 
> ---------------------------------------------------------------------
> commit 69b2f37b0e1cbea5a5b14ab945254d94fdb4495e
> Author: Hans-Joerg Hoexer <hshoexer@genua.de>
> Date:   Wed Jul 24 13:54:17 2024 +0200
> 
>     ccp(4): pledge for ioctl(2)
>     
>     Limit ccp(4) ioctls to processes that pledge vmm.
> 
> diff --git a/sys/arch/amd64/include/conf.h b/sys/arch/amd64/include/conf.h
> index 5a2b10fe45b..7e87c2f539d 100644
> --- a/sys/arch/amd64/include/conf.h
> +++ b/sys/arch/amd64/include/conf.h
> @@ -54,3 +54,6 @@ cdev_decl(pctr);
>  
>  #include "vmm.h"
>  cdev_decl(vmm);
> +
> +#include "ccp.h"
> +cdev_decl(psp);
> diff --git a/sys/dev/ic/ccp.c b/sys/dev/ic/ccp.c
> index 5981ae43450..17e96277c26 100644
> --- a/sys/dev/ic/ccp.c
> +++ b/sys/dev/ic/ccp.c
> @@ -24,6 +24,7 @@
>  #include <sys/malloc.h>
>  #include <sys/kernel.h>
>  #include <sys/timeout.h>
> +#include <sys/pledge.h>
>  
>  #include <machine/bus.h>
>  
> @@ -646,12 +647,30 @@ pspioctl(dev_t dev, u_long cmd, caddr_t data, int flag, struct proc *p)
>  		    psp_snp_get_pstatus((struct psp_snp_platform_status *)data);
>  		break;
>  	default:
> -		printf("%s: unkown ioctl code 0x%lx\n", __func__, cmd);
>  		ret = ENOTTY;
> +		break;
>  	}
>  
>  	rw_exit_write(&ccp_softc->sc_lock);
>  
>  	return (ret);
>  }
> +
> +int
> +pledge_ioctl_psp(struct proc *p, long com)
> +{
> +	switch (com) {
> +	case PSP_IOC_GET_PSTATUS:
> +	case PSP_IOC_DF_FLUSH:
> +	case PSP_IOC_GET_GSTATUS:
> +	case PSP_IOC_LAUNCH_START:
> +	case PSP_IOC_LAUNCH_UPDATE_DATA:
> +	case PSP_IOC_LAUNCH_MEASURE:
> +	case PSP_IOC_LAUNCH_FINISH:
> +	case PSP_IOC_ACTIVATE:
> +		return (0);
> +	default:
> +		return (pledge_fail(p, EPERM, PLEDGE_VMM));
> +	}
> +}
>  #endif	/* __amd64__ */
> diff --git a/sys/dev/ic/ccpvar.h b/sys/dev/ic/ccpvar.h
> index 65efe847912..e8e0514610d 100644
> --- a/sys/dev/ic/ccpvar.h
> +++ b/sys/dev/ic/ccpvar.h
> @@ -284,6 +284,7 @@ int	psp_attach(struct ccp_softc *);
>  int	pspclose(dev_t, int, int, struct proc *);
>  int	pspopen(dev_t, int, int, struct proc *);
>  int	pspioctl(dev_t, u_long, caddr_t, int, struct proc *);
> +int	pledge_ioctl_vmm(struct proc *, long);
>  #endif
>  
>  #endif	/* _KERNEL */
> diff --git a/sys/kern/kern_pledge.c b/sys/kern/kern_pledge.c
> index 6ee11243037..a8601aaa24d 100644
> --- a/sys/kern/kern_pledge.c
> +++ b/sys/kern/kern_pledge.c
> @@ -76,6 +76,7 @@
>  #if NVMM > 0
>  #include <machine/conf.h>
>  #endif
> +#include "ccp.h"
>  #endif
>  
>  #include "drm.h"
> @@ -1349,6 +1350,20 @@ pledge_ioctl(struct proc *p, long com, struct file *fp)
>  	}
>  #endif
>  
> +#if NCCP > 0
> +#if NVMM > 0
> +	if ((pledge & PLEDGE_VMM)) {
> +		if ((fp->f_type == DTYPE_VNODE) &&
> +		    (vp->v_type == VCHR) &&
> +		    (cdevsw[major(vp->v_rdev)].d_open == pspopen)) {
> +			error = pledge_ioctl_psp(p, com);
> +			if (error == 0)
> +				return (0);
> +		}
> +	}
> +#endif
> +#endif
> +
>  	return pledge_fail(p, error, PLEDGE_TTY);
>  }
>  
> diff --git a/sys/sys/pledge.h b/sys/sys/pledge.h
> index 073ad9a050a..db2d78b538d 100644
> --- a/sys/sys/pledge.h
> +++ b/sys/sys/pledge.h
> @@ -134,6 +134,7 @@ int	pledge_socket(struct proc *p, int domain, unsigned int state);
>  int	pledge_ioctl(struct proc *p, long com, struct file *);
>  int	pledge_ioctl_drm(struct proc *p, long com, dev_t device);
>  int	pledge_ioctl_vmm(struct proc *p, long com);
> +int	pledge_ioctl_psp(struct proc *p, long com);
>  int	pledge_flock(struct proc *p);
>  int	pledge_fcntl(struct proc *p, int cmd);
>  int	pledge_swapctl(struct proc *p, int cmd);