linux-kernel - Re: [PATCH] spidev: add platform driver support

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]

Date:   Thu, 27 May 2021 11:10:21 +0100
From:   Mark Brown <broonie@...nel.org>
To:     Christian Gmeiner <christian.gmeiner@...il.com>
Cc:     linux-kernel@...r.kernel.org, linux-spi@...r.kernel.org
Subject: Re: [PATCH] spidev: add platform driver support

On Thu, May 27, 2021 at 10:45:15AM +0200, Christian Gmeiner wrote:

> This makes it possible to use spidev in combination with the
> MFD subsystem. The MFD subsystem add platform_driver devices.

This is a really strange thing to want to do so it needs a
changelog which explains what the goal is and why this is a good
way of accomplishing that goal.

> +static int spidev_platform_probe(struct platform_device *pdev)
> +{
> +	struct device *parent = pdev->dev.parent;
> +	struct spi_device *spi;
> +
> +	if (strcmp(parent->bus->name, "spi"))
> +		return -ENODEV;
> +
> +	spi = to_spi_device(parent);
> +
> +	/* This only works if no drvdata is stored */
> +	if (spi_get_drvdata(spi)) {
> +		dev_err(&pdev->dev, "drvdata is not NULL\n");

Why?

> +		return -EOPNOTSUPP;
> +	}
> +
> +	return spidev_probe(spi);

This really does not seem like a good idea, this is exposing the
entire device to userspace in a completely unstructured fashion
while there will be other drivers controlling the same hardware.
That seems like it's asking for trouble, there's absolutely
nothing ensuring that userspace doesn't break things the drivers
are doing.

I really don't think it makes sense to mix kernel drivers with
unmoderated userspace access to the hardware in a single driver.

> +static struct platform_driver spidev_platfoem_driver = {

platfoem?

Download attachment "signature.asc" of type "application/pgp-signature" (489 bytes)