Re: [PATCH 5/5] platform/x86: Add Intel Software Defined Silicon driver

[Greg KH]

On Thu, Sep 30, 2021 at 06:28:15PM -0700, David E. Box wrote:
> +static long sdsi_device_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
> +{
> +	struct miscdevice *miscdev = file->private_data;
> +	struct sdsi_priv *priv = to_sdsi_priv(miscdev);
> +	void __user *argp = (void __user *)arg;
> +	long ret = -EINVAL;
> +
> +	if (!priv->dev_present)
> +		return -ENODEV;
> +
> +	if (!priv->sdsi_enabled)
> +		return -EPERM;
> +
> +	if (cmd == SDSI_IF_READ_STATE)
> +		return sdsi_if_read_state_cert(priv, argp);
> +
> +	mutex_lock(&priv->akc_lock);
> +	switch (cmd) {
> +	case SDSI_IF_PROVISION_AKC:
> +		/*
> +		 * While writing an authentication certificate disallow other openers
> +		 * from using AKC or CAP.
> +		 */
> +		if (!priv->akc_owner)
> +			priv->akc_owner = file;
> +
> +		if (priv->akc_owner != file) {

Please explain how this test would ever trigger and how you tested it?

What exactly are you trying to protect from here?  If userspace has your
file descriptor, it can do whatever it wants, don't try to be smarter
than it as you will never win.

And why are you using ioctls at all here?  As you are just
reading/writing to the hardware directly, why not just use a binary
sysfs file to be that pipe?  What requires an ioctl at all?

thanks,

greg k-h