From ba8379b220509e9448c00a77cf6c15ac2a559cc7 Mon Sep 17 00:00:00 2001 From: Chris Wright Date: Mon, 20 Nov 2006 15:02:49 -0800 Subject: [PATCH] bridge: fix possible overflow in get_fdb_entries Make sure to properly clamp maxnum to avoid overflow Signed-off-by: Chris Wright Acked-by: Eugene Teo Acked-by: Marcel Holtmann Signed-off-by: Linus Torvalds --- net/bridge/br_ioctl.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) (limited to 'net/bridge') diff --git a/net/bridge/br_ioctl.c b/net/bridge/br_ioctl.c index 4e4119a1213..4c61a7e0a86 100644 --- a/net/bridge/br_ioctl.c +++ b/net/bridge/br_ioctl.c @@ -58,12 +58,13 @@ static int get_fdb_entries(struct net_bridge *br, void __user *userbuf, { int num; void *buf; - size_t size = maxnum * sizeof(struct __fdb_entry); + size_t size; - if (size > PAGE_SIZE) { - size = PAGE_SIZE; + /* Clamp size to PAGE_SIZE, test maxnum to avoid overflow */ + if (maxnum > PAGE_SIZE/sizeof(struct __fdb_entry)) maxnum = PAGE_SIZE/sizeof(struct __fdb_entry); - } + + size = maxnum * sizeof(struct __fdb_entry); buf = kmalloc(size, GFP_USER); if (!buf) -- cgit v1.2.3 From 82e91ffef60e6eba9848fe149ce1eecd2b5aef12 Mon Sep 17 00:00:00 2001 From: Thomas Graf Date: Thu, 9 Nov 2006 15:19:14 -0800 Subject: [NET]: Turn nfmark into generic mark nfmark is being used in various subsystems and has become the defacto mark field for all kinds of packets. Therefore it makes sense to rename it to `mark' and remove the dependency on CONFIG_NETFILTER. Signed-off-by: Thomas Graf Signed-off-by: David S. Miller --- net/bridge/netfilter/ebt_mark.c | 8 ++++---- net/bridge/netfilter/ebt_mark_m.c | 4 ++-- net/bridge/netfilter/ebt_ulog.c | 2 +- 3 files changed, 7 insertions(+), 7 deletions(-) (limited to 'net/bridge') diff --git a/net/bridge/netfilter/ebt_mark.c b/net/bridge/netfilter/ebt_mark.c index b54306a934e..2458638561c 100644 --- a/net/bridge/netfilter/ebt_mark.c +++ b/net/bridge/netfilter/ebt_mark.c @@ -25,13 +25,13 @@ static int ebt_target_mark(struct sk_buff **pskb, unsigned int hooknr, int action = info->target & -16; if (action == MARK_SET_VALUE) - (*pskb)->nfmark = info->mark; + (*pskb)->mark = info->mark; else if (action == MARK_OR_VALUE) - (*pskb)->nfmark |= info->mark; + (*pskb)->mark |= info->mark; else if (action == MARK_AND_VALUE) - (*pskb)->nfmark &= info->mark; + (*pskb)->mark &= info->mark; else - (*pskb)->nfmark ^= info->mark; + (*pskb)->mark ^= info->mark; return info->target | -16; } diff --git a/net/bridge/netfilter/ebt_mark_m.c b/net/bridge/netfilter/ebt_mark_m.c index a6413e4b498..025869ee0b6 100644 --- a/net/bridge/netfilter/ebt_mark_m.c +++ b/net/bridge/netfilter/ebt_mark_m.c @@ -19,8 +19,8 @@ static int ebt_filter_mark(const struct sk_buff *skb, struct ebt_mark_m_info *info = (struct ebt_mark_m_info *) data; if (info->bitmask & EBT_MARK_OR) - return !(!!(skb->nfmark & info->mask) ^ info->invert); - return !(((skb->nfmark & info->mask) == info->mark) ^ info->invert); + return !(!!(skb->mark & info->mask) ^ info->invert); + return !(((skb->mark & info->mask) == info->mark) ^ info->invert); } static int ebt_mark_check(const char *tablename, unsigned int hookmask, diff --git a/net/bridge/netfilter/ebt_ulog.c b/net/bridge/netfilter/ebt_ulog.c index 9f950db3b76..c1af68b5a29 100644 --- a/net/bridge/netfilter/ebt_ulog.c +++ b/net/bridge/netfilter/ebt_ulog.c @@ -168,7 +168,7 @@ static void ebt_ulog_packet(unsigned int hooknr, const struct sk_buff *skb, if (ub->qlen == 1) skb_set_timestamp(ub->skb, &pm->stamp); pm->data_len = copy_len; - pm->mark = skb->nfmark; + pm->mark = skb->mark; pm->hook = hooknr; if (uloginfo->prefix != NULL) strcpy(pm->prefix, uloginfo->prefix); -- cgit v1.2.3 From 339bf98ffc6a8d8eb16fc532ac57ffbced2f8a68 Mon Sep 17 00:00:00 2001 From: Thomas Graf Date: Fri, 10 Nov 2006 14:10:15 -0800 Subject: [NETLINK]: Do precise netlink message allocations where possible Account for the netlink message header size directly in nlmsg_new() instead of relying on the caller calculate it correctly. Replaces error handling of message construction functions when constructing notifications with bug traps since a failure implies a bug in calculating the size of the skb. Signed-off-by: Thomas Graf Acked-by: Paul Moore Signed-off-by: David S. Miller --- net/bridge/br_netlink.c | 21 +++++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) (limited to 'net/bridge') diff --git a/net/bridge/br_netlink.c b/net/bridge/br_netlink.c index 8f661195d09..15d6efbe751 100644 --- a/net/bridge/br_netlink.c +++ b/net/bridge/br_netlink.c @@ -15,6 +15,18 @@ #include #include "br_private.h" +static inline size_t br_nlmsg_size(void) +{ + return NLMSG_ALIGN(sizeof(struct ifinfomsg)) + + nla_total_size(IFNAMSIZ) /* IFLA_IFNAME */ + + nla_total_size(MAX_ADDR_LEN) /* IFLA_ADDRESS */ + + nla_total_size(4) /* IFLA_MASTER */ + + nla_total_size(4) /* IFLA_MTU */ + + nla_total_size(4) /* IFLA_LINK */ + + nla_total_size(1) /* IFLA_OPERSTATE */ + + nla_total_size(1); /* IFLA_PROTINFO */ +} + /* * Create one netlink message for one interface * Contains port and master info as well as carrier and bridge state. @@ -77,19 +89,16 @@ rtattr_failure: void br_ifinfo_notify(int event, struct net_bridge_port *port) { struct sk_buff *skb; - int payload = sizeof(struct ifinfomsg) + 128; int err = -ENOBUFS; pr_debug("bridge notify event=%d\n", event); - skb = nlmsg_new(nlmsg_total_size(payload), GFP_ATOMIC); + skb = nlmsg_new(br_nlmsg_size(), GFP_ATOMIC); if (skb == NULL) goto errout; err = br_fill_ifinfo(skb, port, 0, 0, event, 0); - if (err < 0) { - kfree_skb(skb); - goto errout; - } + /* failure implies BUG in br_nlmsg_size() */ + BUG_ON(err < 0); err = rtnl_notify(skb, 0, RTNLGRP_LINK, NULL, GFP_ATOMIC); errout: -- cgit v1.2.3 From 47c183fa5ea7feebc356da8ccbd9105a41f8e534 Mon Sep 17 00:00:00 2001 From: Al Viro Date: Tue, 14 Nov 2006 21:11:51 -0800 Subject: [BRIDGE]: Annotations. Signed-off-by: Al Viro Signed-off-by: David S. Miller --- net/bridge/br_netfilter.c | 2 +- net/bridge/netfilter/ebt_802_3.c | 2 +- net/bridge/netfilter/ebt_among.c | 22 +++++++++++----------- net/bridge/netfilter/ebt_arp.c | 6 +++--- net/bridge/netfilter/ebt_ip.c | 4 ++-- net/bridge/netfilter/ebt_log.c | 6 +++--- net/bridge/netfilter/ebt_vlan.c | 2 +- 7 files changed, 22 insertions(+), 22 deletions(-) (limited to 'net/bridge') diff --git a/net/bridge/br_netfilter.c b/net/bridge/br_netfilter.c index ac181be13d8..2a5d31b1a19 100644 --- a/net/bridge/br_netfilter.c +++ b/net/bridge/br_netfilter.c @@ -381,7 +381,7 @@ static int check_hbh_len(struct sk_buff *skb) case IPV6_TLV_JUMBO: if (skb->nh.raw[off + 1] != 4 || (off & 3) != 2) goto bad; - pkt_len = ntohl(*(u32 *) (skb->nh.raw + off + 2)); + pkt_len = ntohl(*(__be32 *) (skb->nh.raw + off + 2)); if (pkt_len <= IPV6_MAXPLEN || skb->nh.ipv6h->payload_len) goto bad; diff --git a/net/bridge/netfilter/ebt_802_3.c b/net/bridge/netfilter/ebt_802_3.c index d42f63f5e9f..9abbc09ccdc 100644 --- a/net/bridge/netfilter/ebt_802_3.c +++ b/net/bridge/netfilter/ebt_802_3.c @@ -17,7 +17,7 @@ static int ebt_filter_802_3(const struct sk_buff *skb, const struct net_device * { struct ebt_802_3_info *info = (struct ebt_802_3_info *)data; struct ebt_802_3_hdr *hdr = ebt_802_3_hdr(skb); - uint16_t type = hdr->llc.ui.ctrl & IS_UI ? hdr->llc.ui.type : hdr->llc.ni.type; + __be16 type = hdr->llc.ui.ctrl & IS_UI ? hdr->llc.ui.type : hdr->llc.ni.type; if (info->bitmask & EBT_802_3_SAP) { if (FWINV(info->sap != hdr->llc.ui.ssap, EBT_802_3_SAP)) diff --git a/net/bridge/netfilter/ebt_among.c b/net/bridge/netfilter/ebt_among.c index a614485828a..ce97c4285f9 100644 --- a/net/bridge/netfilter/ebt_among.c +++ b/net/bridge/netfilter/ebt_among.c @@ -15,7 +15,7 @@ #include static int ebt_mac_wormhash_contains(const struct ebt_mac_wormhash *wh, - const char *mac, uint32_t ip) + const char *mac, __be32 ip) { /* You may be puzzled as to how this code works. * Some tricks were used, refer to @@ -70,7 +70,7 @@ static int ebt_mac_wormhash_check_integrity(const struct ebt_mac_wormhash return 0; } -static int get_ip_dst(const struct sk_buff *skb, uint32_t *addr) +static int get_ip_dst(const struct sk_buff *skb, __be32 *addr) { if (eth_hdr(skb)->h_proto == htons(ETH_P_IP)) { struct iphdr _iph, *ih; @@ -81,16 +81,16 @@ static int get_ip_dst(const struct sk_buff *skb, uint32_t *addr) *addr = ih->daddr; } else if (eth_hdr(skb)->h_proto == htons(ETH_P_ARP)) { struct arphdr _arph, *ah; - uint32_t buf, *bp; + __be32 buf, *bp; ah = skb_header_pointer(skb, 0, sizeof(_arph), &_arph); if (ah == NULL || - ah->ar_pln != sizeof(uint32_t) || + ah->ar_pln != sizeof(__be32) || ah->ar_hln != ETH_ALEN) return -1; bp = skb_header_pointer(skb, sizeof(struct arphdr) + - 2 * ETH_ALEN + sizeof(uint32_t), - sizeof(uint32_t), &buf); + 2 * ETH_ALEN + sizeof(__be32), + sizeof(__be32), &buf); if (bp == NULL) return -1; *addr = *bp; @@ -98,7 +98,7 @@ static int get_ip_dst(const struct sk_buff *skb, uint32_t *addr) return 0; } -static int get_ip_src(const struct sk_buff *skb, uint32_t *addr) +static int get_ip_src(const struct sk_buff *skb, __be32 *addr) { if (eth_hdr(skb)->h_proto == htons(ETH_P_IP)) { struct iphdr _iph, *ih; @@ -109,15 +109,15 @@ static int get_ip_src(const struct sk_buff *skb, uint32_t *addr) *addr = ih->saddr; } else if (eth_hdr(skb)->h_proto == htons(ETH_P_ARP)) { struct arphdr _arph, *ah; - uint32_t buf, *bp; + __be32 buf, *bp; ah = skb_header_pointer(skb, 0, sizeof(_arph), &_arph); if (ah == NULL || - ah->ar_pln != sizeof(uint32_t) || + ah->ar_pln != sizeof(__be32) || ah->ar_hln != ETH_ALEN) return -1; bp = skb_header_pointer(skb, sizeof(struct arphdr) + - ETH_ALEN, sizeof(uint32_t), &buf); + ETH_ALEN, sizeof(__be32), &buf); if (bp == NULL) return -1; *addr = *bp; @@ -133,7 +133,7 @@ static int ebt_filter_among(const struct sk_buff *skb, struct ebt_among_info *info = (struct ebt_among_info *) data; const char *dmac, *smac; const struct ebt_mac_wormhash *wh_dst, *wh_src; - uint32_t dip = 0, sip = 0; + __be32 dip = 0, sip = 0; wh_dst = ebt_among_wh_dst(info); wh_src = ebt_among_wh_src(info); diff --git a/net/bridge/netfilter/ebt_arp.c b/net/bridge/netfilter/ebt_arp.c index a6c81d9f73b..9c599800a90 100644 --- a/net/bridge/netfilter/ebt_arp.c +++ b/net/bridge/netfilter/ebt_arp.c @@ -35,10 +35,10 @@ static int ebt_filter_arp(const struct sk_buff *skb, const struct net_device *in return EBT_NOMATCH; if (info->bitmask & (EBT_ARP_SRC_IP | EBT_ARP_DST_IP)) { - uint32_t _addr, *ap; + __be32 _addr, *ap; /* IPv4 addresses are always 4 bytes */ - if (ah->ar_pln != sizeof(uint32_t)) + if (ah->ar_pln != sizeof(__be32)) return EBT_NOMATCH; if (info->bitmask & EBT_ARP_SRC_IP) { ap = skb_header_pointer(skb, sizeof(struct arphdr) + @@ -53,7 +53,7 @@ static int ebt_filter_arp(const struct sk_buff *skb, const struct net_device *in if (info->bitmask & EBT_ARP_DST_IP) { ap = skb_header_pointer(skb, sizeof(struct arphdr) + - 2*ah->ar_hln+sizeof(uint32_t), + 2*ah->ar_hln+sizeof(__be32), sizeof(_addr), &_addr); if (ap == NULL) return EBT_NOMATCH; diff --git a/net/bridge/netfilter/ebt_ip.c b/net/bridge/netfilter/ebt_ip.c index 65b665ce57b..e4c642448e1 100644 --- a/net/bridge/netfilter/ebt_ip.c +++ b/net/bridge/netfilter/ebt_ip.c @@ -20,8 +20,8 @@ #include struct tcpudphdr { - uint16_t src; - uint16_t dst; + __be16 src; + __be16 dst; }; static int ebt_filter_ip(const struct sk_buff *skb, const struct net_device *in, diff --git a/net/bridge/netfilter/ebt_log.c b/net/bridge/netfilter/ebt_log.c index 466ed3440b7..a184f879f25 100644 --- a/net/bridge/netfilter/ebt_log.c +++ b/net/bridge/netfilter/ebt_log.c @@ -38,8 +38,8 @@ static int ebt_log_check(const char *tablename, unsigned int hookmask, struct tcpudphdr { - uint16_t src; - uint16_t dst; + __be16 src; + __be16 dst; }; struct arppayload @@ -130,7 +130,7 @@ ebt_log_packet(unsigned int pf, unsigned int hooknum, * then log the ARP payload */ if (ah->ar_hrd == htons(1) && ah->ar_hln == ETH_ALEN && - ah->ar_pln == sizeof(uint32_t)) { + ah->ar_pln == sizeof(__be32)) { struct arppayload _arpp, *ap; ap = skb_header_pointer(skb, sizeof(_arph), diff --git a/net/bridge/netfilter/ebt_vlan.c b/net/bridge/netfilter/ebt_vlan.c index a2b452862b7..7ee37762296 100644 --- a/net/bridge/netfilter/ebt_vlan.c +++ b/net/bridge/netfilter/ebt_vlan.c @@ -55,7 +55,7 @@ ebt_filter_vlan(const struct sk_buff *skb, unsigned short id; /* VLAN ID, given from frame TCI */ unsigned char prio; /* user_priority, given from frame TCI */ /* VLAN encapsulated Type/Length field, given from orig frame */ - unsigned short encap; + __be16 encap; fp = skb_header_pointer(skb, 0, sizeof(_frame), &_frame); if (fp == NULL) -- cgit v1.2.3 From 3277c39f8d706afb6fefc02f49563a73bbd405b9 Mon Sep 17 00:00:00 2001 From: Al Viro Date: Tue, 14 Nov 2006 21:13:53 -0800 Subject: [NET]: Kill direct includes of asm/checksum.h Signed-off-by: Al Viro Signed-off-by: David S. Miller --- net/bridge/br_netfilter.c | 1 - 1 file changed, 1 deletion(-) (limited to 'net/bridge') diff --git a/net/bridge/br_netfilter.c b/net/bridge/br_netfilter.c index 2a5d31b1a19..ac47ba2ba02 100644 --- a/net/bridge/br_netfilter.c +++ b/net/bridge/br_netfilter.c @@ -40,7 +40,6 @@ #include #include -#include #include "br_private.h" #ifdef CONFIG_SYSCTL #include -- cgit v1.2.3 From 746859625d879688adb99f1e5e8108fea876d369 Mon Sep 17 00:00:00 2001 From: Thomas Graf Date: Mon, 20 Nov 2006 16:20:22 -0800 Subject: [BRIDGE] netlink: Convert bridge netlink code to new netlink interface Removes dependency on buggy rta_buf, fixes a memory corruption bug due to a unvalidated netlink attribute, and simplifies the code. Signed-off-by: Thomas Graf Signed-off-by: David S. Miller --- net/bridge/br_netlink.c | 92 ++++++++++++++++++++----------------------------- 1 file changed, 38 insertions(+), 54 deletions(-) (limited to 'net/bridge') diff --git a/net/bridge/br_netlink.c b/net/bridge/br_netlink.c index 15d6efbe751..a9139682c49 100644 --- a/net/bridge/br_netlink.c +++ b/net/bridge/br_netlink.c @@ -36,51 +36,43 @@ static int br_fill_ifinfo(struct sk_buff *skb, const struct net_bridge_port *por { const struct net_bridge *br = port->br; const struct net_device *dev = port->dev; - struct ifinfomsg *r; + struct ifinfomsg *hdr; struct nlmsghdr *nlh; - unsigned char *b = skb->tail; - u32 mtu = dev->mtu; u8 operstate = netif_running(dev) ? dev->operstate : IF_OPER_DOWN; - u8 portstate = port->state; pr_debug("br_fill_info event %d port %s master %s\n", event, dev->name, br->dev->name); - nlh = NLMSG_NEW(skb, pid, seq, event, sizeof(*r), flags); - r = NLMSG_DATA(nlh); - r->ifi_family = AF_BRIDGE; - r->__ifi_pad = 0; - r->ifi_type = dev->type; - r->ifi_index = dev->ifindex; - r->ifi_flags = dev_get_flags(dev); - r->ifi_change = 0; + nlh = nlmsg_put(skb, pid, seq, event, sizeof(*hdr), flags); + if (nlh == NULL) + return -ENOBUFS; - RTA_PUT(skb, IFLA_IFNAME, strlen(dev->name)+1, dev->name); + hdr = nlmsg_data(nlh); + hdr->ifi_family = AF_BRIDGE; + hdr->__ifi_pad = 0; + hdr->ifi_type = dev->type; + hdr->ifi_index = dev->ifindex; + hdr->ifi_flags = dev_get_flags(dev); + hdr->ifi_change = 0; - RTA_PUT(skb, IFLA_MASTER, sizeof(int), &br->dev->ifindex); + NLA_PUT_STRING(skb, IFLA_IFNAME, dev->name); + NLA_PUT_U32(skb, IFLA_MASTER, br->dev->ifindex); + NLA_PUT_U32(skb, IFLA_MTU, dev->mtu); + NLA_PUT_U8(skb, IFLA_OPERSTATE, operstate); if (dev->addr_len) - RTA_PUT(skb, IFLA_ADDRESS, dev->addr_len, dev->dev_addr); + NLA_PUT(skb, IFLA_ADDRESS, dev->addr_len, dev->dev_addr); - RTA_PUT(skb, IFLA_MTU, sizeof(mtu), &mtu); if (dev->ifindex != dev->iflink) - RTA_PUT(skb, IFLA_LINK, sizeof(int), &dev->iflink); - - - RTA_PUT(skb, IFLA_OPERSTATE, sizeof(operstate), &operstate); + NLA_PUT_U32(skb, IFLA_LINK, dev->iflink); if (event == RTM_NEWLINK) - RTA_PUT(skb, IFLA_PROTINFO, sizeof(portstate), &portstate); - - nlh->nlmsg_len = skb->tail - b; + NLA_PUT_U8(skb, IFLA_PROTINFO, port->state); - return skb->len; - -nlmsg_failure: -rtattr_failure: + return nlmsg_end(skb, nlh); - skb_trim(skb, b - skb->data); - return -EINVAL; +nla_put_failure: + return nlmsg_cancel(skb, nlh); } /* @@ -113,25 +105,18 @@ static int br_dump_ifinfo(struct sk_buff *skb, struct netlink_callback *cb) { struct net_device *dev; int idx; - int s_idx = cb->args[0]; - int err = 0; read_lock(&dev_base_lock); for (dev = dev_base, idx = 0; dev; dev = dev->next) { - struct net_bridge_port *p = dev->br_port; - /* not a bridge port */ - if (!p) - continue; + if (dev->br_port == NULL || idx < cb->args[0]) + goto skip; - if (idx < s_idx) - goto cont; - - err = br_fill_ifinfo(skb, p, NETLINK_CB(cb->skb).pid, - cb->nlh->nlmsg_seq, RTM_NEWLINK, NLM_F_MULTI); - if (err <= 0) + if (br_fill_ifinfo(skb, dev->br_port, NETLINK_CB(cb->skb).pid, + cb->nlh->nlmsg_seq, RTM_NEWLINK, + NLM_F_MULTI) < 0) break; -cont: +skip: ++idx; } read_unlock(&dev_base_lock); @@ -147,26 +132,27 @@ cont: */ static int br_rtm_setlink(struct sk_buff *skb, struct nlmsghdr *nlh, void *arg) { - struct rtattr **rta = arg; - struct ifinfomsg *ifm = NLMSG_DATA(nlh); + struct ifinfomsg *ifm; + struct nlattr *protinfo; struct net_device *dev; struct net_bridge_port *p; u8 new_state; + if (nlmsg_len(nlh) < sizeof(*ifm)) + return -EINVAL; + + ifm = nlmsg_data(nlh); if (ifm->ifi_family != AF_BRIDGE) return -EPFNOSUPPORT; - /* Must pass valid state as PROTINFO */ - if (rta[IFLA_PROTINFO-1]) { - u8 *pstate = RTA_DATA(rta[IFLA_PROTINFO-1]); - new_state = *pstate; - } else + protinfo = nlmsg_find_attr(nlh, sizeof(*ifm), IFLA_PROTINFO); + if (!protinfo || nla_len(protinfo) < sizeof(u8)) return -EINVAL; + new_state = nla_get_u8(protinfo); if (new_state > BR_STATE_BLOCKING) return -EINVAL; - /* Find bridge port */ dev = __dev_get_by_index(ifm->ifi_index); if (!dev) return -ENODEV; @@ -179,10 +165,8 @@ static int br_rtm_setlink(struct sk_buff *skb, struct nlmsghdr *nlh, void *arg) if (p->br->stp_enabled) return -EBUSY; - if (!netif_running(dev)) - return -ENETDOWN; - - if (!netif_carrier_ok(dev) && new_state != BR_STATE_DISABLED) + if (!netif_running(dev) || + (!netif_carrier_ok(dev) && new_state != BR_STATE_DISABLED)) return -ENETDOWN; p->state = new_state; -- cgit v1.2.3 From d12cdc3ccf140bd2febef1c1be92284571da983f Mon Sep 17 00:00:00 2001 From: Bart De Schuymer Date: Wed, 29 Nov 2006 02:35:40 +0100 Subject: [NETFILTER]: ebtables: add --snap-arp option The attached patch adds --snat-arp support, which makes it possible to change the source mac address in both the mac header and the arp header with one rule. Signed-off-by: Bart De Schuymer Signed-off-by: Patrick McHardy --- net/bridge/netfilter/ebt_mark.c | 6 +++--- net/bridge/netfilter/ebt_snat.c | 27 ++++++++++++++++++++++++--- 2 files changed, 27 insertions(+), 6 deletions(-) (limited to 'net/bridge') diff --git a/net/bridge/netfilter/ebt_mark.c b/net/bridge/netfilter/ebt_mark.c index 2458638561c..62d23c7b25e 100644 --- a/net/bridge/netfilter/ebt_mark.c +++ b/net/bridge/netfilter/ebt_mark.c @@ -33,7 +33,7 @@ static int ebt_target_mark(struct sk_buff **pskb, unsigned int hooknr, else (*pskb)->mark ^= info->mark; - return info->target | -16; + return info->target | ~EBT_VERDICT_BITS; } static int ebt_target_mark_check(const char *tablename, unsigned int hookmask, @@ -44,13 +44,13 @@ static int ebt_target_mark_check(const char *tablename, unsigned int hookmask, if (datalen != EBT_ALIGN(sizeof(struct ebt_mark_t_info))) return -EINVAL; - tmp = info->target | -16; + tmp = info->target | ~EBT_VERDICT_BITS; if (BASE_CHAIN && tmp == EBT_RETURN) return -EINVAL; CLEAR_BASE_CHAIN_BIT; if (tmp < -NUM_STANDARD_TARGETS || tmp >= 0) return -EINVAL; - tmp = info->target & -16; + tmp = info->target & ~EBT_VERDICT_BITS; if (tmp != MARK_SET_VALUE && tmp != MARK_OR_VALUE && tmp != MARK_AND_VALUE && tmp != MARK_XOR_VALUE) return -EINVAL; diff --git a/net/bridge/netfilter/ebt_snat.c b/net/bridge/netfilter/ebt_snat.c index cbb33e24ca8..a50722182bf 100644 --- a/net/bridge/netfilter/ebt_snat.c +++ b/net/bridge/netfilter/ebt_snat.c @@ -12,6 +12,8 @@ #include #include #include +#include +#include static int ebt_target_snat(struct sk_buff **pskb, unsigned int hooknr, const struct net_device *in, const struct net_device *out, @@ -31,24 +33,43 @@ static int ebt_target_snat(struct sk_buff **pskb, unsigned int hooknr, *pskb = nskb; } memcpy(eth_hdr(*pskb)->h_source, info->mac, ETH_ALEN); - return info->target; + if (!(info->target & NAT_ARP_BIT) && + eth_hdr(*pskb)->h_proto == htons(ETH_P_ARP)) { + struct arphdr _ah, *ap; + + ap = skb_header_pointer(*pskb, 0, sizeof(_ah), &_ah); + if (ap == NULL) + return EBT_DROP; + if (ap->ar_hln != ETH_ALEN) + goto out; + if (skb_store_bits(*pskb, sizeof(_ah), info->mac,ETH_ALEN)) + return EBT_DROP; + } +out: + return info->target | ~EBT_VERDICT_BITS; } static int ebt_target_snat_check(const char *tablename, unsigned int hookmask, const struct ebt_entry *e, void *data, unsigned int datalen) { struct ebt_nat_info *info = (struct ebt_nat_info *) data; + int tmp; if (datalen != EBT_ALIGN(sizeof(struct ebt_nat_info))) return -EINVAL; - if (BASE_CHAIN && info->target == EBT_RETURN) + tmp = info->target | ~EBT_VERDICT_BITS; + if (BASE_CHAIN && tmp == EBT_RETURN) return -EINVAL; CLEAR_BASE_CHAIN_BIT; if (strcmp(tablename, "nat")) return -EINVAL; if (hookmask & ~(1 << NF_BR_POST_ROUTING)) return -EINVAL; - if (INVALID_TARGET) + + if (tmp < -NUM_STANDARD_TARGETS || tmp >= 0) + return -EINVAL; + tmp = info->target | EBT_VERDICT_BITS; + if ((tmp & ~NAT_ARP_BIT) != ~NAT_ARP_BIT) return -EINVAL; return 0; } -- cgit v1.2.3 From bb2ef25c2c62444b8fdb0346a23658a419803df9 Mon Sep 17 00:00:00 2001 From: Al Viro Date: Thu, 30 Nov 2006 19:22:42 -0800 Subject: [EBTABLES]: Fix wraparounds in ebt_entries verification. We need to verify that a) we are not too close to the end of buffer to dereference b) next entry we'll be checking won't be _before_ our While we are at it, don't subtract unrelated pointers... Signed-off-by: Al Viro Signed-off-by: David S. Miller --- net/bridge/netfilter/ebtables.c | 23 ++++++++++++++++------- 1 file changed, 16 insertions(+), 7 deletions(-) (limited to 'net/bridge') diff --git a/net/bridge/netfilter/ebtables.c b/net/bridge/netfilter/ebtables.c index 9f85666f29f..0dcebf20d6c 100644 --- a/net/bridge/netfilter/ebtables.c +++ b/net/bridge/netfilter/ebtables.c @@ -401,13 +401,17 @@ ebt_check_entry_size_and_hooks(struct ebt_entry *e, struct ebt_entries **hook_entries, unsigned int *n, unsigned int *cnt, unsigned int *totalcnt, unsigned int *udc_cnt, unsigned int valid_hooks) { + unsigned int offset = (char *)e - newinfo->entries; + size_t left = (limit - base) - offset; int i; + if (left < sizeof(unsigned int)) + goto Esmall; + for (i = 0; i < NF_BR_NUMHOOKS; i++) { if ((valid_hooks & (1 << i)) == 0) continue; - if ( (char *)hook_entries[i] - base == - (char *)e - newinfo->entries) + if ((char *)hook_entries[i] == base + offset) break; } /* beginning of a new chain @@ -428,11 +432,8 @@ ebt_check_entry_size_and_hooks(struct ebt_entry *e, return -EINVAL; } /* before we look at the struct, be sure it is not too big */ - if ((char *)hook_entries[i] + sizeof(struct ebt_entries) - > limit) { - BUGPRINT("entries_size too small\n"); - return -EINVAL; - } + if (left < sizeof(struct ebt_entries)) + goto Esmall; if (((struct ebt_entries *)e)->policy != EBT_DROP && ((struct ebt_entries *)e)->policy != EBT_ACCEPT) { /* only RETURN from udc */ @@ -455,6 +456,8 @@ ebt_check_entry_size_and_hooks(struct ebt_entry *e, return 0; } /* a plain old entry, heh */ + if (left < sizeof(struct ebt_entry)) + goto Esmall; if (sizeof(struct ebt_entry) > e->watchers_offset || e->watchers_offset > e->target_offset || e->target_offset >= e->next_offset) { @@ -466,10 +469,16 @@ ebt_check_entry_size_and_hooks(struct ebt_entry *e, BUGPRINT("target size too small\n"); return -EINVAL; } + if (left < e->next_offset) + goto Esmall; (*cnt)++; (*totalcnt)++; return 0; + +Esmall: + BUGPRINT("entries_size too small\n"); + return -EINVAL; } struct ebt_cl_stack -- cgit v1.2.3 From 40642f95f5f818579bc4cc3ee084b033e662d5b3 Mon Sep 17 00:00:00 2001 From: Al Viro Date: Thu, 30 Nov 2006 19:24:12 -0800 Subject: [EBTABLES]: Verify that ebt_entries have zero ->distinguisher. We need that for iterator to work; existing check had been too weak. Signed-off-by: Al Viro Signed-off-by: David S. Miller --- net/bridge/netfilter/ebtables.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) (limited to 'net/bridge') diff --git a/net/bridge/netfilter/ebtables.c b/net/bridge/netfilter/ebtables.c index 0dcebf20d6c..6ab7674ea45 100644 --- a/net/bridge/netfilter/ebtables.c +++ b/net/bridge/netfilter/ebtables.c @@ -417,7 +417,7 @@ ebt_check_entry_size_and_hooks(struct ebt_entry *e, /* beginning of a new chain if i == NF_BR_NUMHOOKS it must be a user defined chain */ if (i != NF_BR_NUMHOOKS || !(e->bitmask & EBT_ENTRY_OR_ENTRIES)) { - if ((e->bitmask & EBT_ENTRY_OR_ENTRIES) != 0) { + if (e->bitmask != 0) { /* we make userspace set this right, so there is no misunderstanding */ BUGPRINT("EBT_ENTRY_OR_ENTRIES shouldn't be set " @@ -500,7 +500,7 @@ ebt_get_udc_positions(struct ebt_entry *e, struct ebt_table_info *newinfo, int i; /* we're only interested in chain starts */ - if (e->bitmask & EBT_ENTRY_OR_ENTRIES) + if (e->bitmask) return 0; for (i = 0; i < NF_BR_NUMHOOKS; i++) { if ((valid_hooks & (1 << i)) == 0) @@ -550,7 +550,7 @@ ebt_cleanup_entry(struct ebt_entry *e, unsigned int *cnt) { struct ebt_entry_target *t; - if ((e->bitmask & EBT_ENTRY_OR_ENTRIES) == 0) + if (e->bitmask == 0) return 0; /* we're done */ if (cnt && (*cnt)-- == 0) @@ -576,7 +576,7 @@ ebt_check_entry(struct ebt_entry *e, struct ebt_table_info *newinfo, int ret; /* don't mess with the struct ebt_entries */ - if ((e->bitmask & EBT_ENTRY_OR_ENTRIES) == 0) + if (e->bitmask == 0) return 0; if (e->bitmask & ~EBT_F_MASK) { @@ -1309,7 +1309,7 @@ static inline int ebt_make_names(struct ebt_entry *e, char *base, char *ubase) char *hlp; struct ebt_entry_target *t; - if ((e->bitmask & EBT_ENTRY_OR_ENTRIES) == 0) + if (e->bitmask == 0) return 0; hlp = ubase - base + (char *)e + e->target_offset; -- cgit v1.2.3 From 98a0824a0f33d051f31ca8ff59e289755b244ede Mon Sep 17 00:00:00 2001 From: Al Viro Date: Thu, 30 Nov 2006 19:24:49 -0800 Subject: [EBTABLES]: Deal with the worst-case behaviour in loop checks. No need to revisit a chain we'd already finished with during the check for current hook. It's either instant loop (which we'd just detected) or a duplicate work. Signed-off-by: Al Viro Signed-off-by: David S. Miller --- net/bridge/netfilter/ebtables.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) (limited to 'net/bridge') diff --git a/net/bridge/netfilter/ebtables.c b/net/bridge/netfilter/ebtables.c index 6ab7674ea45..46ab9b75926 100644 --- a/net/bridge/netfilter/ebtables.c +++ b/net/bridge/netfilter/ebtables.c @@ -717,7 +717,9 @@ static int check_chainloops(struct ebt_entries *chain, struct ebt_cl_stack *cl_s BUGPRINT("loop\n"); return -1; } - /* this can't be 0, so the above test is correct */ + if (cl_s[i].hookmask & (1 << hooknr)) + goto letscontinue; + /* this can't be 0, so the loop test is correct */ cl_s[i].cs.n = pos + 1; pos = 0; cl_s[i].cs.e = ((void *)e + e->next_offset); -- cgit v1.2.3 From 14197d5447afc41fce6b11a91592278cad1a09eb Mon Sep 17 00:00:00 2001 From: Al Viro Date: Thu, 30 Nov 2006 19:25:21 -0800 Subject: [EBTABLES]: Prevent wraparounds in checks for entry components' sizes. Signed-off-by: Al Viro Signed-off-by: David S. Miller --- net/bridge/netfilter/ebtables.c | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) (limited to 'net/bridge') diff --git a/net/bridge/netfilter/ebtables.c b/net/bridge/netfilter/ebtables.c index 46ab9b75926..136ed7d4bd7 100644 --- a/net/bridge/netfilter/ebtables.c +++ b/net/bridge/netfilter/ebtables.c @@ -338,10 +338,11 @@ ebt_check_match(struct ebt_entry_match *m, struct ebt_entry *e, const char *name, unsigned int hookmask, unsigned int *cnt) { struct ebt_match *match; + size_t left = ((char *)e + e->watchers_offset) - (char *)m; int ret; - if (((char *)m) + m->match_size + sizeof(struct ebt_entry_match) > - ((char *)e) + e->watchers_offset) + if (left < sizeof(struct ebt_entry_match) || + left - sizeof(struct ebt_entry_match) < m->match_size) return -EINVAL; match = find_match_lock(m->u.name, &ret, &ebt_mutex); if (!match) @@ -367,10 +368,11 @@ ebt_check_watcher(struct ebt_entry_watcher *w, struct ebt_entry *e, const char *name, unsigned int hookmask, unsigned int *cnt) { struct ebt_watcher *watcher; + size_t left = ((char *)e + e->target_offset) - (char *)w; int ret; - if (((char *)w) + w->watcher_size + sizeof(struct ebt_entry_watcher) > - ((char *)e) + e->target_offset) + if (left < sizeof(struct ebt_entry_watcher) || + left - sizeof(struct ebt_entry_watcher) < w->watcher_size) return -EINVAL; watcher = find_watcher_lock(w->u.name, &ret, &ebt_mutex); if (!watcher) @@ -573,6 +575,7 @@ ebt_check_entry(struct ebt_entry *e, struct ebt_table_info *newinfo, struct ebt_entry_target *t; struct ebt_target *target; unsigned int i, j, hook = 0, hookmask = 0; + size_t gap = e->next_offset - e->target_offset; int ret; /* don't mess with the struct ebt_entries */ @@ -634,8 +637,7 @@ ebt_check_entry(struct ebt_entry *e, struct ebt_table_info *newinfo, t->u.target = target; if (t->u.target == &ebt_standard_target) { - if (e->target_offset + sizeof(struct ebt_standard_target) > - e->next_offset) { + if (gap < sizeof(struct ebt_standard_target)) { BUGPRINT("Standard target size too big\n"); ret = -EFAULT; goto cleanup_watchers; @@ -646,8 +648,7 @@ ebt_check_entry(struct ebt_entry *e, struct ebt_table_info *newinfo, ret = -EFAULT; goto cleanup_watchers; } - } else if ((e->target_offset + t->target_size + - sizeof(struct ebt_entry_target) > e->next_offset) || + } else if (t->target_size > gap - sizeof(struct ebt_entry_target) || (t->u.target->check && t->u.target->check(name, hookmask, e, t->data, t->target_size) != 0)){ module_put(t->u.target->me); -- cgit v1.2.3 From 22b440bf9e717226d0fbaf4f29357cbdd5279de5 Mon Sep 17 00:00:00 2001 From: Al Viro Date: Thu, 30 Nov 2006 19:25:51 -0800 Subject: [EBTABLES]: Split ebt_check_entry_size_and_hooks Split ebt_check_entry_size_and_hooks() in two parts - one that does sanity checks on pointers (basically, checks that we can safely use iterator from now on) and the rest of it (looking into details of entry). The loop applying ebt_check_entry_size_and_hooks() is split in two. Populating newinfo->hook_entry[] is done in the first part. Unused arguments killed. Signed-off-by: Al Viro Signed-off-by: David S. Miller --- net/bridge/netfilter/ebtables.c | 73 +++++++++++++++++++++++++++-------------- 1 file changed, 49 insertions(+), 24 deletions(-) (limited to 'net/bridge') diff --git a/net/bridge/netfilter/ebtables.c b/net/bridge/netfilter/ebtables.c index 136ed7d4bd7..e79c0fbd9e8 100644 --- a/net/bridge/netfilter/ebtables.c +++ b/net/bridge/netfilter/ebtables.c @@ -393,15 +393,11 @@ ebt_check_watcher(struct ebt_entry_watcher *w, struct ebt_entry *e, return 0; } -/* - * this one is very careful, as it is the first function - * to parse the userspace data - */ static inline int -ebt_check_entry_size_and_hooks(struct ebt_entry *e, +__ebt_verify_pointers(struct ebt_entry *e, struct ebt_table_info *newinfo, char *base, char *limit, - struct ebt_entries **hook_entries, unsigned int *n, unsigned int *cnt, - unsigned int *totalcnt, unsigned int *udc_cnt, unsigned int valid_hooks) + struct ebt_entries **hook_entries, + unsigned int valid_hooks) { unsigned int offset = (char *)e - newinfo->entries; size_t left = (limit - base) - offset; @@ -416,8 +412,6 @@ ebt_check_entry_size_and_hooks(struct ebt_entry *e, if ((char *)hook_entries[i] == base + offset) break; } - /* beginning of a new chain - if i == NF_BR_NUMHOOKS it must be a user defined chain */ if (i != NF_BR_NUMHOOKS || !(e->bitmask & EBT_ENTRY_OR_ENTRIES)) { if (e->bitmask != 0) { /* we make userspace set this right, @@ -426,6 +420,45 @@ ebt_check_entry_size_and_hooks(struct ebt_entry *e, "in distinguisher\n"); return -EINVAL; } + if (left < sizeof(struct ebt_entries)) + goto Esmall; + if (i != NF_BR_NUMHOOKS) + newinfo->hook_entry[i] = (struct ebt_entries *)e; + return 0; + } + if (left < sizeof(struct ebt_entry)) + goto Esmall; + if (left < e->next_offset) + goto Esmall; + return 0; + +Esmall: + BUGPRINT("entries_size too small\n"); + return -EINVAL; +} + +/* + * this one is very careful, as it is the first function + * to parse the userspace data + */ +static inline int +ebt_check_entry_size_and_hooks(struct ebt_entry *e, + struct ebt_table_info *newinfo, char *base, + struct ebt_entries **hook_entries, unsigned int *n, unsigned int *cnt, + unsigned int *totalcnt, unsigned int *udc_cnt, unsigned int valid_hooks) +{ + unsigned int offset = (char *)e - newinfo->entries; + int i; + + for (i = 0; i < NF_BR_NUMHOOKS; i++) { + if ((valid_hooks & (1 << i)) == 0) + continue; + if ((char *)hook_entries[i] == base + offset) + break; + } + /* beginning of a new chain + if i == NF_BR_NUMHOOKS it must be a user defined chain */ + if (i != NF_BR_NUMHOOKS || !e->bitmask) { /* this checks if the previous chain has as many entries as it said it has */ if (*n != *cnt) { @@ -433,9 +466,6 @@ ebt_check_entry_size_and_hooks(struct ebt_entry *e, "in the chain\n"); return -EINVAL; } - /* before we look at the struct, be sure it is not too big */ - if (left < sizeof(struct ebt_entries)) - goto Esmall; if (((struct ebt_entries *)e)->policy != EBT_DROP && ((struct ebt_entries *)e)->policy != EBT_ACCEPT) { /* only RETURN from udc */ @@ -447,8 +477,6 @@ ebt_check_entry_size_and_hooks(struct ebt_entry *e, } if (i == NF_BR_NUMHOOKS) /* it's a user defined chain */ (*udc_cnt)++; - else - newinfo->hook_entry[i] = (struct ebt_entries *)e; if (((struct ebt_entries *)e)->counter_offset != *totalcnt) { BUGPRINT("counter_offset != totalcnt"); return -EINVAL; @@ -458,8 +486,6 @@ ebt_check_entry_size_and_hooks(struct ebt_entry *e, return 0; } /* a plain old entry, heh */ - if (left < sizeof(struct ebt_entry)) - goto Esmall; if (sizeof(struct ebt_entry) > e->watchers_offset || e->watchers_offset > e->target_offset || e->target_offset >= e->next_offset) { @@ -471,16 +497,9 @@ ebt_check_entry_size_and_hooks(struct ebt_entry *e, BUGPRINT("target size too small\n"); return -EINVAL; } - if (left < e->next_offset) - goto Esmall; - (*cnt)++; (*totalcnt)++; return 0; - -Esmall: - BUGPRINT("entries_size too small\n"); - return -EINVAL; } struct ebt_cl_stack @@ -776,6 +795,12 @@ static int translate_table(struct ebt_replace *repl, newinfo->entries_size = repl->entries_size; newinfo->nentries = repl->nentries; + ret = EBT_ENTRY_ITERATE(newinfo->entries, newinfo->entries_size, + __ebt_verify_pointers, newinfo, repl->entries, + repl->entries + repl->entries_size, repl->hook_entry, repl->valid_hooks); + if (ret != 0) + return ret; + /* do some early checkings and initialize some things */ i = 0; /* holds the expected nr. of entries for the chain */ j = 0; /* holds the up to now counted entries for the chain */ @@ -784,7 +809,7 @@ static int translate_table(struct ebt_replace *repl, udc_cnt = 0; /* will hold the nr. of user defined chains (udc) */ ret = EBT_ENTRY_ITERATE(newinfo->entries, newinfo->entries_size, ebt_check_entry_size_and_hooks, newinfo, repl->entries, - repl->entries + repl->entries_size, repl->hook_entry, &i, &j, &k, + repl->hook_entry, &i, &j, &k, &udc_cnt, repl->valid_hooks); if (ret != 0) -- cgit v1.2.3 From 70fe9af47ee01a17fe7486f1739f6eac8a14868b Mon Sep 17 00:00:00 2001 From: Al Viro Date: Thu, 30 Nov 2006 19:26:14 -0800 Subject: [EBTABLES]: Pull the loop doing __ebt_verify_pointers() into a separate function. It's easier to expand the iterator here *and* we'll be able to move all uses of ebt_replace from translate_table() into this one. Signed-off-by: Al Viro Signed-off-by: David S. Miller --- net/bridge/netfilter/ebtables.c | 78 ++++++++++++++++++++++------------------- 1 file changed, 41 insertions(+), 37 deletions(-) (limited to 'net/bridge') diff --git a/net/bridge/netfilter/ebtables.c b/net/bridge/netfilter/ebtables.c index e79c0fbd9e8..2eba40f5423 100644 --- a/net/bridge/netfilter/ebtables.c +++ b/net/bridge/netfilter/ebtables.c @@ -393,48 +393,54 @@ ebt_check_watcher(struct ebt_entry_watcher *w, struct ebt_entry *e, return 0; } -static inline int -__ebt_verify_pointers(struct ebt_entry *e, - struct ebt_table_info *newinfo, char *base, char *limit, - struct ebt_entries **hook_entries, - unsigned int valid_hooks) +static int ebt_verify_pointers(struct ebt_replace *repl, + struct ebt_table_info *newinfo) { - unsigned int offset = (char *)e - newinfo->entries; - size_t left = (limit - base) - offset; + unsigned int limit = repl->entries_size; + unsigned int valid_hooks = repl->valid_hooks; + unsigned int offset = 0; int i; - if (left < sizeof(unsigned int)) - goto Esmall; + while (offset < limit) { + size_t left = limit - offset; + struct ebt_entry *e = (void *)newinfo->entries + offset; - for (i = 0; i < NF_BR_NUMHOOKS; i++) { - if ((valid_hooks & (1 << i)) == 0) - continue; - if ((char *)hook_entries[i] == base + offset) + if (left < sizeof(unsigned int)) break; - } - if (i != NF_BR_NUMHOOKS || !(e->bitmask & EBT_ENTRY_OR_ENTRIES)) { - if (e->bitmask != 0) { - /* we make userspace set this right, - so there is no misunderstanding */ - BUGPRINT("EBT_ENTRY_OR_ENTRIES shouldn't be set " - "in distinguisher\n"); - return -EINVAL; + + for (i = 0; i < NF_BR_NUMHOOKS; i++) { + if ((valid_hooks & (1 << i)) == 0) + continue; + if ((char *)repl->hook_entry[i] == repl->entries + offset) + break; } - if (left < sizeof(struct ebt_entries)) - goto Esmall; - if (i != NF_BR_NUMHOOKS) - newinfo->hook_entry[i] = (struct ebt_entries *)e; - return 0; + + if (i != NF_BR_NUMHOOKS || !(e->bitmask & EBT_ENTRY_OR_ENTRIES)) { + if (e->bitmask != 0) { + /* we make userspace set this right, + so there is no misunderstanding */ + BUGPRINT("EBT_ENTRY_OR_ENTRIES shouldn't be set " + "in distinguisher\n"); + return -EINVAL; + } + if (i != NF_BR_NUMHOOKS) + newinfo->hook_entry[i] = (struct ebt_entries *)e; + if (left < sizeof(struct ebt_entries)) + break; + offset += sizeof(struct ebt_entries); + } else { + if (left < sizeof(struct ebt_entry)) + break; + if (left < e->next_offset) + break; + offset += e->next_offset; + } + } + if (offset != limit) { + BUGPRINT("entries_size too small\n"); + return -EINVAL; } - if (left < sizeof(struct ebt_entry)) - goto Esmall; - if (left < e->next_offset) - goto Esmall; return 0; - -Esmall: - BUGPRINT("entries_size too small\n"); - return -EINVAL; } /* @@ -795,9 +801,7 @@ static int translate_table(struct ebt_replace *repl, newinfo->entries_size = repl->entries_size; newinfo->nentries = repl->nentries; - ret = EBT_ENTRY_ITERATE(newinfo->entries, newinfo->entries_size, - __ebt_verify_pointers, newinfo, repl->entries, - repl->entries + repl->entries_size, repl->hook_entry, repl->valid_hooks); + ret = ebt_verify_pointers(repl, newinfo); if (ret != 0) return ret; -- cgit v1.2.3 From e4fd77deac764e17cb1eab8661bcf1413204d04d Mon Sep 17 00:00:00 2001 From: Al Viro Date: Thu, 30 Nov 2006 19:26:35 -0800 Subject: [EBTABLES]: Move more stuff into ebt_verify_pointers(). Take intialization of ->hook_entry[...], ->entries_size and ->nentries over there, pull the check for empty chains into the end of that sucker. Now it's self-contained, so we can move it up in the very beginning of translate_table() *and* we can rely on ->hook_entry[] being properly transliterated after it. Signed-off-by: Al Viro Signed-off-by: David S. Miller --- net/bridge/netfilter/ebtables.c | 38 +++++++++++++++++++------------------- 1 file changed, 19 insertions(+), 19 deletions(-) (limited to 'net/bridge') diff --git a/net/bridge/netfilter/ebtables.c b/net/bridge/netfilter/ebtables.c index 2eba40f5423..7ce190c21dd 100644 --- a/net/bridge/netfilter/ebtables.c +++ b/net/bridge/netfilter/ebtables.c @@ -401,6 +401,12 @@ static int ebt_verify_pointers(struct ebt_replace *repl, unsigned int offset = 0; int i; + for (i = 0; i < NF_BR_NUMHOOKS; i++) + newinfo->hook_entry[i] = NULL; + + newinfo->entries_size = repl->entries_size; + newinfo->nentries = repl->nentries; + while (offset < limit) { size_t left = limit - offset; struct ebt_entry *e = (void *)newinfo->entries + offset; @@ -440,6 +446,15 @@ static int ebt_verify_pointers(struct ebt_replace *repl, BUGPRINT("entries_size too small\n"); return -EINVAL; } + + /* check if all valid hooks have a chain */ + for (i = 0; i < NF_BR_NUMHOOKS; i++) { + if (!newinfo->hook_entry[i] && + (valid_hooks & (1 << i))) { + BUGPRINT("Valid hook without chain\n"); + return -EINVAL; + } + } return 0; } @@ -772,6 +787,10 @@ static int translate_table(struct ebt_replace *repl, int ret; struct ebt_cl_stack *cl_s = NULL; /* used in the checking for chain loops */ + ret = ebt_verify_pointers(repl, newinfo); + if (ret != 0) + return ret; + i = 0; while (i < NF_BR_NUMHOOKS && !(repl->valid_hooks & (1 << i))) i++; @@ -795,16 +814,6 @@ static int translate_table(struct ebt_replace *repl, i = j; } - for (i = 0; i < NF_BR_NUMHOOKS; i++) - newinfo->hook_entry[i] = NULL; - - newinfo->entries_size = repl->entries_size; - newinfo->nentries = repl->nentries; - - ret = ebt_verify_pointers(repl, newinfo); - if (ret != 0) - return ret; - /* do some early checkings and initialize some things */ i = 0; /* holds the expected nr. of entries for the chain */ j = 0; /* holds the up to now counted entries for the chain */ @@ -829,15 +838,6 @@ static int translate_table(struct ebt_replace *repl, return -EINVAL; } - /* check if all valid hooks have a chain */ - for (i = 0; i < NF_BR_NUMHOOKS; i++) { - if (newinfo->hook_entry[i] == NULL && - (repl->valid_hooks & (1 << i))) { - BUGPRINT("Valid hook without chain\n"); - return -EINVAL; - } - } - /* get the location of the udc, put them in an array while we're at it, allocate the chainstack */ if (udc_cnt) { -- cgit v1.2.3 From 1f072c96fdf1a0caa11c6e8078dd96925bd02db5 Mon Sep 17 00:00:00 2001 From: Al Viro Date: Thu, 30 Nov 2006 19:26:53 -0800 Subject: [EBTABLES]: translate_table(): switch direct uses of repl->hook_info to newinfo Since newinfo->hook_table[] already has been set up, we can switch to using it instead of repl->{hook_info,valid_hooks}. Signed-off-by: Al Viro Signed-off-by: David S. Miller --- net/bridge/netfilter/ebtables.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) (limited to 'net/bridge') diff --git a/net/bridge/netfilter/ebtables.c b/net/bridge/netfilter/ebtables.c index 7ce190c21dd..3e1bf716509 100644 --- a/net/bridge/netfilter/ebtables.c +++ b/net/bridge/netfilter/ebtables.c @@ -792,22 +792,22 @@ static int translate_table(struct ebt_replace *repl, return ret; i = 0; - while (i < NF_BR_NUMHOOKS && !(repl->valid_hooks & (1 << i))) + while (i < NF_BR_NUMHOOKS && !newinfo->hook_entry[i]) i++; if (i == NF_BR_NUMHOOKS) { BUGPRINT("No valid hooks specified\n"); return -EINVAL; } - if (repl->hook_entry[i] != (struct ebt_entries *)repl->entries) { + if (newinfo->hook_entry[i] != (struct ebt_entries *)newinfo->entries) { BUGPRINT("Chains don't start at beginning\n"); return -EINVAL; } /* make sure chains are ordered after each other in same order as their corresponding hooks */ for (j = i + 1; j < NF_BR_NUMHOOKS; j++) { - if (!(repl->valid_hooks & (1 << j))) + if (!newinfo->hook_entry[j]) continue; - if ( repl->hook_entry[j] <= repl->hook_entry[i] ) { + if (newinfo->hook_entry[j] <= newinfo->hook_entry[i]) { BUGPRINT("Hook order must be followed\n"); return -EINVAL; } @@ -877,7 +877,7 @@ static int translate_table(struct ebt_replace *repl, /* Check for loops */ for (i = 0; i < NF_BR_NUMHOOKS; i++) - if (repl->valid_hooks & (1 << i)) + if (newinfo->hook_entry[i]) if (check_chainloops(newinfo->hook_entry[i], cl_s, udc_cnt, i, newinfo->entries)) { vfree(cl_s); -- cgit v1.2.3 From 0e795531c5e6d0a7d407b8d9edde47cab13be3ec Mon Sep 17 00:00:00 2001 From: Al Viro Date: Thu, 30 Nov 2006 19:27:13 -0800 Subject: [EBTABLES]: Switch ebt_check_entry_size_and_hooks() to use of newinfo->hook_entry[] kill unused arguments Signed-off-by: Al Viro Signed-off-by: David S. Miller --- net/bridge/netfilter/ebtables.c | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) (limited to 'net/bridge') diff --git a/net/bridge/netfilter/ebtables.c b/net/bridge/netfilter/ebtables.c index 3e1bf716509..ec7709b5c56 100644 --- a/net/bridge/netfilter/ebtables.c +++ b/net/bridge/netfilter/ebtables.c @@ -464,17 +464,14 @@ static int ebt_verify_pointers(struct ebt_replace *repl, */ static inline int ebt_check_entry_size_and_hooks(struct ebt_entry *e, - struct ebt_table_info *newinfo, char *base, - struct ebt_entries **hook_entries, unsigned int *n, unsigned int *cnt, - unsigned int *totalcnt, unsigned int *udc_cnt, unsigned int valid_hooks) + struct ebt_table_info *newinfo, + unsigned int *n, unsigned int *cnt, + unsigned int *totalcnt, unsigned int *udc_cnt) { - unsigned int offset = (char *)e - newinfo->entries; int i; for (i = 0; i < NF_BR_NUMHOOKS; i++) { - if ((valid_hooks & (1 << i)) == 0) - continue; - if ((char *)hook_entries[i] == base + offset) + if ((void *)e == (void *)newinfo->hook_entry[i]) break; } /* beginning of a new chain @@ -821,9 +818,8 @@ static int translate_table(struct ebt_replace *repl, newinfo->nentries afterwards */ udc_cnt = 0; /* will hold the nr. of user defined chains (udc) */ ret = EBT_ENTRY_ITERATE(newinfo->entries, newinfo->entries_size, - ebt_check_entry_size_and_hooks, newinfo, repl->entries, - repl->hook_entry, &i, &j, &k, - &udc_cnt, repl->valid_hooks); + ebt_check_entry_size_and_hooks, newinfo, + &i, &j, &k, &udc_cnt); if (ret != 0) return ret; -- cgit v1.2.3 From 177abc348a00738dbc985df8523d755bf87403d9 Mon Sep 17 00:00:00 2001 From: Al Viro Date: Thu, 30 Nov 2006 19:27:32 -0800 Subject: [EBTABLES]: Clean ebt_get_udc_positions() up. Check for valid_hooks is redundant (newinfo->hook_entry[i] will be NULL if bit i is not set). Kill it, kill unused arguments. Signed-off-by: Al Viro Signed-off-by: David S. Miller --- net/bridge/netfilter/ebtables.c | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) (limited to 'net/bridge') diff --git a/net/bridge/netfilter/ebtables.c b/net/bridge/netfilter/ebtables.c index ec7709b5c56..4d1cf1492ca 100644 --- a/net/bridge/netfilter/ebtables.c +++ b/net/bridge/netfilter/ebtables.c @@ -533,8 +533,7 @@ struct ebt_cl_stack */ static inline int ebt_get_udc_positions(struct ebt_entry *e, struct ebt_table_info *newinfo, - struct ebt_entries **hook_entries, unsigned int *n, unsigned int valid_hooks, - struct ebt_cl_stack *udc) + unsigned int *n, struct ebt_cl_stack *udc) { int i; @@ -542,8 +541,6 @@ ebt_get_udc_positions(struct ebt_entry *e, struct ebt_table_info *newinfo, if (e->bitmask) return 0; for (i = 0; i < NF_BR_NUMHOOKS; i++) { - if ((valid_hooks & (1 << i)) == 0) - continue; if (newinfo->hook_entry[i] == (struct ebt_entries *)e) break; } @@ -861,8 +858,7 @@ static int translate_table(struct ebt_replace *repl, return -ENOMEM; i = 0; /* the i'th udc */ EBT_ENTRY_ITERATE(newinfo->entries, newinfo->entries_size, - ebt_get_udc_positions, newinfo, repl->hook_entry, &i, - repl->valid_hooks, cl_s); + ebt_get_udc_positions, newinfo, &i, cl_s); /* sanity check */ if (i != udc_cnt) { BUGPRINT("i != udc_cnt\n"); -- cgit v1.2.3 From f7da79d99863c044e28483e32c10b394bbd78d21 Mon Sep 17 00:00:00 2001 From: Al Viro Date: Thu, 30 Nov 2006 19:27:48 -0800 Subject: [EBTABLES]: ebt_check_entry() doesn't need valid_hooks We can check newinfo->hook_entry[...] instead. Kill unused argument. Signed-off-by: Al Viro Signed-off-by: David S. Miller --- net/bridge/netfilter/ebtables.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) (limited to 'net/bridge') diff --git a/net/bridge/netfilter/ebtables.c b/net/bridge/netfilter/ebtables.c index 4d1cf1492ca..c4f10b8865a 100644 --- a/net/bridge/netfilter/ebtables.c +++ b/net/bridge/netfilter/ebtables.c @@ -603,7 +603,7 @@ ebt_cleanup_entry(struct ebt_entry *e, unsigned int *cnt) static inline int ebt_check_entry(struct ebt_entry *e, struct ebt_table_info *newinfo, - const char *name, unsigned int *cnt, unsigned int valid_hooks, + const char *name, unsigned int *cnt, struct ebt_cl_stack *cl_s, unsigned int udc_cnt) { struct ebt_entry_target *t; @@ -630,7 +630,7 @@ ebt_check_entry(struct ebt_entry *e, struct ebt_table_info *newinfo, } /* what hook do we belong to? */ for (i = 0; i < NF_BR_NUMHOOKS; i++) { - if ((valid_hooks & (1 << i)) == 0) + if (!newinfo->hook_entry[i]) continue; if ((char *)newinfo->hook_entry[i] < (char *)e) hook = i; @@ -889,8 +889,7 @@ static int translate_table(struct ebt_replace *repl, /* used to know what we need to clean up if something goes wrong */ i = 0; ret = EBT_ENTRY_ITERATE(newinfo->entries, newinfo->entries_size, - ebt_check_entry, newinfo, repl->name, &i, repl->valid_hooks, - cl_s, udc_cnt); + ebt_check_entry, newinfo, repl->name, &i, cl_s, udc_cnt); if (ret != 0) { EBT_ENTRY_ITERATE(newinfo->entries, newinfo->entries_size, ebt_cleanup_entry, &i); -- cgit v1.2.3 From 1bc2326cbe24766d9cb236e63c091cbaecfa2f29 Mon Sep 17 00:00:00 2001 From: Al Viro Date: Thu, 30 Nov 2006 19:28:08 -0800 Subject: [EBTABLES]: Move calls of ebt_verify_pointers() upstream. ... and pass just repl->name to translate_table() Signed-off-by: Al Viro Signed-off-by: David S. Miller --- net/bridge/netfilter/ebtables.c | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) (limited to 'net/bridge') diff --git a/net/bridge/netfilter/ebtables.c b/net/bridge/netfilter/ebtables.c index c4f10b8865a..f0d9ffd4c91 100644 --- a/net/bridge/netfilter/ebtables.c +++ b/net/bridge/netfilter/ebtables.c @@ -774,17 +774,12 @@ letscontinue: } /* do the parsing of the table/chains/entries/matches/watchers/targets, heh */ -static int translate_table(struct ebt_replace *repl, - struct ebt_table_info *newinfo) +static int translate_table(char *name, struct ebt_table_info *newinfo) { unsigned int i, j, k, udc_cnt; int ret; struct ebt_cl_stack *cl_s = NULL; /* used in the checking for chain loops */ - ret = ebt_verify_pointers(repl, newinfo); - if (ret != 0) - return ret; - i = 0; while (i < NF_BR_NUMHOOKS && !newinfo->hook_entry[i]) i++; @@ -889,7 +884,7 @@ static int translate_table(struct ebt_replace *repl, /* used to know what we need to clean up if something goes wrong */ i = 0; ret = EBT_ENTRY_ITERATE(newinfo->entries, newinfo->entries_size, - ebt_check_entry, newinfo, repl->name, &i, cl_s, udc_cnt); + ebt_check_entry, newinfo, name, &i, cl_s, udc_cnt); if (ret != 0) { EBT_ENTRY_ITERATE(newinfo->entries, newinfo->entries_size, ebt_cleanup_entry, &i); @@ -986,7 +981,11 @@ static int do_replace(void __user *user, unsigned int len) /* this can get initialized by translate_table() */ newinfo->chainstack = NULL; - ret = translate_table(&tmp, newinfo); + ret = ebt_verify_pointers(&tmp, newinfo); + if (ret != 0) + goto free_counterstmp; + + ret = translate_table(tmp.name, newinfo); if (ret != 0) goto free_counterstmp; @@ -1185,7 +1184,10 @@ int ebt_register_table(struct ebt_table *table) /* fill in newinfo and parse the entries */ newinfo->chainstack = NULL; - ret = translate_table(table->table, newinfo); + ret = ebt_verify_pointers(table->table, newinfo); + if (ret != 0) + goto free_chainstack; + ret = translate_table(table->table->name, newinfo); if (ret != 0) { BUGPRINT("Translate_table failed\n"); goto free_chainstack; -- cgit v1.2.3 From df07a81e939a0176b125bc83cf22dbb5e380ae9f Mon Sep 17 00:00:00 2001 From: Al Viro Date: Thu, 30 Nov 2006 19:28:25 -0800 Subject: [EBTABLES]: Clean ebt_register_table() up. Signed-off-by: Al Viro Signed-off-by: David S. Miller --- net/bridge/netfilter/ebtables.c | 33 +++++++++++++++++++++------------ 1 file changed, 21 insertions(+), 12 deletions(-) (limited to 'net/bridge') diff --git a/net/bridge/netfilter/ebtables.c b/net/bridge/netfilter/ebtables.c index f0d9ffd4c91..00a89705c1c 100644 --- a/net/bridge/netfilter/ebtables.c +++ b/net/bridge/netfilter/ebtables.c @@ -1156,38 +1156,47 @@ int ebt_register_table(struct ebt_table *table) { struct ebt_table_info *newinfo; struct ebt_table *t; + struct ebt_replace *repl; int ret, i, countersize; + void *p; - if (!table || !table->table ||!table->table->entries || - table->table->entries_size == 0 || - table->table->counters || table->private) { + if (!table || !(repl = table->table) || !repl->entries || + repl->entries_size == 0 || + repl->counters || table->private) { BUGPRINT("Bad table data for ebt_register_table!!!\n"); return -EINVAL; } - countersize = COUNTER_OFFSET(table->table->nentries) * + countersize = COUNTER_OFFSET(repl->nentries) * (highest_possible_processor_id()+1); newinfo = vmalloc(sizeof(*newinfo) + countersize); ret = -ENOMEM; if (!newinfo) return -ENOMEM; - newinfo->entries = vmalloc(table->table->entries_size); - if (!(newinfo->entries)) + p = vmalloc(repl->entries_size); + if (!p) goto free_newinfo; - memcpy(newinfo->entries, table->table->entries, - table->table->entries_size); + memcpy(p, repl->entries, repl->entries_size); + newinfo->entries = p; + + newinfo->entries_size = repl->entries_size; + newinfo->nentries = repl->nentries; if (countersize) memset(newinfo->counters, 0, countersize); /* fill in newinfo and parse the entries */ newinfo->chainstack = NULL; - ret = ebt_verify_pointers(table->table, newinfo); - if (ret != 0) - goto free_chainstack; - ret = translate_table(table->table->name, newinfo); + for (i = 0; i < NF_BR_NUMHOOKS; i++) { + if ((repl->valid_hooks & (1 << i)) == 0) + newinfo->hook_entry[i] = NULL; + else + newinfo->hook_entry[i] = p + + ((char *)repl->hook_entry[i] - repl->entries); + } + ret = translate_table(repl->name, newinfo); if (ret != 0) { BUGPRINT("Translate_table failed\n"); goto free_chainstack; -- cgit v1.2.3 From 1e419cd9953f59d06d7b88d0e2911a68a0044f33 Mon Sep 17 00:00:00 2001 From: Al Viro Date: Thu, 30 Nov 2006 19:28:48 -0800 Subject: [EBTABLES]: Split ebt_replace into user and kernel variants, annotate. Signed-off-by: Al Viro Signed-off-by: David S. Miller --- net/bridge/netfilter/ebtable_broute.c | 2 +- net/bridge/netfilter/ebtable_filter.c | 2 +- net/bridge/netfilter/ebtable_nat.c | 2 +- net/bridge/netfilter/ebtables.c | 19 ++++++++++--------- 4 files changed, 13 insertions(+), 12 deletions(-) (limited to 'net/bridge') diff --git a/net/bridge/netfilter/ebtable_broute.c b/net/bridge/netfilter/ebtable_broute.c index 9a6e548e148..d37ce047893 100644 --- a/net/bridge/netfilter/ebtable_broute.c +++ b/net/bridge/netfilter/ebtable_broute.c @@ -23,7 +23,7 @@ static struct ebt_entries initial_chain = { .policy = EBT_ACCEPT, }; -static struct ebt_replace initial_table = +static struct ebt_replace_kernel initial_table = { .name = "broute", .valid_hooks = 1 << NF_BR_BROUTING, diff --git a/net/bridge/netfilter/ebtable_filter.c b/net/bridge/netfilter/ebtable_filter.c index 3d5bd44f239..127135ead2d 100644 --- a/net/bridge/netfilter/ebtable_filter.c +++ b/net/bridge/netfilter/ebtable_filter.c @@ -30,7 +30,7 @@ static struct ebt_entries initial_chains[] = }, }; -static struct ebt_replace initial_table = +static struct ebt_replace_kernel initial_table = { .name = "filter", .valid_hooks = FILTER_VALID_HOOKS, diff --git a/net/bridge/netfilter/ebtable_nat.c b/net/bridge/netfilter/ebtable_nat.c index 04dd42efda1..9c50488b62e 100644 --- a/net/bridge/netfilter/ebtable_nat.c +++ b/net/bridge/netfilter/ebtable_nat.c @@ -30,7 +30,7 @@ static struct ebt_entries initial_chains[] = } }; -static struct ebt_replace initial_table = +static struct ebt_replace_kernel initial_table = { .name = "nat", .valid_hooks = NAT_VALID_HOOKS, diff --git a/net/bridge/netfilter/ebtables.c b/net/bridge/netfilter/ebtables.c index 00a89705c1c..bee558a4180 100644 --- a/net/bridge/netfilter/ebtables.c +++ b/net/bridge/netfilter/ebtables.c @@ -417,7 +417,8 @@ static int ebt_verify_pointers(struct ebt_replace *repl, for (i = 0; i < NF_BR_NUMHOOKS; i++) { if ((valid_hooks & (1 << i)) == 0) continue; - if ((char *)repl->hook_entry[i] == repl->entries + offset) + if ((char __user *)repl->hook_entry[i] == + repl->entries + offset) break; } @@ -1156,7 +1157,7 @@ int ebt_register_table(struct ebt_table *table) { struct ebt_table_info *newinfo; struct ebt_table *t; - struct ebt_replace *repl; + struct ebt_replace_kernel *repl; int ret, i, countersize; void *p; @@ -1320,33 +1321,33 @@ free_tmp: } static inline int ebt_make_matchname(struct ebt_entry_match *m, - char *base, char *ubase) + char *base, char __user *ubase) { - char *hlp = ubase - base + (char *)m; + char __user *hlp = ubase + ((char *)m - base); if (copy_to_user(hlp, m->u.match->name, EBT_FUNCTION_MAXNAMELEN)) return -EFAULT; return 0; } static inline int ebt_make_watchername(struct ebt_entry_watcher *w, - char *base, char *ubase) + char *base, char __user *ubase) { - char *hlp = ubase - base + (char *)w; + char __user *hlp = ubase + ((char *)w - base); if (copy_to_user(hlp , w->u.watcher->name, EBT_FUNCTION_MAXNAMELEN)) return -EFAULT; return 0; } -static inline int ebt_make_names(struct ebt_entry *e, char *base, char *ubase) +static inline int ebt_make_names(struct ebt_entry *e, char *base, char __user *ubase) { int ret; - char *hlp; + char __user *hlp; struct ebt_entry_target *t; if (e->bitmask == 0) return 0; - hlp = ubase - base + (char *)e + e->target_offset; + hlp = ubase + (((char *)e + e->target_offset) - base); t = (struct ebt_entry_target *)(((char *)e) + e->target_offset); ret = EBT_MATCH_ITERATE(e, ebt_make_matchname, base, ubase); -- cgit v1.2.3