linux-kernel - Re: [PATCH] spi: Ensure memory used for spi_write_then

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] [day] [month] [year] [list]

Message-Id: <b837b2a7-8a88-4782-96d4-1b637bba0664@app.fastmail.com>
Date: Fri, 21 Mar 2025 22:21:03 +0100
From: "Arnd Bergmann" <arnd@...db.de>
To: "Petr Tesarik" <ptesarik@...e.com>
Cc: "Mark Brown" <broonie@...nel.org>,
 "Grant Likely" <grant.likely@...retlab.ca>, linux-kernel@...r.kernel.org,
 linux-spi@...r.kernel.org
Subject: Re: [PATCH] spi: Ensure memory used for spi_write_then_read() is DMA safe

On Fri, Mar 21, 2025, at 15:13, Petr Tesarik wrote:
> On Fri, 21 Mar 2025 13:41:52 +0100
>> Ok, then the question is what we should do about callers that pass
>> in stack data. I can send a patch that adds a WARN_ONCE() or similar,
>> but it would trigger on things like 
>> 
>> static int rt1711h_read16(struct rt1711h_chip *chip, unsigned int reg, u16 *val)
>> {
>>         return regmap_raw_read(chip->data.regmap, reg, val, sizeof(u16));
>> }
>> static int rt1711h_write16(struct rt1711h_chip *chip, unsigned int reg, u16 val)
>> {
>>         return regmap_raw_write(chip->data.regmap, reg, &val, sizeof(u16));
>> }
>> 
>> which happens in a number of drivers but is harmless as long
>> as the driver doesn't actually try to DMA into that buffer.
>
> This sounds like we should push the WARN_ONCE() one level deeper, into
> the DMA code. That's a good idea, actually, because it's always wrong
> to do DMA to a stack address, not just when SPI does it.

This doesn't work for the current SPI code that uses
vmalloc_to_page() in order to deal with vmalloc addresses.
Passing a vmap stack address in here would continue working
on the address from the linear map.

There is already a check_for_stack() assertion in
debug_dma_map_page(), which is meant to catch this problem
in the DMA layer itself, but only when CONFIG_DMA_API_DEBUG
is enabled.

      Arnd