Solomon Peachy
2008-Jul-02 23:09 UTC
[Bridge] [patch] rstpd crashes with GARP/GMRP packets
The attached patch, against Shrinivas's May 7, 2008 snapshot, fixes a
null pointer dereference that occurs when we receive a packet from the
brige interface that bears the STP MACADDR, but is *not* a STP packet.
Specifically, I was receiving GMRP packets (see 802.1D-2004 10.1) from a
3Com switch.
I don't know what we should do with these -- but crashing isn't it. I
can send over a packet dump and more debugging info if desired.
- Solomon
--
Solomon Peachy solomon at linux-wlan.com
AbsoluteValue Systems http://www.linux-wlan.com
721-D North Drive +1 (321) 259-0737 (office)
Melbourne, FL 32934 +1 (321) 259-0286 (fax)
-------------- next part --------------
diff --git a/packages/foss/rstp/brmon.c b/packages/foss/rstp/brmon.c
index d29e7f5..db0d3bb 100644
--- a/packages/foss/rstp/brmon.c
+++ b/packages/foss/rstp/brmon.c
@@ -153,7 +153,7 @@ static int dump_msg(const struct sockaddr_nl *who, struct
nlmsghdr *n,
int newlink = (n->nlmsg_type == RTM_NEWLINK);
int up = 0;
if (newlink && tb[IFLA_OPERSTATE]) {
- int state = *(int*)RTA_DATA(tb[IFLA_OPERSTATE]);
+ int state = *(uint8_t*)RTA_DATA(tb[IFLA_OPERSTATE]);
up = (state == IF_OPER_UP) || (state == IF_OPER_UNKNOWN);
}
diff --git a/packages/foss/rstp/brstate.c b/packages/foss/rstp/brstate.c
index 1fe792e..c31a647 100644
--- a/packages/foss/rstp/brstate.c
+++ b/packages/foss/rstp/brstate.c
@@ -42,7 +42,7 @@ static int br_set_state(struct rtnl_handle *rth, unsigned
ifindex, __u8 state)
req.ifi.ifi_family = AF_BRIDGE;
req.ifi.ifi_index = ifindex;
- addattr32(&req.n, sizeof(req.buf), IFLA_PROTINFO, state);
+ addattr8(&req.n, sizeof(req.buf), IFLA_PROTINFO, state);
return rtnl_talk(rth, &req.n, 0, 0, NULL, NULL, NULL);
}
diff --git a/packages/foss/rstp/include/libnetlink.h
b/packages/foss/rstp/include/libnetlink.h
index 63cc3c8..35d76f0 100644
--- a/packages/foss/rstp/include/libnetlink.h
+++ b/packages/foss/rstp/include/libnetlink.h
@@ -33,6 +33,7 @@ extern int rtnl_talk(struct rtnl_handle *rtnl, struct nlmsghdr
*n, pid_t peer,
extern int rtnl_send(struct rtnl_handle *rth, const char *buf, int);
+extern int addattr8(struct nlmsghdr *n, int maxlen, int type, __u8 data);
extern int addattr32(struct nlmsghdr *n, int maxlen, int type, __u32 data);
extern int addattr_l(struct nlmsghdr *n, int maxlen, int type, const void
*data, int alen);
extern int addraw_l(struct nlmsghdr *n, int maxlen, const void *data, int len);
diff --git a/packages/foss/rstp/libnetlink.c b/packages/foss/rstp/libnetlink.c
index 7752236..aaae102 100644
--- a/packages/foss/rstp/libnetlink.c
+++ b/packages/foss/rstp/libnetlink.c
@@ -508,6 +508,24 @@ int addattr32(struct nlmsghdr *n, int maxlen, int type,
__u32 data)
return 0;
}
+int addattr8(struct nlmsghdr *n, int maxlen, int type, __u8 data)
+{
+ int len = RTA_LENGTH(1);
+ struct rtattr *rta;
+ if (NLMSG_ALIGN(n->nlmsg_len) + len > maxlen) {
+ fprintf(stderr,
+ "addattr32: Error! max allowed bound %d exceeded\n",
+ maxlen);
+ return -1;
+ }
+ rta = NLMSG_TAIL(n);
+ rta->rta_type = type;
+ rta->rta_len = len;
+ memcpy(RTA_DATA(rta), &data, 1);
+ n->nlmsg_len = NLMSG_ALIGN(n->nlmsg_len) + len;
+ return 0;
+}
+
int addattr_l(struct nlmsghdr *n, int maxlen, int type, const void *data,
int alen)
{
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: not available
Url :
http://lists.linux-foundation.org/pipermail/bridge/attachments/20080702/aa092c5f/attachment.pgp
This looks like the earlier patch, for NETLINK size fixes. Could you resend the intended patch. Thanks. On Thu, Jul 3, 2008 at 4:39 AM, Solomon Peachy <solomon at linux-wlan.com> wrote:> The attached patch, against Shrinivas's May 7, 2008 snapshot, fixes a > null pointer dereference that occurs when we receive a packet from the > brige interface that bears the STP MACADDR, but is *not* a STP packet. > > Specifically, I was receiving GMRP packets (see 802.1D-2004 10.1) from a > 3Com switch. > > I don't know what we should do with these -- but crashing isn't it. I > can send over a packet dump and more debugging info if desired.Looks like we aren't validating the BPDU as we should be. Please send any debugging information you have.