netdev - Re: [PATCH lora-next v2 3/8] net: lora: sx1301: convert to passing priv data throughout

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: <ec5207a5-36b0-efb2-b8f3-d6387d7b4347@suse.de>
Date:   Thu, 9 Aug 2018 23:21:57 +0200
From:   Andreas Färber <afaerber@...e.de>
To:     Ben Whitten <benwhitten@...il.com>
Cc:     潘建宏 <starnight@...cu.edu.tw>,
        hasnain.virk@....com, netdev@...r.kernel.org,
        Xue Liu <liuxuenetmail@...il.com>, shess@...sware.de,
        Ben Whitten <ben.whitten@...rdtech.com>,
        Yannick Lanz <yannick.lanz@...ixone.io>
Subject: Re: [PATCH lora-next v2 3/8] net: lora: sx1301: convert to passing
 priv data throughout

Am 09.08.2018 um 23:06 schrieb Ben Whitten:
> On Thu, 9 Aug 2018 at 21:43, Andreas Färber <afaerber@...e.de> wrote:
>> Am 09.08.2018 um 14:33 schrieb Ben Whitten:
>>> @@ -654,22 +646,35 @@ static int sx1301_probe(struct spi_device *spi)
>>>       priv->rst_gpio = rst;
>>>       priv->cur_page = 0xff;
>>>
>>> -     spi_set_drvdata(spi, netdev);
>>> +     spi_set_drvdata(spi, priv);
>>
>> This change seems unnecessary and counter-productive for unregistration.
>>
>> Otherwise applying.
> 
> This is actually pretty critical, as it stands with the two spi masters we use
> spi_get_drvdata on the parent device of the controller to recover the priv
> struct for regmap.
> 
> We may have to include the netdev in the priv data, or do a container_of
> dance to recover netdev in unregistration.
> That said if we wrap things in devm then really our remove function could
> be empty, as we have done with the allocation.

Thanks for quickly noticing. This should compensate:

diff --git a/drivers/net/lora/sx1301.c b/drivers/net/lora/sx1301.c
index 0ba841f8e7cd..43cd2308e41c 100644
--- a/drivers/net/lora/sx1301.c
+++ b/drivers/net/lora/sx1301.c
@@ -164,7 +164,8 @@ static int sx1301_soft_reset(struct sx1301_priv *priv)
 static int sx1301_radio_set_cs(struct spi_controller *ctrl, bool enable)
 {
        struct spi_sx1301 *ssx = spi_controller_get_devdata(ctrl);
-       struct sx1301_priv *priv = spi_get_drvdata(ssx->parent);
+       struct net_device *netdev = spi_get_drvdata(ssx->parent);
+       struct sx1301_priv *priv = netdev_priv(netdev);
        u8 cs;
        int ret;

@@ -204,7 +205,8 @@ static int sx1301_radio_spi_transfer_one(struct
spi_controller *ctrl,
        struct spi_device *spi, struct spi_transfer *xfr)
 {
        struct spi_sx1301 *ssx = spi_controller_get_devdata(ctrl);
-       struct sx1301_priv *priv = spi_get_drvdata(ssx->parent);
+       struct net_device *netdev = spi_get_drvdata(ssx->parent);
+       struct sx1301_priv *priv = netdev_priv(netdev);
        const u8 *tx_buf = xfr->tx_buf;
        u8 *rx_buf = xfr->rx_buf;
        int ret;

Regards,
Andreas

-- 
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)