lib/bitmap.c: add some check to correct the parse result
Subsystems (2)
PATCH 1/1 lib/bitmap.c: add some check to correct the parse result
Sometimes the input from user may cause an unexpected result.
for example, echo "1-3," > /proc/irq/<xxx>/smp_affinity_list. The correct result should be 1-3, however we got 0-4.
To avoid this issue, we check if there is a ready digit. If no valid digit is set, we just continue to the next parse.
--- lib/bitmap.c | 2 ++ 1 file changed, 2 insertions(+)
lib/bitmap.c +2 -0
@@ -561,6 +561,8 @@ static int __bitmap_parselist(const char *buf, unsigned int buflen,return -EINVAL ;if (b >= nmaskbits)return -ERANGE ; +if (unlikely (exp_digit)) +continue ;while (a <= b) {set_bit (a, maskp); a++;
Signature · 1 line
1.9.1
Andrew Morton
maintainerQuoted 18 lines · On Sat, 27 Jun 2015 14:36:18 +0800 Pan Xinhui <[email protected]> wrote:
Sometimes the input from user may cause an unexpected result.
for example, echo "1-3," > /proc/irq/<xxx>/smp_affinity_list. The correct result should be 1-3, however we got 0-4.
To avoid this issue, we check if there is a ready digit. If no valid digit is set, we just continue to the next parse.
Signed-off-by Pan Xinhui <[email protected]>--- lib/bitmap.c | 2 ++ 1 file changed, 2 insertions(+)lib/bitmap.c +0
-0
lib/bitmap.c static int __bitmap_parselist(const char *buf, unsigned int buflen, · line 568
Quoted 8 lines
lib/bitmap.c +2
-0@@ -561,6 +561,8 @@ static int __bitmap_parselist(const char *buf, unsigned int buflen,return -EINVAL ;if (b >= nmaskbits)return -ERANGE ; +if (unlikely (exp_digit)) +continue ;while (a <= b) {set_bit (a, maskp); a++;
This bug might have been fixed by 2528a8b8f457d7 ("__bitmap_parselist:
fix bug in empty string handling"), below. Please check?
commit 2528a8b8f457d7432552d0e2b6f0f4046bb702f4
Author: Chris Metcalf <[email protected]> AuthorDate: Thu Jun 25 15:02:08 2015 -0700 Commit: Linus Torvalds <[email protected]> CommitDate: Thu Jun 25 17:00:40 2015 -0700
__bitmap_parselist: fix bug in empty string handling
bitmap_parselist("", &mask, nmaskbits) will erroneously set bit zero in the mask. The same bug is visible in cpumask_parselist() since it is layered on top of the bitmask code, e.g. if you boot with "isolcpus=", you will actually end up with cpu zero isolated.
The bug was introduced in commit 4b060420a596 ("bitmap, irq: add smp_affinity_list interface to /proc/irq") when bitmap_parselist() was generalized to support userspace as well as kernelspace.
lib/bitmap.c +9 -8
@@ -506,12 +506,12 @@ static int __bitmap_parselist(const char *buf, unsigned int buflen,unsigned a, b;int c, old_c, totaldigits;const char __user __force *ubuf = (const char __user __force *)buf; -int exp_digit, in_range; +int at_start, in_range; totaldigits = c =0 ;bitmap_zero (maskp, nmaskbits); do { - exp_digit = 1; + at_start = 1; in_range = 0; a = b = 0;
@@ -540,11 +540,10 @@ static int __bitmap_parselist(const char *buf, unsigned int buflen,break ;if (c =='-' ) { -if (exp_digit || in_range) +if (at_start || in_range)return -EINVAL ; b =0 ; in_range =1 ; - exp_digit =1 ;continue ; }
@@ -554,16 +553,18 @@ static int __bitmap_parselist(const char *buf, unsigned int buflen, b = b *10 + (c -'0' );if (!in_range) a = b; - exp_digit =0 ; + at_start =0 ; totaldigits++; }if (!(a <= b))return -EINVAL ;if (b >= nmaskbits)return -ERANGE ; -while (a <= b) { -set_bit (a, maskp); - a++; +if (!at_start) { +while (a <= b) { +set_bit (a, maskp); + a++; + } } }while (buflen && c ==',' );return 0 ;
hi, Andrew
thanks for you kind reply :)
Quoted 15 lines · Andrew Morton
Quoted 13 lines · On Sat, 27 Jun 2015 14:36:18 +0800 Pan Xinhui <[email protected]> wrote:
Sometimes the input from user may cause an unexpected result.
for example, echo "1-3," > /proc/irq/<xxx>/smp_affinity_list. The correct result should be 1-3, however we got 0-4.
To avoid this issue, we check if there is a ready digit. If no valid digit is set, we just continue to the next parse.
Signed-off-by Pan Xinhui <[email protected]>--- lib/bitmap.c | 2 ++ 1 file changed, 2 insertions(+)
Quoted 18 lines
lib/bitmap.c static int __bitmap_parselist(const char *buf, unsigned int buflen, · line 568
Quoted 13 lines
lib/bitmap.c +2
-0@@ -561,6 +561,8 @@ static int __bitmap_parselist(const char *buf, unsigned int buflen,return -EINVAL ;if (b >= nmaskbits)return -ERANGE ; +if (unlikely (exp_digit)) +continue ;while (a <= b) {set_bit (a, maskp); a++;This bug might have been fixed by
2528a8b8f457d7("__bitmap_parselist:
fix bug in empty string handling"), below. Please check?
Yes, I noticed it after I sent this patch out. Sorry for not syncing the upstream codes. I am very happy that you reviewed my patch :)
I send out other three patches to make current codes stronger :)
they are
[PATCH 1/3] lib/bitmap.c: correct a code style and do some optimization in __bitmap_parse [PATCH 2/3] lib/bitmap.c: fix a special string handling bug in __bitmap_parselist [PATCH 3/3] lib/bitmap.c: bitmap_parselist can accept string with whitespaces on head or tail Of course I have verified/test them :)
They have minor codes change.
thanks
xinhui
Quoted 79 lines
commit
2528a8b8f457d7432552d0e2b6f0f4046bb702f4Author: Chris Metcalf <[email protected]> AuthorDate: Thu Jun 25 15:02:08 2015 -0700 Commit: Linus Torvalds <[email protected]> CommitDate: Thu Jun 25 17:00:40 2015 -0700
__bitmap_parselist: fix bug in empty string handling
bitmap_parselist("", &mask, nmaskbits)will erroneously set bit zero in the mask. The same bug is visible incpumask_parselist()since it is layered on top of the bitmask code, e.g. if you boot with "isolcpus=", you will actually end up with cpu zero isolated.The bug was introduced in commit
4b060420a596("bitmap, irq: addsmp_affinity_listinterface to/proc/irq") whenbitmap_parselist()was generalized to support userspace as well as kernelspace.Fixes 4b060420a596 ("bitmap, irq: add smp_affinity_list interface to /proc/irq")Signed-off-by Chris Metcalf <[email protected]>Cc Rasmus Villemoes <[email protected]>Signed-off-by Andrew Morton <[email protected]>Signed-off-by Linus Torvalds <[email protected]>lib/bitmap.c +9
-8@@ -506,12 +506,12 @@ static int __bitmap_parselist(const char *buf, unsigned int buflen,unsigned a, b;int c, old_c, totaldigits;const char __user __force *ubuf = (const char __user __force *)buf; -int exp_digit, in_range; +int at_start, in_range; totaldigits = c =0 ;bitmap_zero (maskp, nmaskbits); do { - exp_digit = 1; + at_start = 1; in_range = 0; a = b = 0;@@ -540,11 +540,10 @@ static int __bitmap_parselist(const char *buf, unsigned int buflen,break ;if (c =='-' ) { -if (exp_digit || in_range) +if (at_start || in_range)return -EINVAL ; b =0 ; in_range =1 ; - exp_digit =1 ;continue ; }@@ -554,16 +553,18 @@ static int __bitmap_parselist(const char *buf, unsigned int buflen, b = b *10 + (c -'0' );if (!in_range) a = b; - exp_digit =0 ; + at_start =0 ; totaldigits++; }if (!(a <= b))return -EINVAL ;if (b >= nmaskbits)return -ERANGE ; -while (a <= b) { -set_bit (a, maskp); - a++; +if (!at_start) { +while (a <= b) { +set_bit (a, maskp); + a++; + } } }while (buflen && c ==',' );return 0 ;