linux-kernel - Re: [PATCH V7 3/4] i3c: master: Add endianness support for i3c_readl_fifo() and i3c_writel

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: <13bbd85e-48d2-4163-b9f1-2a2a870d4322@app.fastmail.com>
Date: Tue, 23 Sep 2025 20:51:15 +0200
From: "Arnd Bergmann" <arnd@...db.de>
To: "Manikanta Guntupalli" <manikanta.guntupalli@....com>, git@....com,
 "Michal Simek" <michal.simek@....com>,
 "Alexandre Belloni" <alexandre.belloni@...tlin.com>,
 "Frank Li" <Frank.Li@....com>, "Rob Herring" <robh@...nel.org>,
 krzk+dt@...nel.org, "Conor Dooley" <conor+dt@...nel.org>,
 Przemysław Gaj <pgaj@...ence.com>,
 "Wolfram Sang" <wsa+renesas@...g-engineering.com>,
 tommaso.merciai.xr@...renesas.com, quic_msavaliy@...cinc.com,
 Shyam-sundar.S-k@....com, "Sakari Ailus" <sakari.ailus@...ux.intel.com>,
 "'billy_tsai@...eedtech.com'" <billy_tsai@...eedtech.com>,
 "Kees Cook" <kees@...nel.org>, "Gustavo A. R. Silva" <gustavoars@...nel.org>,
 "Jarkko Nikula" <jarkko.nikula@...ux.intel.com>,
 "Jorge Marques" <jorge.marques@...log.com>,
 "linux-i3c@...ts.infradead.org" <linux-i3c@...ts.infradead.org>,
 devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
 Linux-Arch <linux-arch@...r.kernel.org>, linux-hardening@...r.kernel.org
Cc: radhey.shyam.pandey@....com, srinivas.goud@....com,
 shubhrajyoti.datta@....com, manion05gk@...il.com
Subject: Re: [PATCH V7 3/4] i3c: master: Add endianness support for i3c_readl_fifo()
 and i3c_writel_fifo()

On Tue, Sep 23, 2025, at 17:45, Manikanta Guntupalli wrote:
>  /**
>   * i3c_writel_fifo - Write data buffer to 32bit FIFO
>   * @addr: FIFO Address to write to
>   * @buf: Pointer to the data bytes to write
>   * @nbytes: Number of bytes to write
> + * @endian: Endianness of FIFO write
>   */
>  static inline void i3c_writel_fifo(void __iomem *addr, const void *buf,
> -				   int nbytes)
> +				   int nbytes, enum i3c_fifo_endian endian)
>  {
> -	writesl(addr, buf, nbytes / 4);
> +	if (endian)
> +		writesl_be(addr, buf, nbytes / 4);
> +	else
> +		writesl(addr, buf, nbytes / 4);
> +

This seems counter-intuitive: a FIFO doesn't really have
an endianness, it is instead used to transfer a stream of
bytes, so if the device has a fixed endianess, the
FIFO still needs to be read using a plain writesl().

I see that your writesl_be() has an incorrect definition, which
would lead to the i3c_writel_fifo() function accidentally still
working if both the device and CPU use big-endian registers:

static inline void writesl_be(volatile void __iomem *addr,
			      const void *buffer,
			      unsigned int count)
{
	if (count) {
		const u32 *buf = buffer;
		do {
			__raw_writel((u32 __force)__cpu_to_be32(*buf), addr);
			buf++;
		} while (--count);
	}
}

The __cpu_to_be32() call that you add here means that the
FIFO data is swapped on little-endian CPUs but not swapped
on big-endian ones. Compare this to the normal writesl()
function that never swaps because it writes a byte stream.

>  	if (nbytes & 3) {
>  		u32 tmp = 0;
> 
>  		memcpy(&tmp, buf + (nbytes & ~3), nbytes & 3);
> -		writel(tmp, addr);
> +
> +		if (endian)
> +			writel_be(tmp, addr);
> +		else
> +			writel(tmp, addr);

This bit however seems to fix a bug, but does so in a
confusing way. The way the FIFO registers usually deal
with excess bytes is to put them into the first bytes
of the FIFO register, so this should just be a

       writesl(addr, &tmp, 1);

to write one set of four bytes into the FIFO without
endian-swapping.

Could it be that you are just trying to use a normal
i3c adapter with little-endian registers on a normal
big-endian machine but ran into this bug?

     Arnd