[U-Boot] Potential memory corruption in drivers/net/sh_eth.c ?

[Wolfgang Denk]

Dear Heiko,

[Nobuhiro's mail address fixed]

In message <57076C13.50203 at denx.de> you wrote:
> 
> > I recommend to make this restriction more visible in the code and in
> > the comment, and/or even add a compile time test to guarantee this
> > requirement is met.
> 
> Maybe you try the following patch on your hardware:

I think this is not needed.  I reviewed the code, and if I'm not wrong
the code adds exactly the needed padding to make the structure size a
power of 2.

My complaint is that this requirement is not visible in the code, and
there is no explanation for the magic numbers.  The comment just says

	The size of the tx descriptor is determined by how much
	padding is used.
	4, 20, or 52 bytes of padding can be used

When you look closer, you will see that the currently used struct
size is 12 bytes, so adding 4, 20, or 52 bytes of padding will result
in total sizes of 12+4=16, 12+20=32 resp. 12+52=64 bytes, i. e. all
will result in proper sizes.

But this is not obvious to the reader who does not happen to realize
that this size is used as argument to memalign(), and that memalign()
has such a mandatory requirement() without checking for it, and with
potentially fatal consequences.


[And yes, I think we shoudl augment the memalign() implementation with
a compile time test for proper alignment size... Any volunteers?]

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Die Jahre hinterlassen vielleicht Falten auf der Haut, aber die
Begeisterung zu verlieren hinterlässt Falten auf der Seele.
                                                    - Samuel Ullmann