From: William Gonzalez <gonzalez.williamalexander1@gmail.com>
To: netdev@vger.kernel.org
Cc: stephen@networkplumber.org,
William Gonzalez <gonzalez.williamalexander1@gmail.com>
Subject: [PATCH] bridge: tighten VLAN parsing
Date: Fri, 31 Jul 2026 22:40:51 -0400 [thread overview]
Message-ID: <20260801024051.10581-1-gonzalez.williamalexander1@gmail.com> (raw)
---
bridge/fdb.c | 45 +++++++++++--------
bridge/mdb.c | 21 ++++++---
bridge/vlan.c | 113 ++++++++++++++++++------------------------------
include/utils.h | 2 +
lib/utils.c | 51 ++++++++++++++++++++++
5 files changed, 136 insertions(+), 96 deletions(-)
diff --git a/bridge/fdb.c b/bridge/fdb.c
index 05f093b5..fadd82ae 100644
--- a/bridge/fdb.c
+++ b/bridge/fdb.c
@@ -364,10 +364,14 @@ static int fdb_show(int argc, char **argv)
NEXT_ARG();
br = *argv;
} else if (strcmp(*argv, "vlan") == 0) {
+ __u16 vid;
+
NEXT_ARG();
if (filter_vlan)
duparg("vlan", *argv);
- filter_vlan = atoi(*argv);
+ if (get_vlan(&vid, *argv))
+ invarg("invalid vlan", *argv);
+ filter_vlan = vid;
} else if (strcmp(*argv, "state") == 0) {
unsigned int state;
@@ -474,7 +478,8 @@ static int fdb_modify(int cmd, int flags, int argc, char **argv)
unsigned long src_vni = ~0;
unsigned int via = 0;
char *endptr;
- short vid = -1;
+ __u16 vid = 0;
+ bool vid_set = false;
__u32 nhid = 0;
while (argc > 0) {
@@ -537,10 +542,12 @@ static int fdb_modify(int cmd, int flags, int argc, char **argv)
req.ndm.ndm_state |= NUD_REACHABLE;
req.ndm.ndm_state &= ~NUD_NOARP;
} else if (matches(*argv, "vlan") == 0) {
- if (vid >= 0)
+ if (vid_set)
duparg2("vlan", *argv);
NEXT_ARG();
- vid = atoi(*argv);
+ if (get_vlan(&vid, *argv))
+ invarg("invalid vlan", *argv);
+ vid_set = true;
} else if (matches(*argv, "use") == 0) {
req.ndm.ndm_flags |= NTF_USE;
} else if (matches(*argv, "extern_learn") == 0) {
@@ -595,7 +602,7 @@ static int fdb_modify(int cmd, int flags, int argc, char **argv)
if (dst_ok)
addattr_l(&req.n, sizeof(req), NDA_DST, &dst.data, dst.bytelen);
- if (vid >= 0)
+ if (vid_set)
addattr16(&req.n, sizeof(req), NDA_VLAN, vid);
if (nhid > 0)
addattr32(&req.n, sizeof(req), NDA_NH_ID, nhid);
@@ -644,7 +651,8 @@ static int fdb_get(int argc, char **argv)
char abuf[ETH_ALEN];
int br_ifindex = 0;
char *addr = NULL;
- short vlan = -1;
+ __u16 vlan = 0;
+ bool vlan_set = false;
char *endptr;
int ret;
@@ -669,10 +677,12 @@ static int fdb_get(int argc, char **argv)
} else if (matches(*argv, "master") == 0) {
req.ndm.ndm_flags |= NTF_MASTER;
} else if (matches(*argv, "vlan") == 0) {
- if (vlan >= 0)
+ if (vlan_set)
duparg2("vlan", *argv);
NEXT_ARG();
- vlan = atoi(*argv);
+ if (get_vlan(&vlan, *argv))
+ invarg("invalid vlan", *argv);
+ vlan_set = true;
} else if (matches(*argv, "dynamic") == 0) {
filter_dynamic = 1;
} else {
@@ -702,7 +712,7 @@ static int fdb_get(int argc, char **argv)
addattr_l(&req.n, sizeof(req), NDA_LLADDR, abuf, ETH_ALEN);
- if (vlan >= 0)
+ if (vlan_set)
addattr16(&req.n, sizeof(req), NDA_VLAN, vlan);
if (vni != ~0)
@@ -758,7 +768,9 @@ static int fdb_flush(int argc, char **argv)
};
unsigned short ndm_state_mask = 0;
unsigned short ndm_flags_mask = 0;
- short vid = -1, brport_ifidx = -1;
+ short brport_ifidx = -1;
+ __u16 vid = 0;
+ bool vid_set = false;
char *d = NULL, *brport = NULL;
unsigned short ndm_flags = 0;
unsigned short ndm_state = 0;
@@ -832,10 +844,12 @@ static int fdb_flush(int argc, char **argv)
NEXT_ARG();
brport = *argv;
} else if (strcmp(*argv, "vlan") == 0) {
- if (vid >= 0)
+ if (vid_set)
duparg2("vlan", *argv);
NEXT_ARG();
- vid = atoi(*argv);
+ if (get_vlan(&vid, *argv))
+ invarg("invalid vlan", *argv);
+ vid_set = true;
} else if (strcmp(*argv, "src_vni") == 0) {
NEXT_ARG();
src_vni = strtoul(*argv, &endptr, 0);
@@ -901,11 +915,6 @@ static int fdb_flush(int argc, char **argv)
}
}
- if (vid >= 4096) {
- fprintf(stderr, "Invalid VLAN ID \"%hu\"\n", vid);
- return -1;
- }
-
/* if self and master were not specified assume self */
if (!(ndm_flags & (NTF_SELF | NTF_MASTER)))
ndm_flags |= NTF_SELF;
@@ -914,7 +923,7 @@ static int fdb_flush(int argc, char **argv)
req.ndm.ndm_state = ndm_state;
if (brport_ifidx > -1)
addattr32(&req.n, sizeof(req), NDA_IFINDEX, brport_ifidx);
- if (vid > -1)
+ if (vid_set)
addattr16(&req.n, sizeof(req), NDA_VLAN, vid);
if (src_vni != ~0)
addattr32(&req.n, sizeof(req), NDA_SRC_VNI, src_vni);
diff --git a/bridge/mdb.c b/bridge/mdb.c
index 112deeb9..b1d6ec28 100644
--- a/bridge/mdb.c
+++ b/bridge/mdb.c
@@ -452,10 +452,14 @@ static int mdb_show(int argc, char **argv)
duparg("dev", *argv);
filter_dev = *argv;
} else if (strcmp(*argv, "vid") == 0) {
+ __u16 vid;
+
NEXT_ARG();
if (filter_vlan)
duparg("vid", *argv);
- filter_vlan = atoi(*argv);
+ if (get_vlan(&vid, *argv))
+ invarg("invalid vlan", *argv);
+ filter_vlan = vid;
}
argc--; argv++;
}
@@ -706,7 +710,7 @@ static int mdb_modify(int cmd, int flags, int argc, char **argv)
char *src_list = NULL, *proto = NULL, *dst = NULL;
struct br_mdb_entry entry = {};
bool set_attrs = false;
- short vid = 0;
+ __u16 vid = 0;
while (argc > 0) {
if (strcmp(*argv, "dev") == 0) {
@@ -725,7 +729,8 @@ static int mdb_modify(int cmd, int flags, int argc, char **argv)
;/* nothing */
} else if (strcmp(*argv, "vid") == 0) {
NEXT_ARG();
- vid = atoi(*argv);
+ if (get_vlan(&vid, *argv))
+ invarg("invalid vlan", *argv);
} else if (strcmp(*argv, "src") == 0) {
NEXT_ARG();
src = *argv;
@@ -868,7 +873,7 @@ static int mdb_get(int argc, char **argv)
struct br_mdb_entry entry = {};
struct nlmsghdr *answer;
bool get_attrs = false;
- short vid = 0;
+ __u16 vid = 0;
int ret = 0;
while (argc > 0) {
@@ -880,7 +885,8 @@ static int mdb_get(int argc, char **argv)
grp = *argv;
} else if (strcmp(*argv, "vid") == 0) {
NEXT_ARG();
- vid = atoi(*argv);
+ if (get_vlan(&vid, *argv))
+ invarg("invalid vlan", *argv);
} else if (strcmp(*argv, "src") == 0) {
NEXT_ARG();
src = *argv;
@@ -963,7 +969,7 @@ static int mdb_flush(int argc, char **argv)
struct br_mdb_entry entry = {};
unsigned short state_mask = 0;
bool set_attrs = false;
- short vid = 0;
+ __u16 vid = 0;
while (argc > 0) {
if (strcmp(*argv, "dev") == 0) {
@@ -974,7 +980,8 @@ static int mdb_flush(int argc, char **argv)
p = *argv;
} else if (strcmp(*argv, "vid") == 0) {
NEXT_ARG();
- vid = atoi(*argv);
+ if (get_vlan(&vid, *argv))
+ invarg("invalid vlan", *argv);
} else if (strcmp(*argv, "src_vni") == 0) {
NEXT_ARG();
src_vni = *argv;
diff --git a/bridge/vlan.c b/bridge/vlan.c
index 09c01153..0f0c8333 100644
--- a/bridge/vlan.c
+++ b/bridge/vlan.c
@@ -165,8 +165,10 @@ static int vlan_modify(int cmd, int argc, char **argv)
.ifm.ifi_family = PF_BRIDGE,
};
char *d = NULL;
- short vid = -1;
- short vid_end = -1;
+ __u16 vid = 0;
+ __u16 vid_end = 0;
+ bool vid_set = false;
+ bool vid_range = false;
struct rtattr *afspec;
struct bridge_vlan_info vinfo = {};
bool tunnel_info_set = false;
@@ -179,19 +181,15 @@ static int vlan_modify(int cmd, int argc, char **argv)
NEXT_ARG();
d = *argv;
} else if (strcmp(*argv, "vid") == 0) {
- char *p;
-
NEXT_ARG();
- p = strchr(*argv, '-');
- if (p) {
- *p = '\0';
- p++;
- vid = atoi(*argv);
- vid_end = atoi(p);
+ if (vid_set)
+ duparg2("vid", *argv);
+ if (get_vlan_range(&vid, &vid_end, *argv))
+ invarg("invalid vlan", *argv);
+ vid_set = true;
+ vid_range = vid_end != vid;
+ if (vid_range)
vinfo.flags |= BRIDGE_VLAN_INFO_RANGE_BEGIN;
- } else {
- vid = atoi(*argv);
- }
} else if (strcmp(*argv, "self") == 0) {
flags |= BRIDGE_FLAGS_SELF;
} else if (strcmp(*argv, "master") == 0) {
@@ -213,7 +211,7 @@ static int vlan_modify(int cmd, int argc, char **argv)
argc--; argv++;
}
- if (d == NULL || vid == -1) {
+ if (d == NULL || !vid_set) {
fprintf(stderr, "Device and VLAN ID are required arguments.\n");
return -1;
}
@@ -224,13 +222,8 @@ static int vlan_modify(int cmd, int argc, char **argv)
return -1;
}
- if (vid < 1 || vid > 4094) {
- fprintf(stderr, "Invalid VLAN ID \"%d\"\n", vid);
- return -1;
- }
-
- if (vinfo.flags & BRIDGE_VLAN_INFO_RANGE_BEGIN) {
- if (vid_end == -1 || vid_end >= 4096 || vid >= vid_end) {
+ if (vid_range) {
+ if (vid_end <= vid) {
fprintf(stderr, "Invalid VLAN range \"%hu-%hu\"\n",
vid, vid_end);
return -1;
@@ -248,10 +241,12 @@ static int vlan_modify(int cmd, int argc, char **argv)
addattr16(&req.n, sizeof(req), IFLA_BRIDGE_FLAGS, flags);
if (tunnel_info_set)
- add_tunnel_info_range(&req.n, sizeof(req), vid, vid_end,
+ add_tunnel_info_range(&req.n, sizeof(req), vid,
+ vid_range ? vid_end : -1,
tun_id_start, tun_id_end);
else
- add_vlan_info_range(&req.n, sizeof(req), vid, vid_end,
+ add_vlan_info_range(&req.n, sizeof(req), vid,
+ vid_range ? vid_end : -1,
vinfo.flags);
addattr_nest_end(&req.n, afspec);
@@ -277,7 +272,7 @@ static int vlan_option_set(int argc, char **argv)
struct bridge_vlan_info vinfo = {};
struct rtattr *afspec;
char *d = NULL;
- short vid = -1;
+ __u16 vid = 0;
afspec = addattr_nest(&req.n, sizeof(req), BRIDGE_VLANDB_ENTRY);
afspec->rta_type |= NLA_F_NESTED;
@@ -293,35 +288,19 @@ static int vlan_option_set(int argc, char **argv)
return -1;
}
} else if (strcmp(*argv, "vid") == 0) {
- short vid_end = -1;
- char *p;
+ __u16 vid_end = 0;
+ bool vid_range = false;
NEXT_ARG();
- p = strchr(*argv, '-');
- if (p) {
- *p = '\0';
- p++;
- vid = atoi(*argv);
- vid_end = atoi(p);
- if (vid >= vid_end || vid_end >= 4096) {
- fprintf(stderr, "Invalid VLAN range \"%hu-%hu\"\n",
- vid, vid_end);
- return -1;
- }
- } else {
- vid = atoi(*argv);
- }
- if (vid >= 4096) {
- fprintf(stderr, "Invalid VLAN ID \"%hu\"\n",
- vid);
- return -1;
- }
+ if (get_vlan_range(&vid, &vid_end, *argv))
+ invarg("invalid vlan", *argv);
+ vid_range = vid_end != vid;
vinfo.flags = BRIDGE_VLAN_INFO_ONLY_OPTS;
vinfo.vid = vid;
addattr_l(&req.n, sizeof(req), BRIDGE_VLANDB_ENTRY_INFO,
&vinfo, sizeof(vinfo));
- if (vid_end != -1)
+ if (vid_range)
addattr16(&req.n, sizeof(req),
BRIDGE_VLANDB_ENTRY_RANGE, vid_end);
} else if (strcmp(*argv, "state") == 0) {
@@ -400,9 +379,9 @@ static int vlan_global_option_set(int argc, char **argv)
.bvm.family = PF_BRIDGE,
};
struct rtattr *afspec;
- short vid_end = -1;
+ __u16 vid_end = 0;
char *d = NULL;
- short vid = -1;
+ __u16 vid = 0;
__u64 val64;
__u32 val32;
__u16 val16;
@@ -422,31 +401,15 @@ static int vlan_global_option_set(int argc, char **argv)
return -1;
}
} else if (strcmp(*argv, "vid") == 0) {
- char *p;
+ bool vid_range = false;
NEXT_ARG();
- p = strchr(*argv, '-');
- if (p) {
- *p = '\0';
- p++;
- vid = atoi(*argv);
- vid_end = atoi(p);
- if (vid >= vid_end || vid_end >= 4096) {
- fprintf(stderr, "Invalid VLAN range \"%hu-%hu\"\n",
- vid, vid_end);
- return -1;
- }
- } else {
- vid = atoi(*argv);
- }
- if (vid >= 4096) {
- fprintf(stderr, "Invalid VLAN ID \"%hu\"\n",
- vid);
- return -1;
- }
+ if (get_vlan_range(&vid, &vid_end, *argv))
+ invarg("invalid vlan", *argv);
+ vid_range = vid_end != vid;
addattr16(&req.n, sizeof(req), BRIDGE_VLANDB_GOPTS_ID,
vid);
- if (vid_end != -1)
+ if (vid_range)
addattr16(&req.n, sizeof(req),
BRIDGE_VLANDB_GOPTS_RANGE, vid_end);
} else if (strcmp(*argv, "mcast_snooping") == 0) {
@@ -1102,10 +1065,14 @@ static int vlan_show(int argc, char **argv, int subject)
duparg("dev", *argv);
filter_dev = *argv;
} else if (strcmp(*argv, "vid") == 0) {
+ __u16 vid;
+
NEXT_ARG();
if (filter_vlan)
duparg("vid", *argv);
- filter_vlan = atoi(*argv);
+ if (get_vlan(&vid, *argv))
+ invarg("invalid vlan", *argv);
+ filter_vlan = vid;
}
argc--; argv++;
}
@@ -1219,10 +1186,14 @@ static int vlan_global_show(int argc, char **argv)
duparg("dev", *argv);
filter_dev = *argv;
} else if (strcmp(*argv, "vid") == 0) {
+ __u16 vid;
+
NEXT_ARG();
if (filter_vlan)
duparg("vid", *argv);
- filter_vlan = atoi(*argv);
+ if (get_vlan(&vid, *argv))
+ invarg("invalid vlan", *argv);
+ filter_vlan = vid;
}
argc--; argv++;
}
diff --git a/include/utils.h b/include/utils.h
index e4e318e2..06d5242f 100644
--- a/include/utils.h
+++ b/include/utils.h
@@ -151,6 +151,8 @@ int get_u32(__u32 *val, const char *arg, int base);
int get_s32(__s32 *val, const char *arg, int base);
int get_u16(__u16 *val, const char *arg, int base);
int get_u8(__u8 *val, const char *arg, int base);
+int get_vlan(__u16 *val, const char *arg);
+int get_vlan_range(__u16 *start, __u16 *end, const char *arg);
int get_be64(__be64 *val, const char *arg, int base);
int get_be32(__be32 *val, const char *arg, int base);
int get_be16(__be16 *val, const char *arg, int base);
diff --git a/lib/utils.c b/lib/utils.c
index 1215fe31..debba751 100644
--- a/lib/utils.c
+++ b/lib/utils.c
@@ -387,6 +387,57 @@ int get_u16(__u16 *val, const char *arg, int base)
return 0;
}
+int get_vlan(__u16 *val, const char *arg)
+{
+ if (get_u16(val, arg, 10))
+ return -1;
+
+ if (*val < 1 || *val > 4094)
+ return -1;
+
+ return 0;
+}
+
+int get_vlan_range(__u16 *start, __u16 *end, const char *arg)
+{
+ char *start_arg;
+ const char *sep;
+ size_t len;
+ int ret;
+
+ if (!arg)
+ return -1;
+
+ sep = strchr(arg, '-');
+ if (!sep) {
+ ret = get_vlan(start, arg);
+ if (!ret)
+ *end = *start;
+ return ret;
+ }
+
+ len = sep - arg;
+ if (!len || !sep[1])
+ return -1;
+
+ start_arg = malloc(len + 1);
+ if (!start_arg)
+ return -1;
+
+ memcpy(start_arg, arg, len);
+ start_arg[len] = '\0';
+
+ ret = get_vlan(start, start_arg) || get_vlan(end, sep + 1);
+ free(start_arg);
+ if (ret)
+ return -1;
+
+ if (*start >= *end)
+ return -1;
+
+ return 0;
+}
+
int get_u8(__u8 *val, const char *arg, int base)
{
unsigned long res;
--
2.39.3 (Apple Git-145)
next reply other threads:[~2026-08-01 2:40 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-08-01 2:40 William Gonzalez [this message]
2026-08-02 18:44 ` [PATCH] bridge: tighten VLAN parsing Stephen Hemminger
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20260801024051.10581-1-gonzalez.williamalexander1@gmail.com \
--to=gonzalez.williamalexander1@gmail.com \
--cc=netdev@vger.kernel.org \
--cc=stephen@networkplumber.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox