[B.A.T.M.A.N.] [PATCH next 1/2] batman-adv: check return type of unicast packet preparations
27 Jul
2013
27 Jul
'13
1:24 a.m.
batadv_send_skb_prepare_unicast(_4addr) might reallocate the skb's data.
And if it tries to do so then this can potentially fail.
We shouldn't continue working on this skb in such a case.
Signed-off-by: Linus Lüssing <linus.luessing(a)web.de>
---
unicast.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/unicast.c b/unicast.c
index dc8b5d4..4c5a1aa 100644
--- a/unicast.c
+++ b/unicast.c
@@ -428,11 +428,13 @@ find_router:
switch (packet_type) {
case BATADV_UNICAST:
- batadv_unicast_prepare_skb(skb, orig_node);
+ if (!batadv_unicast_prepare_skb(skb, orig_node))
+ goto out;
break;
case BATADV_UNICAST_4ADDR:
- batadv_unicast_4addr_prepare_skb(bat_priv, skb, orig_node,
- packet_subtype);
+ if (!batadv_unicast_4addr_prepare_skb(bat_priv, skb, orig_node,
+ packet_subtype))
+ goto out;
break;
default:
/* this function supports UNICAST and UNICAST_4ADDR only. It
--
1.7.10.4
27 Jul
27 Jul
1:24 a.m.
New subject: [B.A.T.M.A.N.] [PATCH next 2/2] batman-adv: fix potential kernel paging error for unicast transmissions
batadv_send_skb_prepare_unicast(_4addr) might reallocate the skb's
data. If it does then our ethhdr pointer is not valid anymore in
batadv_send_skb_unicast(), resulting in a kernel paging error.
This patch fixes this issue by storing the few bytes we are interested
in on the stack before modifying the skb.
Signed-off-by: Linus Lüssing <linus.luessing(a)web.de>
---
unicast.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/unicast.c b/unicast.c
index 4c5a1aa..c636fd5 100644
--- a/unicast.c
+++ b/unicast.c
@@ -395,6 +395,7 @@ int batadv_unicast_generic_send_skb(struct batadv_priv *bat_priv,
struct sk_buff *skb, int packet_type,
int packet_subtype)
{
+ uint8_t dest[ETH_ALEN];
struct ethhdr *ethhdr = (struct ethhdr *)skb->data;
struct batadv_unicast_packet *unicast_packet;
struct batadv_orig_node *orig_node;
@@ -426,6 +427,8 @@ find_router:
if (!neigh_node)
goto out;
+ memcpy(dest, ethhdr->h_dest, sizeof(dest));
+
switch (packet_type) {
case BATADV_UNICAST:
if (!batadv_unicast_prepare_skb(skb, orig_node))
@@ -450,7 +453,7 @@ find_router:
* try to reroute it because the ttvn contained in the header is less
* than the current one
*/
- if (batadv_tt_global_client_is_roaming(bat_priv, ethhdr->h_dest))
+ if (batadv_tt_global_client_is_roaming(bat_priv, dest))
unicast_packet->ttvn = unicast_packet->ttvn - 1;
dev_mtu = neigh_node->if_incoming->net_dev->mtu;
--
1.7.10.4
28 Jul
28 Jul
8:17 p.m.
New subject: [B.A.T.M.A.N.] [PATCH next 2/2] batman-adv: fix potential kernel paging error for unicast transmissions
Il 27.07.2013 03:24 Linus Lüssing ha scritto:
batadv_send_skb_prepare_unicast(_4addr) might
reallocate the skb's
data. If it does then our ethhdr pointer is not valid anymore in
batadv_send_skb_unicast(), resulting in a kernel paging error.
This patch fixes this issue by storing the few bytes we are interested
in on the stack before modifying the skb.
Good fix! thanks!
However, I think it would be nice to send another patch aiming master
which could polish this situation a bit better: e.g. by calling
skb_reset_mac_header() in the batadv_send_skb_prepare_unicast_*
functions and then get the Ethernet header with eth_hdr() right after
having changed the skb.
Signed-off-by: Linus Lüssing <linus.luessing(a)web.de>
Acked-by: Antonio Quartulli <ordex(a)autistici.org>
--
Antonio Quartulli
2 Aug
2 Aug
4:05 a.m.
New subject: [B.A.T.M.A.N.] [PATCH next 2/2] batman-adv: fix potential kernel paging error for unicast transmissions
On Monday, July 29, 2013 04:17:57 Antonio Quartulli wrote:
Il 27.07.2013 03:24 Linus Lüssing ha scritto:
I like that approach because it seems cleaner that way. Is there a reason not
do it right away ?
Cheers,
Marek
batadv_send_skb_prepare_unicast(_4addr) might
reallocate the skb's
data. If it does then our ethhdr pointer is not valid anymore in
batadv_send_skb_unicast(), resulting in a kernel paging error.
This patch fixes this issue by storing the few bytes we are interested
in on the stack before modifying the skb.
Good fix! thanks!
However, I think it would be nice to send another patch aiming master
which could polish this situation a bit better: e.g. by calling
skb_reset_mac_header() in the batadv_send_skb_prepare_unicast_*
functions and then get the Ethernet header with eth_hdr() right after
having changed the skb.
7:18 a.m.
New subject: [B.A.T.M.A.N.] [PATCH next 2/2] batman-adv: fix potential kernel paging error for unicast transmissions
On Fri, Aug 02, 2013 at 12:05:28PM +0800, Marek Lindner wrote:
On Monday, July 29, 2013 04:17:57 Antonio Quartulli
wrote:
I thought the second approach would consist in a bigger patch and so I preferred
to send this to net and the bigger patch to net-next later.
But you may be right. The change I suggested is not really big.
Linus, would you like to provide "the next" patch so that we can clearly
understand if it is worth sending to net or not?
Cheers,
--
Antonio Quartulli
..each of us alone is worth nothing..
Ernesto "Che" Guevara
Il 27.07.2013 03:24 Linus Lüssing ha scritto:
I like that approach because it seems cleaner that way. Is there a reason not
do it right away ? batadv_send_skb_prepare_unicast(_4addr) might
reallocate the skb's
data. If it does then our ethhdr pointer is not valid anymore in
batadv_send_skb_unicast(), resulting in a kernel paging error.
This patch fixes this issue by storing the few bytes we are interested
in on the stack before modifying the skb.
Good fix! thanks!
However, I think it would be nice to send another patch aiming master
which could polish this situation a bit better: e.g. by calling
skb_reset_mac_header() in the batadv_send_skb_prepare_unicast_*
functions and then get the Ethernet header with eth_hdr() right after
having changed the skb.
8:04 a.m.
New subject: [B.A.T.M.A.N.] [PATCH next 2/2] batman-adv: fix potential kernel paging error for unicast transmissions
On Fri, Aug 02, 2013 at 09:18:06AM +0200, Antonio Quartulli wrote:
On Fri, Aug 02, 2013 at 12:05:28PM +0800, Marek
Lindner wrote:
Sure! I think it could actually be a lot easier than I thought (if I
didn't misread or miss something in the skb code).
On Monday, July 29, 2013 04:17:57 Antonio
Quartulli wrote:
I thought the second approach would consist in a bigger patch and so I preferred
to send this to net and the bigger patch to net-next later.
But you may be right. The change I suggested is not really big.
Linus, would you like to provide "the next" patch so that we can clearly
understand if it is worth sending to net or not? Il 27.07.2013 03:24 Linus Lüssing ha scritto:
I like that approach because it seems cleaner that way. Is there a reason not
do it right away ? batadv_send_skb_prepare_unicast(_4addr) might
reallocate the skb's
data. If it does then our ethhdr pointer is not valid anymore in
batadv_send_skb_unicast(), resulting in a kernel paging error.
This patch fixes this issue by storing the few bytes we are interested
in on the stack before modifying the skb.
Good fix! thanks!
However, I think it would be nice to send another patch aiming master
which could polish this situation a bit better: e.g. by calling
skb_reset_mac_header() in the batadv_send_skb_prepare_unicast_*
functions and then get the Ethernet header with eth_hdr() right after
having changed the skb.
Cheers,
--
Antonio Quartulli
..each of us alone is worth nothing..
Ernesto "Che" Guevara
28 Jul
28 Jul
8:14 p.m.
New subject: [B.A.T.M.A.N.] [PATCH next 1/2] batman-adv: check return type of unicast packet preparations
Il 27.07.2013 03:24 Linus Lüssing ha scritto:
batadv_send_skb_prepare_unicast(_4addr) might
reallocate the skb's
data.
In next these functions are named a bit differently. I think this can be
fixed while merging the patch.
And if it tries to do so then this can potentially
fail.
We shouldn't continue working on this skb in such a case.
Signed-off-by: Linus Lüssing <linus.luessing(a)web.de>
Acked-by: Antonio Quartulli <ordex(a)autistici.org>
--
Antonio Quartulli
2 Aug
2 Aug
4:02 a.m.
New subject: [B.A.T.M.A.N.] [PATCH next 1/2] batman-adv: check return type of unicast packet preparations
On Monday, July 29, 2013 04:14:23 Antonio Quartulli wrote:
Il 27.07.2013 03:24 Linus Lüssing ha scritto:
Applied in revision d781a70.
Thanks,
Marek
batadv_send_skb_prepare_unicast(_4addr) might
reallocate the skb's
data.
In next these functions are named a bit differently. I think this can be
fixed while merging the patch.
And if it tries to do so then this can
potentially fail.
We shouldn't continue working on this skb in such a case.
Signed-off-by: Linus Lüssing <linus.luessing(a)web.de>
Acked-by: Antonio Quartulli <ordex(a)autistici.org>
3521
days inactive
3527
days old
b.a.t.m.a.n@lists.open-mesh.org
7 comments
3 participants
participants (3)
-
Antonio Quartulli -
Linus Lüssing -
Marek Lindner