* [PATCH] MIPS: BCM63XX: properly handle mac address octet overflow
@ 2012-09-18 9:32 Jonas Gorski
2012-09-18 11:51 ` Sergei Shtylyov
2012-09-18 12:09 ` [PATCH V2] " Jonas Gorski
0 siblings, 2 replies; 5+ messages in thread
From: Jonas Gorski @ 2012-09-18 9:32 UTC (permalink / raw)
To: Ralf Baechle; +Cc: linux-mips, Maxime Bizon, Florian Fainelli
While calculating the mac address the pointer for the current octet was
never reset back to the least significant one after being decremented
because of an octet overflow. This resulted in the code continuing to
increment at the current octet, potentially generating duplicate or
invalid mac addresses.
As a second issue the pointer was allowed to advance up to the most
significant octet, modifying the OUI, and potentially changing the type
of mac address.
Rewrite the code so it resets the pointer to the least significant
in each outer loop step, and bails out when the least significant octet
of the OUI is reached.
Signed-off-by: Jonas Gorski <jonas.gorski@gmail.com>
---
arch/mips/bcm63xx/boards/board_bcm963xx.c | 16 +++++++++-------
1 file changed, 9 insertions(+), 7 deletions(-)
diff --git a/arch/mips/bcm63xx/boards/board_bcm963xx.c b/arch/mips/bcm63xx/boards/board_bcm963xx.c
index ea4ea77..f0fcec6 100644
--- a/arch/mips/bcm63xx/boards/board_bcm963xx.c
+++ b/arch/mips/bcm63xx/boards/board_bcm963xx.c
@@ -720,7 +720,7 @@ const char *board_get_name(void)
*/
static int board_get_mac_address(u8 *mac)
{
- u8 *p;
+ u8 *oui;
int count;
if (mac_addr_used >= nvram.mac_addr_count) {
@@ -729,21 +729,23 @@ static int board_get_mac_address(u8 *mac)
}
memcpy(mac, nvram.mac_addr_base, ETH_ALEN);
- p = mac + ETH_ALEN - 1;
+ oui = mac + ETH_ALEN/2 - 1;
count = mac_addr_used;
while (count--) {
+ p = mac + ETH_ALEN - 1;
+
do {
(*p)++;
if (*p != 0)
break;
p--;
- } while (p != mac);
- }
+ } while (p != oui);
- if (p == mac) {
- printk(KERN_ERR PFX "unable to fetch mac address\n");
- return -ENODEV;
+ if (p == oui) {
+ printk(KERN_ERR PFX "unable to fetch mac address\n");
+ return -ENODEV;
+ }
}
mac_addr_used++;
--
1.7.10.4
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] MIPS: BCM63XX: properly handle mac address octet overflow
2012-09-18 9:32 [PATCH] MIPS: BCM63XX: properly handle mac address octet overflow Jonas Gorski
@ 2012-09-18 11:51 ` Sergei Shtylyov
2012-09-18 11:56 ` Jonas Gorski
2012-09-18 12:09 ` [PATCH V2] " Jonas Gorski
1 sibling, 1 reply; 5+ messages in thread
From: Sergei Shtylyov @ 2012-09-18 11:51 UTC (permalink / raw)
To: Jonas Gorski; +Cc: Ralf Baechle, linux-mips, Maxime Bizon, Florian Fainelli
Hello.
On 18-09-2012 13:32, Jonas Gorski wrote:
> While calculating the mac address the pointer for the current octet was
> never reset back to the least significant one after being decremented
> because of an octet overflow. This resulted in the code continuing to
> increment at the current octet, potentially generating duplicate or
> invalid mac addresses.
> As a second issue the pointer was allowed to advance up to the most
> significant octet, modifying the OUI, and potentially changing the type
> of mac address.
> Rewrite the code so it resets the pointer to the least significant
> in each outer loop step, and bails out when the least significant octet
> of the OUI is reached.
> Signed-off-by: Jonas Gorski <jonas.gorski@gmail.com>
> ---
> arch/mips/bcm63xx/boards/board_bcm963xx.c | 16 +++++++++-------
> 1 file changed, 9 insertions(+), 7 deletions(-)
> diff --git a/arch/mips/bcm63xx/boards/board_bcm963xx.c b/arch/mips/bcm63xx/boards/board_bcm963xx.c
> index ea4ea77..f0fcec6 100644
> --- a/arch/mips/bcm63xx/boards/board_bcm963xx.c
> +++ b/arch/mips/bcm63xx/boards/board_bcm963xx.c
> @@ -720,7 +720,7 @@ const char *board_get_name(void)
> */
> static int board_get_mac_address(u8 *mac)
> {
> - u8 *p;
> + u8 *oui;
> int count;
>
> if (mac_addr_used >= nvram.mac_addr_count) {
> @@ -729,21 +729,23 @@ static int board_get_mac_address(u8 *mac)
> }
>
> memcpy(mac, nvram.mac_addr_base, ETH_ALEN);
> - p = mac + ETH_ALEN - 1;
> + oui = mac + ETH_ALEN/2 - 1;
> count = mac_addr_used;
>
> while (count--) {
> + p = mac + ETH_ALEN - 1;
But didn't you remove 'p' above? Did you compile this?
WBR, Sergei
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] MIPS: BCM63XX: properly handle mac address octet overflow
2012-09-18 11:51 ` Sergei Shtylyov
@ 2012-09-18 11:56 ` Jonas Gorski
0 siblings, 0 replies; 5+ messages in thread
From: Jonas Gorski @ 2012-09-18 11:56 UTC (permalink / raw)
To: Sergei Shtylyov; +Cc: Ralf Baechle, linux-mips, Maxime Bizon, Florian Fainelli
On 18 September 2012 13:51, Sergei Shtylyov <sshtylyov@mvista.com> wrote:
> Hello.
>
>
> On 18-09-2012 13:32, Jonas Gorski wrote:
>
>> While calculating the mac address the pointer for the current octet was
>> never reset back to the least significant one after being decremented
>> because of an octet overflow. This resulted in the code continuing to
>> increment at the current octet, potentially generating duplicate or
>> invalid mac addresses.
>
>
>> As a second issue the pointer was allowed to advance up to the most
>> significant octet, modifying the OUI, and potentially changing the type
>> of mac address.
>
>
>> Rewrite the code so it resets the pointer to the least significant
>> in each outer loop step, and bails out when the least significant octet
>> of the OUI is reached.
>
>
>> Signed-off-by: Jonas Gorski <jonas.gorski@gmail.com>
>> ---
>> arch/mips/bcm63xx/boards/board_bcm963xx.c | 16 +++++++++-------
>> 1 file changed, 9 insertions(+), 7 deletions(-)
>
>
>> diff --git a/arch/mips/bcm63xx/boards/board_bcm963xx.c
>> b/arch/mips/bcm63xx/boards/board_bcm963xx.c
>> index ea4ea77..f0fcec6 100644
>> --- a/arch/mips/bcm63xx/boards/board_bcm963xx.c
>> +++ b/arch/mips/bcm63xx/boards/board_bcm963xx.c
>> @@ -720,7 +720,7 @@ const char *board_get_name(void)
>> */
>> static int board_get_mac_address(u8 *mac)
>> {
>> - u8 *p;
>> + u8 *oui;
>> int count;
>>
>> if (mac_addr_used >= nvram.mac_addr_count) {
>> @@ -729,21 +729,23 @@ static int board_get_mac_address(u8 *mac)
>> }
>>
>> memcpy(mac, nvram.mac_addr_base, ETH_ALEN);
>> - p = mac + ETH_ALEN - 1;
>> + oui = mac + ETH_ALEN/2 - 1;
>> count = mac_addr_used;
>>
>> while (count--) {
>> + p = mac + ETH_ALEN - 1;
>
>
> But didn't you remove 'p' above? Did you compile this?
Argh. Yes, but the wrong version (my "user space" one to test it). Let
me try that again ... . Thanks for catching it.
Jonas
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH V2] MIPS: BCM63XX: properly handle mac address octet overflow
2012-09-18 9:32 [PATCH] MIPS: BCM63XX: properly handle mac address octet overflow Jonas Gorski
2012-09-18 11:51 ` Sergei Shtylyov
@ 2012-09-18 12:09 ` Jonas Gorski
2012-09-23 17:34 ` Ralf Baechle
1 sibling, 1 reply; 5+ messages in thread
From: Jonas Gorski @ 2012-09-18 12:09 UTC (permalink / raw)
To: Ralf Baechle; +Cc: linux-mips, Maxime Bizon, Florian Fainelli, Sergei Shtylyov
While calculating the mac address the pointer for the current octet was
never reset back to the least significant one after being decremented
because of an octet overflow. This resulted in the code continuing to
increment at the current octet, potentially generating duplicate or
invalid mac addresses.
As a second issue the pointer was allowed to advance up to the most
significant octet, modifying the OUI, and potentially changing the type
of mac address.
Rewrite the code so it resets the pointer to the least significant
in each outer loop step, and bails out when the least significant octet
of the OUI is reached.
Signed-off-by: Jonas Gorski <jonas.gorski@gmail.com>
---
V1 -> V2: add a missing variable declaration breaking the compilation.
arch/mips/bcm63xx/boards/board_bcm963xx.c | 16 +++++++++-------
1 file changed, 9 insertions(+), 7 deletions(-)
diff --git a/arch/mips/bcm63xx/boards/board_bcm963xx.c b/arch/mips/bcm63xx/boards/board_bcm963xx.c
index ea4ea77..442ba96 100644
--- a/arch/mips/bcm63xx/boards/board_bcm963xx.c
+++ b/arch/mips/bcm63xx/boards/board_bcm963xx.c
@@ -720,7 +720,7 @@ const char *board_get_name(void)
*/
static int board_get_mac_address(u8 *mac)
{
- u8 *p;
+ u8 *oui;
int count;
if (mac_addr_used >= nvram.mac_addr_count) {
@@ -729,21 +729,23 @@ static int board_get_mac_address(u8 *mac)
}
memcpy(mac, nvram.mac_addr_base, ETH_ALEN);
- p = mac + ETH_ALEN - 1;
+ oui = mac + ETH_ALEN/2 - 1;
count = mac_addr_used;
while (count--) {
+ u8 *p = mac + ETH_ALEN - 1;
+
do {
(*p)++;
if (*p != 0)
break;
p--;
- } while (p != mac);
- }
+ } while (p != oui);
- if (p == mac) {
- printk(KERN_ERR PFX "unable to fetch mac address\n");
- return -ENODEV;
+ if (p == oui) {
+ printk(KERN_ERR PFX "unable to fetch mac address\n");
+ return -ENODEV;
+ }
}
mac_addr_used++;
--
1.7.10.4
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH V2] MIPS: BCM63XX: properly handle mac address octet overflow
2012-09-18 12:09 ` [PATCH V2] " Jonas Gorski
@ 2012-09-23 17:34 ` Ralf Baechle
0 siblings, 0 replies; 5+ messages in thread
From: Ralf Baechle @ 2012-09-23 17:34 UTC (permalink / raw)
To: Jonas Gorski; +Cc: linux-mips, Maxime Bizon, Florian Fainelli, Sergei Shtylyov
Applied. Thanks,
Ralf
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2012-09-23 17:35 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-09-18 9:32 [PATCH] MIPS: BCM63XX: properly handle mac address octet overflow Jonas Gorski
2012-09-18 11:51 ` Sergei Shtylyov
2012-09-18 11:56 ` Jonas Gorski
2012-09-18 12:09 ` [PATCH V2] " Jonas Gorski
2012-09-23 17:34 ` Ralf Baechle
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox