[U-Boot] [PATCH 1/2] net/eth.c: make eth_get_dev_by_name(NULL) safe

[Mike Frysinger]

On Thu, Jul 7, 2011 at 02:12, Helmut Raiger wrote:
> On 07/06/2011 09:38 PM, Mike Frysinger wrote:
>> On Wednesday, July 06, 2011 03:15:08 Helmut Raiger wrote:
>>> On 07/05/2011 05:44 AM, Mike Frysinger wrote:
>>>> On Monday, July 04, 2011 06:29:51 helmut.raiger at hale.at wrote:
>>>>> eth_get_dev_by_name() is not safe to use for devname being NULL
>>>>> as it uses strcmp. This patch makes it return NULL if devname NULL
>>>>> is passed.
>>>>
>>>> i'm not sure about this.  passing NULL is wrong, and the caller should
>>>> catch that shouldnt it ?
>>>
>>> So what is your suggestion how to deal with it?
>>
>> in what situation is eth_get_dev_by_name(NULL) being called ?  my
>> suggestion
>> would be to fix that call point since it's doing something wrong.
>
> I couldn't find a situation where this might be the case. But as Luca
> Ceresoli pointed out in his e-mail, somewhere up the thread, that he tested
> for devname being NULL in his miiphy_read and write routines, I checked
> eth_get_dev_by_name() and found that it is vulnerable to passing a NULL
> pointer, hence the fix.

those NULL checks should not be necessary either.  a correctly written
networking driver should only register itself with the miiphy layer
when it has successfully registered itself with the eth layer.  thus
any of the miiphy callbacks should always come in with a name that is
found via eth_get_dev_by_name().

checking for NULLs here and gracefully returning is unnecessary
overhead imo as you're only catering to broken code.  fix the broken
drivers instead.

by your logic, why put the NULL check in eth_get_dev_by_name() ?  why
not handle strcmp(NULL, NULL) too ?  then eth_get_dev_by_name() would
automatically get "fixed" as would all other call points.
-mike