* [PATCH 0/2] net: spider_net: fix possible bitops errors
@ 2014-10-03 15:01 ` Antoine Tenart
0 siblings, 0 replies; 17+ messages in thread
From: Antoine Tenart @ 2014-10-03 15:01 UTC (permalink / raw)
To: dan.carpenter, kou.ishizaki, jens
Cc: Antoine Tenart, netdev, linux-arm-kernel, linux-kernel
Hi,
Dan reported a possible signedness issue on the pxa168_eth driver. While
having a look at it, I came across a similar problem in the spider_net
driver.
Here is one proposal to fix it. The first patch rework the
spider_net_set_mac() function by removing the spider_net_get_mac_address()
call and using memcpy() to set netdev->dev_addr (which is what's done in
lots of Ethernet drivers) and the second one fix the actual signedness
issue.
If for any reason you really want to keep a call to
spider_net_get_mac_address() because the memcpy() is somehow not good
enough here, we can also come up with a solution involving a temporary
unsigned char variable.
I couldn't test these changes, so please do.
Thanks,
Antoine
Antoine Tenart (2):
net: spider_net: do not read mac address again after setting it
net: spider_net: avoid using signed char for bitops
drivers/net/ethernet/toshiba/spider_net.c | 42 ++++---------------------------
1 file changed, 5 insertions(+), 37 deletions(-)
--
1.9.1
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH] net: pxa168_eth: avoid using signed char for bitops
2014-10-03 15:01 ` Antoine Tenart
(?)
@ 2014-10-03 15:01 ` Antoine Tenart
-1 siblings, 0 replies; 17+ messages in thread
From: Antoine Tenart @ 2014-10-03 15:01 UTC (permalink / raw)
To: linux-arm-kernel
Signedness bugs may occur when using signed char for bitops,
depending on if the highest bit is ever used.
Signed-off-by: Antoine Tenart <antoine.tenart@free-electrons.com>
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
---
Dan reported a static checker warning:
http://marc.info/?l=kernel-janitors&m=141218246222535&w=2
This patch fixes it. Instead of using sa->sa_data we now use
dev->dev_addr (of type unsigned char *) to avoid possible
signedness issues.
drivers/net/ethernet/marvell/pxa168_eth.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/net/ethernet/marvell/pxa168_eth.c b/drivers/net/ethernet/marvell/pxa168_eth.c
index 24de41231593..c3b209cd0660 100644
--- a/drivers/net/ethernet/marvell/pxa168_eth.c
+++ b/drivers/net/ethernet/marvell/pxa168_eth.c
@@ -634,12 +634,12 @@ static int pxa168_eth_set_mac_address(struct net_device *dev, void *addr)
memcpy(oldMac, dev->dev_addr, ETH_ALEN);
memcpy(dev->dev_addr, sa->sa_data, ETH_ALEN);
- mac_h = sa->sa_data[0] << 24;
- mac_h |= sa->sa_data[1] << 16;
- mac_h |= sa->sa_data[2] << 8;
- mac_h |= sa->sa_data[3];
- mac_l = sa->sa_data[4] << 8;
- mac_l |= sa->sa_data[5];
+ mac_h = dev->dev_addr[0] << 24;
+ mac_h |= dev->dev_addr[1] << 16;
+ mac_h |= dev->dev_addr[2] << 8;
+ mac_h |= dev->dev_addr[3];
+ mac_l = dev->dev_addr[4] << 8;
+ mac_l |= dev->dev_addr[5];
wrl(pep, MAC_ADDR_HIGH, mac_h);
wrl(pep, MAC_ADDR_LOW, mac_l);
--
1.9.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH] net: pxa168_eth: avoid using signed char for bitops
@ 2014-10-03 15:01 ` Antoine Tenart
0 siblings, 0 replies; 17+ messages in thread
From: Antoine Tenart @ 2014-10-03 15:01 UTC (permalink / raw)
To: dan.carpenter
Cc: thomas.petazzoni, zmxu, netdev, Antoine Tenart, linux-kernel,
alexandre.belloni, jszhang, linux-arm-kernel,
sebastian.hesselbarth
Signedness bugs may occur when using signed char for bitops,
depending on if the highest bit is ever used.
Signed-off-by: Antoine Tenart <antoine.tenart@free-electrons.com>
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
---
Dan reported a static checker warning:
http://marc.info/?l=kernel-janitors&m=141218246222535&w=2
This patch fixes it. Instead of using sa->sa_data we now use
dev->dev_addr (of type unsigned char *) to avoid possible
signedness issues.
drivers/net/ethernet/marvell/pxa168_eth.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/net/ethernet/marvell/pxa168_eth.c b/drivers/net/ethernet/marvell/pxa168_eth.c
index 24de41231593..c3b209cd0660 100644
--- a/drivers/net/ethernet/marvell/pxa168_eth.c
+++ b/drivers/net/ethernet/marvell/pxa168_eth.c
@@ -634,12 +634,12 @@ static int pxa168_eth_set_mac_address(struct net_device *dev, void *addr)
memcpy(oldMac, dev->dev_addr, ETH_ALEN);
memcpy(dev->dev_addr, sa->sa_data, ETH_ALEN);
- mac_h = sa->sa_data[0] << 24;
- mac_h |= sa->sa_data[1] << 16;
- mac_h |= sa->sa_data[2] << 8;
- mac_h |= sa->sa_data[3];
- mac_l = sa->sa_data[4] << 8;
- mac_l |= sa->sa_data[5];
+ mac_h = dev->dev_addr[0] << 24;
+ mac_h |= dev->dev_addr[1] << 16;
+ mac_h |= dev->dev_addr[2] << 8;
+ mac_h |= dev->dev_addr[3];
+ mac_l = dev->dev_addr[4] << 8;
+ mac_l |= dev->dev_addr[5];
wrl(pep, MAC_ADDR_HIGH, mac_h);
wrl(pep, MAC_ADDR_LOW, mac_l);
--
1.9.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH] net: pxa168_eth: avoid using signed char for bitops
@ 2014-10-03 15:01 ` Antoine Tenart
0 siblings, 0 replies; 17+ messages in thread
From: Antoine Tenart @ 2014-10-03 15:01 UTC (permalink / raw)
To: dan.carpenter
Cc: Antoine Tenart, sebastian.hesselbarth, alexandre.belloni,
thomas.petazzoni, zmxu, jszhang, netdev, linux-arm-kernel,
linux-kernel
Signedness bugs may occur when using signed char for bitops,
depending on if the highest bit is ever used.
Signed-off-by: Antoine Tenart <antoine.tenart@free-electrons.com>
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
---
Dan reported a static checker warning:
http://marc.info/?l=kernel-janitors&m=141218246222535&w=2
This patch fixes it. Instead of using sa->sa_data we now use
dev->dev_addr (of type unsigned char *) to avoid possible
signedness issues.
drivers/net/ethernet/marvell/pxa168_eth.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/net/ethernet/marvell/pxa168_eth.c b/drivers/net/ethernet/marvell/pxa168_eth.c
index 24de41231593..c3b209cd0660 100644
--- a/drivers/net/ethernet/marvell/pxa168_eth.c
+++ b/drivers/net/ethernet/marvell/pxa168_eth.c
@@ -634,12 +634,12 @@ static int pxa168_eth_set_mac_address(struct net_device *dev, void *addr)
memcpy(oldMac, dev->dev_addr, ETH_ALEN);
memcpy(dev->dev_addr, sa->sa_data, ETH_ALEN);
- mac_h = sa->sa_data[0] << 24;
- mac_h |= sa->sa_data[1] << 16;
- mac_h |= sa->sa_data[2] << 8;
- mac_h |= sa->sa_data[3];
- mac_l = sa->sa_data[4] << 8;
- mac_l |= sa->sa_data[5];
+ mac_h = dev->dev_addr[0] << 24;
+ mac_h |= dev->dev_addr[1] << 16;
+ mac_h |= dev->dev_addr[2] << 8;
+ mac_h |= dev->dev_addr[3];
+ mac_l = dev->dev_addr[4] << 8;
+ mac_l |= dev->dev_addr[5];
wrl(pep, MAC_ADDR_HIGH, mac_h);
wrl(pep, MAC_ADDR_LOW, mac_l);
--
1.9.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH] net: pxa168_eth: avoid using signed char for bitops
2014-10-03 15:01 ` Antoine Tenart
@ 2014-10-03 15:07 ` Antoine Tenart
-1 siblings, 0 replies; 17+ messages in thread
From: Antoine Tenart @ 2014-10-03 15:07 UTC (permalink / raw)
To: linux-arm-kernel
On Fri, Oct 03, 2014 at 05:01:54PM +0200, Antoine Tenart wrote:
> Signedness bugs may occur when using signed char for bitops,
> depending on if the highest bit is ever used.
>
> Signed-off-by: Antoine Tenart <antoine.tenart@free-electrons.com>
> Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
> ---
>
> Dan reported a static checker warning:
> http://marc.info/?l=kernel-janitors&m=141218246222535&w=2
>
> This patch fixes it. Instead of using sa->sa_data we now use
> dev->dev_addr (of type unsigned char *) to avoid possible
> signedness issues.
Well, this patch shouldn't be in this thread. I'm resending it.
Sorry for the noise.
Antoine
>
> drivers/net/ethernet/marvell/pxa168_eth.c | 12 ++++++------
> 1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/net/ethernet/marvell/pxa168_eth.c b/drivers/net/ethernet/marvell/pxa168_eth.c
> index 24de41231593..c3b209cd0660 100644
> --- a/drivers/net/ethernet/marvell/pxa168_eth.c
> +++ b/drivers/net/ethernet/marvell/pxa168_eth.c
> @@ -634,12 +634,12 @@ static int pxa168_eth_set_mac_address(struct net_device *dev, void *addr)
> memcpy(oldMac, dev->dev_addr, ETH_ALEN);
> memcpy(dev->dev_addr, sa->sa_data, ETH_ALEN);
>
> - mac_h = sa->sa_data[0] << 24;
> - mac_h |= sa->sa_data[1] << 16;
> - mac_h |= sa->sa_data[2] << 8;
> - mac_h |= sa->sa_data[3];
> - mac_l = sa->sa_data[4] << 8;
> - mac_l |= sa->sa_data[5];
> + mac_h = dev->dev_addr[0] << 24;
> + mac_h |= dev->dev_addr[1] << 16;
> + mac_h |= dev->dev_addr[2] << 8;
> + mac_h |= dev->dev_addr[3];
> + mac_l = dev->dev_addr[4] << 8;
> + mac_l |= dev->dev_addr[5];
> wrl(pep, MAC_ADDR_HIGH, mac_h);
> wrl(pep, MAC_ADDR_LOW, mac_l);
>
> --
> 1.9.1
>
--
Antoine T?nart, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] net: pxa168_eth: avoid using signed char for bitops
@ 2014-10-03 15:07 ` Antoine Tenart
0 siblings, 0 replies; 17+ messages in thread
From: Antoine Tenart @ 2014-10-03 15:07 UTC (permalink / raw)
To: dan.carpenter
Cc: Antoine Tenart, sebastian.hesselbarth, alexandre.belloni,
thomas.petazzoni, zmxu, jszhang, netdev, linux-arm-kernel,
linux-kernel
On Fri, Oct 03, 2014 at 05:01:54PM +0200, Antoine Tenart wrote:
> Signedness bugs may occur when using signed char for bitops,
> depending on if the highest bit is ever used.
>
> Signed-off-by: Antoine Tenart <antoine.tenart@free-electrons.com>
> Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
> ---
>
> Dan reported a static checker warning:
> http://marc.info/?l=kernel-janitors&m=141218246222535&w=2
>
> This patch fixes it. Instead of using sa->sa_data we now use
> dev->dev_addr (of type unsigned char *) to avoid possible
> signedness issues.
Well, this patch shouldn't be in this thread. I'm resending it.
Sorry for the noise.
Antoine
>
> drivers/net/ethernet/marvell/pxa168_eth.c | 12 ++++++------
> 1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/net/ethernet/marvell/pxa168_eth.c b/drivers/net/ethernet/marvell/pxa168_eth.c
> index 24de41231593..c3b209cd0660 100644
> --- a/drivers/net/ethernet/marvell/pxa168_eth.c
> +++ b/drivers/net/ethernet/marvell/pxa168_eth.c
> @@ -634,12 +634,12 @@ static int pxa168_eth_set_mac_address(struct net_device *dev, void *addr)
> memcpy(oldMac, dev->dev_addr, ETH_ALEN);
> memcpy(dev->dev_addr, sa->sa_data, ETH_ALEN);
>
> - mac_h = sa->sa_data[0] << 24;
> - mac_h |= sa->sa_data[1] << 16;
> - mac_h |= sa->sa_data[2] << 8;
> - mac_h |= sa->sa_data[3];
> - mac_l = sa->sa_data[4] << 8;
> - mac_l |= sa->sa_data[5];
> + mac_h = dev->dev_addr[0] << 24;
> + mac_h |= dev->dev_addr[1] << 16;
> + mac_h |= dev->dev_addr[2] << 8;
> + mac_h |= dev->dev_addr[3];
> + mac_l = dev->dev_addr[4] << 8;
> + mac_l |= dev->dev_addr[5];
> wrl(pep, MAC_ADDR_HIGH, mac_h);
> wrl(pep, MAC_ADDR_LOW, mac_l);
>
> --
> 1.9.1
>
--
Antoine Ténart, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 1/2] net: spider_net: do not read mac address again after setting it
2014-10-03 15:01 ` Antoine Tenart
@ 2014-10-03 15:01 ` Antoine Tenart
-1 siblings, 0 replies; 17+ messages in thread
From: Antoine Tenart @ 2014-10-03 15:01 UTC (permalink / raw)
To: linux-arm-kernel
This patch removes the spider_net_get_mac_address() call at the end of
the spider_net_set_mac() function. The dev->dev_addr is instead updated
with a memcpy() from sa->sa_data.
Since spider_net_get_mac_address() is not used anywhere else, this patch
also removes the function.
Signed-off-by: Antoine Tenart <antoine.tenart@free-electrons.com>
---
drivers/net/ethernet/toshiba/spider_net.c | 36 ++-----------------------------
1 file changed, 2 insertions(+), 34 deletions(-)
diff --git a/drivers/net/ethernet/toshiba/spider_net.c b/drivers/net/ethernet/toshiba/spider_net.c
index 3e38f67c6011..713313e15c68 100644
--- a/drivers/net/ethernet/toshiba/spider_net.c
+++ b/drivers/net/ethernet/toshiba/spider_net.c
@@ -267,34 +267,6 @@ spider_net_set_promisc(struct spider_net_card *card)
}
/**
- * spider_net_get_mac_address - read mac address from spider card
- * @card: device structure
- *
- * reads MAC address from GMACUNIMACU and GMACUNIMACL registers
- */
-static int
-spider_net_get_mac_address(struct net_device *netdev)
-{
- struct spider_net_card *card = netdev_priv(netdev);
- u32 macl, macu;
-
- macl = spider_net_read_reg(card, SPIDER_NET_GMACUNIMACL);
- macu = spider_net_read_reg(card, SPIDER_NET_GMACUNIMACU);
-
- netdev->dev_addr[0] = (macu >> 24) & 0xff;
- netdev->dev_addr[1] = (macu >> 16) & 0xff;
- netdev->dev_addr[2] = (macu >> 8) & 0xff;
- netdev->dev_addr[3] = macu & 0xff;
- netdev->dev_addr[4] = (macl >> 8) & 0xff;
- netdev->dev_addr[5] = macl & 0xff;
-
- if (!is_valid_ether_addr(&netdev->dev_addr[0]))
- return -EINVAL;
-
- return 0;
-}
-
-/**
* spider_net_get_descr_status -- returns the status of a descriptor
* @descr: descriptor to look at
*
@@ -1345,6 +1317,8 @@ spider_net_set_mac(struct net_device *netdev, void *p)
if (!is_valid_ether_addr(addr->sa_data))
return -EADDRNOTAVAIL;
+ memcpy(netdev->dev_addr, addr->sa_data, ETH_ALEN);
+
/* switch off GMACTPE and GMACRPE */
regvalue = spider_net_read_reg(card, SPIDER_NET_GMACOPEMD);
regvalue &= ~((1 << 5) | (1 << 6));
@@ -1364,12 +1338,6 @@ spider_net_set_mac(struct net_device *netdev, void *p)
spider_net_set_promisc(card);
- /* look up, whether we have been successful */
- if (spider_net_get_mac_address(netdev))
- return -EADDRNOTAVAIL;
- if (memcmp(netdev->dev_addr,addr->sa_data,netdev->addr_len))
- return -EADDRNOTAVAIL;
-
return 0;
}
--
1.9.1
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH 1/2] net: spider_net: do not read mac address again after setting it
@ 2014-10-03 15:01 ` Antoine Tenart
0 siblings, 0 replies; 17+ messages in thread
From: Antoine Tenart @ 2014-10-03 15:01 UTC (permalink / raw)
To: dan.carpenter, kou.ishizaki, jens
Cc: Antoine Tenart, netdev, linux-arm-kernel, linux-kernel
This patch removes the spider_net_get_mac_address() call at the end of
the spider_net_set_mac() function. The dev->dev_addr is instead updated
with a memcpy() from sa->sa_data.
Since spider_net_get_mac_address() is not used anywhere else, this patch
also removes the function.
Signed-off-by: Antoine Tenart <antoine.tenart@free-electrons.com>
---
drivers/net/ethernet/toshiba/spider_net.c | 36 ++-----------------------------
1 file changed, 2 insertions(+), 34 deletions(-)
diff --git a/drivers/net/ethernet/toshiba/spider_net.c b/drivers/net/ethernet/toshiba/spider_net.c
index 3e38f67c6011..713313e15c68 100644
--- a/drivers/net/ethernet/toshiba/spider_net.c
+++ b/drivers/net/ethernet/toshiba/spider_net.c
@@ -267,34 +267,6 @@ spider_net_set_promisc(struct spider_net_card *card)
}
/**
- * spider_net_get_mac_address - read mac address from spider card
- * @card: device structure
- *
- * reads MAC address from GMACUNIMACU and GMACUNIMACL registers
- */
-static int
-spider_net_get_mac_address(struct net_device *netdev)
-{
- struct spider_net_card *card = netdev_priv(netdev);
- u32 macl, macu;
-
- macl = spider_net_read_reg(card, SPIDER_NET_GMACUNIMACL);
- macu = spider_net_read_reg(card, SPIDER_NET_GMACUNIMACU);
-
- netdev->dev_addr[0] = (macu >> 24) & 0xff;
- netdev->dev_addr[1] = (macu >> 16) & 0xff;
- netdev->dev_addr[2] = (macu >> 8) & 0xff;
- netdev->dev_addr[3] = macu & 0xff;
- netdev->dev_addr[4] = (macl >> 8) & 0xff;
- netdev->dev_addr[5] = macl & 0xff;
-
- if (!is_valid_ether_addr(&netdev->dev_addr[0]))
- return -EINVAL;
-
- return 0;
-}
-
-/**
* spider_net_get_descr_status -- returns the status of a descriptor
* @descr: descriptor to look at
*
@@ -1345,6 +1317,8 @@ spider_net_set_mac(struct net_device *netdev, void *p)
if (!is_valid_ether_addr(addr->sa_data))
return -EADDRNOTAVAIL;
+ memcpy(netdev->dev_addr, addr->sa_data, ETH_ALEN);
+
/* switch off GMACTPE and GMACRPE */
regvalue = spider_net_read_reg(card, SPIDER_NET_GMACOPEMD);
regvalue &= ~((1 << 5) | (1 << 6));
@@ -1364,12 +1338,6 @@ spider_net_set_mac(struct net_device *netdev, void *p)
spider_net_set_promisc(card);
- /* look up, whether we have been successful */
- if (spider_net_get_mac_address(netdev))
- return -EADDRNOTAVAIL;
- if (memcmp(netdev->dev_addr,addr->sa_data,netdev->addr_len))
- return -EADDRNOTAVAIL;
-
return 0;
}
--
1.9.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 2/2] net: spider_net: avoid using signed char for bitops
2014-10-03 15:01 ` Antoine Tenart
@ 2014-10-03 15:01 ` Antoine Tenart
-1 siblings, 0 replies; 17+ messages in thread
From: Antoine Tenart @ 2014-10-03 15:01 UTC (permalink / raw)
To: linux-arm-kernel
Signedness bugs may occur when using signed char for bitops,
depending on if the highest bit is ever used.
Signed-off-by: Antoine Tenart <antoine.tenart@free-electrons.com>
---
drivers/net/ethernet/toshiba/spider_net.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/toshiba/spider_net.c b/drivers/net/ethernet/toshiba/spider_net.c
index 713313e15c68..8e9371a3388a 100644
--- a/drivers/net/ethernet/toshiba/spider_net.c
+++ b/drivers/net/ethernet/toshiba/spider_net.c
@@ -1325,9 +1325,9 @@ spider_net_set_mac(struct net_device *netdev, void *p)
spider_net_write_reg(card, SPIDER_NET_GMACOPEMD, regvalue);
/* write mac */
- macu = (addr->sa_data[0]<<24) + (addr->sa_data[1]<<16) +
- (addr->sa_data[2]<<8) + (addr->sa_data[3]);
- macl = (addr->sa_data[4]<<8) + (addr->sa_data[5]);
+ macu = (netdev->dev_addr[0]<<24) + (netdev->dev_addr[1]<<16) +
+ (netdev->dev_addr[2]<<8) + (netdev->dev_addr[3]);
+ macl = (netdev->dev_addr[4]<<8) + (netdev->dev_addr[5]);
spider_net_write_reg(card, SPIDER_NET_GMACUNIMACU, macu);
spider_net_write_reg(card, SPIDER_NET_GMACUNIMACL, macl);
--
1.9.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 2/2] net: spider_net: avoid using signed char for bitops
@ 2014-10-03 15:01 ` Antoine Tenart
0 siblings, 0 replies; 17+ messages in thread
From: Antoine Tenart @ 2014-10-03 15:01 UTC (permalink / raw)
To: dan.carpenter, kou.ishizaki, jens
Cc: Antoine Tenart, netdev, linux-arm-kernel, linux-kernel
Signedness bugs may occur when using signed char for bitops,
depending on if the highest bit is ever used.
Signed-off-by: Antoine Tenart <antoine.tenart@free-electrons.com>
---
drivers/net/ethernet/toshiba/spider_net.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/toshiba/spider_net.c b/drivers/net/ethernet/toshiba/spider_net.c
index 713313e15c68..8e9371a3388a 100644
--- a/drivers/net/ethernet/toshiba/spider_net.c
+++ b/drivers/net/ethernet/toshiba/spider_net.c
@@ -1325,9 +1325,9 @@ spider_net_set_mac(struct net_device *netdev, void *p)
spider_net_write_reg(card, SPIDER_NET_GMACOPEMD, regvalue);
/* write mac */
- macu = (addr->sa_data[0]<<24) + (addr->sa_data[1]<<16) +
- (addr->sa_data[2]<<8) + (addr->sa_data[3]);
- macl = (addr->sa_data[4]<<8) + (addr->sa_data[5]);
+ macu = (netdev->dev_addr[0]<<24) + (netdev->dev_addr[1]<<16) +
+ (netdev->dev_addr[2]<<8) + (netdev->dev_addr[3]);
+ macl = (netdev->dev_addr[4]<<8) + (netdev->dev_addr[5]);
spider_net_write_reg(card, SPIDER_NET_GMACUNIMACU, macu);
spider_net_write_reg(card, SPIDER_NET_GMACUNIMACL, macl);
--
1.9.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 2/2] net: spider_net: avoid using signed char for bitops
2014-10-03 15:01 ` Antoine Tenart
@ 2014-10-03 15:15 ` Arnd Bergmann
-1 siblings, 0 replies; 17+ messages in thread
From: Arnd Bergmann @ 2014-10-03 15:15 UTC (permalink / raw)
To: linux-arm-kernel
On Friday 03 October 2014 17:01:56 Antoine Tenart wrote:
> Signedness bugs may occur when using signed char for bitops,
> depending on if the highest bit is ever used.
>
> Signed-off-by: Antoine Tenart <antoine.tenart@free-electrons.com>
>
It's probably worth mentioning here that this cannot happen in this
driver because it's only used in one powerpc-specific chip, and 'char'
is always 'unsigned' on powerpc.
Arnd
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 2/2] net: spider_net: avoid using signed char for bitops
@ 2014-10-03 15:15 ` Arnd Bergmann
0 siblings, 0 replies; 17+ messages in thread
From: Arnd Bergmann @ 2014-10-03 15:15 UTC (permalink / raw)
To: linux-arm-kernel
Cc: Antoine Tenart, dan.carpenter, kou.ishizaki, jens, netdev,
linux-kernel
On Friday 03 October 2014 17:01:56 Antoine Tenart wrote:
> Signedness bugs may occur when using signed char for bitops,
> depending on if the highest bit is ever used.
>
> Signed-off-by: Antoine Tenart <antoine.tenart@free-electrons.com>
>
It's probably worth mentioning here that this cannot happen in this
driver because it's only used in one powerpc-specific chip, and 'char'
is always 'unsigned' on powerpc.
Arnd
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 2/2] net: spider_net: avoid using signed char for bitops
2014-10-03 15:15 ` Arnd Bergmann
@ 2014-10-03 15:17 ` Antoine Tenart
-1 siblings, 0 replies; 17+ messages in thread
From: Antoine Tenart @ 2014-10-03 15:17 UTC (permalink / raw)
To: linux-arm-kernel
Hi Arnd,
On Fri, Oct 03, 2014 at 05:15:05PM +0200, Arnd Bergmann wrote:
> On Friday 03 October 2014 17:01:56 Antoine Tenart wrote:
> > Signedness bugs may occur when using signed char for bitops,
> > depending on if the highest bit is ever used.
> >
> > Signed-off-by: Antoine Tenart <antoine.tenart@free-electrons.com>
> >
>
> It's probably worth mentioning here that this cannot happen in this
> driver because it's only used in one powerpc-specific chip, and 'char'
> is always 'unsigned' on powerpc.
Oh, that's good to know :)
Antoine
--
Antoine T?nart, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 2/2] net: spider_net: avoid using signed char for bitops
@ 2014-10-03 15:17 ` Antoine Tenart
0 siblings, 0 replies; 17+ messages in thread
From: Antoine Tenart @ 2014-10-03 15:17 UTC (permalink / raw)
To: Arnd Bergmann
Cc: linux-arm-kernel, Antoine Tenart, dan.carpenter, kou.ishizaki,
jens, netdev, linux-kernel
Hi Arnd,
On Fri, Oct 03, 2014 at 05:15:05PM +0200, Arnd Bergmann wrote:
> On Friday 03 October 2014 17:01:56 Antoine Tenart wrote:
> > Signedness bugs may occur when using signed char for bitops,
> > depending on if the highest bit is ever used.
> >
> > Signed-off-by: Antoine Tenart <antoine.tenart@free-electrons.com>
> >
>
> It's probably worth mentioning here that this cannot happen in this
> driver because it's only used in one powerpc-specific chip, and 'char'
> is always 'unsigned' on powerpc.
Oh, that's good to know :)
Antoine
--
Antoine Ténart, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 0/2] net: spider_net: fix possible bitops errors
2014-10-03 15:01 ` Antoine Tenart
@ 2014-10-06 1:15 ` David Miller
-1 siblings, 0 replies; 17+ messages in thread
From: David Miller @ 2014-10-06 1:15 UTC (permalink / raw)
To: linux-arm-kernel
From: Antoine Tenart <antoine.tenart@free-electrons.com>
Date: Fri, 3 Oct 2014 17:01:53 +0200
> Dan reported a possible signedness issue on the pxa168_eth driver. While
> having a look at it, I came across a similar problem in the spider_net
> driver.
>
> Here is one proposal to fix it. The first patch rework the
> spider_net_set_mac() function by removing the spider_net_get_mac_address()
> call and using memcpy() to set netdev->dev_addr (which is what's done in
> lots of Ethernet drivers) and the second one fix the actual signedness
> issue.
>
> If for any reason you really want to keep a call to
> spider_net_get_mac_address() because the memcpy() is somehow not good
> enough here, we can also come up with a solution involving a temporary
> unsigned char variable.
Series applied, thanks.
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 0/2] net: spider_net: fix possible bitops errors
@ 2014-10-06 1:15 ` David Miller
0 siblings, 0 replies; 17+ messages in thread
From: David Miller @ 2014-10-06 1:15 UTC (permalink / raw)
To: antoine.tenart
Cc: dan.carpenter, kou.ishizaki, jens, netdev, linux-arm-kernel,
linux-kernel
From: Antoine Tenart <antoine.tenart@free-electrons.com>
Date: Fri, 3 Oct 2014 17:01:53 +0200
> Dan reported a possible signedness issue on the pxa168_eth driver. While
> having a look at it, I came across a similar problem in the spider_net
> driver.
>
> Here is one proposal to fix it. The first patch rework the
> spider_net_set_mac() function by removing the spider_net_get_mac_address()
> call and using memcpy() to set netdev->dev_addr (which is what's done in
> lots of Ethernet drivers) and the second one fix the actual signedness
> issue.
>
> If for any reason you really want to keep a call to
> spider_net_get_mac_address() because the memcpy() is somehow not good
> enough here, we can also come up with a solution involving a temporary
> unsigned char variable.
Series applied, thanks.
^ permalink raw reply [flat|nested] 17+ messages in thread