-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA512 On 09/25/14 15:40, Mateusz Guzik wrote: > On Thu, Sep 25, 2014 at 10:37:28PM +0000, Xin LI wrote: >> Author: delphij Date: Thu Sep 25 22:37:27 2014 New Revision: >> 272144 URL: http://svnweb.freebsd.org/changeset/base/272144 >> >> Log: The strtol(3) family of functions would set errno when it >> hits one. Check errno and handle it as invalid input. >> > > But this requires explicitely setting errno to 0 before strto* > call, otherwise you cannot know whether errno is meaningful when > you test it.
Fixed in r272145, good catch! > Also it looks like the code would use some deduplications with > macros or something. I think the code can use some refactor. Does the attached patch look good to you? Cheers, - -- Xin LI <delp...@delphij.net> https://www.delphij.net/ FreeBSD - The Power to Serve! Live free or die -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0 iQIcBAEBCgAGBQJUJLEeAAoJEJW2GBstM+nsSD0P/3Oe4gzNc1AYy14cg8ewZ0nL w75gPr9rMABxMXk7tGut6qS+W1vTZ32g9rCpzE+7ra1A8kDZ9l+VkJbUwkN2nr/i udsMq+qQ1qB+dCsVJLm8Pjsh/g+AVlSxXdHrkD1u49XhANZQUGt9NBaO0ah84c5J vaZ+Sq7cR6fF2VrZDJ0MLxW8tHLgW6hIVORvyiohvOqhzBHOFihDEDcoUOamzFZI txqI2oxjuhGy+T1V4dVYso/PgYhhnFC4L9Lvx5E9EqUZrVzckPP1PYXiZiQR/i7H ZEV5RgxHOGXEYNVDZXoEQthE1iP3pzFhe/xNURzlUqZIwgPeJ6Q2xz+As/C0yXlT +LxmXdQ/zsQmRu5b2m7TvBjBp18znAR2c2mKbvHS3EX6UWEmUKwcb2CREm0AbnQL kFrLRsksJfxD1TCkfmqysOa6WSkvRQpJ4csZdKf9y5nZFh/yjdSnzJULLKphpbzr npc9ZYJ9m7bLj7OZF5UPwEr0JfTnRDn6VnMqKiUTgXpHk5Lpm7vm5WefdOWMyTmL nwtmU3SyaJdLBrWARY1ZyN0ENmzTrs3sjCyWvhwlAomYIVuahT1oCuaRYIDyzito mThiFbuRA+ufz6m8OcIXeiGLxkimdIHm68ZreNZlBHA/R/bH84vLiIN0HRNhgAOy bgc97yetD5PL8NhRC8G/ =PWTt -----END PGP SIGNATURE-----
Index: sysctl.c
===================================================================
--- sysctl.c (revision 272145)
+++ sysctl.c (working copy)
@@ -57,6 +57,7 @@ static const char rcsid[] =
#include <machine/pc/bios.h>
#endif
+#include <assert.h>
#include <ctype.h>
#include <err.h>
#include <errno.h>
@@ -80,8 +81,32 @@ static int show_var(int *, int);
static int sysctl_all(int *oid, int len);
static int name2oid(const char *, int *);
-static int set_IK(const char *, int *);
+static int strIKtoi(const char *, char **);
+static int ctl_sign[CTLTYPE+1] = {
+ [CTLTYPE_INT] = 1,
+ [CTLTYPE_LONG] = 1,
+ [CTLTYPE_S64] = 1,
+};
+
+static int ctl_size[CTLTYPE+1] = {
+ [CTLTYPE_INT] = sizeof(int),
+ [CTLTYPE_UINT] = sizeof(u_int),
+ [CTLTYPE_LONG] = sizeof(long),
+ [CTLTYPE_ULONG] = sizeof(u_long),
+ [CTLTYPE_S64] = sizeof(int64_t),
+ [CTLTYPE_U64] = sizeof(uint64_t),
+};
+
+static const char *ctl_typename[CTLTYPE+1] = {
+ [CTLTYPE_INT] = "integer",
+ [CTLTYPE_UINT] = "unsigned integer",
+ [CTLTYPE_LONG] = "long integer",
+ [CTLTYPE_ULONG] = "unsigned long",
+ [CTLTYPE_S64] = "int64_t",
+ [CTLTYPE_U64] = "uint64_t",
+};
+
static void
usage(void)
{
@@ -191,7 +216,8 @@ static int
parse(const char *string, int lineno)
{
int len, i, j;
- void *newval = 0;
+ const void *newval = NULL;
+ const char *newvalstr = NULL;
int intval;
unsigned int uintval;
long longval;
@@ -230,7 +256,7 @@ parse(const char *string, int lineno)
cp[strlen(cp) - 1] = '\0';
cp++;
}
- newval = cp;
+ newvalstr = cp;
newsize = strlen(cp);
}
len = name2oid(bufp, mib);
@@ -254,7 +280,7 @@ parse(const char *string, int lineno)
exit(1);
}
- if (newval == NULL || dflag) {
+ if (newvalstr == NULL || dflag) {
if ((kind & CTLTYPE) == CTLTYPE_NODE) {
if (dflag) {
i = show_var(mib, len);
@@ -288,10 +314,22 @@ parse(const char *string, int lineno)
(kind & CTLTYPE) == CTLTYPE_ULONG ||
(kind & CTLTYPE) == CTLTYPE_S64 ||
(kind & CTLTYPE) == CTLTYPE_U64) {
- if (strlen(newval) == 0) {
+ if (strlen(newvalstr) == 0) {
warnx("empty numeric value");
return (1);
}
+ } else if ((kind & CTLTYPE) != CTLTYPE_STRING) {
+ /*
+ * The only acceptable types are:
+ * CTLTYPE_INT, CTLTYPE_UINT,
+ * CTLTYPE_LONG, CTLTYPE_ULONG,
+ * CTLTYPE_S64, CTLTYPE_U64 and
+ * CTLTYPE_STRING.
+ */
+ warnx("oid '%s' is type %d,"
+ " cannot set that%s", bufp,
+ kind & CTLTYPE, line);
+ return (1);
}
errno = 0;
@@ -298,91 +336,53 @@ parse(const char *string, int lineno)
switch (kind & CTLTYPE) {
case CTLTYPE_INT:
- if (strcmp(fmt, "IK") == 0) {
- if (!set_IK(newval, &intval)) {
- warnx("invalid value '%s'%s",
- (char *)newval, line);
- return (1);
- }
- } else {
- intval = (int)strtol(newval, &endptr,
+ if (strcmp(fmt, "IK") == 0)
+ intval = strIKtoi(newvalstr, &endptr);
+ else
+ intval = (int)strtol(newvalstr, &endptr,
0);
- if (errno != 0 || endptr == newval ||
- *endptr != '\0') {
- warnx("invalid integer '%s'%s",
- (char *)newval, line);
- return (1);
- }
- }
newval = &intval;
newsize = sizeof(intval);
break;
case CTLTYPE_UINT:
- uintval = (int) strtoul(newval, &endptr, 0);
- if (errno != 0 || endptr == newval ||
- *endptr != '\0') {
- warnx("invalid unsigned integer '%s'%s",
- (char *)newval, line);
- return (1);
- }
+ uintval = (int) strtoul(newvalstr, &endptr, 0);
newval = &uintval;
newsize = sizeof(uintval);
break;
case CTLTYPE_LONG:
- longval = strtol(newval, &endptr, 0);
- if (errno != 0 || endptr == newval ||
- *endptr != '\0') {
- warnx("invalid long integer '%s'%s",
- (char *)newval, line);
- return (1);
- }
+ longval = strtol(newvalstr, &endptr, 0);
newval = &longval;
newsize = sizeof(longval);
break;
case CTLTYPE_ULONG:
- ulongval = strtoul(newval, &endptr, 0);
- if (errno != 0 || endptr == newval ||
- *endptr != '\0') {
- warnx("invalid unsigned long integer"
- " '%s'%s", (char *)newval, line);
- return (1);
- }
+ ulongval = strtoul(newvalstr, &endptr, 0);
newval = &ulongval;
newsize = sizeof(ulongval);
break;
case CTLTYPE_STRING:
+ newval = newvalstr;
break;
case CTLTYPE_S64:
- i64val = strtoimax(newval, &endptr, 0);
- if (errno != 0 || endptr == newval ||
- *endptr != '\0') {
- warnx("invalid int64_t '%s'%s",
- (char *)newval, line);
- return (1);
- }
+ i64val = strtoimax(newvalstr, &endptr, 0);
newval = &i64val;
newsize = sizeof(i64val);
break;
case CTLTYPE_U64:
- u64val = strtoumax(newval, &endptr, 0);
- if (errno != 0 || endptr == newval ||
- *endptr != '\0') {
- warnx("invalid uint64_t '%s'%s",
- (char *)newval, line);
- return (1);
- }
+ u64val = strtoumax(newvalstr, &endptr, 0);
newval = &u64val;
newsize = sizeof(u64val);
break;
- case CTLTYPE_OPAQUE:
- /* FALLTHROUGH */
default:
- warnx("oid '%s' is type %d,"
- " cannot set that%s", bufp,
- kind & CTLTYPE, line);
+ /* NOTREACHED */
return (1);
}
+ if (errno != 0 || endptr == newvalstr || *endptr != '\0') {
+ warnx("invalid %s '%s'%s", ctl_typename[kind & CTLTYPE],
+ newvalstr, line);
+ return (1);
+ }
+
i = show_var(mib, len);
if (sysctl(mib, len, 0, 0, newval, newsize) == -1) {
if (!i && !bflag)
@@ -665,33 +665,35 @@ S_bios_smap_xattr(size_t l2, void *p)
#endif
static int
-set_IK(const char *str, int *val)
+strIKtoi(const char *str, char **endptrp)
{
+ int kelv;
float temp;
- int len, kelv;
+ size_t len;
const char *p;
- char *endptr;
- if ((len = strlen(str)) == 0)
- return (0);
+ assert(errno == 0);
+
+ len = strlen(str);
+ /* caller already checked this */
+ assert(len > 0);
+
p = &str[len - 1];
- errno = 0;
if (*p == 'C' || *p == 'F') {
- temp = strtof(str, &endptr);
- if (errno != 0 || endptr == str ||
- endptr != p)
- return (0);
- if (*p == 'F')
- temp = (temp - 32) * 5 / 9;
- kelv = temp * 10 + 2732;
+ temp = strtof(str, endptrp);
+ if (*endptrp != str && *endptrp == p && errno != 0) {
+ if (*p == 'F')
+ temp = (temp - 32) * 5 / 9;
+ return (temp * 10 + 2732);
+ }
} else {
- kelv = (int)strtol(str, &endptr, 10);
- if (errno != 0 || endptr == str ||
- *endptr != '\0')
- return (0);
+ kelv = (int)strtol(str, endptrp, 10);
+ if (*endptrp != str && *endptrp == p && errno != 0)
+ return (kelv);
}
- *val = kelv;
- return (1);
+
+ errno = ERANGE;
+ return (0);
}
/*
@@ -746,21 +748,6 @@ oidfmt(int *oid, int len, char *fmt, u_int *kind)
return (0);
}
-static int ctl_sign[CTLTYPE+1] = {
- [CTLTYPE_INT] = 1,
- [CTLTYPE_LONG] = 1,
- [CTLTYPE_S64] = 1,
-};
-
-static int ctl_size[CTLTYPE+1] = {
- [CTLTYPE_INT] = sizeof(int),
- [CTLTYPE_UINT] = sizeof(u_int),
- [CTLTYPE_LONG] = sizeof(long),
- [CTLTYPE_ULONG] = sizeof(u_long),
- [CTLTYPE_S64] = sizeof(int64_t),
- [CTLTYPE_U64] = sizeof(int64_t),
-};
-
/*
* This formats and outputs the value of one variable
*
sysctl.diff.sig
Description: Binary data
_______________________________________________ svn-src-head@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
