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

Hi,

Sorry, reply was too late.

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.

Indeed. I will write more infomation about this padding.

Best regards, Nobuhiro

2016-04-08 17:55 GMT+09:00 Wolfgang Denk wd@denx.de:

Dear Heiko,

[Nobuhiro's mail address fixed]

In message 57076C13.50203@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@denx.de Die Jahre hinterlassen vielleicht Falten auf der Haut, aber die Begeisterung zu verlieren hinterlässt Falten auf der Seele. - Samuel Ullmann