| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Thu, 1 Jan 2015 08:54:32 +0000
From: "Arad, Ronen" <ronen.arad@...el.com>
To: "roopa@...ulusnetworks.com" <roopa@...ulusnetworks.com>,
"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
"hemminger@...tta.com" <hemminger@...tta.com>,
"vyasevic@...hat.com" <vyasevic@...hat.com>
CC: "sfeldma@...il.com" <sfeldma@...il.com>,
"wkok@...ulusnetworks.com" <wkok@...ulusnetworks.com>
Subject: RE: [PATCH net-next v2 2/2] bridge: modify bridge af spec parser to
accomodate vlan list and ranges
>-----Original Message-----
>From: netdev-owner@...r.kernel.org [mailto:netdev-owner@...r.kernel.org] On
>Behalf Of roopa@...ulusnetworks.com
>Sent: Wednesday, December 31, 2014 6:49 PM
>To: netdev@...r.kernel.org; hemminger@...tta.com; vyasevic@...hat.com
>Cc: sfeldma@...il.com; wkok@...ulusnetworks.com; Roopa Prabhu
>Subject: [PATCH net-next v2 2/2] bridge: modify bridge af spec parser to
>accomodate vlan list and ranges
>
>From: Roopa Prabhu <roopa@...ulusnetworks.com>
>
>This patch modifies br_afspec to parse incoming IFLA_BRIDGE_VLAN_INFO_LIST
>
>Signed-off-by: Roopa Prabhu <roopa@...ulusnetworks.com>
>---
> net/bridge/br_netlink.c | 115 ++++++++++++++++++++++++++++++++++------------
>-
> 1 file changed, 85 insertions(+), 30 deletions(-)
>
>diff --git a/net/bridge/br_netlink.c b/net/bridge/br_netlink.c
>index 492ef6a..bcba9d2 100644
>--- a/net/bridge/br_netlink.c
>+++ b/net/bridge/br_netlink.c
>@@ -226,53 +226,108 @@ static const struct nla_policy
>ifla_br_policy[IFLA_MAX+1] = {
> [IFLA_BRIDGE_VLAN_INFO_LIST] = { .type = NLA_NESTED, },
> };
>
>+static int br_vlan_info(struct net_bridge *br, struct net_bridge_port *p,
>+ int cmd, struct bridge_vlan_info *vinfo)
>+{
>+ int err = 0;
>+
>+ switch (cmd) {
>+ case RTM_SETLINK:
>+ if (p) {
>+ err = nbp_vlan_add(p, vinfo->vid, vinfo->flags);
>+ if (err)
>+ break;
>+
>+ if (vinfo->flags & BRIDGE_VLAN_INFO_MASTER)
>+ err = br_vlan_add(p->br, vinfo->vid,
>+ vinfo->flags);
>+ } else {
>+ err = br_vlan_add(br, vinfo->vid, vinfo->flags);
>+ }
>+ break;
>+
>+ case RTM_DELLINK:
>+ if (p) {
>+ nbp_vlan_delete(p, vinfo->vid);
>+ if (vinfo->flags & BRIDGE_VLAN_INFO_MASTER)
>+ br_vlan_delete(p->br, vinfo->vid);
>+ } else {
>+ br_vlan_delete(br, vinfo->vid);
>+ }
>+ break;
>+ }
>+
>+ return err;
>+}
>+
> static int br_afspec(struct net_bridge *br,
> struct net_bridge_port *p,
> struct nlattr *af_spec,
> int cmd)
> {
> struct nlattr *tb[IFLA_BRIDGE_MAX+1];
>+ struct nlattr *attr;
> int err = 0;
>+ int rem;
>
> err = nla_parse_nested(tb, IFLA_BRIDGE_MAX, af_spec, ifla_br_policy);
> if (err)
> return err;
>
> if (tb[IFLA_BRIDGE_VLAN_INFO]) {
>- struct bridge_vlan_info *vinfo;
>-
>- vinfo = nla_data(tb[IFLA_BRIDGE_VLAN_INFO]);
>-
>- if (!vinfo->vid || vinfo->vid >= VLAN_VID_MASK)
>- return -EINVAL;
>-
>- switch (cmd) {
>- case RTM_SETLINK:
>- if (p) {
>- err = nbp_vlan_add(p, vinfo->vid, vinfo->flags);
>- if (err)
>- break;
>-
>- if (vinfo->flags & BRIDGE_VLAN_INFO_MASTER)
>- err = br_vlan_add(p->br, vinfo->vid,
>- vinfo->flags);
>- } else
>- err = br_vlan_add(br, vinfo->vid, vinfo->flags);
>-
>- break;
>-
>- case RTM_DELLINK:
>- if (p) {
>- nbp_vlan_delete(p, vinfo->vid);
>- if (vinfo->flags & BRIDGE_VLAN_INFO_MASTER)
>- br_vlan_delete(p->br, vinfo->vid);
>- } else
>- br_vlan_delete(br, vinfo->vid);
>- break;
>+ attr = tb[IFLA_BRIDGE_VLAN_INFO];
>+ if (nla_len(attr) != sizeof(struct bridge_vlan_info))
>+ goto err_inval;
>+
>+ err = br_vlan_info(br, p, cmd,
>+ (struct bridge_vlan_info *)nla_data(attr));
>+
>+ } else if (tb[IFLA_BRIDGE_VLAN_INFO_LIST]) {
>+ struct bridge_vlan_info *vinfo_start = NULL;
>+ struct bridge_vlan_info *vinfo = NULL;
>+
>+ nla_for_each_nested(attr, tb[IFLA_BRIDGE_VLAN_INFO_LIST], rem) {
>+ if (nla_len(attr) != sizeof(struct bridge_vlan_info) ||
>+ nla_type(attr) != IFLA_BRIDGE_VLAN_INFO)
>+ goto err_inval;
>+ vinfo = nla_data(attr);
>+ if (vinfo->flags & BRIDGE_VLAN_INFO_RANGE_START) {
>+ if (vinfo_start)
>+ goto err_inval;
>+ vinfo_start = vinfo;
>+ continue;
>+ }
>+
>+ if (vinfo_start) {
>+ int v;
>+
>+ if (!(vinfo->flags & BRIDGE_VLAN_INFO_RANGE_END))
>+ goto err_inval;
>+
>+ if (vinfo->vid < vinfo_start->vid)
This check rejects inverted range. However it allows the RANGE_START and
RANGE_END vinfos to have the same vid. Isn't it inconsistent with the rejection
of a single vinfo with both RANGE_START and RANGE_END set?
>+ goto err_inval;
Are additional validation such as consistency of flags between the RANGE_START
and RANGE_END vinfos is needed here?
The loop below applies flags (more precisely all data except for vid) from the
RANGE_START vinfo to all vids in the range. All data from the RANGE_END vinfo
is ignored.
>+
>+ for (v = vinfo_start->vid; v <= vinfo->vid;
>+ v++) {
>+ vinfo_start->vid = v;
This changes the vinfo with RANGE_START flag within the incoming message. Would
it be better to left the input message unmodified and use a local copy of
struct bridge_vlan_info?
>+ err = br_vlan_info(br, p, cmd,
>+ vinfo_start);
>+ if (err)
>+ break;
>+ }
>+ vinfo_start = NULL;
>+ } else {
>+ err = br_vlan_info(br, p, cmd, vinfo);
>+ }
>+ if (err)
>+ break;
> }
> }
>
> return err;
>+
>+err_inval:
>+ return -EINVAL;
> }
>
> static const struct nla_policy br_port_policy[IFLA_BRPORT_MAX + 1] = {
>--
>1.7.10.4
>
>--
>To unsubscribe from this list: send the line "unsubscribe netdev" in
>the body of a message to majordomo@...r.kernel.org
>More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists