* [PATCH 1/4] bitmap_parselist: don't calculate length of the input string
2018-12-23 9:44 [PATCH 0/4] rework bitmap_parselist Yuri Norov
@ 2018-12-23 9:44 ` Yuri Norov
2018-12-23 9:44 ` [PATCH 2/4] bitmap_parselist: move part of logic to helpers Yuri Norov
` (2 subsequent siblings)
3 siblings, 0 replies; 9+ messages in thread
From: Yuri Norov @ 2018-12-23 9:44 UTC (permalink / raw)
To: Andrew Morton, Andy Shevchenko, Rasmus Villemoes, Arnd Bergmann,
Kees Cook, Matthew Wilcox, Tetsuo Handa
Cc: Yuri Norov, linux-kernel@vger.kernel.org
bitmap_parselist() calculates length of the input string before passing
it to the __bitmap_parselist(). But the end-of-line condition is checked
for every character in __bitmap_parselist() anyway. So doing it in wrapper
is a simple waste of time.
Signed-off-by: Yury Norov <ynorov@marvell.com>
---
lib/bitmap.c | 5 +----
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/lib/bitmap.c b/lib/bitmap.c
index eead55aa7170..ad43ba397c58 100644
--- a/lib/bitmap.c
+++ b/lib/bitmap.c
@@ -614,10 +614,7 @@ static int __bitmap_parselist(const char *buf, unsigned int buflen,
int bitmap_parselist(const char *bp, unsigned long *maskp, int nmaskbits)
{
- char *nl = strchrnul(bp, '\n');
- int len = nl - bp;
-
- return __bitmap_parselist(bp, len, 0, maskp, nmaskbits);
+ return __bitmap_parselist(bp, UINT_MAX, 0, maskp, nmaskbits);
}
EXPORT_SYMBOL(bitmap_parselist);
--
2.17.1
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH 2/4] bitmap_parselist: move part of logic to helpers
2018-12-23 9:44 [PATCH 0/4] rework bitmap_parselist Yuri Norov
2018-12-23 9:44 ` [PATCH 1/4] bitmap_parselist: don't calculate length of the input string Yuri Norov
@ 2018-12-23 9:44 ` Yuri Norov
2018-12-23 9:44 ` [PATCH 3/4] bitmap_parselist: rework input string parser Yuri Norov
2018-12-23 9:45 ` [PATCH 4/4] test_bitmap: add testcases for bitmap_parselist Yuri Norov
3 siblings, 0 replies; 9+ messages in thread
From: Yuri Norov @ 2018-12-23 9:44 UTC (permalink / raw)
To: Andrew Morton, Andy Shevchenko, Rasmus Villemoes, Arnd Bergmann,
Kees Cook, Matthew Wilcox, Tetsuo Handa
Cc: Yuri Norov, linux-kernel@vger.kernel.org
Move region checking and setting functionality of __bitmap_parselist()
to helpers.
Signed-off-by: Yury Norov <ynorov@marvell.com>
---
lib/bitmap.c | 64 +++++++++++++++++++++++++++++++++++++++++++---------
1 file changed, 53 insertions(+), 11 deletions(-)
diff --git a/lib/bitmap.c b/lib/bitmap.c
index ad43ba397c58..a60fd9723677 100644
--- a/lib/bitmap.c
+++ b/lib/bitmap.c
@@ -477,6 +477,42 @@ int bitmap_print_to_pagebuf(bool list, char *buf, const unsigned long *maskp,
}
EXPORT_SYMBOL(bitmap_print_to_pagebuf);
+/*
+ * Region 9-38:4/10 describes the following bitmap structure:
+ * 0 9 12 18 38
+ * .........****......****......****......
+ * ^ ^ ^ ^
+ * start off grlen end
+ */
+struct region {
+ unsigned int start;
+ unsigned int off;
+ unsigned int grlen;
+ unsigned int end;
+};
+
+static int bitmap_set_region(const struct region *r,
+ unsigned long *bitmap, int nbits)
+{
+ unsigned int start;
+
+ if (r->end >= nbits)
+ return -ERANGE;
+
+ for (start = r->start; start <= r->end; start += r->grlen)
+ bitmap_set(bitmap, start, min(r->end - start + 1, r->off));
+
+ return 0;
+}
+
+static int bitmap_check_region(const struct region *r)
+{
+ if (r->start > r->end || r->grlen == 0 || r->off > r->grlen)
+ return -EINVAL;
+
+ return 0;
+}
+
/**
* __bitmap_parselist - convert list format ASCII string to bitmap
* @buf: read nul-terminated user string from this buffer
@@ -507,10 +543,11 @@ static int __bitmap_parselist(const char *buf, unsigned int buflen,
int nmaskbits)
{
unsigned int a, b, old_a, old_b;
- unsigned int group_size, used_size, off;
+ unsigned int group_size, used_size;
int c, old_c, totaldigits, ndigits;
const char __user __force *ubuf = (const char __user __force *)buf;
- int at_start, in_range, in_partial_range;
+ int at_start, in_range, in_partial_range, ret;
+ struct region r;
totaldigits = c = 0;
old_a = old_b = 0;
@@ -599,15 +636,20 @@ static int __bitmap_parselist(const char *buf, unsigned int buflen,
/* if no digit is after '-', it's wrong*/
if (at_start && in_range)
return -EINVAL;
- if (!(a <= b) || group_size == 0 || !(used_size <= group_size))
- return -EINVAL;
- if (b >= nmaskbits)
- return -ERANGE;
- while (a <= b) {
- off = min(b - a + 1, used_size);
- bitmap_set(maskp, a, off);
- a += group_size;
- }
+
+ r.start = a;
+ r.off = used_size;
+ r.grlen = group_size;
+ r.end = b;
+
+ ret = bitmap_check_region(&r);
+ if (ret)
+ return ret;
+
+ ret = bitmap_set_region(&r, maskp, nmaskbits);
+ if (ret)
+ return ret;
+
} while (buflen && c == ',');
return 0;
}
--
2.17.1
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH 3/4] bitmap_parselist: rework input string parser
2018-12-23 9:44 [PATCH 0/4] rework bitmap_parselist Yuri Norov
2018-12-23 9:44 ` [PATCH 1/4] bitmap_parselist: don't calculate length of the input string Yuri Norov
2018-12-23 9:44 ` [PATCH 2/4] bitmap_parselist: move part of logic to helpers Yuri Norov
@ 2018-12-23 9:44 ` Yuri Norov
2018-12-23 13:02 ` kbuild test robot
2019-01-09 16:01 ` Andy Shevchenko
2018-12-23 9:45 ` [PATCH 4/4] test_bitmap: add testcases for bitmap_parselist Yuri Norov
3 siblings, 2 replies; 9+ messages in thread
From: Yuri Norov @ 2018-12-23 9:44 UTC (permalink / raw)
To: Andrew Morton, Andy Shevchenko, Rasmus Villemoes, Arnd Bergmann,
Kees Cook, Matthew Wilcox, Tetsuo Handa
Cc: Yuri Norov, linux-kernel@vger.kernel.org
The requirement for this rework is to keep the __bitmap_parselist()
copy-less and single-pass but make it more readable and maintainable by
splitting into logical parts and removing explicit nested cycles and
opaque local variables.
__bitmap_parselist() can parse userspace inputs and therefore we cannot
use simple_strtoul() to parse numbers.
Signed-off-by: Yury Norov <ynorov@marvell.com>
---
lib/bitmap.c | 247 ++++++++++++++++++++++++++++++---------------------
1 file changed, 148 insertions(+), 99 deletions(-)
diff --git a/lib/bitmap.c b/lib/bitmap.c
index a60fd9723677..edc7068c28a6 100644
--- a/lib/bitmap.c
+++ b/lib/bitmap.c
@@ -513,6 +513,140 @@ static int bitmap_check_region(const struct region *r)
return 0;
}
+static long get_char(char *c, const char *str, int is_user)
+{
+ if (unlikely(is_user))
+ return __get_user(*c, str);
+
+ *c = *str;
+ return 0;
+}
+
+static const char *bitmap_getnum(unsigned int *num, const char *str,
+ const char *const end, int is_user)
+{
+ unsigned int n = 0;
+ const char *_str;
+ long ret;
+ char c;
+
+ for (_str = str, *num = 0; _str <= end; _str++) {
+ ret = get_char(&c, _str, is_user);
+ if (ret)
+ return (char *)ret;
+
+ if (!isdigit(c)) {
+ if (_str == str)
+ return (char *)-EINVAL;
+
+ goto out;
+ }
+
+ *num = *num * 10 + (c - '0');
+ if (*num < n)
+ return (char *)-EOVERFLOW;
+
+ n = *num;
+ }
+
+out:
+ return _str;
+}
+
+static const char *bitmap_find_region(const char *str,
+ const char *const end, int is_user)
+{
+ long ret;
+ char c;
+
+ for (; str < end; str++) {
+ ret = get_char(&c, str, is_user);
+ if (ret)
+ return (char *)ret;
+
+ /* Unexpected end of line. */
+ if (c == 0 || c == '\n')
+ return NULL;
+
+ /*
+ * The format allows commas and whitespases
+ * at the beginning of the region, so skip it.
+ */
+ if (!isspace(c) && c != ',')
+ break;
+ }
+
+ return str;
+}
+
+static const char *bitmap_parse_region(struct region *r, const char *str,
+ const char *const end, int is_user)
+{
+ long ret;
+ char c;
+
+ str = bitmap_getnum(&r->start, str, end, is_user);
+ if (IS_ERR(str))
+ return str;
+
+ ret = get_char(&c, str, is_user);
+ if (ret)
+ return (char *)ret;
+
+ if (c == 0 || c == '\n') {
+ str = end + 1;
+ goto no_end;
+ }
+
+ if (isspace(c) || c == ',')
+ goto no_end;
+
+ if (c != '-')
+ return (char *)-EINVAL;
+
+ str = bitmap_getnum(&r->end, str + 1, end, is_user);
+ if (IS_ERR(str))
+ return str;
+
+ ret = get_char(&c, str, is_user);
+ if (ret)
+ return (char *)ret;
+
+ if (c == 0 || c == '\n') {
+ str = end + 1;
+ goto no_pattern;
+ }
+
+ if (isspace(c) || c == ',')
+ goto no_pattern;
+
+ if (c != ':')
+ return (char *)-EINVAL;
+
+ str = bitmap_getnum(&r->off, str + 1, end, is_user);
+ if (IS_ERR(str))
+ return str;
+
+ ret = get_char(&c, str, is_user);
+ if (ret)
+ return (char *)ret;
+
+ if (c != '/')
+ return (char *)-EINVAL;
+
+ str = bitmap_getnum(&r->grlen, str + 1, end, is_user);
+
+ return str;
+
+no_end:
+ r->end = r->start;
+no_pattern:
+ r->off = r->end + 1;
+ r->grlen = r->end + 1;
+
+ return (char *)str;
+}
+
/**
* __bitmap_parselist - convert list format ASCII string to bitmap
* @buf: read nul-terminated user string from this buffer
@@ -534,113 +668,28 @@ static int bitmap_check_region(const struct region *r)
*
* Returns: 0 on success, -errno on invalid input strings. Error values:
*
- * - ``-EINVAL``: second number in range smaller than first
+ * - ``-EINVAL``: wrong region format
* - ``-EINVAL``: invalid character in string
* - ``-ERANGE``: bit number specified too large for mask
+ * - ``-EOVERFLOW``: integer overflow in the input parameters
*/
-static int __bitmap_parselist(const char *buf, unsigned int buflen,
+static int __bitmap_parselist(const char *buf, const char *const end,
int is_user, unsigned long *maskp,
int nmaskbits)
{
- unsigned int a, b, old_a, old_b;
- unsigned int group_size, used_size;
- int c, old_c, totaldigits, ndigits;
- const char __user __force *ubuf = (const char __user __force *)buf;
- int at_start, in_range, in_partial_range, ret;
struct region r;
+ long ret;
- totaldigits = c = 0;
- old_a = old_b = 0;
- group_size = used_size = 0;
bitmap_zero(maskp, nmaskbits);
- do {
- at_start = 1;
- in_range = 0;
- in_partial_range = 0;
- a = b = 0;
- ndigits = totaldigits;
-
- /* Get the next cpu# or a range of cpu#'s */
- while (buflen) {
- old_c = c;
- if (is_user) {
- if (__get_user(c, ubuf++))
- return -EFAULT;
- } else
- c = *buf++;
- buflen--;
- if (isspace(c))
- continue;
-
- /* A '\0' or a ',' signal the end of a cpu# or range */
- if (c == '\0' || c == ',')
- break;
- /*
- * whitespaces between digits are not allowed,
- * but it's ok if whitespaces are on head or tail.
- * when old_c is whilespace,
- * if totaldigits == ndigits, whitespace is on head.
- * if whitespace is on tail, it should not run here.
- * as c was ',' or '\0',
- * the last code line has broken the current loop.
- */
- if ((totaldigits != ndigits) && isspace(old_c))
- return -EINVAL;
-
- if (c == '/') {
- used_size = a;
- at_start = 1;
- in_range = 0;
- a = b = 0;
- continue;
- }
-
- if (c == ':') {
- old_a = a;
- old_b = b;
- at_start = 1;
- in_range = 0;
- in_partial_range = 1;
- a = b = 0;
- continue;
- }
-
- if (c == '-') {
- if (at_start || in_range)
- return -EINVAL;
- b = 0;
- in_range = 1;
- at_start = 1;
- continue;
- }
- if (!isdigit(c))
- return -EINVAL;
-
- b = b * 10 + (c - '0');
- if (!in_range)
- a = b;
- at_start = 0;
- totaldigits++;
- }
- if (ndigits == totaldigits)
- continue;
- if (in_partial_range) {
- group_size = a;
- a = old_a;
- b = old_b;
- old_a = old_b = 0;
- } else {
- used_size = group_size = b - a + 1;
- }
- /* if no digit is after '-', it's wrong*/
- if (at_start && in_range)
- return -EINVAL;
+ while (buf && buf < end) {
+ buf = bitmap_find_region(buf, end, is_user);
+ if (buf == NULL)
+ return 0;
- r.start = a;
- r.off = used_size;
- r.grlen = group_size;
- r.end = b;
+ buf = bitmap_parse_region(&r, buf, end, is_user);
+ if (IS_ERR(buf))
+ return (long)buf;
ret = bitmap_check_region(&r);
if (ret)
@@ -649,14 +698,14 @@ static int __bitmap_parselist(const char *buf, unsigned int buflen,
ret = bitmap_set_region(&r, maskp, nmaskbits);
if (ret)
return ret;
+ }
- } while (buflen && c == ',');
return 0;
}
int bitmap_parselist(const char *bp, unsigned long *maskp, int nmaskbits)
{
- return __bitmap_parselist(bp, UINT_MAX, 0, maskp, nmaskbits);
+ return __bitmap_parselist(bp, (char *)SIZE_MAX, 0, maskp, nmaskbits);
}
EXPORT_SYMBOL(bitmap_parselist);
@@ -683,7 +732,7 @@ int bitmap_parselist_user(const char __user *ubuf,
if (!access_ok(VERIFY_READ, ubuf, ulen))
return -EFAULT;
return __bitmap_parselist((const char __force *)ubuf,
- ulen, 1, maskp, nmaskbits);
+ ubuf + ulen - 1, 1, maskp, nmaskbits);
}
EXPORT_SYMBOL(bitmap_parselist_user);
--
2.17.1
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH 3/4] bitmap_parselist: rework input string parser
2018-12-23 9:44 ` [PATCH 3/4] bitmap_parselist: rework input string parser Yuri Norov
@ 2018-12-23 13:02 ` kbuild test robot
2019-01-09 16:01 ` Andy Shevchenko
1 sibling, 0 replies; 9+ messages in thread
From: kbuild test robot @ 2018-12-23 13:02 UTC (permalink / raw)
To: Yuri Norov
Cc: kbuild-all, Andrew Morton, Andy Shevchenko, Rasmus Villemoes,
Arnd Bergmann, Kees Cook, Matthew Wilcox, Tetsuo Handa,
Yuri Norov, linux-kernel@vger.kernel.org
[-- Attachment #1: Type: text/plain, Size: 17446 bytes --]
Hi Yuri,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on linus/master]
[also build test WARNING on v4.20-rc7 next-20181221]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Yuri-Norov/rework-bitmap_parselist/20181223-175529
reproduce: make htmldocs
All warnings (new ones prefixed by >>):
WARNING: convert(1) not found, for SVG to PDF conversion install ImageMagick (https://www.imagemagick.org)
lib/bitmap.c:679: warning: Excess function parameter 'buflen' description in '__bitmap_parselist'
lib/bitmap.c:680: warning: Excess function parameter 'buflen' description in '__bitmap_parselist'
>> lib/bitmap.c:680: warning: Function parameter or member 'end' not described in '__bitmap_parselist'
lib/bitmap.c:680: warning: Excess function parameter 'buflen' description in '__bitmap_parselist'
include/linux/rcutree.h:1: warning: no structured comments found
kernel/rcu/tree.c:684: warning: Excess function parameter 'irq' description in 'rcu_nmi_exit'
include/linux/srcu.h:175: warning: Function parameter or member 'p' not described in 'srcu_dereference_notrace'
include/linux/srcu.h:175: warning: Function parameter or member 'sp' not described in 'srcu_dereference_notrace'
include/linux/gfp.h:1: warning: no structured comments found
include/net/cfg80211.h:2838: warning: cannot understand function prototype: 'struct cfg80211_ftm_responder_stats '
include/net/cfg80211.h:2838: warning: cannot understand function prototype: 'struct cfg80211_ftm_responder_stats '
include/net/cfg80211.h:2838: warning: cannot understand function prototype: 'struct cfg80211_ftm_responder_stats '
include/net/cfg80211.h:2838: warning: cannot understand function prototype: 'struct cfg80211_ftm_responder_stats '
include/net/cfg80211.h:2838: warning: cannot understand function prototype: 'struct cfg80211_ftm_responder_stats '
include/net/cfg80211.h:2838: warning: cannot understand function prototype: 'struct cfg80211_ftm_responder_stats '
include/net/cfg80211.h:2838: warning: cannot understand function prototype: 'struct cfg80211_ftm_responder_stats '
include/net/cfg80211.h:2838: warning: cannot understand function prototype: 'struct cfg80211_ftm_responder_stats '
include/net/cfg80211.h:2838: warning: cannot understand function prototype: 'struct cfg80211_ftm_responder_stats '
include/net/cfg80211.h:2838: warning: cannot understand function prototype: 'struct cfg80211_ftm_responder_stats '
include/net/cfg80211.h:2838: warning: cannot understand function prototype: 'struct cfg80211_ftm_responder_stats '
include/net/cfg80211.h:2838: warning: cannot understand function prototype: 'struct cfg80211_ftm_responder_stats '
include/net/cfg80211.h:2838: warning: cannot understand function prototype: 'struct cfg80211_ftm_responder_stats '
include/net/cfg80211.h:4439: warning: Function parameter or member 'wext.ibss' not described in 'wireless_dev'
include/net/cfg80211.h:4439: warning: Function parameter or member 'wext.connect' not described in 'wireless_dev'
include/net/cfg80211.h:4439: warning: Function parameter or member 'wext.keys' not described in 'wireless_dev'
include/net/cfg80211.h:4439: warning: Function parameter or member 'wext.ie' not described in 'wireless_dev'
include/net/cfg80211.h:4439: warning: Function parameter or member 'wext.ie_len' not described in 'wireless_dev'
include/net/cfg80211.h:4439: warning: Function parameter or member 'wext.bssid' not described in 'wireless_dev'
include/net/cfg80211.h:4439: warning: Function parameter or member 'wext.ssid' not described in 'wireless_dev'
include/net/cfg80211.h:4439: warning: Function parameter or member 'wext.default_key' not described in 'wireless_dev'
include/net/cfg80211.h:4439: warning: Function parameter or member 'wext.default_mgmt_key' not described in 'wireless_dev'
include/net/cfg80211.h:4439: warning: Function parameter or member 'wext.prev_bssid_valid' not described in 'wireless_dev'
include/net/cfg80211.h:2838: warning: cannot understand function prototype: 'struct cfg80211_ftm_responder_stats '
include/net/cfg80211.h:2838: warning: cannot understand function prototype: 'struct cfg80211_ftm_responder_stats '
include/net/cfg80211.h:2838: warning: cannot understand function prototype: 'struct cfg80211_ftm_responder_stats '
include/net/cfg80211.h:2838: warning: cannot understand function prototype: 'struct cfg80211_ftm_responder_stats '
include/net/cfg80211.h:2838: warning: cannot understand function prototype: 'struct cfg80211_ftm_responder_stats '
include/net/cfg80211.h:2838: warning: cannot understand function prototype: 'struct cfg80211_ftm_responder_stats '
include/net/cfg80211.h:2838: warning: cannot understand function prototype: 'struct cfg80211_ftm_responder_stats '
include/net/cfg80211.h:2838: warning: cannot understand function prototype: 'struct cfg80211_ftm_responder_stats '
include/net/cfg80211.h:2838: warning: cannot understand function prototype: 'struct cfg80211_ftm_responder_stats '
include/net/cfg80211.h:2838: warning: cannot understand function prototype: 'struct cfg80211_ftm_responder_stats '
include/net/cfg80211.h:2838: warning: cannot understand function prototype: 'struct cfg80211_ftm_responder_stats '
include/net/cfg80211.h:2838: warning: cannot understand function prototype: 'struct cfg80211_ftm_responder_stats '
include/net/cfg80211.h:2838: warning: cannot understand function prototype: 'struct cfg80211_ftm_responder_stats '
include/net/cfg80211.h:2838: warning: cannot understand function prototype: 'struct cfg80211_ftm_responder_stats '
include/net/cfg80211.h:2838: warning: cannot understand function prototype: 'struct cfg80211_ftm_responder_stats '
include/net/cfg80211.h:2838: warning: cannot understand function prototype: 'struct cfg80211_ftm_responder_stats '
include/net/cfg80211.h:2838: warning: cannot understand function prototype: 'struct cfg80211_ftm_responder_stats '
include/net/cfg80211.h:2838: warning: cannot understand function prototype: 'struct cfg80211_ftm_responder_stats '
include/net/cfg80211.h:2838: warning: cannot understand function prototype: 'struct cfg80211_ftm_responder_stats '
include/net/cfg80211.h:2838: warning: cannot understand function prototype: 'struct cfg80211_ftm_responder_stats '
include/net/cfg80211.h:2838: warning: cannot understand function prototype: 'struct cfg80211_ftm_responder_stats '
include/net/cfg80211.h:2838: warning: cannot understand function prototype: 'struct cfg80211_ftm_responder_stats '
include/net/cfg80211.h:2838: warning: cannot understand function prototype: 'struct cfg80211_ftm_responder_stats '
include/net/cfg80211.h:2838: warning: cannot understand function prototype: 'struct cfg80211_ftm_responder_stats '
include/net/cfg80211.h:2838: warning: cannot understand function prototype: 'struct cfg80211_ftm_responder_stats '
include/net/cfg80211.h:2838: warning: cannot understand function prototype: 'struct cfg80211_ftm_responder_stats '
include/net/cfg80211.h:2838: warning: cannot understand function prototype: 'struct cfg80211_ftm_responder_stats '
include/net/cfg80211.h:2838: warning: cannot understand function prototype: 'struct cfg80211_ftm_responder_stats '
include/net/cfg80211.h:2838: warning: cannot understand function prototype: 'struct cfg80211_ftm_responder_stats '
include/net/cfg80211.h:2838: warning: cannot understand function prototype: 'struct cfg80211_ftm_responder_stats '
include/net/cfg80211.h:2838: warning: cannot understand function prototype: 'struct cfg80211_ftm_responder_stats '
include/net/cfg80211.h:2838: warning: cannot understand function prototype: 'struct cfg80211_ftm_responder_stats '
include/net/cfg80211.h:2838: warning: cannot understand function prototype: 'struct cfg80211_ftm_responder_stats '
include/net/cfg80211.h:2838: warning: cannot understand function prototype: 'struct cfg80211_ftm_responder_stats '
include/net/cfg80211.h:2838: warning: cannot understand function prototype: 'struct cfg80211_ftm_responder_stats '
include/net/cfg80211.h:2838: warning: cannot understand function prototype: 'struct cfg80211_ftm_responder_stats '
include/net/cfg80211.h:2838: warning: cannot understand function prototype: 'struct cfg80211_ftm_responder_stats '
include/net/cfg80211.h:2838: warning: cannot understand function prototype: 'struct cfg80211_ftm_responder_stats '
include/net/cfg80211.h:2838: warning: cannot understand function prototype: 'struct cfg80211_ftm_responder_stats '
include/net/cfg80211.h:2838: warning: cannot understand function prototype: 'struct cfg80211_ftm_responder_stats '
include/net/cfg80211.h:2838: warning: cannot understand function prototype: 'struct cfg80211_ftm_responder_stats '
include/net/cfg80211.h:2838: warning: cannot understand function prototype: 'struct cfg80211_ftm_responder_stats '
include/net/cfg80211.h:2838: warning: cannot understand function prototype: 'struct cfg80211_ftm_responder_stats '
include/net/cfg80211.h:2838: warning: cannot understand function prototype: 'struct cfg80211_ftm_responder_stats '
include/net/cfg80211.h:2838: warning: cannot understand function prototype: 'struct cfg80211_ftm_responder_stats '
include/net/cfg80211.h:2838: warning: cannot understand function prototype: 'struct cfg80211_ftm_responder_stats '
include/net/cfg80211.h:2838: warning: cannot understand function prototype: 'struct cfg80211_ftm_responder_stats '
include/net/cfg80211.h:2838: warning: cannot understand function prototype: 'struct cfg80211_ftm_responder_stats '
include/net/cfg80211.h:2838: warning: cannot understand function prototype: 'struct cfg80211_ftm_responder_stats '
include/net/cfg80211.h:2838: warning: cannot understand function prototype: 'struct cfg80211_ftm_responder_stats '
include/net/cfg80211.h:2838: warning: cannot understand function prototype: 'struct cfg80211_ftm_responder_stats '
include/net/cfg80211.h:2838: warning: cannot understand function prototype: 'struct cfg80211_ftm_responder_stats '
include/net/cfg80211.h:2838: warning: cannot understand function prototype: 'struct cfg80211_ftm_responder_stats '
include/net/cfg80211.h:2838: warning: cannot understand function prototype: 'struct cfg80211_ftm_responder_stats '
include/net/cfg80211.h:2838: warning: cannot understand function prototype: 'struct cfg80211_ftm_responder_stats '
include/net/cfg80211.h:2838: warning: cannot understand function prototype: 'struct cfg80211_ftm_responder_stats '
include/net/cfg80211.h:2838: warning: cannot understand function prototype: 'struct cfg80211_ftm_responder_stats '
include/net/cfg80211.h:2838: warning: cannot understand function prototype: 'struct cfg80211_ftm_responder_stats '
include/net/cfg80211.h:2838: warning: cannot understand function prototype: 'struct cfg80211_ftm_responder_stats '
include/net/cfg80211.h:2838: warning: cannot understand function prototype: 'struct cfg80211_ftm_responder_stats '
include/net/cfg80211.h:2838: warning: cannot understand function prototype: 'struct cfg80211_ftm_responder_stats '
include/net/cfg80211.h:2838: warning: cannot understand function prototype: 'struct cfg80211_ftm_responder_stats '
include/net/cfg80211.h:2838: warning: cannot understand function prototype: 'struct cfg80211_ftm_responder_stats '
include/net/cfg80211.h:2838: warning: cannot understand function prototype: 'struct cfg80211_ftm_responder_stats '
include/net/cfg80211.h:2838: warning: cannot understand function prototype: 'struct cfg80211_ftm_responder_stats '
include/net/cfg80211.h:2838: warning: cannot understand function prototype: 'struct cfg80211_ftm_responder_stats '
include/net/cfg80211.h:2838: warning: cannot understand function prototype: 'struct cfg80211_ftm_responder_stats '
include/net/cfg80211.h:2838: warning: cannot understand function prototype: 'struct cfg80211_ftm_responder_stats '
include/net/cfg80211.h:2838: warning: cannot understand function prototype: 'struct cfg80211_ftm_responder_stats '
include/net/cfg80211.h:2838: warning: cannot understand function prototype: 'struct cfg80211_ftm_responder_stats '
include/net/cfg80211.h:2838: warning: cannot understand function prototype: 'struct cfg80211_ftm_responder_stats '
vim +680 lib/bitmap.c
32eb0e6b5 Yuri Norov 2018-12-23 649
5aaba3631 Sudeep Holla 2014-09-30 650 /**
4b060420a Mike Travis 2011-05-24 651 * __bitmap_parselist - convert list format ASCII string to bitmap
b0825ee3a Randy Dunlap 2011-06-15 652 * @buf: read nul-terminated user string from this buffer
4b060420a Mike Travis 2011-05-24 653 * @buflen: buffer size in bytes. If string is smaller than this
4b060420a Mike Travis 2011-05-24 654 * then it must be terminated with a \0.
4b060420a Mike Travis 2011-05-24 655 * @is_user: location of buffer, 0 indicates kernel space
6e1907ffd Randy Dunlap 2006-06-25 656 * @maskp: write resulting mask here
^1da177e4 Linus Torvalds 2005-04-16 657 * @nmaskbits: number of bits in mask to be written
^1da177e4 Linus Torvalds 2005-04-16 658 *
^1da177e4 Linus Torvalds 2005-04-16 659 * Input format is a comma-separated list of decimal numbers and
^1da177e4 Linus Torvalds 2005-04-16 660 * ranges. Consecutively set bits are shown as two hyphen-separated
^1da177e4 Linus Torvalds 2005-04-16 661 * decimal numbers, the smallest and largest bit numbers set in
^1da177e4 Linus Torvalds 2005-04-16 662 * the range.
2d13e6ca4 Noam Camus 2016-10-11 663 * Optionally each range can be postfixed to denote that only parts of it
2d13e6ca4 Noam Camus 2016-10-11 664 * should be set. The range will divided to groups of specific size.
2d13e6ca4 Noam Camus 2016-10-11 665 * From each group will be used only defined amount of bits.
2d13e6ca4 Noam Camus 2016-10-11 666 * Syntax: range:used_size/group_size
2d13e6ca4 Noam Camus 2016-10-11 667 * Example: 0-1023:2/256 ==> 0,1,256,257,512,513,768,769
^1da177e4 Linus Torvalds 2005-04-16 668 *
40bf19a8d Mauro Carvalho Chehab 2017-03-30 669 * Returns: 0 on success, -errno on invalid input strings. Error values:
40bf19a8d Mauro Carvalho Chehab 2017-03-30 670 *
32eb0e6b5 Yuri Norov 2018-12-23 671 * - ``-EINVAL``: wrong region format
40bf19a8d Mauro Carvalho Chehab 2017-03-30 672 * - ``-EINVAL``: invalid character in string
40bf19a8d Mauro Carvalho Chehab 2017-03-30 673 * - ``-ERANGE``: bit number specified too large for mask
32eb0e6b5 Yuri Norov 2018-12-23 674 * - ``-EOVERFLOW``: integer overflow in the input parameters
^1da177e4 Linus Torvalds 2005-04-16 675 */
32eb0e6b5 Yuri Norov 2018-12-23 676 static int __bitmap_parselist(const char *buf, const char *const end,
4b060420a Mike Travis 2011-05-24 677 int is_user, unsigned long *maskp,
4b060420a Mike Travis 2011-05-24 678 int nmaskbits)
^1da177e4 Linus Torvalds 2005-04-16 @679 {
87ac23b87 Yuri Norov 2018-12-23 @680 struct region r;
32eb0e6b5 Yuri Norov 2018-12-23 681 long ret;
^1da177e4 Linus Torvalds 2005-04-16 682
^1da177e4 Linus Torvalds 2005-04-16 683 bitmap_zero(maskp, nmaskbits);
4b060420a Mike Travis 2011-05-24 684
32eb0e6b5 Yuri Norov 2018-12-23 685 while (buf && buf < end) {
32eb0e6b5 Yuri Norov 2018-12-23 686 buf = bitmap_find_region(buf, end, is_user);
32eb0e6b5 Yuri Norov 2018-12-23 687 if (buf == NULL)
32eb0e6b5 Yuri Norov 2018-12-23 688 return 0;
87ac23b87 Yuri Norov 2018-12-23 689
32eb0e6b5 Yuri Norov 2018-12-23 690 buf = bitmap_parse_region(&r, buf, end, is_user);
32eb0e6b5 Yuri Norov 2018-12-23 691 if (IS_ERR(buf))
32eb0e6b5 Yuri Norov 2018-12-23 692 return (long)buf;
87ac23b87 Yuri Norov 2018-12-23 693
87ac23b87 Yuri Norov 2018-12-23 694 ret = bitmap_check_region(&r);
87ac23b87 Yuri Norov 2018-12-23 695 if (ret)
87ac23b87 Yuri Norov 2018-12-23 696 return ret;
87ac23b87 Yuri Norov 2018-12-23 697
87ac23b87 Yuri Norov 2018-12-23 698 ret = bitmap_set_region(&r, maskp, nmaskbits);
87ac23b87 Yuri Norov 2018-12-23 699 if (ret)
87ac23b87 Yuri Norov 2018-12-23 700 return ret;
32eb0e6b5 Yuri Norov 2018-12-23 701 }
87ac23b87 Yuri Norov 2018-12-23 702
^1da177e4 Linus Torvalds 2005-04-16 703 return 0;
^1da177e4 Linus Torvalds 2005-04-16 704 }
4b060420a Mike Travis 2011-05-24 705
:::::: The code at line 680 was first introduced by commit
:::::: 87ac23b8755f8cbef41f1befaee43d76bbfb2cc9 bitmap_parselist: move part of logic to helpers
:::::: TO: Yuri Norov <ynorov@marvell.com>
:::::: CC: 0day robot <lkp@intel.com>
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 6598 bytes --]
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH 3/4] bitmap_parselist: rework input string parser
2018-12-23 9:44 ` [PATCH 3/4] bitmap_parselist: rework input string parser Yuri Norov
2018-12-23 13:02 ` kbuild test robot
@ 2019-01-09 16:01 ` Andy Shevchenko
2019-01-09 17:16 ` Yuri Norov
1 sibling, 1 reply; 9+ messages in thread
From: Andy Shevchenko @ 2019-01-09 16:01 UTC (permalink / raw)
To: Yuri Norov
Cc: Andrew Morton, Rasmus Villemoes, Arnd Bergmann, Kees Cook,
Matthew Wilcox, Tetsuo Handa, linux-kernel@vger.kernel.org
On Sun, Dec 23, 2018 at 09:44:55AM +0000, Yuri Norov wrote:
> The requirement for this rework is to keep the __bitmap_parselist()
> copy-less and single-pass but make it more readable and maintainable by
> splitting into logical parts and removing explicit nested cycles and
> opaque local variables.
>
> __bitmap_parselist() can parse userspace inputs and therefore we cannot
> use simple_strtoul() to parse numbers.
I see two issues with this patch:
- you are using IS_ERR() but ain't utilizing PTR_ERR(), ERR_PTR() ones
- you remove lines here which you added in the same series.
Second one shows ordering issue of logical changes.
>
> Signed-off-by: Yury Norov <ynorov@marvell.com>
> ---
> lib/bitmap.c | 247 ++++++++++++++++++++++++++++++---------------------
> 1 file changed, 148 insertions(+), 99 deletions(-)
>
> diff --git a/lib/bitmap.c b/lib/bitmap.c
> index a60fd9723677..edc7068c28a6 100644
> --- a/lib/bitmap.c
> +++ b/lib/bitmap.c
> @@ -513,6 +513,140 @@ static int bitmap_check_region(const struct region *r)
> return 0;
> }
>
> +static long get_char(char *c, const char *str, int is_user)
> +{
> + if (unlikely(is_user))
> + return __get_user(*c, str);
> +
> + *c = *str;
> + return 0;
> +}
> +
> +static const char *bitmap_getnum(unsigned int *num, const char *str,
> + const char *const end, int is_user)
> +{
> + unsigned int n = 0;
> + const char *_str;
> + long ret;
> + char c;
> +
> + for (_str = str, *num = 0; _str <= end; _str++) {
> + ret = get_char(&c, _str, is_user);
> + if (ret)
> + return (char *)ret;
> +
> + if (!isdigit(c)) {
> + if (_str == str)
> + return (char *)-EINVAL;
> +
> + goto out;
> + }
> +
> + *num = *num * 10 + (c - '0');
> + if (*num < n)
> + return (char *)-EOVERFLOW;
> +
> + n = *num;
> + }
> +
> +out:
> + return _str;
> +}
> +
> +static const char *bitmap_find_region(const char *str,
> + const char *const end, int is_user)
> +{
> + long ret;
> + char c;
> +
> + for (; str < end; str++) {
> + ret = get_char(&c, str, is_user);
> + if (ret)
> + return (char *)ret;
> +
> + /* Unexpected end of line. */
> + if (c == 0 || c == '\n')
> + return NULL;
> +
> + /*
> + * The format allows commas and whitespases
> + * at the beginning of the region, so skip it.
> + */
> + if (!isspace(c) && c != ',')
> + break;
> + }
> +
> + return str;
> +}
> +
> +static const char *bitmap_parse_region(struct region *r, const char *str,
> + const char *const end, int is_user)
> +{
> + long ret;
> + char c;
> +
> + str = bitmap_getnum(&r->start, str, end, is_user);
> + if (IS_ERR(str))
> + return str;
> +
> + ret = get_char(&c, str, is_user);
> + if (ret)
> + return (char *)ret;
> +
> + if (c == 0 || c == '\n') {
> + str = end + 1;
> + goto no_end;
> + }
> +
> + if (isspace(c) || c == ',')
> + goto no_end;
> +
> + if (c != '-')
> + return (char *)-EINVAL;
> +
> + str = bitmap_getnum(&r->end, str + 1, end, is_user);
> + if (IS_ERR(str))
> + return str;
> +
> + ret = get_char(&c, str, is_user);
> + if (ret)
> + return (char *)ret;
> +
> + if (c == 0 || c == '\n') {
> + str = end + 1;
> + goto no_pattern;
> + }
> +
> + if (isspace(c) || c == ',')
> + goto no_pattern;
> +
> + if (c != ':')
> + return (char *)-EINVAL;
> +
> + str = bitmap_getnum(&r->off, str + 1, end, is_user);
> + if (IS_ERR(str))
> + return str;
> +
> + ret = get_char(&c, str, is_user);
> + if (ret)
> + return (char *)ret;
> +
> + if (c != '/')
> + return (char *)-EINVAL;
> +
> + str = bitmap_getnum(&r->grlen, str + 1, end, is_user);
> +
> + return str;
> +
> +no_end:
> + r->end = r->start;
> +no_pattern:
> + r->off = r->end + 1;
> + r->grlen = r->end + 1;
> +
> + return (char *)str;
> +}
> +
> /**
> * __bitmap_parselist - convert list format ASCII string to bitmap
> * @buf: read nul-terminated user string from this buffer
> @@ -534,113 +668,28 @@ static int bitmap_check_region(const struct region *r)
> *
> * Returns: 0 on success, -errno on invalid input strings. Error values:
> *
> - * - ``-EINVAL``: second number in range smaller than first
> + * - ``-EINVAL``: wrong region format
> * - ``-EINVAL``: invalid character in string
> * - ``-ERANGE``: bit number specified too large for mask
> + * - ``-EOVERFLOW``: integer overflow in the input parameters
> */
> -static int __bitmap_parselist(const char *buf, unsigned int buflen,
> +static int __bitmap_parselist(const char *buf, const char *const end,
> int is_user, unsigned long *maskp,
> int nmaskbits)
> {
> - unsigned int a, b, old_a, old_b;
> - unsigned int group_size, used_size;
> - int c, old_c, totaldigits, ndigits;
> - const char __user __force *ubuf = (const char __user __force *)buf;
> - int at_start, in_range, in_partial_range, ret;
> struct region r;
> + long ret;
>
> - totaldigits = c = 0;
> - old_a = old_b = 0;
> - group_size = used_size = 0;
> bitmap_zero(maskp, nmaskbits);
> - do {
> - at_start = 1;
> - in_range = 0;
> - in_partial_range = 0;
> - a = b = 0;
> - ndigits = totaldigits;
> -
> - /* Get the next cpu# or a range of cpu#'s */
> - while (buflen) {
> - old_c = c;
> - if (is_user) {
> - if (__get_user(c, ubuf++))
> - return -EFAULT;
> - } else
> - c = *buf++;
> - buflen--;
> - if (isspace(c))
> - continue;
> -
> - /* A '\0' or a ',' signal the end of a cpu# or range */
> - if (c == '\0' || c == ',')
> - break;
> - /*
> - * whitespaces between digits are not allowed,
> - * but it's ok if whitespaces are on head or tail.
> - * when old_c is whilespace,
> - * if totaldigits == ndigits, whitespace is on head.
> - * if whitespace is on tail, it should not run here.
> - * as c was ',' or '\0',
> - * the last code line has broken the current loop.
> - */
> - if ((totaldigits != ndigits) && isspace(old_c))
> - return -EINVAL;
> -
> - if (c == '/') {
> - used_size = a;
> - at_start = 1;
> - in_range = 0;
> - a = b = 0;
> - continue;
> - }
> -
> - if (c == ':') {
> - old_a = a;
> - old_b = b;
> - at_start = 1;
> - in_range = 0;
> - in_partial_range = 1;
> - a = b = 0;
> - continue;
> - }
> -
> - if (c == '-') {
> - if (at_start || in_range)
> - return -EINVAL;
> - b = 0;
> - in_range = 1;
> - at_start = 1;
> - continue;
> - }
>
> - if (!isdigit(c))
> - return -EINVAL;
> -
> - b = b * 10 + (c - '0');
> - if (!in_range)
> - a = b;
> - at_start = 0;
> - totaldigits++;
> - }
> - if (ndigits == totaldigits)
> - continue;
> - if (in_partial_range) {
> - group_size = a;
> - a = old_a;
> - b = old_b;
> - old_a = old_b = 0;
> - } else {
> - used_size = group_size = b - a + 1;
> - }
> - /* if no digit is after '-', it's wrong*/
> - if (at_start && in_range)
> - return -EINVAL;
> + while (buf && buf < end) {
> + buf = bitmap_find_region(buf, end, is_user);
> + if (buf == NULL)
> + return 0;
>
> - r.start = a;
> - r.off = used_size;
> - r.grlen = group_size;
> - r.end = b;
> + buf = bitmap_parse_region(&r, buf, end, is_user);
> + if (IS_ERR(buf))
> + return (long)buf;
>
> ret = bitmap_check_region(&r);
> if (ret)
> @@ -649,14 +698,14 @@ static int __bitmap_parselist(const char *buf, unsigned int buflen,
> ret = bitmap_set_region(&r, maskp, nmaskbits);
> if (ret)
> return ret;
> + }
>
> - } while (buflen && c == ',');
> return 0;
> }
>
> int bitmap_parselist(const char *bp, unsigned long *maskp, int nmaskbits)
> {
> - return __bitmap_parselist(bp, UINT_MAX, 0, maskp, nmaskbits);
> + return __bitmap_parselist(bp, (char *)SIZE_MAX, 0, maskp, nmaskbits);
> }
> EXPORT_SYMBOL(bitmap_parselist);
>
> @@ -683,7 +732,7 @@ int bitmap_parselist_user(const char __user *ubuf,
> if (!access_ok(VERIFY_READ, ubuf, ulen))
> return -EFAULT;
> return __bitmap_parselist((const char __force *)ubuf,
> - ulen, 1, maskp, nmaskbits);
> + ubuf + ulen - 1, 1, maskp, nmaskbits);
> }
> EXPORT_SYMBOL(bitmap_parselist_user);
>
> --
> 2.17.1
>
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH 3/4] bitmap_parselist: rework input string parser
2019-01-09 16:01 ` Andy Shevchenko
@ 2019-01-09 17:16 ` Yuri Norov
0 siblings, 0 replies; 9+ messages in thread
From: Yuri Norov @ 2019-01-09 17:16 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Andrew Morton, Rasmus Villemoes, Arnd Bergmann, Kees Cook,
Matthew Wilcox, Tetsuo Handa, linux-kernel@vger.kernel.org,
yury.norov@gmail.com
On Wed, Jan 09, 2019 at 06:01:46PM +0200, Andy Shevchenko wrote:
> On Sun, Dec 23, 2018 at 09:44:55AM +0000, Yuri Norov wrote:
> > The requirement for this rework is to keep the __bitmap_parselist()
> > copy-less and single-pass but make it more readable and maintainable by
> > splitting into logical parts and removing explicit nested cycles and
> > opaque local variables.
> >
> > __bitmap_parselist() can parse userspace inputs and therefore we cannot
> > use simple_strtoul() to parse numbers.
>
> I see two issues with this patch:
> - you are using IS_ERR() but ain't utilizing PTR_ERR(), ERR_PTR() ones
OK. Will use them in v2.
> - you remove lines here which you added in the same series.
>
> Second one shows ordering issue of logical changes.
Do you mean this chunk?
> > - r.start = a;
> > - r.off = used_size;
> > - r.grlen = group_size;
> > - r.end = b;
It's because I split refactoring into 2 parts for the sake of
readability. Patch #2 may be applied independently if #3 will
be considered inappropriate, that's why I ordered it prior to
#3, and it caused the need for removing that lines. I don't
think it's too ugly though, and I'd prefer to keep 2 and 3
separated with the cost of this little mess.
Reversing the order looks tricky at the first glance, but I'm
OK to join #2 and #3 if it will be considered worthy. Would
work both ways.
> > Signed-off-by: Yury Norov <ynorov@marvell.com>
> > ---
> > lib/bitmap.c | 247 ++++++++++++++++++++++++++++++---------------------
> > 1 file changed, 148 insertions(+), 99 deletions(-)
> >
> > diff --git a/lib/bitmap.c b/lib/bitmap.c
> > index a60fd9723677..edc7068c28a6 100644
> > --- a/lib/bitmap.c
> > +++ b/lib/bitmap.c
> > @@ -513,6 +513,140 @@ static int bitmap_check_region(const struct region *r)
> > return 0;
> > }
> >
> > +static long get_char(char *c, const char *str, int is_user)
> > +{
> > + if (unlikely(is_user))
> > + return __get_user(*c, str);
> > +
> > + *c = *str;
> > + return 0;
> > +}
> > +
> > +static const char *bitmap_getnum(unsigned int *num, const char *str,
> > + const char *const end, int is_user)
> > +{
> > + unsigned int n = 0;
> > + const char *_str;
> > + long ret;
> > + char c;
> > +
> > + for (_str = str, *num = 0; _str <= end; _str++) {
> > + ret = get_char(&c, _str, is_user);
> > + if (ret)
> > + return (char *)ret;
> > +
> > + if (!isdigit(c)) {
> > + if (_str == str)
> > + return (char *)-EINVAL;
> > +
> > + goto out;
> > + }
> > +
> > + *num = *num * 10 + (c - '0');
> > + if (*num < n)
> > + return (char *)-EOVERFLOW;
> > +
> > + n = *num;
> > + }
> > +
> > +out:
> > + return _str;
> > +}
> > +
> > +static const char *bitmap_find_region(const char *str,
> > + const char *const end, int is_user)
> > +{
> > + long ret;
> > + char c;
> > +
> > + for (; str < end; str++) {
> > + ret = get_char(&c, str, is_user);
> > + if (ret)
> > + return (char *)ret;
> > +
> > + /* Unexpected end of line. */
> > + if (c == 0 || c == '\n')
> > + return NULL;
> > +
> > + /*
> > + * The format allows commas and whitespases
> > + * at the beginning of the region, so skip it.
> > + */
> > + if (!isspace(c) && c != ',')
> > + break;
> > + }
> > +
> > + return str;
> > +}
> > +
> > +static const char *bitmap_parse_region(struct region *r, const char *str,
> > + const char *const end, int is_user)
> > +{
> > + long ret;
> > + char c;
> > +
> > + str = bitmap_getnum(&r->start, str, end, is_user);
> > + if (IS_ERR(str))
> > + return str;
> > +
> > + ret = get_char(&c, str, is_user);
> > + if (ret)
> > + return (char *)ret;
> > +
> > + if (c == 0 || c == '\n') {
> > + str = end + 1;
> > + goto no_end;
> > + }
> > +
> > + if (isspace(c) || c == ',')
> > + goto no_end;
> > +
> > + if (c != '-')
> > + return (char *)-EINVAL;
> > +
> > + str = bitmap_getnum(&r->end, str + 1, end, is_user);
> > + if (IS_ERR(str))
> > + return str;
> > +
> > + ret = get_char(&c, str, is_user);
> > + if (ret)
> > + return (char *)ret;
> > +
> > + if (c == 0 || c == '\n') {
> > + str = end + 1;
> > + goto no_pattern;
> > + }
> > +
> > + if (isspace(c) || c == ',')
> > + goto no_pattern;
> > +
> > + if (c != ':')
> > + return (char *)-EINVAL;
> > +
> > + str = bitmap_getnum(&r->off, str + 1, end, is_user);
> > + if (IS_ERR(str))
> > + return str;
> > +
> > + ret = get_char(&c, str, is_user);
> > + if (ret)
> > + return (char *)ret;
> > +
> > + if (c != '/')
> > + return (char *)-EINVAL;
> > +
> > + str = bitmap_getnum(&r->grlen, str + 1, end, is_user);
> > +
> > + return str;
> > +
> > +no_end:
> > + r->end = r->start;
> > +no_pattern:
> > + r->off = r->end + 1;
> > + r->grlen = r->end + 1;
> > +
> > + return (char *)str;
This cast looks unneeded.
> > +}
> > +
> > /**
> > * __bitmap_parselist - convert list format ASCII string to bitmap
> > * @buf: read nul-terminated user string from this buffer
> > @@ -534,113 +668,28 @@ static int bitmap_check_region(const struct region *r)
> > *
> > * Returns: 0 on success, -errno on invalid input strings. Error values:
> > *
> > - * - ``-EINVAL``: second number in range smaller than first
> > + * - ``-EINVAL``: wrong region format
> > * - ``-EINVAL``: invalid character in string
> > * - ``-ERANGE``: bit number specified too large for mask
> > + * - ``-EOVERFLOW``: integer overflow in the input parameters
> > */
> > -static int __bitmap_parselist(const char *buf, unsigned int buflen,
> > +static int __bitmap_parselist(const char *buf, const char *const end,
> > int is_user, unsigned long *maskp,
> > int nmaskbits)
> > {
> > - unsigned int a, b, old_a, old_b;
> > - unsigned int group_size, used_size;
> > - int c, old_c, totaldigits, ndigits;
> > - const char __user __force *ubuf = (const char __user __force *)buf;
> > - int at_start, in_range, in_partial_range, ret;
> > struct region r;
> > + long ret;
> >
> > - totaldigits = c = 0;
> > - old_a = old_b = 0;
> > - group_size = used_size = 0;
> > bitmap_zero(maskp, nmaskbits);
> > - do {
> > - at_start = 1;
> > - in_range = 0;
> > - in_partial_range = 0;
> > - a = b = 0;
> > - ndigits = totaldigits;
> > -
> > - /* Get the next cpu# or a range of cpu#'s */
> > - while (buflen) {
> > - old_c = c;
> > - if (is_user) {
> > - if (__get_user(c, ubuf++))
> > - return -EFAULT;
> > - } else
> > - c = *buf++;
> > - buflen--;
> > - if (isspace(c))
> > - continue;
> > -
> > - /* A '\0' or a ',' signal the end of a cpu# or range */
> > - if (c == '\0' || c == ',')
> > - break;
> > - /*
> > - * whitespaces between digits are not allowed,
> > - * but it's ok if whitespaces are on head or tail.
> > - * when old_c is whilespace,
> > - * if totaldigits == ndigits, whitespace is on head.
> > - * if whitespace is on tail, it should not run here.
> > - * as c was ',' or '\0',
> > - * the last code line has broken the current loop.
> > - */
> > - if ((totaldigits != ndigits) && isspace(old_c))
> > - return -EINVAL;
> > -
> > - if (c == '/') {
> > - used_size = a;
> > - at_start = 1;
> > - in_range = 0;
> > - a = b = 0;
> > - continue;
> > - }
> > -
> > - if (c == ':') {
> > - old_a = a;
> > - old_b = b;
> > - at_start = 1;
> > - in_range = 0;
> > - in_partial_range = 1;
> > - a = b = 0;
> > - continue;
> > - }
> > -
> > - if (c == '-') {
> > - if (at_start || in_range)
> > - return -EINVAL;
> > - b = 0;
> > - in_range = 1;
> > - at_start = 1;
> > - continue;
> > - }
> >
> > - if (!isdigit(c))
> > - return -EINVAL;
> > -
> > - b = b * 10 + (c - '0');
> > - if (!in_range)
> > - a = b;
> > - at_start = 0;
> > - totaldigits++;
> > - }
> > - if (ndigits == totaldigits)
> > - continue;
> > - if (in_partial_range) {
> > - group_size = a;
> > - a = old_a;
> > - b = old_b;
> > - old_a = old_b = 0;
> > - } else {
> > - used_size = group_size = b - a + 1;
> > - }
> > - /* if no digit is after '-', it's wrong*/
> > - if (at_start && in_range)
> > - return -EINVAL;
> > + while (buf && buf < end) {
> > + buf = bitmap_find_region(buf, end, is_user);
> > + if (buf == NULL)
> > + return 0;
> >
> > - r.start = a;
> > - r.off = used_size;
> > - r.grlen = group_size;
> > - r.end = b;
> > + buf = bitmap_parse_region(&r, buf, end, is_user);
> > + if (IS_ERR(buf))
> > + return (long)buf;
> >
> > ret = bitmap_check_region(&r);
> > if (ret)
> > @@ -649,14 +698,14 @@ static int __bitmap_parselist(const char *buf, unsigned int buflen,
> > ret = bitmap_set_region(&r, maskp, nmaskbits);
> > if (ret)
> > return ret;
> > + }
> >
> > - } while (buflen && c == ',');
> > return 0;
> > }
> >
> > int bitmap_parselist(const char *bp, unsigned long *maskp, int nmaskbits)
> > {
> > - return __bitmap_parselist(bp, UINT_MAX, 0, maskp, nmaskbits);
> > + return __bitmap_parselist(bp, (char *)SIZE_MAX, 0, maskp, nmaskbits);
> > }
> > EXPORT_SYMBOL(bitmap_parselist);
> >
> > @@ -683,7 +732,7 @@ int bitmap_parselist_user(const char __user *ubuf,
> > if (!access_ok(VERIFY_READ, ubuf, ulen))
> > return -EFAULT;
> > return __bitmap_parselist((const char __force *)ubuf,
> > - ulen, 1, maskp, nmaskbits);
> > + ubuf + ulen - 1, 1, maskp, nmaskbits);
> > }
> > EXPORT_SYMBOL(bitmap_parselist_user);
> >
> > --
> > 2.17.1
> >
>
Thanks,
Yury
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 4/4] test_bitmap: add testcases for bitmap_parselist
2018-12-23 9:44 [PATCH 0/4] rework bitmap_parselist Yuri Norov
` (2 preceding siblings ...)
2018-12-23 9:44 ` [PATCH 3/4] bitmap_parselist: rework input string parser Yuri Norov
@ 2018-12-23 9:45 ` Yuri Norov
2018-12-23 12:41 ` kbuild test robot
3 siblings, 1 reply; 9+ messages in thread
From: Yuri Norov @ 2018-12-23 9:45 UTC (permalink / raw)
To: Andrew Morton, Andy Shevchenko, Rasmus Villemoes, Arnd Bergmann,
Kees Cook, Matthew Wilcox, Tetsuo Handa
Cc: Yuri Norov, linux-kernel@vger.kernel.org
Add tests for non-number character, empty regions, integer overflow.
Signed-off-by: Yury Norov <ynorov@marvell.com>
---
lib/test_bitmap.c | 18 +++++++++++++++++-
1 file changed, 17 insertions(+), 1 deletion(-)
diff --git a/lib/test_bitmap.c b/lib/test_bitmap.c
index 6cd7d0740005..7580dd6ac599 100644
--- a/lib/test_bitmap.c
+++ b/lib/test_bitmap.c
@@ -224,7 +224,8 @@ static const unsigned long exp[] __initconst = {
BITMAP_FROM_U64(0xffffffff),
BITMAP_FROM_U64(0xfffffffe),
BITMAP_FROM_U64(0x3333333311111111ULL),
- BITMAP_FROM_U64(0xffffffff77777777ULL)
+ BITMAP_FROM_U64(0xffffffff77777777ULL),
+ BITMAP_FROM_U64(0),
};
static const unsigned long exp2[] __initconst = {
@@ -247,19 +248,34 @@ static const struct test_bitmap_parselist parselist_tests[] __initconst = {
{0, "1-31:4/4", &exp[9 * step], 32, 0},
{0, "0-31:1/4,32-63:2/4", &exp[10 * step], 64, 0},
{0, "0-31:3/4,32-63:4/4", &exp[11 * step], 64, 0},
+ {0, " ,, 0-31:3/4 ,, 32-63:4/4 ,, ", &exp[11 * step], 64, 0},
{0, "0-31:1/4,32-63:2/4,64-95:3/4,96-127:4/4", exp2, 128, 0},
{0, "0-2047:128/256", NULL, 2048, PARSE_TIME},
+ {0, "", &exp[12], 8, 0},
+ {0, "\n", &exp[12], 8, 0},
+ {0, ",, ,, , , ,", &exp[12], 8, 0},
+ {0, " , ,, , , ", &exp[12], 8, 0},
+ {0, " , ,, , , \n", &exp[12], 8, 0},
+
{-EINVAL, "-1", NULL, 8, 0},
{-EINVAL, "-0", NULL, 8, 0},
{-EINVAL, "10-1", NULL, 8, 0},
{-EINVAL, "0-31:", NULL, 8, 0},
{-EINVAL, "0-31:0", NULL, 8, 0},
+ {-EINVAL, "0-31:0/", NULL, 8, 0},
{-EINVAL, "0-31:0/0", NULL, 8, 0},
{-EINVAL, "0-31:1/0", NULL, 8, 0},
{-EINVAL, "0-31:10/1", NULL, 8, 0},
+ {-EOVERFLOW, "0-98765432123456789:10/1", NULL, 8, 0},
+
+ {-EINVAL, "a-31", NULL, 8, 0},
+ {-EINVAL, "0-a1", NULL, 8, 0},
+ {-EINVAL, "a-31:10/1", NULL, 8, 0},
+ {-EINVAL, "0-31:a/1", NULL, 8, 0},
+ {-EINVAL, "0-\n", NULL, 8, 0},
};
static void __init test_bitmap_parselist(void)
--
2.17.1
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH 4/4] test_bitmap: add testcases for bitmap_parselist
2018-12-23 9:45 ` [PATCH 4/4] test_bitmap: add testcases for bitmap_parselist Yuri Norov
@ 2018-12-23 12:41 ` kbuild test robot
0 siblings, 0 replies; 9+ messages in thread
From: kbuild test robot @ 2018-12-23 12:41 UTC (permalink / raw)
To: Yuri Norov
Cc: kbuild-all, Andrew Morton, Andy Shevchenko, Rasmus Villemoes,
Arnd Bergmann, Kees Cook, Matthew Wilcox, Tetsuo Handa,
Yuri Norov, linux-kernel@vger.kernel.org
[-- Attachment #1: Type: text/plain, Size: 2004 bytes --]
Hi Yuri,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on linus/master]
[also build test WARNING on v4.20-rc7 next-20181221]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Yuri-Norov/rework-bitmap_parselist/20181223-175529
config: x86_64-allmodconfig (attached as .config)
compiler: gcc-7 (Debian 7.3.0-1) 7.3.0
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64
All warnings (new ones prefixed by >>):
>> lib//bitmap.c:519:24: warning: incorrect type in argument 1 (different address spaces)
lib//bitmap.c:519:24: expected void const volatile [noderef] <asn:1> *
lib//bitmap.c:519:24: got char const *str
lib//bitmap.c:735:53: warning: incorrect type in argument 2 (different address spaces)
lib//bitmap.c:735:53: expected char const *const end
lib//bitmap.c:735:53: got char const [noderef] <asn:1> *
include/linux/slab.h:332:43: warning: dubious: x & !y
vim +519 lib//bitmap.c
87ac23b8 Yuri Norov 2018-12-23 515
32eb0e6b Yuri Norov 2018-12-23 516 static long get_char(char *c, const char *str, int is_user)
32eb0e6b Yuri Norov 2018-12-23 517 {
32eb0e6b Yuri Norov 2018-12-23 518 if (unlikely(is_user))
32eb0e6b Yuri Norov 2018-12-23 @519 return __get_user(*c, str);
32eb0e6b Yuri Norov 2018-12-23 520
32eb0e6b Yuri Norov 2018-12-23 521 *c = *str;
32eb0e6b Yuri Norov 2018-12-23 522 return 0;
32eb0e6b Yuri Norov 2018-12-23 523 }
32eb0e6b Yuri Norov 2018-12-23 524
:::::: The code at line 519 was first introduced by commit
:::::: 32eb0e6b5e705132d4efef268c65730a2f2b9c41 bitmap_parselist: rework input string parser
:::::: TO: Yuri Norov <ynorov@marvell.com>
:::::: CC: 0day robot <lkp@intel.com>
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 66645 bytes --]
^ permalink raw reply [flat|nested] 9+ messages in thread