netdev - Re: [PATCH net-next 1/4] bnx2x: refactor nvram read procedure

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]

Date:	Tue, 16 Apr 2013 01:31:51 +0200
From:	Francois Romieu <romieu@...zoreil.com>
To:	Dmitry Kravkov <dmitry@...adcom.com>
Cc:	"davem@...emloft.net" <davem@...emloft.net>,
	"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
	Eilon Greenstein <eilong@...adcom.com>
Subject: Re: [PATCH net-next 1/4] bnx2x: refactor nvram read procedure

Dmitry Kravkov <dmitry@...adcom.com> :
[...]
> > You memcpy a u32 to an array of bytes instead of copying it byte after
> > byte with proper shift operators and now you are paving the way for more
> > endianess headaches. I'd rather avoid the memcpy when readying data for
> > ethtool in the first place.
> In case of _be data I don't see any issue with copying _be32 it into byte array.

The modified code (bnx2x_nvram_read) will not be copying __be32 but u32.
It issmuggling _be data behind neutral u32 and using casting when the
kernel has provided cpu_to_{le / be} helpers for years. Think of type
checking as kind of messed up as soon as u32 *ret_val contains a _le or
_be data depending on the value of bool to_be.

I don't want to worry about the endianness of a u32. If it's a u32 (u16,
s32, whatever), I only want to think of bytes in it through '>>' or '<<'
operators. No need to remember the semantic of the last bnx2x_nvram_read_dword
parameter, if it's internal, external, nor assume that the lower layers
enforce some ability to memcpy it blindly. So I'd just favor using cpu order
as soon and as much as possible.

Of course it's your driver, whence your maintenance choices.

-- 
Ueimor
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html