From: Hans-Jörg Höxer <Hans-Joerg_Hoexer@genua.de>
Subject: Re: [EXT] AMD SEV 1/5: ccp(4): pledge for ioctl(2
To: Hans-Jörg Höxer <Hans-Joerg_Hoexer@genua.de>, <tech@openbsd.org>
Date: Thu, 29 Aug 2024 11:15:55 +0200

Hi,

On Thu, Aug 29, 2024 at 10:14:12AM +1000, Jonathan Gray wrote:
> On Thu, Aug 29, 2024 at 09:28:50AM +1000, Jonathan Gray wrote:
> > >  
> > > +#if NCCP > 0
> > > +#if NVMM > 0
> > 
> > can't this be only #if NCCP > 0?

yes, there's actually no need for depending on NVMM

> > if it can't, #if NCCP > 0 && NVMM > 0 would be better
> 
> pspopen is under #ifdef __amd64__
> 
> #if defined(__amd64__) && NCCP > 0
> 
> to make this build if arm64 gets vmm
> (or drop the ifdef in ccp.c)

it's not visible from the diff -- the chunk size is too short -- but the
include statement for ccp.h is already within #if defined(__amd64__).
Along with the statement for vmm.h.  So this should be safe already.
Nonetheless, making this explicit as you suggest makes sense.

updated diff below.
------------------------------------------------------------------------
commit 675c633aceffbb7b329fa9b4ac34ee5708d9bf53
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/kern/kern_pledge.c b/sys/kern/kern_pledge.c
index 6ee11243037..1e1204ad472 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,18 @@ pledge_ioctl(struct proc *p, long com, struct file *fp)
 	}
 #endif
 
+#if defined(__amd64__) && NCCP > 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
+
 	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);