| 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: Tue, 12 Apr 2011 10:31:24 -0700
From: Stephen Hemminger <shemminger@...tta.com>
To: tgraf@...hat.com
Cc: netdev@...r.kernel.org
Subject: Re: [RFC] iproute2: Fix meta match u32 with 0xffffffff
On Tue, 12 Apr 2011 19:22:05 +0200
Thomas Graf <tgraf@...hat.com> wrote:
> On Tue, 2011-04-12 at 08:19 -0700, Stephen Hemminger wrote:
>
> > >
> > > This is definitely much better but we still can't parse ULONG_MAX
> > > as string representative. Checking glibc docs, the only way to do it is
> > > to ignore the return value for error checking and look errno.
> > >
> >
> > I think the error case is ret == ULONG_MAX && errno == ERANGE
> > If there is no error, then strtoul doesn't set errno.
>
> That's ok too but your patch adds ret == ULONG_MAX || errno == ERANGE
> which will not allow to parse ULONG_MAX as a string. You probably
> still have to clear errno to 0 before calling strtoul in case
> ULONG_MAX is meant as legitimate return value but a previous glibc
> function has left errno set to ERANGE.
>
I changed to && in final version.
Still needs more work because most of the code assumes that
unsigned long can be used for mask etc, when infact the code only
allows for u32.
>From 63da1ed89970af18383c7318deb38bdb062fe74e Mon Sep 17 00:00:00 2001
From: Stephen Hemminger <shemminger@...tta.com>
Date: Mon, 11 Apr 2011 11:48:55 -0700
Subject: [PATCH 1/2] Fix meta match u32 with 0xffffffff
The value 0xffffffff is a valid mask and bstrtoul() would return
ULONG_MAX which was the error value.
---
tc/em_cmp.c | 12 ++++--------
tc/em_meta.c | 9 +++------
tc/em_nbyte.c | 6 ++----
tc/em_u32.c | 19 +++++++++----------
tc/m_ematch.c | 17 +++++++++++------
tc/m_ematch.h | 2 +-
6 files changed, 30 insertions(+), 35 deletions(-)
diff --git a/tc/em_cmp.c b/tc/em_cmp.c
index 6addce0..af3e591 100644
--- a/tc/em_cmp.c
+++ b/tc/em_cmp.c
@@ -69,8 +69,7 @@ static int cmp_parse_eopt(struct nlmsghdr *n, struct tcf_ematch_hdr *hdr,
return PARSE_ERR(a, "cmp: missing argument");
a = bstr_next(a);
- offset = bstrtoul(a);
- if (offset == ULONG_MAX)
+ if (bstrtoul(a, &offset) < 0)
return PARSE_ERR(a, "cmp: invalid offset, " \
"must be numeric");
@@ -82,8 +81,7 @@ static int cmp_parse_eopt(struct nlmsghdr *n, struct tcf_ematch_hdr *hdr,
layer = parse_layer(a);
if (layer == INT_MAX) {
- layer = bstrtoul(a);
- if (layer == ULONG_MAX)
+ if (bstrtoul(a, &layer) < 0)
return PARSE_ERR(a, "cmp: invalid " \
"layer");
}
@@ -96,8 +94,7 @@ static int cmp_parse_eopt(struct nlmsghdr *n, struct tcf_ematch_hdr *hdr,
return PARSE_ERR(a, "cmp: missing argument");
a = bstr_next(a);
- mask = bstrtoul(a);
- if (mask == ULONG_MAX)
+ if (bstrtoul(a, &mask) < 0)
return PARSE_ERR(a, "cmp: invalid mask");
} else if (!bstrcmp(a, "trans")) {
cmp.flags |= TCF_EM_CMP_TRANS;
@@ -115,8 +112,7 @@ static int cmp_parse_eopt(struct nlmsghdr *n, struct tcf_ematch_hdr *hdr,
return PARSE_ERR(a, "cmp: missing argument");
a = bstr_next(a);
- value = bstrtoul(a);
- if (value == ULONG_MAX)
+ if (bstrtoul(a, &value) < 0)
return PARSE_ERR(a, "cmp: invalid value");
value_present = 1;
diff --git a/tc/em_meta.c b/tc/em_meta.c
index 033e29f..276223a 100644
--- a/tc/em_meta.c
+++ b/tc/em_meta.c
@@ -260,8 +260,7 @@ parse_object(struct bstr *args, struct bstr *arg, struct tcf_meta_val *obj,
return bstr_next(arg);
}
- num = bstrtoul(arg);
- if (num != ULONG_MAX) {
+ if (bstrtoul(arg, &num) < 0) {
obj->kind = TCF_META_TYPE_INT << 12;
obj->kind |= TCF_META_ID_VALUE;
*dst = (unsigned long) num;
@@ -318,8 +317,7 @@ compatible:
}
a = bstr_next(a);
- shift = bstrtoul(a);
- if (shift == ULONG_MAX) {
+ if (bstrtoul(a, &shift) < 0) {
PARSE_ERR(a, "meta: invalid shift, must " \
"be numeric");
return PARSE_FAILURE;
@@ -336,8 +334,7 @@ compatible:
}
a = bstr_next(a);
- mask = bstrtoul(a);
- if (mask == ULONG_MAX) {
+ if (bstrtoul(a, &mask) < 0) {
PARSE_ERR(a, "meta: invalid mask, must be " \
"numeric");
return PARSE_FAILURE;
diff --git a/tc/em_nbyte.c b/tc/em_nbyte.c
index 87f3e9d..9a52ffc 100644
--- a/tc/em_nbyte.c
+++ b/tc/em_nbyte.c
@@ -63,8 +63,7 @@ static int nbyte_parse_eopt(struct nlmsghdr *n, struct tcf_ematch_hdr *hdr,
return PARSE_ERR(a, "nbyte: missing argument");
a = bstr_next(a);
- offset = bstrtoul(a);
- if (offset == ULONG_MAX)
+ if (bstrtoul(a, &offset) < 0)
return PARSE_ERR(a, "nbyte: invalid offset, " \
"must be numeric");
@@ -76,8 +75,7 @@ static int nbyte_parse_eopt(struct nlmsghdr *n, struct tcf_ematch_hdr *hdr,
layer = parse_layer(a);
if (layer == INT_MAX) {
- layer = bstrtoul(a);
- if (layer == ULONG_MAX)
+ if (bstrtoul(a, &layer) < 0)
return PARSE_ERR(a, "nbyte: invalid " \
"layer");
}
diff --git a/tc/em_u32.c b/tc/em_u32.c
index 21ed70f..88b5fa1 100644
--- a/tc/em_u32.c
+++ b/tc/em_u32.c
@@ -33,6 +33,7 @@ static void u32_print_usage(FILE *fd)
"Example: u32(u16 0x1122 0xffff at nexthdr+4)\n");
}
+
static int u32_parse_eopt(struct nlmsghdr *n, struct tcf_ematch_hdr *hdr,
struct bstr *args)
{
@@ -62,16 +63,14 @@ static int u32_parse_eopt(struct nlmsghdr *n, struct tcf_ematch_hdr *hdr,
if (a == NULL)
return PARSE_ERR(a, "u32: missing key");
- key = bstrtoul(a);
- if (key == ULONG_MAX)
+ if (bstrtoul(a, &key) < 0)
return PARSE_ERR(a, "u32: invalid key, must be numeric");
a = bstr_next(a);
if (a == NULL)
return PARSE_ERR(a, "u32: missing mask");
- mask = bstrtoul(a);
- if (mask == ULONG_MAX)
+ if (bstrtoul(a, &mask) < 0)
return PARSE_ERR(a, "u32: invalid mask, must be numeric");
a = bstr_next(a);
@@ -92,12 +91,12 @@ static int u32_parse_eopt(struct nlmsghdr *n, struct tcf_ematch_hdr *hdr,
a = bstr_next(a);
if (a == NULL)
return PARSE_ERR(a, "u32: missing offset");
- offset = bstrtoul(a);
- } else
- offset = bstrtoul(a);
-
- if (offset == ULONG_MAX)
- return PARSE_ERR(a, "u32: invalid offset");
+ if (bstrtoul(a, &offset) < 0)
+ return PARSE_ERR(a, "u32: invalid offset");
+ } else {
+ if (bstrtoul(a, &offset) < 0)
+ return PARSE_ERR(a, "u32: invalid offset");
+ }
if (a->next)
return PARSE_ERR(a->next, "u32: unexpected trailer");
diff --git a/tc/m_ematch.c b/tc/m_ematch.c
index 4c3acf8..b7eed18 100644
--- a/tc/m_ematch.c
+++ b/tc/m_ematch.c
@@ -510,20 +510,25 @@ struct bstr * bstr_alloc(const char *text)
return b;
}
-unsigned long bstrtoul(const struct bstr *b)
+int bstrtoul(const struct bstr *b, unsigned long *lp)
{
char *inv = NULL;
- unsigned long l;
char buf[b->len+1];
+ if (b->len == 0)
+ return -EINVAL;
+
memcpy(buf, b->data, b->len);
buf[b->len] = '\0';
- l = strtoul(buf, &inv, 0);
- if (l == ULONG_MAX || inv == buf)
- return ULONG_MAX;
+ *lp = strtoul(buf, &inv, 0);
+ if (inv == buf)
+ return -EINVAL;
+
+ if (*lp == ULONG_MAX && errno == ERANGE)
+ return -ERANGE;
- return l;
+ return 0;
}
void bstr_print(FILE *fd, const struct bstr *b, int ascii)
diff --git a/tc/m_ematch.h b/tc/m_ematch.h
index 5036e9b..e676290 100644
--- a/tc/m_ematch.h
+++ b/tc/m_ematch.h
@@ -49,7 +49,7 @@ static inline struct bstr *bstr_next(struct bstr *b)
return b->next;
}
-extern unsigned long bstrtoul(const struct bstr *b);
+extern int bstrtoul(const struct bstr *b, unsigned long *lp);
extern void bstr_print(FILE *fd, const struct bstr *b, int ascii);
--
1.7.1
--
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