[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <87618083B2453E4A8714035B62D67992337C63A8@FMSMSX102.amr.corp.intel.com>
Date: Wed, 20 Feb 2013 00:26:37 +0000
From: "Tantilov, Emil S" <emil.s.tantilov@...el.com>
To: Ben Hutchings <bhutchings@...arflare.com>,
"Kirsher, Jeffrey T" <jeffrey.t.kirsher@...el.com>,
Aurélien Guillaume <footplus@...il.com>
CC: "davem@...emloft.net" <davem@...emloft.net>,
"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
"gospo@...hat.com" <gospo@...hat.com>,
"sassmann@...hat.com" <sassmann@...hat.com>
Subject: RE: [net-next 13/15] ixgbe: implement SFF diagnostic monitoring via
ethtool
>-----Original Message-----
>From: Ben Hutchings [mailto:bhutchings@...arflare.com]
>Sent: Tuesday, February 19, 2013 2:14 PM
>To: Kirsher, Jeffrey T; Aurélien Guillaume
>Cc: davem@...emloft.net; netdev@...r.kernel.org; gospo@...hat.com;
>sassmann@...hat.com; Tantilov, Emil S
>Subject: Re: [net-next 13/15] ixgbe: implement SFF diagnostic monitoring
>via ethtool
>
>On Sat, 2013-02-16 at 00:33 -0800, Jeff Kirsher wrote:
>> From: Aurélien Guillaume <footplus@...il.com>
>>
>> This patch adds support for reading data from SFP+ modules over i2c.
>[...]
>> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c
>> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c
>[...]
>> +static int ixgbe_get_module_eeprom(struct net_device *dev,
>> + struct ethtool_eeprom *ee,
>> + u8 *data)
>> +{
>> + struct ixgbe_adapter *adapter = netdev_priv(dev);
>> + struct ixgbe_hw *hw = &adapter->hw;
>> + u32 status = IXGBE_ERR_PHY_ADDR_INVALID;
>> + u8 databyte = 0xFF;
>> + int i = 0;
>> + int ret_val = 0;
>> +
>> + /* ixgbe_get_module_info is called before this function in all
>> + * cases, so we do not need any checks we already do above,
>> + * and can trust ee->len to be a known value.
>> + */
>[...]
>
>Whatever makes you think that?
>
>All you are guaranteed is that:
> ee->offset <= ee->offset + ee->len <= eeprom_len
>Same as for the regular EEPROM read function.
>
>This implementation doesn't result in a heap overrun at present because
>the caller allocates a whole page as a buffer. But you should not
>assume that it's safe to write more than ee->len bytes, nor that
>ee->offset == 0.
>
>Ben.
Thanks Ben, we'll address this in a follow up patch.
Emil
Powered by blists - more mailing lists