rstpd crashes with GARP/GMRP packets
PATCH 1/1 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
Signature · 4 lines
Solomon Peachy [email protected] AbsoluteValue Systems http://www.linux-wlan.com 721-D North Drive +1 (321) 259-0737 (office) Melbourne, FL 32934 +1 (321) 259-0286 (fax)
packages/foss/rstp/brmon.c +1 -1
@@ -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 ); }
packages/foss/rstp/brstate.c +1 -1
@@ -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 ); }
packages/foss/rstp/include/libnetlink.h +1 -0
@@ -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);
packages/foss/rstp/libnetlink.c +18 -0
@@ -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) {
This looks like the earlier patch, for NETLINK size fixes. Could you resend the intended patch. Thanks.
Quoted 9 lines · Solomon Peachy
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.
Whoops, the correct patch is attached.
Quoted 16 lines · Srinivas M.A.
This looks like the earlier patch, for NETLINK size fixes. Could you resend the intended patch. Thanks.
Quoted 9 lines · Solomon Peachy
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.
Signature · 4 lines
Solomon Peachy [email protected] AbsoluteValue Systems http://www.linux-wlan.com 721-D North Drive +1 (321) 259-0737 (office) Melbourne, FL 32934 +1 (321) 259-0286 (fax)
packages/foss/rstp/bridge_track.c +6 -0
@@ -448,6 +448,12 @@ void bridge_bpdu_rcv(int if_index, const unsigned char *data, int len)return ; TST(ifc->up,); + +/* Note, we can receive packets via the bridge interface +that are not STP, eg GMRP */ +if (!ifc->master) +return ; +if (!ifc->master->stp_up)return ;
Thanks. I put in a modified patch, checking ifc->is_bridge, so that the bridge and port specific fields of struct ifdata can be put into a variant record at some point.
commit 8172012df1d981ef1934ede430c4936eae50e5f7
Author: Srinivas Aji <[email protected]> Date: Thu Jul 3 22:45:03 2008 +0530
Fix crash when BPDU received on bridge if.
We could receive non STP BPDU's on bridge interface, or even STP BPDUs if STP is off for the bridge. The crash is avoided by checking for bridge interface and not processing further in that case.
Patch based on one from Solomon Peachy <[email protected]>
bridge_track.c +6 -0
@@ -447,6 +447,12 @@ void bridge_bpdu_rcv(int if_index, const unsigned char *datif (!ifc)return ; +/* We could get a STP BPDU from a bridge interface if STP is off, +or a non STP packet as well, e.g. GMRP, even when STP is on. +Don't process it, else we will crash at ifc->master->stp_up */ +if (ifc->is_bridge) +return ; + TST(ifc->up,);if (!ifc->master->stp_up)return ;