netdev - Re: [PATCH v2] net: phy: mdio-gpio: fix access that may sleep

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: <93d2cb1c-47ee-0b9b-472a-a7fa8a2dfd3d@cogentembedded.com>
Date:   Wed, 14 Nov 2018 12:20:18 +0300
From:   Sergei Shtylyov <sergei.shtylyov@...entembedded.com>
To:     Martin Schiller <ms@....tdt.de>, netdev@...r.kernel.org,
        linux-kernel@...r.kernel.org
Cc:     andrew@...n.ch, f.fainelli@...il.com, davem@...emloft.net
Subject: Re: [PATCH v2] net: phy: mdio-gpio: fix access that may sleep

Hello!

On 14.11.2018 9:37, Martin Schiller wrote:

> This commit re-enables support for slow GPIO pins. It was initially
> introduced by commit
> 	2d6c9091ab7630dfcf34417c6683ce4764d7d40a
> and got lost by commit
> 	7e5fbd1e0700f1bdb94508f84ec2aeb01eed7b12

    This is not how you cite a commit: 12 digits is enough and they should
be followed by the commit summary enclosed in ("").

> 
> Also add a warning about slow GPIO pins like it is done in i2c-gpio.
> 
> Signed-off-by: Martin Schiller <ms@....tdt.de>
> ---
> v2:
>   - fixed copy/paste bug in warning about slow GPIO pins
> ---
>   drivers/net/phy/mdio-gpio.c | 15 ++++++++++-----
>   1 file changed, 10 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/net/phy/mdio-gpio.c b/drivers/net/phy/mdio-gpio.c
> index 33265747bf39..6c1cca14689b 100644
> --- a/drivers/net/phy/mdio-gpio.c
> +++ b/drivers/net/phy/mdio-gpio.c
[...]
> @@ -162,6 +162,11 @@ static int mdio_gpio_probe(struct platform_device *pdev)
>   	if (ret)
>   		return ret;
>   
> +	if (gpiod_cansleep(bitbang->mdc) || gpiod_cansleep(bitbang->mdio)
> +		|| gpiod_cansleep(bitbang->mdo))

    The line continued that way blends with the branch below, the networking
code uses a different style of line continuation where the continuation line
starts immediately below the 1st gpiod_cansleep() call. Also, || should be
left on the 1st line...

> +		dev_warn(&pdev->dev, "Slow GPIO pins might wreak havoc into"
> +				     "MDIO bus timing");
> +
>   	if (pdev->dev.of_node) {
>   		bus_id = of_alias_get_id(pdev->dev.of_node, "mdio-gpio");
>   		if (bus_id < 0) {

MBR, Sergei