Patchwork [U-Boot] api: net: fix length check in eth_receive()

login
register
mail settings
Submitter Michael Walle
Date June 22, 2012, 9:24 p.m.
Message ID <1340400268-7403-1-git-send-email-michael@walle.cc>
Download mbox | patch
Permalink /patch/166677/
State Accepted
Delegated to: Joe Hershberger
Headers show

Comments

Michael Walle - June 22, 2012, 9:24 p.m.
If the requested length is too small to hold the received packet,
eth_receive() will return -1 and will leave the packet in the receive
buffers. Instead of returning an error in this case, we return the first
portion of the received packet and remove it from the receive buffers.

This fixes FreeBSD's ubldr. Without this patch it will just stop receiving
packets if the NIC receives more than PKTBUFSRX too large packets.

Signed-off-by: Michael Walle <michael@walle.cc>
Cc: Joe Hershberger <joe.hershberger@gmail.com>
Cc: Rafal Jaworowski <raj@semihalf.com>
Cc: Piotr Kruszynski <ppk@semihalf.com>
---
 net/eth.c |    5 +----
 1 files changed, 1 insertions(+), 4 deletions(-)
Joe Hershberger - June 25, 2012, 8:25 p.m.
Hi Michael,

On Fri, Jun 22, 2012 at 4:24 PM, Michael Walle <michael@walle.cc> wrote:
> If the requested length is too small to hold the received packet,
> eth_receive() will return -1 and will leave the packet in the receive
> buffers. Instead of returning an error in this case, we return the first
> portion of the received packet and remove it from the receive buffers.
>
> This fixes FreeBSD's ubldr. Without this patch it will just stop receiving
> packets if the NIC receives more than PKTBUFSRX too large packets.
>
> Signed-off-by: Michael Walle <michael@walle.cc>
> Cc: Joe Hershberger <joe.hershberger@gmail.com>
> Cc: Rafal Jaworowski <raj@semihalf.com>
> Cc: Piotr Kruszynski <ppk@semihalf.com>
> ---
>  net/eth.c |    5 +----
>  1 files changed, 1 insertions(+), 4 deletions(-)
>
> diff --git a/net/eth.c b/net/eth.c
> index d526264..09249c9 100644
> --- a/net/eth.c
> +++ b/net/eth.c
> @@ -486,10 +486,7 @@ int eth_receive(void *packet, int length)
>                        return -1;
>        }
>
> -       if (length < eth_rcv_bufs[eth_rcv_current].length)
> -               return -1;
> -
> -       length = eth_rcv_bufs[eth_rcv_current].length;
> +       length = min(eth_rcv_bufs[eth_rcv_current].length, length);

Is it really a good idea to return a runt packet to the network
protocols?  I understand wanting to drop the too-big packet, but I
think you should simply drop that packet before returning the error,
but still error.

-Joe
Michael Walle - June 25, 2012, 8:50 p.m.
[removed Piotr Kruszynski from CC because mail address does not exist anymore]

Hi Joe,

Am Montag 25 Juni 2012, 22:25:56 schrieb Joe Hershberger:
> Hi Michael,
> 
> On Fri, Jun 22, 2012 at 4:24 PM, Michael Walle <michael@walle.cc> wrote:
> > If the requested length is too small to hold the received packet,
> > eth_receive() will return -1 and will leave the packet in the receive
> > buffers. Instead of returning an error in this case, we return the first
> > portion of the received packet and remove it from the receive buffers.
> > 
> > This fixes FreeBSD's ubldr. Without this patch it will just stop
> > receiving packets if the NIC receives more than PKTBUFSRX too large
> > packets.
> > 
> > Signed-off-by: Michael Walle <michael@walle.cc>
> > Cc: Joe Hershberger <joe.hershberger@gmail.com>
> > Cc: Rafal Jaworowski <raj@semihalf.com>
> > Cc: Piotr Kruszynski <ppk@semihalf.com>
> > ---
> >  net/eth.c |    5 +----
> >  1 files changed, 1 insertions(+), 4 deletions(-)
> > 
> > diff --git a/net/eth.c b/net/eth.c
> > index d526264..09249c9 100644
> > --- a/net/eth.c
> > +++ b/net/eth.c
> > @@ -486,10 +486,7 @@ int eth_receive(void *packet, int length)
> >                        return -1;
> >        }
> > 
> > -       if (length < eth_rcv_bufs[eth_rcv_current].length)
> > -               return -1;
> > -
> > -       length = eth_rcv_bufs[eth_rcv_current].length;
> > +       length = min(eth_rcv_bufs[eth_rcv_current].length, length);
> 
> Is it really a good idea to return a runt packet to the network
> protocols?  I understand wanting to drop the too-big packet, but I
> think you should simply drop that packet before returning the error,
> but still error.

its the same behaviour as recv() with udp packets, isn't it?
 see http://pubs.opengroup.org/onlinepubs/009695399/functions/recv.html

But i'm not set on this, maybe rafal could clarify this. My main point is to 
make ubldr working ;)
Joe Hershberger - Sept. 28, 2012, 3:48 p.m.
Hi Michael,

On Mon, Jun 25, 2012 at 3:50 PM, Michael Walle <michael@walle.cc> wrote:
>
> [removed Piotr Kruszynski from CC because mail address does not exist anymore]
>
> Hi Joe,
>
> Am Montag 25 Juni 2012, 22:25:56 schrieb Joe Hershberger:
>> Hi Michael,
>>
>> On Fri, Jun 22, 2012 at 4:24 PM, Michael Walle <michael@walle.cc> wrote:
>> > If the requested length is too small to hold the received packet,
>> > eth_receive() will return -1 and will leave the packet in the receive
>> > buffers. Instead of returning an error in this case, we return the first
>> > portion of the received packet and remove it from the receive buffers.
>> >
>> > This fixes FreeBSD's ubldr. Without this patch it will just stop
>> > receiving packets if the NIC receives more than PKTBUFSRX too large
>> > packets.
>> >
>> > Signed-off-by: Michael Walle <michael@walle.cc>
>> > Cc: Joe Hershberger <joe.hershberger@gmail.com>
>> > Cc: Rafal Jaworowski <raj@semihalf.com>
>> > Cc: Piotr Kruszynski <ppk@semihalf.com>
>> > ---

Applied, thanks.

-Joe

Patch

diff --git a/net/eth.c b/net/eth.c
index d526264..09249c9 100644
--- a/net/eth.c
+++ b/net/eth.c
@@ -486,10 +486,7 @@  int eth_receive(void *packet, int length)
 			return -1;
 	}
 
-	if (length < eth_rcv_bufs[eth_rcv_current].length)
-		return -1;
-
-	length = eth_rcv_bufs[eth_rcv_current].length;
+	length = min(eth_rcv_bufs[eth_rcv_current].length, length);
 
 	for (i = 0; i < length; i++)
 		p[i] = eth_rcv_bufs[eth_rcv_current].data[i];