lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAOD=uF7=28Bx4eSpGfP-uTND42k1xKE3tvQ6+exSJha9Yjf-_w@mail.gmail.com>
Date:	Sat, 10 Mar 2012 00:20:12 +0530
From:	santosh prasad nayak <santoshprasadnayak@...il.com>
To:	Rajesh Borundia <rajesh.borundia@...gic.com>
Cc:	Sony Chacko <sony.chacko@...gic.com>,
	netdev <netdev@...r.kernel.org>,
	linux-kernel <linux-kernel@...r.kernel.org>,
	"kernel-janitors@...r.kernel.org" <kernel-janitors@...r.kernel.org>
Subject: Re: [PATCH 3/3] netxen: qlogic ethernet : Fix Endian Bug.

On Fri, Mar 9, 2012 at 10:04 PM, Rajesh Borundia
<rajesh.borundia@...gic.com> wrote:
>
>> -----Original Message-----
>> From: santosh nayak [mailto:santoshprasadnayak@...il.com]
>> Sent: Saturday, March 03, 2012 9:18 PM
>> To: Sony Chacko
>> Cc: Rajesh Borundia; netdev; linux-kernel; kernel-
>> janitors@...r.kernel.org; Santosh Nayak
>> Subject: [PATCH 3/3] netxen: qlogic ethernet : Fix Endian Bug.
>>
>> From: Santosh Nayak <santoshprasadnayak@...il.com>
>>
>> Fix endian bug.
>>
>> Signed-off-by: Santosh Nayak <santoshprasadnayak@...il.com>
>> ---
>>  drivers/net/ethernet/qlogic/netxen/netxen_nic.h    |    4 ++--
>>  drivers/net/ethernet/qlogic/netxen/netxen_nic_hw.c |   12 +++++++-----
>>  .../net/ethernet/qlogic/netxen/netxen_nic_main.c   |    2 +-
>>  3 files changed, 10 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/qlogic/netxen/netxen_nic.h
>> b/drivers/net/ethernet/qlogic/netxen/netxen_nic.h
>> index 2eeac32..b5de8a7 100644
>> --- a/drivers/net/ethernet/qlogic/netxen/netxen_nic.h
>> +++ b/drivers/net/ethernet/qlogic/netxen/netxen_nic.h
>> @@ -954,7 +954,7 @@ typedef struct nx_mac_list_s {
>>
>>  struct nx_vlan_ip_list {
>>       struct list_head list;
>> -     u32 ip_addr;
>> +     __be32 ip_addr;
>>  };
>>
>>  /*
>> @@ -1780,7 +1780,7 @@ int netxen_process_rcv_ring(struct
>> nx_host_sds_ring *sds_ring, int max);
>>  void netxen_p3_free_mac_list(struct netxen_adapter *adapter);
>>  int netxen_config_intr_coalesce(struct netxen_adapter *adapter);
>>  int netxen_config_rss(struct netxen_adapter *adapter, int enable);
>> -int netxen_config_ipaddr(struct netxen_adapter *adapter, u32 ip, int
>> cmd);
>> +int netxen_config_ipaddr(struct netxen_adapter *adapter, __be32 ip,
>> int cmd);
>>  int netxen_linkevent_request(struct netxen_adapter *adapter, int
>> enable);
>>  void netxen_advert_link_change(struct netxen_adapter *adapter, int
>> linkup);
>>  void netxen_pci_camqm_read_2M(struct netxen_adapter *, u64, u64 *);
>> diff --git a/drivers/net/ethernet/qlogic/netxen/netxen_nic_hw.c
>> b/drivers/net/ethernet/qlogic/netxen/netxen_nic_hw.c
>> index 6f37470..0f81287 100644
>> --- a/drivers/net/ethernet/qlogic/netxen/netxen_nic_hw.c
>> +++ b/drivers/net/ethernet/qlogic/netxen/netxen_nic_hw.c
>> @@ -909,10 +909,11 @@ int netxen_config_rss(struct netxen_adapter
>> *adapter, int enable)
>>       return rv;
>>  }
>>
>> -int netxen_config_ipaddr(struct netxen_adapter *adapter, u32 ip, int
>> cmd)
>> +int netxen_config_ipaddr(struct netxen_adapter *adapter, __be32 ip,
>> int cmd)
>>  {
>>       nx_nic_req_t req;
>>       u64 word;
>> +     u64 ip_addr;
>>       int rv;
>>
>>       memset(&req, 0, sizeof(nx_nic_req_t));
>> @@ -922,7 +923,8 @@ int netxen_config_ipaddr(struct netxen_adapter
>> *adapter, u32 ip, int cmd)
>>       req.req_hdr = cpu_to_le64(word);
>>
>>       req.words[0] = cpu_to_le64(cmd);
>> -     req.words[1] = cpu_to_le64(ip);
>> +     ip_addr = be32_to_cpu(ip);
>> +     *(__be64 *)&req.words[1] = cpu_to_be64(ip_addr);
>


> Adapter requires ip value in big endian stored at lower 32 bit address.
> The cpu_to_be64 will swap the lower 32 bit with higher 32 bit and adapter
> Will get incorrect ip addr. Instead u can do.
>
>       U32 *ip_addr;
>       ip_addr = (u32 *)&req.words[1];
>       *ip_addr = ip;


1.  It looks incomplete.
    In the function call "netxen_send_cmd_descs" we have to pass "&req" as
   2nd argument  not  "ipaddr".

2. Your above suggestion is with assumption that the data type of 2nd
argument "ip"
     in "netxen_config_ipaddr()" is still "u32".  This is not true.

     Some days back you suggested to change the data type to "__be32".
 In the present patch
     the "ip"  is in "__be32" format i.e already in Big endian format
as per requirement.
     We need to only convert this 32 bit to 64 bit.  There are two ways:

     a.   *(__be64 *)&req.words[1] = ip;   // auto conversion

     b.   *(__be64 *)&req.words[1] = cpu_to_be64(be32_to_cpu(ip));
            // explicit conversion.


 Please correct me if I am wrong.


regards
Santosh




>
>
>>
>>       rv = netxen_send_cmd_descs(adapter, (struct cmd_desc_type0
>> *)&req, 1);
>>       if (rv != 0) {
>> @@ -1050,7 +1052,7 @@ int netxen_get_flash_mac_addr(struct
>> netxen_adapter *adapter, u64 *mac)
>>       if (netxen_get_flash_block(adapter, offset, sizeof(u64), pmac) ==
>> -1)
>>               return -1;
>>
>> -     if (*mac == cpu_to_le64(~0ULL)) {
>> +     if (*mac == ~0ULL) {
>
> *mac is in little endian format so compare it with cpu_to_le64.
>
>>
>>               offset = NX_OLD_MAC_ADDR_OFFSET +
>>                       (adapter->portnum * sizeof(u64));
>> @@ -1059,7 +1061,7 @@ int netxen_get_flash_mac_addr(struct
>> netxen_adapter *adapter, u64 *mac)
>>                                       offset, sizeof(u64), pmac) == -1)
>>                       return -1;
>>
>> -             if (*mac == cpu_to_le64(~0ULL))
>> +             if (*mac == ~0ULL)
>
> *mac here is in little endian format so compare it with cpu_to_le64.
>
>>                       return -1;
>>       }
>>       return 0;
>> @@ -2178,7 +2180,7 @@ lock_try:
>>               NX_WR_DUMP_REG(FLASH_ROM_WINDOW, adapter->ahw.pci_base0,
>> waddr);
>>               raddr = FLASH_ROM_DATA + (fl_addr & 0x0000FFFF);
>>               NX_RD_DUMP_REG(raddr, adapter->ahw.pci_base0, &val);
>> -             *data_buff++ = cpu_to_le32(val);
>> +             *data_buff++ = val;
>
> It should be cpu_to_le32 as it is returned to tool which requires
> output in little endian.
>
>>               fl_addr += sizeof(val);
>>       }
>>       readl((void __iomem *)(adapter->ahw.pci_base0 +
>> NX_FLASH_SEM2_ULK));
>> diff --git a/drivers/net/ethernet/qlogic/netxen/netxen_nic_main.c
>> b/drivers/net/ethernet/qlogic/netxen/netxen_nic_main.c
>> index 8dc4a134..70783b4 100644
>> --- a/drivers/net/ethernet/qlogic/netxen/netxen_nic_main.c
>> +++ b/drivers/net/ethernet/qlogic/netxen/netxen_nic_main.c
>> @@ -818,7 +818,7 @@ netxen_check_options(struct netxen_adapter
>> *adapter)
>>                       adapter->driver_mismatch = 1;
>>                       return;
>>               }
>> -             ptr32[i] = cpu_to_le32(val);
>> +             ptr32[i] = val;
>
> Here val should be in little endian (cpu_to_le32) as it will be referenced by byte array to print serial number.
>
>>               offset += sizeof(u32);
>>       }
>>
>> --
>> 1.7.4.4
>>
>
>
> Sorry for Late reply.
>
> Rajesh
>
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ