linux-kernel - Re: [PATCH 1/2] Initial support for Allwinner's Security ID fuses

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:	Fri, 24 May 2013 23:50:38 +0200
From:	Oliver Schinagl <oliver+list@...inagl.nl>
To:	Oliver Schinagl <oliver+list@...inagl.nl>
CC:	Maxime Ripard <maxime.ripard@...e-electrons.com>, arnd@...db.de,
	gregkh@...uxfoundation.org, linux-kernel@...r.kernel.org,
	linux-arm-kernel@...ts.infradead.org
Subject: Re: [PATCH 1/2] Initial support for Allwinner's Security ID fuses

On 05/18/13 19:19, Oliver Schinagl wrote:
<snip>
>>> +
>>> +
>>> +/* We read the entire key, using a look up table. Returned is only the
>>> + * requested byte. This is of course slower then it could be and uses 4 times
>>> + * more reads as needed but keeps code a little simpler.
>>> + */
>>> +u8 sunxi_sid_read_byte(const int key)
>>> +{
>>> +	u32 sid_key;
>>> +	u8 ret;
>>> +
>>> +	ret = 0;
>>> +	if (likely((key <= SUNXI_SID_SIZE))) {
>>> +		sid_key = ioread32(p->sid_base + keys_lut[key >> 2]);
>>> +		switch (key % 4) {
>>> +		case 0:
>>> +			ret = (sid_key >> 24) & 0xff;
>>> +			break;
>>> +		case 1:
>>> +			ret = (sid_key >> 16) & 0xff;
>>> +			break;
>>> +		case 2:
>>> +			ret = (sid_key >> 8) & 0xff;
>>> +			break;
>>> +		case 3:
>>> +			ret = sid_key & 0xff;
>>> +			break;
>>> +		}
>>> +	}
>>
>> Come on, you can do better. This lookup table is useless.
> I didn't want to depend on the fixed layout of memory, but consider it
> removed.
But i'm not smart enough :p

We can either use the look up table (which does have benefits as its 
potentially more future proof), or do some ((key >> 2) << 2) to 'drop' 
the LSB's that we want to ignore (unless there's some smarter way).

Personally, I think the LUT is a little cleaner and more readable, but I 
guess if you look at poor efficiency, the lut costs some memory, the 
left/right shift cost an additional >> 2 ... what you prefer.
>>
>> Also, why the first key is the one with the MSBs?
>> I'd expect that the key 0 is the one holding the LSBs.
> Strangely enough, they have swapped the MSB and LSB bytes. I double
> checked it with u-boot and yep, swapped. Though in the end, if we write
> stuff there and we read stuff from there, order doesn't matter? So what
> do we prefer. Have it so that it makes sense and ignore how u-boot reads
> it, or correct it and be consistent?
>
You had me confused and I was looking at this for a little while. 
Bit-ordering does not change, Byte endianness is a different story of 
course. As it is now, I decided to use Big endianess. So now a 32bit key 
looks like:
0x162367c7 and if we read one byte at a time, we get 0x16, 0x23, 0x67 
and 0xc7. I made a comment that data is read as Big endian. If it is 
important, for eeprom data, to be stored little endian, I'll obviously 
change it per request.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/