Hi,
i would propose some smaller corrections.
The following changes since commit 53320fe3bb1b1eef1aaff8dd47aae530ebeeb1e5:
batman-adv: Return hna count on local buffer fill (2010-12-20 10:32:03 -0800)
are available in the git repository at: git://git.open-mesh.org/ecsv/linux-merge.git for-david
Jesper Juhl (1): batman-adv: Even Batman should not dereference NULL pointers
Sven Eckelmann (1): batman-adv: Use "__attribute__" shortcut macros
net/batman-adv/main.h | 6 +++--- net/batman-adv/packet.h | 14 +++++++------- net/batman-adv/types.h | 4 ++-- net/batman-adv/unicast.c | 6 ++++-- 4 files changed, 16 insertions(+), 14 deletions(-)
From: Jesper Juhl jj@chaosbits.net
There's a problem in net/batman-adv/unicast.c::frag_send_skb(). dev_alloc_skb() allocates memory and may fail, thus returning NULL. If this happens we'll pass a NULL pointer on to skb_split() which in turn hands it to skb_split_inside_header() from where it gets passed to skb_put() that lets skb_tail_pointer() play with it and that function dereferences it. And thus the bat dies.
While I was at it I also moved the call to dev_alloc_skb() above the assignment to 'unicast_packet' since there's no reason to do that assignment if the memory allocation fails.
Signed-off-by: Jesper Juhl jj@chaosbits.net Signed-off-by: Sven Eckelmann sven@narfation.org --- net/batman-adv/unicast.c | 6 ++++-- 1 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/net/batman-adv/unicast.c b/net/batman-adv/unicast.c index dc2e28b..ee41fef 100644 --- a/net/batman-adv/unicast.c +++ b/net/batman-adv/unicast.c @@ -229,10 +229,12 @@ int frag_send_skb(struct sk_buff *skb, struct bat_priv *bat_priv, if (!bat_priv->primary_if) goto dropped;
- unicast_packet = (struct unicast_packet *) skb->data; + frag_skb = dev_alloc_skb(data_len - (data_len / 2) + ucf_hdr_len); + if (!frag_skb) + goto dropped;
+ unicast_packet = (struct unicast_packet *) skb->data; memcpy(&tmp_uc, unicast_packet, uc_hdr_len); - frag_skb = dev_alloc_skb(data_len - (data_len / 2) + ucf_hdr_len); skb_split(skb, frag_skb, data_len / 2);
if (my_skb_head_push(skb, ucf_hdr_len - uc_hdr_len) < 0 ||
Linux 2.6.21 defines different macros for __attribute__ which are also used inside batman-adv. The next version of checkpatch.pl warns about the usage of __attribute__((packed))).
Linux 2.6.33 defines an extra macro __always_unused which is used to assist source code analyzers and can be used to removed the last existing __attribute__ inside the source code.
Signed-off-by: Sven Eckelmann sven@narfation.org --- net/batman-adv/main.h | 6 +++--- net/batman-adv/packet.h | 14 +++++++------- net/batman-adv/types.h | 4 ++-- 3 files changed, 12 insertions(+), 12 deletions(-)
diff --git a/net/batman-adv/main.h b/net/batman-adv/main.h index d4d9926..65106fb 100644 --- a/net/batman-adv/main.h +++ b/net/batman-adv/main.h @@ -151,9 +151,9 @@ int debug_log(struct bat_priv *bat_priv, char *fmt, ...); } \ while (0) #else /* !CONFIG_BATMAN_ADV_DEBUG */ -static inline void bat_dbg(char type __attribute__((unused)), - struct bat_priv *bat_priv __attribute__((unused)), - char *fmt __attribute__((unused)), ...) +static inline void bat_dbg(char type __always_unused, + struct bat_priv *bat_priv __always_unused, + char *fmt __always_unused, ...) { } #endif diff --git a/net/batman-adv/packet.h b/net/batman-adv/packet.h index b49fdf7..2284e81 100644 --- a/net/batman-adv/packet.h +++ b/net/batman-adv/packet.h @@ -63,7 +63,7 @@ struct batman_packet { uint8_t num_hna; uint8_t gw_flags; /* flags related to gateway class */ uint8_t align; -} __attribute__((packed)); +} __packed;
#define BAT_PACKET_LEN sizeof(struct batman_packet)
@@ -76,7 +76,7 @@ struct icmp_packet { uint8_t orig[6]; uint16_t seqno; uint8_t uid; -} __attribute__((packed)); +} __packed;
#define BAT_RR_LEN 16
@@ -93,14 +93,14 @@ struct icmp_packet_rr { uint8_t uid; uint8_t rr_cur; uint8_t rr[BAT_RR_LEN][ETH_ALEN]; -} __attribute__((packed)); +} __packed;
struct unicast_packet { uint8_t packet_type; uint8_t version; /* batman version field */ uint8_t dest[6]; uint8_t ttl; -} __attribute__((packed)); +} __packed;
struct unicast_frag_packet { uint8_t packet_type; @@ -110,7 +110,7 @@ struct unicast_frag_packet { uint8_t flags; uint8_t orig[6]; uint16_t seqno; -} __attribute__((packed)); +} __packed;
struct bcast_packet { uint8_t packet_type; @@ -118,7 +118,7 @@ struct bcast_packet { uint8_t orig[6]; uint8_t ttl; uint32_t seqno; -} __attribute__((packed)); +} __packed;
struct vis_packet { uint8_t packet_type; @@ -131,6 +131,6 @@ struct vis_packet { * neighbors */ uint8_t target_orig[6]; /* who should receive this packet */ uint8_t sender_orig[6]; /* who sent or rebroadcasted this packet */ -} __attribute__((packed)); +} __packed;
#endif /* _NET_BATMAN_ADV_PACKET_H_ */ diff --git a/net/batman-adv/types.h b/net/batman-adv/types.h index 97cb23d..bf3f6f5 100644 --- a/net/batman-adv/types.h +++ b/net/batman-adv/types.h @@ -246,13 +246,13 @@ struct vis_info { /* this packet might be part of the vis send queue. */ struct sk_buff *skb_packet; /* vis_info may follow here*/ -} __attribute__((packed)); +} __packed;
struct vis_info_entry { uint8_t src[ETH_ALEN]; uint8_t dest[ETH_ALEN]; uint8_t quality; /* quality = 0 means HNA */ -} __attribute__((packed)); +} __packed;
struct recvlist_node { struct list_head list;
From: Sven Eckelmann sven@narfation.org Date: Sun, 16 Jan 2011 03:38:45 +0100
The following changes since commit 53320fe3bb1b1eef1aaff8dd47aae530ebeeb1e5:
batman-adv: Return hna count on local buffer fill (2010-12-20 10:32:03 -0800)
are available in the git repository at: git://git.open-mesh.org/ecsv/linux-merge.git for-david
Pulled, thanks Sven.
b.a.t.m.a.n@lists.open-mesh.org
-
David Miller -
Sven Eckelmann