Re: [patch] pch_gbe: memory corruption calling pch_gbe_validate_option()

[Dan Carpenter <dan.carpenter@oracle.com>]

[-- Attachment #1: Type: text/plain, Size: 1264 bytes --]

On Mon, Mar 05, 2012 at 11:04:49AM +0530, santosh prasad nayak wrote:
> Dan,
> 
>         Your fix may introduce new bug.
> 
>          hw->phy.autoneg_advertised = tmp
> 
>        Assigning signed integer to unsigned short  leads to bit truncation.

In this case it doesn't.  It's going to be 0x2f or less.  I
obviously checked this before I submitted the patch.

>        Is it safe for both Big-endian and little endian format ?

It's CPU endian in both cases.

> 
>        AutoNeg is initialized with a Negative value (OPTION_UNSET)
>         Won't it create any issue with above assignment ?
> 

Nope.  It gets set to 0x2f inside pch_gbe_validate_option().

> 
> The simpler fix is to make "autoneg_advertised" signed integer.
> 
> struct pch_gbe_phy_info {
>         u32 addr;
>         u32 id;
>         u32 revision;
>         u32 reset_delay_us;
>         u16 autoneg_advertised;     // ==> int autoneg_advertised
> };
> 

The better fix would be to change pch_gbe_validate_option() so it
isn't so easy to call improperly.  I would have done that except
that probably we won't introduce many more callers, it seemed
like a lot of work and I don't have the hardware to test the
results.

regards,
dan carpenter


[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]