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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <YVa46eU1VX7CM+Xd@kroah.com>
Date:   Fri, 1 Oct 2021 09:29:45 +0200
From:   Greg KH <gregkh@...uxfoundation.org>
To:     "David E. Box" <david.e.box@...ux.intel.com>
Cc:     lee.jones@...aro.org, hdegoede@...hat.com, mgross@...ux.intel.com,
        bhelgaas@...gle.com, andriy.shevchenko@...ux.intel.com,
        srinivas.pandruvada@...el.com, linux-kernel@...r.kernel.org,
        linux-doc@...r.kernel.org, platform-driver-x86@...r.kernel.org,
        linux-pci@...r.kernel.org
Subject: Re: [PATCH 5/5] platform/x86: Add Intel Software Defined Silicon
 driver

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