* [PATCH] lib/bitmap.c: add some check to correct the parse result @ 2015-06-27 6:36 Pan Xinhui 2015-07-09 22:57 ` Andrew Morton 0 siblings, 1 reply; 3+ messages in thread From: Pan Xinhui @ 2015-06-27 6:36 UTC (permalink / raw) To: linux-kernel; +Cc: akpm, linux, tj, mnipxh 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 <xinhuix.pan@intel.com> --- lib/bitmap.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lib/bitmap.c b/lib/bitmap.c index 64c0926..3c489c1 100644 --- a/lib/bitmap.c +++ b/lib/bitmap.c @@ -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++; -- 1.9.1 ^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] lib/bitmap.c: add some check to correct the parse result 2015-06-27 6:36 [PATCH] lib/bitmap.c: add some check to correct the parse result Pan Xinhui @ 2015-07-09 22:57 ` Andrew Morton 2015-07-10 6:03 ` Pan Xinhui 0 siblings, 1 reply; 3+ messages in thread From: Andrew Morton @ 2015-07-09 22:57 UTC (permalink / raw) To: Pan Xinhui; +Cc: linux-kernel, linux, tj, mnipxh, Chris Metcalf On Sat, 27 Jun 2015 14:36:18 +0800 Pan Xinhui <xinhuix.pan@intel.com> 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 <xinhuix.pan@intel.com> > --- > lib/bitmap.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/lib/bitmap.c b/lib/bitmap.c > index 64c0926..3c489c1 100644 > --- a/lib/bitmap.c > +++ b/lib/bitmap.c > @@ -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 <cmetcalf@ezchip.com> AuthorDate: Thu Jun 25 15:02:08 2015 -0700 Commit: Linus Torvalds <torvalds@linux-foundation.org> 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. Fixes: 4b060420a596 ("bitmap, irq: add smp_affinity_list interface to /proc/irq") Signed-off-by: Chris Metcalf <cmetcalf@ezchip.com> Cc: Rasmus Villemoes <linux@rasmusvillemoes.dk> Signed-off-by: Andrew Morton <akpm@linux-foundation.org> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org> diff --git a/lib/bitmap.c b/lib/bitmap.c index 64c0926..40162f8 100644 --- a/lib/bitmap.c +++ b/lib/bitmap.c @@ -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; ^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] lib/bitmap.c: add some check to correct the parse result 2015-07-09 22:57 ` Andrew Morton @ 2015-07-10 6:03 ` Pan Xinhui 0 siblings, 0 replies; 3+ messages in thread From: Pan Xinhui @ 2015-07-10 6:03 UTC (permalink / raw) To: Andrew Morton; +Cc: linux-kernel, linux, tj, mnipxh, Chris Metcalf hi, Andrew thanks for you kind reply :) On 2015年07月10日 06:57, Andrew Morton wrote: > On Sat, 27 Jun 2015 14:36:18 +0800 Pan Xinhui <xinhuix.pan@intel.com> 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 <xinhuix.pan@intel.com> >> --- >> lib/bitmap.c | 2 ++ >> 1 file changed, 2 insertions(+) >> >> diff --git a/lib/bitmap.c b/lib/bitmap.c >> index 64c0926..3c489c1 100644 >> --- a/lib/bitmap.c >> +++ b/lib/bitmap.c >> @@ -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 > commit 2528a8b8f457d7432552d0e2b6f0f4046bb702f4 > Author: Chris Metcalf <cmetcalf@ezchip.com> > AuthorDate: Thu Jun 25 15:02:08 2015 -0700 > Commit: Linus Torvalds <torvalds@linux-foundation.org> > 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. > > Fixes: 4b060420a596 ("bitmap, irq: add smp_affinity_list interface to /proc/irq") > Signed-off-by: Chris Metcalf <cmetcalf@ezchip.com> > Cc: Rasmus Villemoes <linux@rasmusvillemoes.dk> > Signed-off-by: Andrew Morton <akpm@linux-foundation.org> > Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org> > > diff --git a/lib/bitmap.c b/lib/bitmap.c > index 64c0926..40162f8 100644 > --- a/lib/bitmap.c > +++ b/lib/bitmap.c > @@ -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; > ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2015-07-10 6:06 UTC | newest] Thread overview: 3+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2015-06-27 6:36 [PATCH] lib/bitmap.c: add some check to correct the parse result Pan Xinhui 2015-07-09 22:57 ` Andrew Morton 2015-07-10 6:03 ` Pan Xinhui
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox