[PATCH] grub-core/net/arp.c: trim arp packets with abnormal size.

[PATCH] grub-core/net/arp.c: trim arp packets with abnormal size.

[Paulo Flabiano Smorigo]

* grub-core/net/arp.c (grub_net_arp_receive): Trim arp packets with abnormal size.

GRUB uses arp request to create the arp response. If the incoming packet is
foobared, GRUB needs to trim the arp response packet before sending it.

This is just a fix in time for 2.02. I'm planning to rewrite the arp response code
after the release.

diff --git a/ChangeLog b/ChangeLog
index cfa9c83..10e9c3d 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,11 @@
+2014-01-31  Paulo Flabiano Smorigo  <pfsmorigo@br.ibm.com>
+
+	* grub-core/net/arp.c (grub_net_arp_receive): Trim arp packets with
+	abnormal size.
+
+	GRUB uses arp request to create the arp response. If the incoming packet is
+	foobared, GRUB needs to trim the arp response packet before sending it.
+
 2014-01-29  Vladimir Serbinenko  <phcoder@gmail.com>
 
 	* grub-core/disk/ahci.c: Increase timeout. Some SSDs take up to
diff --git a/grub-core/net/arp.c b/grub-core/net/arp.c
index d62d0cc..77581f4 100644
--- a/grub-core/net/arp.c
+++ b/grub-core/net/arp.c
@@ -162,6 +162,12 @@ grub_net_arp_receive (struct grub_net_buff *nb,
     if (grub_net_addr_cmp (&inf->address, &target_addr) == 0
 	&& grub_be_to_cpu16 (arp_header->op) == ARP_REQUEST)
       {
+        if ((nb->tail - nb->data) > 50)
+          {
+            grub_dprintf ("net", "arp packet with abnormal size (%ld bytes).\n",
+                         nb->tail - nb->data);
+            nb->tail = nb->data + 50;
+          }
 	grub_net_link_level_address_t target;
 	/* We've already checked that pln is either 4 or 16.  */
 	char tmp[16];

-- 
Paulo Flabiano Smorigo
IBM Linux Technology Center

Re: [PATCH] grub-core/net/arp.c: trim arp packets with abnormal size.

[Andrey Borzenkov]

В Fri, 31 Jan 2014 13:35:11 -0200
Paulo Flabiano Smorigo <pfsmorigo@linux.vnet.ibm.com> пишет:

> * grub-core/net/arp.c (grub_net_arp_receive): Trim arp packets with abnormal size.
> 
> GRUB uses arp request to create the arp response. If the incoming packet is
> foobared, GRUB needs to trim the arp response packet before sending it.
> 
> This is just a fix in time for 2.02. I'm planning to rewrite the arp response code
> after the release.
> 
> diff --git a/ChangeLog b/ChangeLog
> index cfa9c83..10e9c3d 100644
> --- a/ChangeLog
> +++ b/ChangeLog
> @@ -1,3 +1,11 @@
> +2014-01-31  Paulo Flabiano Smorigo  <pfsmorigo@br.ibm.com>
> +
> +	* grub-core/net/arp.c (grub_net_arp_receive): Trim arp packets with
> +	abnormal size.
> +
> +	GRUB uses arp request to create the arp response. If the incoming packet is
> +	foobared, GRUB needs to trim the arp response packet before sending it.
> +
>  2014-01-29  Vladimir Serbinenko  <phcoder@gmail.com>
>  
>  	* grub-core/disk/ahci.c: Increase timeout. Some SSDs take up to
> diff --git a/grub-core/net/arp.c b/grub-core/net/arp.c
> index d62d0cc..77581f4 100644
> --- a/grub-core/net/arp.c
> +++ b/grub-core/net/arp.c
> @@ -162,6 +162,12 @@ grub_net_arp_receive (struct grub_net_buff *nb,
>      if (grub_net_addr_cmp (&inf->address, &target_addr) == 0
>  	&& grub_be_to_cpu16 (arp_header->op) == ARP_REQUEST)
>        {
> +        if ((nb->tail - nb->data) > 50)

Using large ping size is quite common during network troubleshooting.
This looks like rather arbitrary restriction. I'd say - if we were able
to receive packet we should be able to also send packet of the same
size; what problem does it fix?

> +          {
> +            grub_dprintf ("net", "arp packet with abnormal size (%ld bytes).\n",
> +                         nb->tail - nb->data);
> +            nb->tail = nb->data + 50;
> +          }
>  	grub_net_link_level_address_t target;
>  	/* We've already checked that pln is either 4 or 16.  */
>  	char tmp[16];
> 

Re: [PATCH] grub-core/net/arp.c: trim arp packets with abnormal size.

[Paulo Flabiano Smorigo]

Fri, Jan 31, 2014 at 08:02:07PM +0400, Andrey Borzenkov wrote:
> В Fri, 31 Jan 2014 13:35:11 -0200
> Paulo Flabiano Smorigo <pfsmorigo@linux.vnet.ibm.com> пишет:
> 
> > * grub-core/net/arp.c (grub_net_arp_receive): Trim arp packets with abnormal size.
> > 
> > GRUB uses arp request to create the arp response. If the incoming packet is
> > foobared, GRUB needs to trim the arp response packet before sending it.
> > 
> > This is just a fix in time for 2.02. I'm planning to rewrite the arp response code
> > after the release.
> > 
> > diff --git a/ChangeLog b/ChangeLog
> > index cfa9c83..10e9c3d 100644
> > --- a/ChangeLog
> > +++ b/ChangeLog
> > @@ -1,3 +1,11 @@
> > +2014-01-31  Paulo Flabiano Smorigo  <pfsmorigo@br.ibm.com>
> > +
> > +	* grub-core/net/arp.c (grub_net_arp_receive): Trim arp packets with
> > +	abnormal size.
> > +
> > +	GRUB uses arp request to create the arp response. If the incoming packet is
> > +	foobared, GRUB needs to trim the arp response packet before sending it.
> > +
> >  2014-01-29  Vladimir Serbinenko  <phcoder@gmail.com>
> >  
> >  	* grub-core/disk/ahci.c: Increase timeout. Some SSDs take up to
> > diff --git a/grub-core/net/arp.c b/grub-core/net/arp.c
> > index d62d0cc..77581f4 100644
> > --- a/grub-core/net/arp.c
> > +++ b/grub-core/net/arp.c
> > @@ -162,6 +162,12 @@ grub_net_arp_receive (struct grub_net_buff *nb,
> >      if (grub_net_addr_cmp (&inf->address, &target_addr) == 0
> >  	&& grub_be_to_cpu16 (arp_header->op) == ARP_REQUEST)
> >        {
> > +        if ((nb->tail - nb->data) > 50)
> 
> Using large ping size is quite common during network troubleshooting.
> This looks like rather arbitrary restriction. I'd say - if we were able
> to receive packet we should be able to also send packet of the same
> size; what problem does it fix?

I hit a firmware bug where after a while every packet comes with MTU 
size (~1500 bytes). GRUB handles those foobared packets well and the
only problem is the case that I described above:

ARP responses are built using ARP request as its base. ARP responses
with trash are usually ignored by the network and my patch only trim the arp
responses to the corrent size, nothing more.

> 
> > +          {
> > +            grub_dprintf ("net", "arp packet with abnormal size (%ld bytes).\n",
> > +                         nb->tail - nb->data);
> > +            nb->tail = nb->data + 50;
> > +          }
> >  	grub_net_link_level_address_t target;
> >  	/* We've already checked that pln is either 4 or 16.  */
> >  	char tmp[16];
> > 
> 
> 
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel

-- 
Paulo Flabiano Smorigo
IBM Linux Technology Center

Re: [PATCH] grub-core/net/arp.c: trim arp packets with abnormal size.

[Vladimir 'φ-coder/phcoder' Serbinenko]

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

On 31.01.2014 16:35, Paulo Flabiano Smorigo wrote:
> * grub-core/net/arp.c (grub_net_arp_receive): Trim arp packets with abnormal size.
> 
> GRUB uses arp request to create the arp response. If the incoming packet is
> foobared, GRUB needs to trim the arp response packet before sending it.
> 
> This is just a fix in time for 2.02. I'm planning to rewrite the arp response code
> after the release.
> 
> diff --git a/ChangeLog b/ChangeLog
> index cfa9c83..10e9c3d 100644
> --- a/ChangeLog
> +++ b/ChangeLog
> @@ -1,3 +1,11 @@
> +2014-01-31  Paulo Flabiano Smorigo  <pfsmorigo@br.ibm.com>
> +
> +	* grub-core/net/arp.c (grub_net_arp_receive): Trim arp packets with
> +	abnormal size.
> +
> +	GRUB uses arp request to create the arp response. If the incoming packet is
> +	foobared, GRUB needs to trim the arp response packet before sending it.
> +
>  2014-01-29  Vladimir Serbinenko  <phcoder@gmail.com>
>  
>  	* grub-core/disk/ahci.c: Increase timeout. Some SSDs take up to
> diff --git a/grub-core/net/arp.c b/grub-core/net/arp.c
> index d62d0cc..77581f4 100644
> --- a/grub-core/net/arp.c
> +++ b/grub-core/net/arp.c
> @@ -162,6 +162,12 @@ grub_net_arp_receive (struct grub_net_buff *nb,
>      if (grub_net_addr_cmp (&inf->address, &target_addr) == 0
>  	&& grub_be_to_cpu16 (arp_header->op) == ARP_REQUEST)
>        {
> +        if ((nb->tail - nb->data) > 50)
> +          {
> +            grub_dprintf ("net", "arp packet with abnormal size (%ld bytes).\n",
> +                         nb->tail - nb->data);
> +            nb->tail = nb->data + 50;
> +          }
This is not right solution, I already commented on it. Instead you need
to recompute arp length from its fields.
>  	grub_net_link_level_address_t target;
>  	/* We've already checked that pln is either 4 or 16.  */
>  	char tmp[16];
> 



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

Re: [PATCH] grub-core/net/arp.c: trim arp packets with abnormal size.

[Vladimir 'φ-coder/phcoder' Serbinenko]

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

On 31.01.2014 17:02, Andrey Borzenkov wrote:
> Using large ping size is quite common during network troubleshooting.
> This looks like rather arbitrary restriction.
ARP != ICMP



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