From: Sven Eckelmann <sven@narfation.org>
To: Antonio Quartulli <a@unstable.cc>
Cc: The list for a Better Approach To Mobile Ad-hoc Networking
<b.a.t.m.a.n@lists.open-mesh.org>,
Marek Lindner <mareklindner@neomailbox.ch>
Subject: Re: [B.A.T.M.A.N.] [PATCH] batman-adv: Check skb size before using encapsulated ETH+VLAN header
Date: Sun, 28 Feb 2016 10:20:30 +0100 [thread overview]
Message-ID: <2289020.8mL0hEvS8q@sven-edge> (raw)
In-Reply-To: <20160228090227.GJ28478@prodigo>
[-- Attachment #1: Type: text/plain, Size: 1644 bytes --]
On Sunday 28 February 2016 17:02:27 Antonio Quartulli wrote:
> On Sun, Feb 28, 2016 at 07:36:40AM +0100, Sven Eckelmann wrote:
> > On Sunday 28 February 2016 08:49:02 Marek Lindner wrote:
> > > On Friday, February 26, 2016 17:56:13 Sven Eckelmann wrote:
> > >
> > > In its current form that check is useless because
> > > batadv_recv_unicast_packet() already calls batadv_check_unicast_packet()
> > > which does the same
> > > pskb_may_pull(skb, hdr_size). Am I overlooking something ?
> >
> > Looks like it also only checks the size of the batman-adv header and the
> > content of the outer (not the encapsulated) ethernet header.
>
> I think you are both right here :-) in batadv_check_unicast_packet() we
> perform a may_pull with the same hdr_size, therefore it here once again
> is not useful. It is not really related to this patch, but I think that
> the removal of the useless pskb_may_pull() in interface_rx() could be
> done here since you are cleaning up all the pulls in this function.
Ok, I misunderstood his comment. This one is not necessary when each path to
this function uses batadv_check_unicast_packet or does "if
(!pskb_may_pull(skb, hdr_size))" directly. The only callers are
batadv_recv_bcast_packet (does "if (unlikely(!pskb_may_pull(skb, hdr_size)))")
and batadv_recv_unicast_packet (calls batadv_check_unicast_packet). I would
say that an extra patch for that can be introduced later to remove it. But it
should not be part of this patch because this fix should not contain cleanup
stuff ("batman-adv header size check") which is not related to the
encapsulated ethernet (vlan) header.
Kind regards,
Sven
[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
next prev parent reply other threads:[~2016-02-28 9:20 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-02-26 16:56 [B.A.T.M.A.N.] [PATCH] batman-adv: Check skb size before using encapsulated ETH+VLAN header Sven Eckelmann
2016-02-28 0:49 ` Marek Lindner
2016-02-28 6:36 ` Sven Eckelmann
2016-02-28 9:02 ` Antonio Quartulli
2016-02-28 9:20 ` Sven Eckelmann [this message]
2016-02-28 9:42 ` Antonio Quartulli
2016-03-20 9:57 ` Marek Lindner
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=2289020.8mL0hEvS8q@sven-edge \
--to=sven@narfation.org \
--cc=a@unstable.cc \
--cc=b.a.t.m.a.n@lists.open-mesh.org \
--cc=mareklindner@neomailbox.ch \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox