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
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:	Wed, 01 Jul 2015 11:17:47 +0800
From:	Pan Xinhui <xinhuix.pan@...el.com>
To:	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
CC:	Yury Norov <yury.norov@...il.com>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Rasmus Villemoes <linux@...musvillemoes.dk>, tj@...nel.org,
	peterz@...radead.org, sudeep.holla@....com, mina86@...a86.com,
	"mnipxh@....com" <mnipxh@....com>,
	Alexey Klimov <klimov.linux@...il.com>
Subject: Re: [PATCH V3] lib/bitmap.c: rewrite __bitmap_parse && __bitmap_parselist

hi, all
	after a deep think, allocing a new buf is ugly.
new patch "lib/bitmap.c: fix some parsing issues and code style" is sent out.

any comments is welcome. :)

thanks
xinhui

On 2015年06月30日 18:02, Pan Xinhui wrote:
>
> add __bitmap_parse_common to match any contents and return expected reslut.
>
> as __bitmap_parse_common need NULL-terminated string, we alloc a new buf.
>
> this patch also fix an unexpected parse result issue in __bitmap_parselist.
>
> Signed-off-by: Pan Xinhui <xinhuix.pan@...el.com>
> ---
>   lib/bitmap.c | 238 +++++++++++++++++++++++++++++++++--------------------------
>   1 file changed, 134 insertions(+), 104 deletions(-)
> ---
> change log
> v3:
> __bitmap_parselist now allow some extra input, like ",2, 3,,,,  5-8", at least one digit inside.
> input like " " ", " is still not allowed.
>
> V2:
> __bitmap_parse_common need *base* to parse correct result
>
> V1:
> add __bitmap_parse_common and rewrite __bitmap_parse && __bitmap_parselist
> ---
>
> diff --git a/lib/bitmap.c b/lib/bitmap.c
> index 64c0926..f2095e1d 100644
> --- a/lib/bitmap.c
> +++ b/lib/bitmap.c
> @@ -16,6 +16,8 @@
>   #include <asm/page.h>
>   #include <asm/uaccess.h>
>
> +#include <linux/parser.h>
> +#include <linux/slab.h>
>   /*
>    * bitmaps provide an array of bits, implemented using an an
>    * array of unsigned longs.  The number of valid bits in a
> @@ -331,6 +333,58 @@ again:
>   EXPORT_SYMBOL(bitmap_find_next_zero_area_off);
>
>   /*
> + * __bitmap_parse_common - parse expected number from buf
> + * Return 0 on success.
> + * there two patterns.
> + * if buf's contents did not match any of them, reutrn equivalent error.
> + * Notice buf's contents may be changed.
> + */
> +static int __bitmap_parse_common(char *buf, unsigned int buflen,
> +         unsigned long *a, unsigned long *b, int base)
> +{
> +    int ret;
> +    int token;
> +    const match_table_t table = {
> +        {
> +            .token = 1,
> +            .pattern = "%x",
> +        },
> +        {
> +            .token = 2,
> +            .pattern = "%x-%x",
> +        },
> +        {
> +            .token = 0,
> +            .pattern = NULL,
> +        }
> +    };
> +    substring_t substr[MAX_OPT_ARGS];
> +
> +    if (!buflen || !a)
> +        return -EINVAL;
> +
> +    token = match_token((char *)buf, table, substr);
> +    switch (token) {
> +    case 1:
> +        *substr[0].to = '\0';
> +        ret = kstrtoul(substr[0].from, base, a);
> +        if (b)
> +            *b = *a;
> +        break;
> +    case 2:
> +        *substr[0].to = '\0';
> +        *substr[1].to = '\0';
> +        ret = kstrtoul(substr[0].from, base, a);
> +        ret |= b ? kstrtoul(substr[1].from, base, b) : -EINVAL;
> +        break;
> +    default:
> +        ret = -EINVAL;
> +        break;
> +    }
> +    return ret;
> +}
> +
> +/*
>    * Bitmap printing & parsing functions: first version by Nadia Yvette Chambers,
>    * second version by Paul Jackson, third by Joe Korty.
>    */
> @@ -359,57 +413,45 @@ int __bitmap_parse(const char *buf, unsigned int buflen,
>           int is_user, unsigned long *maskp,
>           int nmaskbits)
>   {
> -    int c, old_c, totaldigits, ndigits, nchunks, nbits;
> +    int nchunks, nbits, ret;
> +    unsigned long a;
>       u32 chunk;
>       const char __user __force *ubuf = (const char __user __force *)buf;
> +    char *kbuf, *endp;
> +
> +    if (!buflen)
> +        return -EINVAL;
> +    kbuf = kmalloc(buflen + 1, GFP_KERNEL);
> +    if (!kbuf)
> +        return -ENOMEM;
> +    if (is_user) {
> +        if (copy_from_user(kbuf, ubuf, buflen) != 0) {
> +            kfree(kbuf);
> +            return -EFAULT;
> +        }
> +    } else
> +        memcpy(kbuf, buf, buflen);
> +    kbuf[buflen] = '\0';
> +    buf = strim(kbuf);
>
>       bitmap_zero(maskp, nmaskbits);
>
> -    nchunks = nbits = totaldigits = c = 0;
> +    nchunks = nbits = 0;
>       do {
> -        chunk = ndigits = 0;
> -
> -        /* Get the next chunk of the bitmap */
> -        while (buflen) {
> -            old_c = c;
> -            if (is_user) {
> -                if (__get_user(c, ubuf++))
> -                    return -EFAULT;
> -            }
> -            else
> -                c = *buf++;
> -            buflen--;
> -            if (isspace(c))
> -                continue;
> -
> -            /*
> -             * If the last character was a space and the current
> -             * character isn't '\0', we've got embedded whitespace.
> -             * This is a no-no, so throw an error.
> -             */
> -            if (totaldigits && c && isspace(old_c))
> -                return -EINVAL;
> -
> -            /* A '\0' or a ',' signal the end of the chunk */
> -            if (c == '\0' || c == ',')
> -                break;
> -
> -            if (!isxdigit(c))
> -                return -EINVAL;
> -
> -            /*
> -             * Make sure there are at least 4 free bits in 'chunk'.
> -             * If not, this hexdigit will overflow 'chunk', so
> -             * throw an error.
> -             */
> -            if (chunk & ~((1UL << (CHUNKSZ - 4)) - 1))
> -                return -EOVERFLOW;
> -
> -            chunk = (chunk << 4) | hex_to_bin(c);
> -            ndigits++; totaldigits++;
> +        endp = strchr(buf, ',');
> +        if (endp)
> +            *endp = '\0';
> +        ret = __bitmap_parse_common((char *)buf, strlen(buf),
> +                                &a, NULL, 16);
> +        if (ret)
> +            break;
> +        buf = endp + 1;
> +
> +        if (unlikely(a > U32_MAX)) {
> +            ret = -ERANGE;
> +            break;
>           }
> -        if (ndigits == 0)
> -            return -EINVAL;
> +        chunk = (u32)a;
>           if (nchunks == 0 && chunk == 0)
>               continue;
>
> @@ -417,11 +459,13 @@ int __bitmap_parse(const char *buf, unsigned int buflen,
>           *maskp |= chunk;
>           nchunks++;
>           nbits += (nchunks == 1) ? nbits_to_hold_value(chunk) : CHUNKSZ;
> -        if (nbits > nmaskbits)
> -            return -EOVERFLOW;
> -    } while (buflen && c == ',');
> -
> -    return 0;
> +        if (nbits > nmaskbits) {
> +            ret = -EOVERFLOW;
> +            break;
> +        }
> +    } while (endp);
> +    kfree(kbuf);
> +    return ret;
>   }
>   EXPORT_SYMBOL(__bitmap_parse);
>
> @@ -503,70 +547,56 @@ static int __bitmap_parselist(const char *buf, unsigned int buflen,
>           int is_user, unsigned long *maskp,
>           int nmaskbits)
>   {
> -    unsigned a, b;
> -    int c, old_c, totaldigits;
> +    unsigned long a, b;
> +    int ret = -EINVAL;
>       const char __user __force *ubuf = (const char __user __force *)buf;
> -    int exp_digit, in_range;
> +    char *kbuf, *endp, *ibuf;
> +
> +    if (!buflen)
> +        return -EINVAL;
> +    ibuf = kbuf = kmalloc(buflen + 1, GFP_KERNEL);
> +    if (!kbuf)
> +        return -ENOMEM;
> +    if (is_user) {
> +        if (copy_from_user(kbuf, ubuf, buflen) != 0) {
> +            kfree(kbuf);
> +            return -EFAULT;
> +        }
> +    } else
> +        memcpy(kbuf, buf, buflen);
> +    kbuf[buflen] = '\0';
>
> -    totaldigits = c = 0;
>       bitmap_zero(maskp, nmaskbits);
>       do {
> -        exp_digit = 1;
> -        in_range = 0;
> -        a = b = 0;
> -
> -        /* 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;
> -
> -            /*
> -             * If the last character was a space and the current
> -             * character isn't '\0', we've got embedded whitespace.
> -             * This is a no-no, so throw an error.
> -             */
> -            if (totaldigits && c && isspace(old_c))
> -                return -EINVAL;
> -
> -            /* A '\0' or a ',' signal the end of a cpu# or range */
> -            if (c == '\0' || c == ',')
> -                break;
> -
> -            if (c == '-') {
> -                if (exp_digit || in_range)
> -                    return -EINVAL;
> -                b = 0;
> -                in_range = 1;
> -                exp_digit = 1;
> -                continue;
> -            }
> -
> -            if (!isdigit(c))
> -                return -EINVAL;
> -
> -            b = b * 10 + (c - '0');
> -            if (!in_range)
> -                a = b;
> -            exp_digit = 0;
> -            totaldigits++;
> +        endp = strchr(ibuf, ',');
> +        if (endp)
> +            *endp = '\0';
> +        ibuf = strim(ibuf);
> +        if (*ibuf == 0) {
> +            ibuf = endp + 1;
> +            continue;
> +        }
> +        ret = __bitmap_parse_common(ibuf, strlen(ibuf),
> +                                &a, &b, 0);
> +        if (ret)
> +            break;
> +        ibuf = endp + 1;
> +
> +        if (!(a <= b)) {
> +            ret = -EINVAL;
> +            break;
> +        }
> +        if (b >= nmaskbits) {
> +            ret = -ERANGE;
> +            break;
>           }
> -        if (!(a <= b))
> -            return -EINVAL;
> -        if (b >= nmaskbits)
> -            return -ERANGE;
>           while (a <= b) {
>               set_bit(a, maskp);
>               a++;
>           }
> -    } while (buflen && c == ',');
> -    return 0;
> +    } while (endp);
> +    kfree(kbuf);
> +    return ret;
>   }
>
>   int bitmap_parselist(const char *bp, unsigned long *maskp, int nmaskbits)
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ