[PATCH] bug fix for bootp

[PATCH] bug fix for bootp

[Bean]

[-- Attachment #1: Type: text/plain, Size: 161 bytes --]

Hi,

DHCP offer can use multicast address, so instead of comparing the
target address, we should compare the mac field in the dhcp packet.

-- 
Best wishes
Bean

[-- Attachment #2: bootp.txt --]
[-- Type: text/plain, Size: 939 bytes --]

=== modified file 'grub-core/net/ip.c'
--- grub-core/net/ip.c	2012-02-09 22:43:43 +0000
+++ grub-core/net/ip.c	2012-04-29 04:13:32 +0000
@@ -240,7 +240,7 @@
 	FOR_NET_NETWORK_LEVEL_INTERFACES (inf)
 	  if (inf->card == card
 	      && inf->address.type == GRUB_NET_NETWORK_LEVEL_PROTOCOL_DHCP_RECV
-	      && grub_net_hwaddr_cmp (&inf->hwaddress, hwaddress) == 0)
+	      && inf->hwaddress.type == GRUB_NET_LINK_LEVEL_PROTOCOL_ETHERNET)
 	    {
 	      if (udph->chksum)
 		{
@@ -266,7 +266,11 @@
 	      err = grub_netbuff_pull (nb, sizeof (*udph));
 	      if (err)
 		return err;
-	      grub_net_process_dhcp (nb, inf->card);
+	      struct grub_net_bootp_packet *dhcp =
+		(struct grub_net_bootp_packet *) nb->data;
+	      if (grub_memcmp(inf->hwaddress.mac, &dhcp->mac_addr,
+			      sizeof(inf->hwaddress.mac)) == 0)
+		grub_net_process_dhcp (nb, inf->card);
 	      grub_netbuff_free (nb);
 	      return GRUB_ERR_NONE;
 	    }

Re: [PATCH] bug fix for bootp

[Vladimir 'φ-coder/phcoder' Serbinenko]

[-- Attachment #1: Type: text/plain, Size: 547 bytes --]

On 29.04.2012 06:17, Bean wrote:
> Hi,
>
> DHCP offer can use multicast address, so instead of comparing the
> target address, we should compare the mac field in the dhcp packet.
Could you adjust this patch to the fact that there may be several
simultaneous bootp active requests from GRUB for different MAC addresses?
>
>
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel


-- 
Regards
Vladimir 'φ-coder/phcoder' Serbinenko



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 294 bytes --]

Re: [PATCH] bug fix for bootp

[Bean]

Hi,

It should be ok, since it's inside	FOR_NET_NETWORK_LEVEL_INTERFACES
(inf) loop and compare inf->hwaddress.mac to the current dhcp response
packet.

2012/4/30 Vladimir 'φ-coder/phcoder' Serbinenko <phcoder@gmail.com>:
> On 29.04.2012 06:17, Bean wrote:
>> Hi,
>>
>> DHCP offer can use multicast address, so instead of comparing the
>> target address, we should compare the mac field in the dhcp packet.
> Could you adjust this patch to the fact that there may be several
> simultaneous bootp active requests from GRUB for different MAC addresses?
>>
>>
>> _______________________________________________
>> Grub-devel mailing list
>> Grub-devel@gnu.org
>> https://lists.gnu.org/mailman/listinfo/grub-devel
>
>
> --
> Regards
> Vladimir 'φ-coder/phcoder' Serbinenko
>
>
>
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel
>



-- 
Best wishes
Bean

Re: [PATCH] bug fix for bootp

[Vladimir 'φ-coder/phcoder' Serbinenko]

[-- Attachment #1: Type: text/plain, Size: 1140 bytes --]

On 29.04.2012 20:26, Bean wrote:
> Hi,
>
> It should be ok, since it's inside	FOR_NET_NETWORK_LEVEL_INTERFACES
> (inf) loop and compare inf->hwaddress.mac to the current dhcp response
> packet.
It's not. You return even if packet didn't match.
> 2012/4/30 Vladimir 'φ-coder/phcoder' Serbinenko <phcoder@gmail.com>:
>> On 29.04.2012 06:17, Bean wrote:
>>> Hi,
>>>
>>> DHCP offer can use multicast address, so instead of comparing the
>>> target address, we should compare the mac field in the dhcp packet.
>> Could you adjust this patch to the fact that there may be several
>> simultaneous bootp active requests from GRUB for different MAC addresses?
>>>
>>> _______________________________________________
>>> Grub-devel mailing list
>>> Grub-devel@gnu.org
>>> https://lists.gnu.org/mailman/listinfo/grub-devel
>>
>> --
>> Regards
>> Vladimir 'φ-coder/phcoder' Serbinenko
>>
>>
>>
>> _______________________________________________
>> Grub-devel mailing list
>> Grub-devel@gnu.org
>> https://lists.gnu.org/mailman/listinfo/grub-devel
>>
>
>


-- 
Regards
Vladimir 'φ-coder/phcoder' Serbinenko



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 294 bytes --]

Re: [PATCH] bug fix for bootp

[Bean]

2012/4/30 Vladimir 'φ-coder/phcoder' Serbinenko <phcoder@gmail.com>:
> On 29.04.2012 20:26, Bean wrote:
>> Hi,
>>
>> It should be ok, since it's inside    FOR_NET_NETWORK_LEVEL_INTERFACES
>> (inf) loop and compare inf->hwaddress.mac to the current dhcp response
>> packet.
> It's not. You return even if packet didn't match.

Hi,

Oh i see, i forget about the return a few lines beyond.

-- 
Best wishes
Bean

Re: [PATCH] bug fix for bootp

[Bean]

[-- Attachment #1: Type: text/plain, Size: 579 bytes --]

2012/4/30 Bean <bean123ch@gmail.com>:
> 2012/4/30 Vladimir 'φ-coder/phcoder' Serbinenko <phcoder@gmail.com>:
>> On 29.04.2012 20:26, Bean wrote:
>>> Hi,
>>>
>>> It should be ok, since it's inside    FOR_NET_NETWORK_LEVEL_INTERFACES
>>> (inf) loop and compare inf->hwaddress.mac to the current dhcp response
>>> packet.
>> It's not. You return even if packet didn't match.
>
> Hi,
>
> Oh i see, i forget about the return a few lines beyond.

Hi,

oh, there seems to be a missing grub_netbuff_free here as well, pls
check if this is ok.

-- 
Best wishes
Bean

[-- Attachment #2: bootp.txt --]
[-- Type: text/plain, Size: 1298 bytes --]

=== modified file 'grub-core/net/ip.c'
--- grub-core/net/ip.c	2012-02-09 22:43:43 +0000
+++ grub-core/net/ip.c	2012-04-29 18:56:57 +0000
@@ -240,7 +240,7 @@
 	FOR_NET_NETWORK_LEVEL_INTERFACES (inf)
 	  if (inf->card == card
 	      && inf->address.type == GRUB_NET_NETWORK_LEVEL_PROTOCOL_DHCP_RECV
-	      && grub_net_hwaddr_cmp (&inf->hwaddress, hwaddress) == 0)
+	      && inf->hwaddress.type == GRUB_NET_LINK_LEVEL_PROTOCOL_ETHERNET)
 	    {
 	      if (udph->chksum)
 		{
@@ -257,18 +257,25 @@
 				    "Expected %x, got %x\n", 
 				    grub_be_to_cpu16 (expected),
 				    grub_be_to_cpu16 (chk));
-		      grub_netbuff_free (nb);
-		      return GRUB_ERR_NONE;
+		      break;
 		    }
 		  udph->chksum = chk;
 		}
 
 	      err = grub_netbuff_pull (nb, sizeof (*udph));
 	      if (err)
-		return err;
-	      grub_net_process_dhcp (nb, inf->card);
-	      grub_netbuff_free (nb);
-	      return GRUB_ERR_NONE;
+		{
+		  grub_netbuff_free (nb);
+		  return err;
+		}
+	      struct grub_net_bootp_packet *dhcp =
+		(struct grub_net_bootp_packet *) nb->data;
+	      if (grub_memcmp(inf->hwaddress.mac, &dhcp->mac_addr,
+			      sizeof(inf->hwaddress.mac)) == 0)
+		{
+		  grub_net_process_dhcp (nb, inf->card);
+		  break;
+		}
 	    }
 	grub_netbuff_free (nb);
 	return GRUB_ERR_NONE;

Re: [PATCH] bug fix for bootp

[Vladimir 'φ-coder/phcoder' Serbinenko]

[-- Attachment #1: Type: text/plain, Size: 956 bytes --]

On 29.04.2012 20:59, Bean wrote:
> 2012/4/30 Bean <bean123ch@gmail.com>:
>> 2012/4/30 Vladimir 'φ-coder/phcoder' Serbinenko <phcoder@gmail.com>:
>>> On 29.04.2012 20:26, Bean wrote:
>>>> Hi,
>>>>
>>>> It should be ok, since it's inside    FOR_NET_NETWORK_LEVEL_INTERFACES
>>>> (inf) loop and compare inf->hwaddress.mac to the current dhcp response
>>>> packet.
>>> It's not. You return even if packet didn't match.
>> Hi,
>>
>> Oh i see, i forget about the return a few lines beyond.
> Hi,
>
> oh, there seems to be a missing grub_netbuff_free here as well, pls
> check if this is ok.
The patch is still incorrect. Namely the problem is with netbuff_pull
being called in every loop. I've committed a fixed version.
>
>
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel


-- 
Regards
Vladimir 'φ-coder/phcoder' Serbinenko



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 294 bytes --]