Commit f2f17175 authored by David S. Miller's avatar David S. Miller

Merge branch 'bridge-stale-ptrs'

Nikolay Aleksandrov says:

====================
net: bridge: fix possible stale skb pointers

In the bridge driver we have a couple of places which call pskb_may_pull
but we've cached skb pointers before that and use them after which can
lead to out-of-bounds/stale pointer use. I've had these in my "to fix"
list for some time and now we got a report (patch 01) so here they are.
Patches 02-04 are fixes based on code inspection. Also patch 01 was
tested by Martin Weinelt, Martin if you don't mind please add your
tested-by tag to it by replying with Tested-by: name <email>.
I've also briefly tested the set by trying to exercise those code paths.
====================
Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
parents 13e04fbf 2446a68a
...@@ -74,7 +74,6 @@ int br_handle_frame_finish(struct net *net, struct sock *sk, struct sk_buff *skb ...@@ -74,7 +74,6 @@ int br_handle_frame_finish(struct net *net, struct sock *sk, struct sk_buff *skb
struct net_bridge_fdb_entry *dst = NULL; struct net_bridge_fdb_entry *dst = NULL;
struct net_bridge_mdb_entry *mdst; struct net_bridge_mdb_entry *mdst;
bool local_rcv, mcast_hit = false; bool local_rcv, mcast_hit = false;
const unsigned char *dest;
struct net_bridge *br; struct net_bridge *br;
u16 vid = 0; u16 vid = 0;
...@@ -92,10 +91,9 @@ int br_handle_frame_finish(struct net *net, struct sock *sk, struct sk_buff *skb ...@@ -92,10 +91,9 @@ int br_handle_frame_finish(struct net *net, struct sock *sk, struct sk_buff *skb
br_fdb_update(br, p, eth_hdr(skb)->h_source, vid, false); br_fdb_update(br, p, eth_hdr(skb)->h_source, vid, false);
local_rcv = !!(br->dev->flags & IFF_PROMISC); local_rcv = !!(br->dev->flags & IFF_PROMISC);
dest = eth_hdr(skb)->h_dest; if (is_multicast_ether_addr(eth_hdr(skb)->h_dest)) {
if (is_multicast_ether_addr(dest)) {
/* by definition the broadcast is also a multicast address */ /* by definition the broadcast is also a multicast address */
if (is_broadcast_ether_addr(dest)) { if (is_broadcast_ether_addr(eth_hdr(skb)->h_dest)) {
pkt_type = BR_PKT_BROADCAST; pkt_type = BR_PKT_BROADCAST;
local_rcv = true; local_rcv = true;
} else { } else {
...@@ -145,7 +143,7 @@ int br_handle_frame_finish(struct net *net, struct sock *sk, struct sk_buff *skb ...@@ -145,7 +143,7 @@ int br_handle_frame_finish(struct net *net, struct sock *sk, struct sk_buff *skb
} }
break; break;
case BR_PKT_UNICAST: case BR_PKT_UNICAST:
dst = br_fdb_find_rcu(br, dest, vid); dst = br_fdb_find_rcu(br, eth_hdr(skb)->h_dest, vid);
default: default:
break; break;
} }
......
...@@ -911,6 +911,7 @@ static int br_ip4_multicast_igmp3_report(struct net_bridge *br, ...@@ -911,6 +911,7 @@ static int br_ip4_multicast_igmp3_report(struct net_bridge *br,
int type; int type;
int err = 0; int err = 0;
__be32 group; __be32 group;
u16 nsrcs;
ih = igmpv3_report_hdr(skb); ih = igmpv3_report_hdr(skb);
num = ntohs(ih->ngrec); num = ntohs(ih->ngrec);
...@@ -924,8 +925,9 @@ static int br_ip4_multicast_igmp3_report(struct net_bridge *br, ...@@ -924,8 +925,9 @@ static int br_ip4_multicast_igmp3_report(struct net_bridge *br,
grec = (void *)(skb->data + len - sizeof(*grec)); grec = (void *)(skb->data + len - sizeof(*grec));
group = grec->grec_mca; group = grec->grec_mca;
type = grec->grec_type; type = grec->grec_type;
nsrcs = ntohs(grec->grec_nsrcs);
len += ntohs(grec->grec_nsrcs) * 4; len += nsrcs * 4;
if (!ip_mc_may_pull(skb, len)) if (!ip_mc_may_pull(skb, len))
return -EINVAL; return -EINVAL;
...@@ -946,7 +948,7 @@ static int br_ip4_multicast_igmp3_report(struct net_bridge *br, ...@@ -946,7 +948,7 @@ static int br_ip4_multicast_igmp3_report(struct net_bridge *br,
src = eth_hdr(skb)->h_source; src = eth_hdr(skb)->h_source;
if ((type == IGMPV3_CHANGE_TO_INCLUDE || if ((type == IGMPV3_CHANGE_TO_INCLUDE ||
type == IGMPV3_MODE_IS_INCLUDE) && type == IGMPV3_MODE_IS_INCLUDE) &&
ntohs(grec->grec_nsrcs) == 0) { nsrcs == 0) {
br_ip4_multicast_leave_group(br, port, group, vid, src); br_ip4_multicast_leave_group(br, port, group, vid, src);
} else { } else {
err = br_ip4_multicast_add_group(br, port, group, vid, err = br_ip4_multicast_add_group(br, port, group, vid,
...@@ -983,7 +985,8 @@ static int br_ip6_multicast_mld2_report(struct net_bridge *br, ...@@ -983,7 +985,8 @@ static int br_ip6_multicast_mld2_report(struct net_bridge *br,
len = skb_transport_offset(skb) + sizeof(*icmp6h); len = skb_transport_offset(skb) + sizeof(*icmp6h);
for (i = 0; i < num; i++) { for (i = 0; i < num; i++) {
__be16 *nsrcs, _nsrcs; __be16 *_nsrcs, __nsrcs;
u16 nsrcs;
nsrcs_offset = len + offsetof(struct mld2_grec, grec_nsrcs); nsrcs_offset = len + offsetof(struct mld2_grec, grec_nsrcs);
...@@ -991,12 +994,13 @@ static int br_ip6_multicast_mld2_report(struct net_bridge *br, ...@@ -991,12 +994,13 @@ static int br_ip6_multicast_mld2_report(struct net_bridge *br,
nsrcs_offset + sizeof(_nsrcs)) nsrcs_offset + sizeof(_nsrcs))
return -EINVAL; return -EINVAL;
nsrcs = skb_header_pointer(skb, nsrcs_offset, _nsrcs = skb_header_pointer(skb, nsrcs_offset,
sizeof(_nsrcs), &_nsrcs); sizeof(__nsrcs), &__nsrcs);
if (!nsrcs) if (!_nsrcs)
return -EINVAL; return -EINVAL;
grec_len = struct_size(grec, grec_src, ntohs(*nsrcs)); nsrcs = ntohs(*_nsrcs);
grec_len = struct_size(grec, grec_src, nsrcs);
if (!ipv6_mc_may_pull(skb, len + grec_len)) if (!ipv6_mc_may_pull(skb, len + grec_len))
return -EINVAL; return -EINVAL;
...@@ -1021,7 +1025,7 @@ static int br_ip6_multicast_mld2_report(struct net_bridge *br, ...@@ -1021,7 +1025,7 @@ static int br_ip6_multicast_mld2_report(struct net_bridge *br,
src = eth_hdr(skb)->h_source; src = eth_hdr(skb)->h_source;
if ((grec->grec_type == MLD2_CHANGE_TO_INCLUDE || if ((grec->grec_type == MLD2_CHANGE_TO_INCLUDE ||
grec->grec_type == MLD2_MODE_IS_INCLUDE) && grec->grec_type == MLD2_MODE_IS_INCLUDE) &&
ntohs(*nsrcs) == 0) { nsrcs == 0) {
br_ip6_multicast_leave_group(br, port, &grec->grec_mca, br_ip6_multicast_leave_group(br, port, &grec->grec_mca,
vid, src); vid, src);
} else { } else {
...@@ -1275,7 +1279,6 @@ static int br_ip6_multicast_query(struct net_bridge *br, ...@@ -1275,7 +1279,6 @@ static int br_ip6_multicast_query(struct net_bridge *br,
u16 vid) u16 vid)
{ {
unsigned int transport_len = ipv6_transport_len(skb); unsigned int transport_len = ipv6_transport_len(skb);
const struct ipv6hdr *ip6h = ipv6_hdr(skb);
struct mld_msg *mld; struct mld_msg *mld;
struct net_bridge_mdb_entry *mp; struct net_bridge_mdb_entry *mp;
struct mld2_query *mld2q; struct mld2_query *mld2q;
...@@ -1319,7 +1322,7 @@ static int br_ip6_multicast_query(struct net_bridge *br, ...@@ -1319,7 +1322,7 @@ static int br_ip6_multicast_query(struct net_bridge *br,
if (is_general_query) { if (is_general_query) {
saddr.proto = htons(ETH_P_IPV6); saddr.proto = htons(ETH_P_IPV6);
saddr.u.ip6 = ip6h->saddr; saddr.u.ip6 = ipv6_hdr(skb)->saddr;
br_multicast_query_received(br, port, &br->ip6_other_query, br_multicast_query_received(br, port, &br->ip6_other_query,
&saddr, max_delay); &saddr, max_delay);
......
...@@ -143,7 +143,6 @@ void br_send_tcn_bpdu(struct net_bridge_port *p) ...@@ -143,7 +143,6 @@ void br_send_tcn_bpdu(struct net_bridge_port *p)
void br_stp_rcv(const struct stp_proto *proto, struct sk_buff *skb, void br_stp_rcv(const struct stp_proto *proto, struct sk_buff *skb,
struct net_device *dev) struct net_device *dev)
{ {
const unsigned char *dest = eth_hdr(skb)->h_dest;
struct net_bridge_port *p; struct net_bridge_port *p;
struct net_bridge *br; struct net_bridge *br;
const unsigned char *buf; const unsigned char *buf;
...@@ -172,7 +171,7 @@ void br_stp_rcv(const struct stp_proto *proto, struct sk_buff *skb, ...@@ -172,7 +171,7 @@ void br_stp_rcv(const struct stp_proto *proto, struct sk_buff *skb,
if (p->state == BR_STATE_DISABLED) if (p->state == BR_STATE_DISABLED)
goto out; goto out;
if (!ether_addr_equal(dest, br->group_addr)) if (!ether_addr_equal(eth_hdr(skb)->h_dest, br->group_addr))
goto out; goto out;
if (p->flags & BR_BPDU_GUARD) { if (p->flags & BR_BPDU_GUARD) {
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment