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]

Message-ID: <20181220094148.GA13180@lunn.ch>
Date:   Thu, 20 Dec 2018 10:41:48 +0100
From:   Andrew Lunn <andrew@...n.ch>
To:     Florian Fainelli <f.fainelli@...il.com>
Cc:     Marek Vasut <marex@...x.de>, 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 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.

Thanks
       Andrew