linux-kernel - 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: <65427fd4.df0a0220.28d26.1955@mx.google.com>
Date:   Wed, 1 Nov 2023 17:41:54 +0100
From:   Christian Marangi <ansuelsmth@...il.com>
To:     Andrew Lunn <andrew@...n.ch>
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

On Wed, Nov 01, 2023 at 05:32:29PM +0100, Andrew Lunn wrote:
> > > > +	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.
>

There are plenty of firmware around so it can be checked by from what I
have, it looks like they are word aligned... Ok I will use the
get_unaligned and add a comment saying that we assume the iram and dram
section are always word aligned.

Is it ok?

-- 
	Ansuel