[GIT,PULL,net-next,01/17] ndisc: Fix size calculation for headers.

We used to allocate MAX_HEADER bytes more than needed but
reserved hlen only (not MAX_HEADER + hlen) and the MAX_HEADER
was left behind.

Signed-off-by: YOSHIFUJI Hideaki <yoshfuji@linux-ipv6.org>
---
 net/ipv6/ndisc.c |    6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

From: YOSHIFUJI Hideaki <yoshfuji@linux-ipv6.org>
Date: Tue, 18 Dec 2012 19:52:04 +0900

> We used to allocate MAX_HEADER bytes more than needed but
> reserved hlen only (not MAX_HEADER + hlen) and the MAX_HEADER
> was left behind.
> 
> Signed-off-by: YOSHIFUJI Hideaki <yoshfuji@linux-ipv6.org>

This change is wrong.

The MAX_HEADER is being used in order to accomodate any
possible encapsulation that may occur.

I'm really disappointed that the very first patch in this series is
buggy, and you didn't even bother to repost the absolutely required
"00/17" email for this series which is where you're supposed to give a
top-level overview of your changes as well as any GIT pull request.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

From: David Miller <davem@davemloft.net>
Date: Tue, 18 Dec 2012 16:23:38 -0800 (PST)

> I'm really disappointed that the very first patch in this series is
> buggy, and you didn't even bother to repost the absolutely required
> "00/17" email for this series which is where you're supposed to give a
> top-level overview of your changes as well as any GIT pull request.

Also, right now the net-next tree is not open, so these changes
aren't even appropriate to be submitting right now.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

David Miller wrote:
> From: David Miller <davem@davemloft.net>
> Date: Tue, 18 Dec 2012 16:23:38 -0800 (PST)
> 
>> I'm really disappointed that the very first patch in this series is
>> buggy, and you didn't even bother to repost the absolutely required
>> "00/17" email for this series which is where you're supposed to give a
>> top-level overview of your changes as well as any GIT pull request.
> 
> Also, right now the net-next tree is not open, so these changes
> aren't even appropriate to be submitting right now.

Sorry about this, and I'll repost later.

--yoshfuji
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

David Miller wrote:
> From: YOSHIFUJI Hideaki <yoshfuji@linux-ipv6.org>
> Date: Tue, 18 Dec 2012 19:52:04 +0900
> 
>> We used to allocate MAX_HEADER bytes more than needed but
>> reserved hlen only (not MAX_HEADER + hlen) and the MAX_HEADER
>> was left behind.
>>
>> Signed-off-by: YOSHIFUJI Hideaki <yoshfuji@linux-ipv6.org>
> 
> This change is wrong.
> 
> The MAX_HEADER is being used in order to accomodate any
> possible encapsulation that may occur.

As I tried to explain in the commit message, ndisc_build_skb() does tI would say his:

|        int hlen = LL_RESERVED_SPACE(dev);
:
|        skb = sock_alloc_send_skb(sk,
|                                  (MAX_HEADER + sizeof(struct ipv6hdr) +
|                                   len + hlen + tlen),
|                                  1, &err);
|        if (!skb) {
|                ND_PRINTK(0, err, "ND: %s failed to allocate an skb, err=%d\n",
|                          __func__, err);
|                return NULL;
|        }
|
|        skb_reserve(skb, hlen);

This means, MAX_HEADER has been placed at the TAIL of
the buffer, instead of the HEAD of the buffer, like this.

head        data                   tail                        end
+--------------------------------------------------------------+
+           |                      |          |                |
+--------------------------------------------------------------+
|<- hlen  ->|<---ipv6 packet------>|<--tlen-->|<--MAX_HEADER-->|
|  = LL_
     RESERVED_
     SPACE(dev)

MAX_HEADER will not be used at all and I would say it is waste of
memory.

Or do you expect something like this?

+--------------------------------------------------------------+
+                |           |                      |          |
+--------------------------------------------------------------+
|<--MAX_HEADER-->|<---hlen-->|<---ipv6 packet------>|<--tlen-->|
head                       data                   tail       end

or like this:

+--------------------------------------------------+
+                |                      |          |
+--------------------------------------------------+
|<--MAX_HEADER-->|<---ipv6 packet------>|<--tlen-->|
head           data                   tail       end

If so, we should (re)visit almost all users of
sock_alloc_send_skb() and friends, anyway, I think.

--yoshfuji
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html