| 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
| ||
|
Message-ID: <13A253B3F9BEFE43B93C09CF75F63CAA81A8AB3C4D@MNEXMB1.qlogic.org>
Date: Mon, 12 Mar 2012 01:19:34 -0500
From: Rajesh Borundia <rajesh.borundia@...gic.com>
To: santosh prasad nayak <santoshprasadnayak@...il.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.
> -----Original Message-----
> From: santosh prasad nayak [mailto:santoshprasadnayak@...il.com]
> Sent: Sunday, March 11, 2012 2:47 PM
> To: Rajesh Borundia
> Cc: Sony Chacko; netdev; linux-kernel; kernel-janitors@...r.kernel.org
> Subject: Re: [PATCH 3/3] netxen: qlogic ethernet : Fix Endian Bug.
>
> Thanks Rajesh for clarification.
> Included all your inputs in the following patch.
> This is for review not a formal one. Once review is done I will send a
> formal patch.
>
>
Looks fine to me.
>
> 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..59d5ee7 100644
> --- a/drivers/net/ethernet/qlogic/netxen/netxen_nic_hw.c
> +++ b/drivers/net/ethernet/qlogic/netxen/netxen_nic_hw.c
> @@ -909,7 +909,7 @@ 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;
> @@ -922,7 +922,7 @@ 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);
> + memcpy(&req.words[1], &ip, sizeof(u32));
>
> rv = netxen_send_cmd_descs(adapter, (struct cmd_desc_type0
> *)&req, 1);
> if (rv != 0) {
> @@ -1050,7 +1050,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 (*(__le64 *)mac == cpu_to_le64(~0ULL)) {
>
> offset = NX_OLD_MAC_ADDR_OFFSET +
> (adapter->portnum * sizeof(u64));
> @@ -1059,7 +1059,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 (*(__le64 *)mac == cpu_to_le64(~0ULL))
> return -1;
> }
> return 0;
> @@ -2155,7 +2155,7 @@ static u32 netxen_md_rd_crb(struct
> netxen_adapter *adapter,
> static u32
> netxen_md_rdrom(struct netxen_adapter *adapter,
> struct netxen_minidump_entry_rdrom
> - *romEntry, u32 *data_buff)
> + *romEntry, __le32 *data_buff)
> {
> int i, count = 0;
> u32 size, lck_val;
> diff --git a/drivers/net/ethernet/qlogic/netxen/netxen_nic_main.c
> b/drivers/net/ethernet/qlogic/netxen/netxen_nic_main.c
> index 7648995..65a718f 100644
> --- a/drivers/net/ethernet/qlogic/netxen/netxen_nic_main.c
> +++ b/drivers/net/ethernet/qlogic/netxen/netxen_nic_main.c
> @@ -805,12 +805,12 @@ netxen_check_options(struct netxen_adapter
> *adapter)
> char brd_name[NETXEN_MAX_SHORT_NAME];
> char serial_num[32];
> int i, offset, val, err;
> - int *ptr32;
> + __le32 *ptr32;
> struct pci_dev *pdev = adapter->pdev;
>
> adapter->driver_mismatch = 0;
>
> - ptr32 = (int *)&serial_num;
> + ptr32 = (__le32 *)&serial_num;
> offset = NX_FW_SERIAL_NUM_OFFSET;
> for (i = 0; i < 8; i++) {
> if (netxen_rom_fast_read(adapter, offset, &val) == -1) {
>
>
>
> On Sun, Mar 11, 2012 at 12:31 AM, Rajesh Borundia
> <rajesh.borundia@...gic.com> wrote:
> >
> >
> >> -----Original Message-----
> >> From: santosh prasad nayak [mailto:santoshprasadnayak@...il.com]
> >> Sent: Saturday, March 10, 2012 12:20 AM
> >> To: Rajesh Borundia
> >> Cc: Sony Chacko; netdev; linux-kernel; 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".
> >
> > I should have sent a patch. This piece of code was just to show how
> to
> > copy ip addr in req.words[1].
> >
> >>
> >> 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:
> >>
> > No I did not assume that ip is u32, ip is still __be32(ip value is
> in form of big endian)
> > though I should have mentioned it explicitly. But the ip value
> should be copied to lower 32 bit of req.words[1].
> >
> >
> >> a. *(__be64 *)&req.words[1] = ip; // auto conversion
> > In big endian machine MSB is copied into MSB first. So ip will get
> copied into higher 32 bit of
> > req.words[1] but adapter requires it in lower 32 bit.
> >>
> >> b. *(__be64 *)&req.words[1] = cpu_to_be64(be32_to_cpu(ip));
> >> // explicit conversion.
> >>
> > If you follow second cpu_to_be64 it will swap lower 32 bit of ip with
> higher 32 bit in little endian machine.
> > But adapter requires it in lower 32 bit.
> >
> > Simple solution to copy ip in req.words[1] could be
> memcpy(&req.words[1], &ip, sizeof(u32));
> >
> >
> >>
> >> 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