netdev - Re: [RFT][PATCH 4/7] net: dsa: microchip: Remove dev->txbuf

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, 20 Dec 2018 15:35:39 +0100
From:   Marek Vasut <marex@...x.de>
To:     Andrew Lunn <andrew@...n.ch>,
        Florian Fainelli <f.fainelli@...il.com>
Cc:     netdev@...r.kernel.org, Tristram Ha <Tristram.Ha@...rochip.com>,
        Woojung Huh <Woojung.Huh@...rochip.com>
Subject: Re: [RFT][PATCH 4/7] net: dsa: microchip: Remove dev->txbuf

On 12/20/2018 10:41 AM, Andrew Lunn wrote:
> On Wed, Dec 19, 2018 at 05:20:33PM -0800, Florian Fainelli wrote:
>> On 12/19/18 5:06 PM, Marek Vasut wrote:
>>> Previous patches unconver that ksz_spi_write() is always ever called
>>> with len = 1, 2 or 4. We can thus drop the if (len > SPI_TX_BUF_LEN)
>>> check and we can also drop the allocation of the txbuf which is part
>>> of the driver data. This wastes 256 bytes for no reason and can be
>>> replaced with 8-byte stack allocated buffer, which is what this patch
>>> does. This is an intermediate step though, which will go away after
>>> regmap conversion.
>>
>> dev is a kmalloc'd buffer, so dev->txbuf is a DMA-able buffer, that
>> could be presumably why it was used in the first place, DMA from the
>> stack is not something safe, but I did not check the core SPI layer or
>> the SPI bus master driver to see whether they do take care of that already.
> 
> Hi Marek
> 
> This is also something i requested for the i2c layering. An i2c bus
> master might use DMA. So you cannot use stack space, or the i2c core
> needs to copy the data to somewhere which is DMA'able.
> 
> Please could you check what regmap offers. If regmap does the copies,
> than this buffer can be removed, but only after the change to regmap
> is performed. You should not remove it before swapping to regmap,
> otherwise you can introducing a regression, which somebody might hit
> during a git bisect.

This code goes away with the regmap completely, so I'll just reorder the
patches and remove it in 6/7 . The 6/7 will need rework anyway, I talked
to Mark Brown about this crazy register layout of this switch and it
seems we'll need a regmap per register width (sigh).

-- 
Best regards,
Marek Vasut