Re: [PATCH 3/4] selftests/sgx: Harden test enclave API

[Jo Van Bulck]

On 20.07.23 19:32, Jarkko Sakkinen wrote:

> > +#define PANIC()								\
> > +	asm("ud2\n\t")
>
> any reason not to use static inline function?

Thanks for the suggestion, no reason in this case. Apart perhaps that 
it's only 1 line of code and an inline function may seem a bit like a 
waste (given that gcc does not inline until certain optimization 
levels). I can surely change it to static inline void panic(void) if you 
prefer?

> > +#define SAFE_COPY_STRUCT(u_arg, t_cp)					\ >> +	do {								\
> > +		/* 1. check if the argument lies entirely outside */	\
> > +		if (!is_outside_enclave((void *)u_arg, sizeof(*t_cp)))	\
> > +			PANIC();					\
> > +		/* 2. copy the argument inside to prevent TOCTOU */	\
> > +		memcpy(t_cp, u_arg, sizeof(*t_cp));			\
> > +	} while (0)
> > +

This could be made into a static inline function, but then t_cp would 
have to be type void* and sizeof(*t_cp) won't work anymore and a third 
parameter to pass the sizeof would be needed, which would require the 
caller to pass it correctly. Hence, a macro seems "safer" to me here in 
this instance as it requires only 2 arguments. Agreed?

> > +#define ASSERT_INSIDE_ENCLAVE(u_arg, size)				\
> > +	do {								\
> > +		if (!is_inside_enclave(((void *)(u_arg)), size))	\
> > +			PANIC();					\
> > +	} while (0)

This macro could certainly be turned into a static inline void function 
if preferred.

Best,
Jo