Skip to contentSkip to conversation

: Fix RTNLGRP_IPV6_IFINFO netlink notifications

IPV6 Patrick McHardy 2 participants 3 messages Jun 8, 2006 – Jun 13, 2006 1 message not in the archive

Subsystems (3)
Networking Network drivers IPv4/IPv6
lore
Share & tools
lore

Patrick McHardy

author
#0 lore raw

[IPV6]: Fix RTNLGRP_IPV6_IFINFO netlink notifications

The allocated skb is too small for the message, which results in an EINVAL getting reported to the (innocent) multicast listeners.

Signed-off-by Patrick McHardy <[email protected]>

---
commit f71bc8ace692302ad3a80d84b8cbccbb14e88fe5
tree 21ccfa89e911ff8e6bda04ab817367f2ac4e3173
parent 1def630a6a49dda5bc89dfbd86656293640456f0
author Patrick McHardy <[email protected]> Wed, 07 Jun 2006 09:25:26 +0200 committer Patrick McHardy <[email protected]> Wed, 07 Jun 2006 09:25:26 +0200

 net/ipv6/addrconf.c |    3 +--
 1 files changed, 1 insertions(+), 2 deletions(-)
net/ipv6/addrconf.c +1 -2
index 445006e..cc81cab 100644
@@ -3235,8 +3235,7 @@ static int inet6_dump_ifinfo(struct sk_b
 void inet6_ifinfo_notify(int event, struct inet6_dev *idev)
 {
 	struct sk_buff *skb;
-	/* 128 bytes ?? */
-	int size = NLMSG_SPACE(sizeof(struct ifinfomsg)+128);
+	int size = NLMSG_SPACE(sizeof(struct ifinfomsg)+172);
 	
 	skb = alloc_skb(size, GFP_ATOMIC);
 	if (!skb) {
#1 lore raw

Hello.

In article <[email protected]> (at Fri, 09 Jun 2006 00:56:46 +0200), Patrick McHardy <[email protected]> says:

Okay, thanks.
I think it is better to eliminate these magic numbers.

How about this?

Signed-off-by YOSHIFUJI Hideaki <[email protected]>
net/ipv6/addrconf.c +23 -4
index 445006e..b5267c6 100644
@@ -2860,6 +2860,11 @@ inet6_rtm_newaddr(struct sk_buff *skb, s
 	return inet6_addr_add(ifm->ifa_index, pfx, ifm->ifa_prefixlen);
 }
 
+/* Maximum length of ifa_cacheinfo attributes */
+#define INET6_IFADDR_RTA_SPACE \
+		RTA_SPACE(16) /* IFA_ADDRESS */ + \
+		RTA_SPACE(sizeof(struct ifa_cacheinfo)) /* CACHEINFO */
+
 static int inet6_fill_ifaddr(struct sk_buff *skb, struct inet6_ifaddr *ifa,
 			     u32 pid, u32 seq, int event, unsigned int flags)
 {

@@ -3092,7 +3097,7 @@ static int inet6_dump_ifacaddr(struct sk
 static void inet6_ifa_notify(int event, struct inet6_ifaddr *ifa)
 {
 	struct sk_buff *skb;
-	int size = NLMSG_SPACE(sizeof(struct ifaddrmsg)+128);
+	int size = NLMSG_SPACE(sizeof(struct ifaddrmsg) + INET6_IFADDR_RTA_SPACE);
 
 	skb = alloc_skb(size, GFP_ATOMIC);
 	if (!skb) {

@@ -3142,6 +3147,16 @@ static void inline ipv6_store_devconf(st
 #endif
 }
 
+/* Maximum length of ifinfomsg attributes */
+#define INET6_IFINFO_RTA_SPACE \
+		RTA_SPACE(IFNAMSIZ) /* IFNAME */ + \
+		RTA_SPACE(MAX_ADDR_LEN) /* ADDRESS */ +	\
+		RTA_SPACE(sizeof(u32)) /* MTU */ + \
+		RTA_SPACE(0) /* PROTINFO */ + \
+		RTA_SPACE(sizeof(u32)) /* FLAGS */ + \
+		RTA_SPACE(sizeof(struct ifla_cacheinfo)) /* CACHEINFO */ + \
+		RTA_SPACE(sizeof(__s32[DEVCONF_MAX])) /* CONF */
+
 static int inet6_fill_ifinfo(struct sk_buff *skb, struct inet6_dev *idev, 
 			     u32 pid, u32 seq, int event, unsigned int flags)
 {

@@ -3235,8 +3250,7 @@ static int inet6_dump_ifinfo(struct sk_b
 void inet6_ifinfo_notify(int event, struct inet6_dev *idev)
 {
 	struct sk_buff *skb;
-	/* 128 bytes ?? */
-	int size = NLMSG_SPACE(sizeof(struct ifinfomsg)+128);
+	int size = NLMSG_SPACE(sizeof(struct ifinfomsg) + INET6_IFINFO_RTA_SPACE);
 	
 	skb = alloc_skb(size, GFP_ATOMIC);
 	if (!skb) {

@@ -3252,6 +3266,11 @@ void inet6_ifinfo_notify(int event, stru
 	netlink_broadcast(rtnl, skb, 0, RTNLGRP_IPV6_IFINFO, GFP_ATOMIC);
 }
 
+/* Maximum length of prefix_cacheinfo attributes */
+#define INET6_PREFIX_RTA_SPACE \
+		RTA_SPACE(sizeof(((struct prefix_info *)NULL)->prefix)) /* ADDRESS */ + \
+		RTA_SPACE(sizeof(struct prefix_cacheinfo)) /* CACHEINFO */
+
 static int inet6_fill_prefix(struct sk_buff *skb, struct inet6_dev *idev,
 			struct prefix_info *pinfo, u32 pid, u32 seq, 
 			int event, unsigned int flags)

@@ -3296,7 +3315,7 @@ static void inet6_prefix_notify(int even
 			 struct prefix_info *pinfo)
 {
 	struct sk_buff *skb;
-	int size = NLMSG_SPACE(sizeof(struct prefixmsg)+128);
+	int size = NLMSG_SPACE(sizeof(struct prefixmsg) + INET6_PREFIX_RTA_SPACE);
 
 	skb = alloc_skb(size, GFP_ATOMIC);
 	if (!skb) {
Signature · 2 lines
YOSHIFUJI Hideaki @ USAGI Project  <[email protected]>
GPG-FP  : 9022 65EB 1ECF 3AD1 0BDF  80D8 4807 F894 E062 0EEA

A message that is not in the archive (sent off-list or not yet received): [email protected]

#3 lore raw

In article <[email protected]> (at Tue, 13 Jun 2006 18:03:22 +0200), Patrick McHardy <[email protected]> says:

Quoted 5 lines

Yes, thats better. I also thing we shouldn't return -EINVAL but -ENOSPC or something like that, -EINVAL usually indicated that the user sent an invalid message, which isn't the case here. Maybe a WARN_ON would also be appropriate, since this condition always means there is a bug.

Yes, but that's another issue.
Of course, you can cook up patch for it.

Quoted 5 lines

Review

Quoted 5 lines

How about this?

(quoted diff) +3 -0
+/* Maximum length of ifinfomsg attributes */
+#define INET6_IFINFO_RTA_SPACE \
+		RTA_SPACE(IFNAMSIZ) /* IFNAME */ + \

:

Quoted 2 lines

This doesn't seem to account for IFLA_LINK. Other than that it looks fine too me.

[IPV6]: Sum real space for RTAs.

This patch fixes RTNLGRP_IPV6_IFINFO netlink notifications. Issue pointed out by Patrick McHardy <[email protected]>.

Signed-off-by YOSHIFUJI Hideaki <[email protected]>
net/ipv6/addrconf.c +24 -4
index 445006e..c2c26fa 100644
@@ -2860,6 +2860,11 @@ inet6_rtm_newaddr(struct sk_buff *skb, s
 	return inet6_addr_add(ifm->ifa_index, pfx, ifm->ifa_prefixlen);
 }
 
+/* Maximum length of ifa_cacheinfo attributes */
+#define INET6_IFADDR_RTA_SPACE \
+		RTA_SPACE(16) /* IFA_ADDRESS */ + \
+		RTA_SPACE(sizeof(struct ifa_cacheinfo)) /* CACHEINFO */
+
 static int inet6_fill_ifaddr(struct sk_buff *skb, struct inet6_ifaddr *ifa,
 			     u32 pid, u32 seq, int event, unsigned int flags)
 {

@@ -3092,7 +3097,7 @@ static int inet6_dump_ifacaddr(struct sk
 static void inet6_ifa_notify(int event, struct inet6_ifaddr *ifa)
 {
 	struct sk_buff *skb;
-	int size = NLMSG_SPACE(sizeof(struct ifaddrmsg)+128);
+	int size = NLMSG_SPACE(sizeof(struct ifaddrmsg) + INET6_IFADDR_RTA_SPACE);
 
 	skb = alloc_skb(size, GFP_ATOMIC);
 	if (!skb) {

@@ -3142,6 +3147,17 @@ #endif
 #endif
 }
 
+/* Maximum length of ifinfomsg attributes */
+#define INET6_IFINFO_RTA_SPACE \
+		RTA_SPACE(IFNAMSIZ) /* IFNAME */ + \
+		RTA_SPACE(MAX_ADDR_LEN) /* ADDRESS */ +	\
+		RTA_SPACE(sizeof(u32)) /* MTU */ + \
+		RTA_SPACE(sizeof(int)) /* LINK */ + \
+		RTA_SPACE(0) /* PROTINFO */ + \
+		RTA_SPACE(sizeof(u32)) /* FLAGS */ + \
+		RTA_SPACE(sizeof(struct ifla_cacheinfo)) /* CACHEINFO */ + \
+		RTA_SPACE(sizeof(__s32[DEVCONF_MAX])) /* CONF */
+
 static int inet6_fill_ifinfo(struct sk_buff *skb, struct inet6_dev *idev, 
 			     u32 pid, u32 seq, int event, unsigned int flags)
 {

@@ -3235,8 +3251,7 @@ static int inet6_dump_ifinfo(struct sk_b
 void inet6_ifinfo_notify(int event, struct inet6_dev *idev)
 {
 	struct sk_buff *skb;
-	/* 128 bytes ?? */
-	int size = NLMSG_SPACE(sizeof(struct ifinfomsg)+128);
+	int size = NLMSG_SPACE(sizeof(struct ifinfomsg) + INET6_IFINFO_RTA_SPACE);
 	
 	skb = alloc_skb(size, GFP_ATOMIC);
 	if (!skb) {

@@ -3252,6 +3267,11 @@ void inet6_ifinfo_notify(int event, stru
 	netlink_broadcast(rtnl, skb, 0, RTNLGRP_IPV6_IFINFO, GFP_ATOMIC);
 }
 
+/* Maximum length of prefix_cacheinfo attributes */
+#define INET6_PREFIX_RTA_SPACE \
+		RTA_SPACE(sizeof(((struct prefix_info *)NULL)->prefix)) /* ADDRESS */ + \
+		RTA_SPACE(sizeof(struct prefix_cacheinfo)) /* CACHEINFO */
+
 static int inet6_fill_prefix(struct sk_buff *skb, struct inet6_dev *idev,
 			struct prefix_info *pinfo, u32 pid, u32 seq, 
 			int event, unsigned int flags)

@@ -3296,7 +3316,7 @@ static void inet6_prefix_notify(int even
 			 struct prefix_info *pinfo)
 {
 	struct sk_buff *skb;
-	int size = NLMSG_SPACE(sizeof(struct prefixmsg)+128);
+	int size = NLMSG_SPACE(sizeof(struct prefixmsg) + INET6_PREFIX_RTA_SPACE);
 
 	skb = alloc_skb(size, GFP_ATOMIC);
 	if (!skb) {