netdev - Re: [net-next PATCH v2 1/2] net: phy: aquantia: add firmware load support

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: <34a0b76e-aa0e-4148-ba01-c3b4608f17f7@lunn.ch>
Date: Wed, 1 Nov 2023 17:32:29 +0100
From: Andrew Lunn <andrew@...n.ch>
To: Christian Marangi <ansuelsmth@...il.com>
Cc: "David S. Miller" <davem@...emloft.net>,
	Eric Dumazet <edumazet@...gle.com>,
	Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
	Rob Herring <robh+dt@...nel.org>,
	Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
	Conor Dooley <conor+dt@...nel.org>,
	Heiner Kallweit <hkallweit1@...il.com>,
	Russell King <linux@...linux.org.uk>, netdev@...r.kernel.org,
	devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
	Robert Marko <robimarko@...il.com>
Subject: Re: [net-next PATCH v2 1/2] net: phy: aquantia: add firmware load
 support

> > > +	for (pos = 0; pos < len; pos += min(sizeof(u32), len - pos)) {
> > > +		u32 word = 0;
> > > +
> > > +		memcpy(&word, data + pos, min(sizeof(u32), len - pos));
> > 
> > Rather than do a memcpy, use the get_unaligned_ macros. They might map
> > to a memcpy(), but some architectures can do unaligned accesses
> > without problems.
> > 
> 
> I don't think this is doable for this loop, think we would end up in
> some funny situation where for the last run we have to copy less than
> u32. (get_unaligned would always take u32 of data and that would end up
> reading more than requested) Am I wrong?

Does it happen in practice that the last chunk is not 4 bytes?  Since
this is firmware, its probably produced by some sort of linker, and
they often round segments to words. Could you take a look at the
firmware images you have access to and see if this is true?

It could be we do need to keep with the memcpy, but it would be nice
if we could limit it to words, at least until somebody has a firmware
which is not word aligned.

      Andrew