Re: [PATCH -mm] mm, swap: Fix bad swap file entry warning

2019-05-31 Thread Yuri Norov
(Resend as LKML didn't take outlook settings.)

> On Fri, 2019-05-31 at 11:27 -0700, Dexuan-Linux Cui wrote:
> > Hi,
> > Did you know about the panic reported here:
> >  https://marc.info/?t=15593077303=1=2
> > 
> > "Kernel panic - not syncing: stack-protector: Kernel stack is
> > corrupted in: write_irq_affinity.isra> "
> > 
> > This panic is reported on PowerPC and x86.
> > 
> > In the case of x86, we see a lot of "get_swap_device: Bad swap file entry"
> > errors before the panic:
> > 
> > ...
> > [   24.404693] get_swap_device: Bad swap file entry 5801
> > [   24.408702] get_swap_device: Bad swap file entry 5c01
> > [   24.412510] get_swap_device: Bad swap file entry 6001
> > [   24.416519] get_swap_device: Bad swap file entry 6401
> > [   24.420217] get_swap_device: Bad swap file entry 6801
> > [   24.423921] get_swap_device: Bad swap file entry 6c01

[..]

I don't have a panic, but I observe many lines like this.

> Looks familiar,
>
> https://lore.kernel.org/lkml/1559242868.6132.35.ca...@lca.pw/
>
> I suppose Andrew might be better of reverting the whole series first before 
> Yury
> came up with a right fix, so that other people who is testing linux-next don't
> need to waste time for the same problem.

I didn't observe any problems with this series on top of 5.1, and there's a fix 
for swap\
that eliminates the problem on top of current next for me:
https://lkml.org/lkml/2019/5/30/1630

Could you please test my series with the patch above?

Thanks,
Yury

 

Re: [PATCH 3/6] bitmap_parselist: rework input string parser

2019-03-26 Thread Yuri Norov
+ Mike Travis 
+ Thomas Gleixner 

--
On Tue, Mar 26, 2019 at 12:07:45AM +0300, Yury 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.

> So, all above depends to what memory we access kernel / user space.
> Perhaps we can get copy of memory of a given size and then parse it in kernel 
> space always?

> --
> With Best Regards,
> Andy Shevchenko

What I missed during rework is that we have only one caller of *parselist_user  
-
it's write_irq_affinity() introduced by Mike Travis in kernel/irq/proc.c. It 
doesn't look
like a hot path as it's file operations handler. If no objections from Mike or 
Thomas,
I think it would make sense to copy_from_user() the userspace data at the 
beginning
in sake of simplicity of __bitmap_parselist(), as you suggested above.

Yury


Re: [PATCH 3/4] bitmap_parselist: rework input string parser

2019-01-09 Thread Yuri Norov
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 
> > ---
> >  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(, _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(, 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(>start, str, end, is_user);
> > +   if (IS_ERR(str))
> > +   return str;
> > +
> > +   ret = get_char(, 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 (

[PATCH] arm64: introduce AUDIT_ARCH_AARCH64ILP32 for ilp32

2019-01-08 Thread Yuri Norov
Make syscall_get_arch() distinguish arm64 and arm64/ilp32 by adding
AUDIT_ARCH_AARCH64ILP32.

Sugested-by: Andy Lutomirski 
Signed-off-by: Yury Norov 
---
 arch/arm64/include/asm/syscall.h | 3 +++
 include/uapi/linux/audit.h   | 1 +
 2 files changed, 4 insertions(+)

diff --git a/arch/arm64/include/asm/syscall.h b/arch/arm64/include/asm/syscall.h
index 73fbe08763b0..77578d703cb1 100644
--- a/arch/arm64/include/asm/syscall.h
+++ b/arch/arm64/include/asm/syscall.h
@@ -126,6 +126,9 @@ static inline int syscall_get_arch(void)
if (is_a32_compat_task())
return AUDIT_ARCH_ARM;
 
+   if (is_ilp32_compat_task())
+   return AUDIT_ARCH_AARCH64ILP32;
+
return AUDIT_ARCH_AARCH64;
 }
 
diff --git a/include/uapi/linux/audit.h b/include/uapi/linux/audit.h
index 818ae690ab79..624127147404 100644
--- a/include/uapi/linux/audit.h
+++ b/include/uapi/linux/audit.h
@@ -374,6 +374,7 @@ enum {
 #define __AUDIT_ARCH_LE   0x4000
 
 #define AUDIT_ARCH_AARCH64 (EM_AARCH64|__AUDIT_ARCH_64BIT|__AUDIT_ARCH_LE)
+#define AUDIT_ARCH_AARCH64ILP32(EM_AARCH64|__AUDIT_ARCH_LE)
 #define AUDIT_ARCH_ALPHA   (EM_ALPHA|__AUDIT_ARCH_64BIT|__AUDIT_ARCH_LE)
 #define AUDIT_ARCH_ARM (EM_ARM|__AUDIT_ARCH_LE)
 #define AUDIT_ARCH_ARMEB   (EM_ARM)
-- 
2.17.1



Re: [PATCH v9 00/24] ILP32 for ARM64

2019-01-07 Thread Yuri Norov
On Mon, Jan 07, 2019 at 09:48:44AM -0800, Andy Lutomirski wrote:
> 
> 
> > On Jan 7, 2019, at 7:50 AM, Yuri Norov  wrote:
> > 
> > Hi all,
> > 
> >> On Wed, May 16, 2018 at 11:18:45AM +0300, Yury Norov wrote:
> >> This series enables AARCH64 with ILP32 mode.
> >> 
> >> As supporting work, it introduces ARCH_32BIT_OFF_T configuration
> >> option that is enabled for existing 32-bit architectures but disabled
> >> for new arches (so 64-bit off_t userspace type is used by new userspace).
> >> Also it deprecates getrlimit and setrlimit syscalls prior to prlimit64.
> >> 
> >> Based on kernel v4.16. Tested with LTP, glibc testsuite, trinity, lmbench,
> >> CPUSpec.
> >> 
> >> This series on github: 
> >> https://github.com/norov/linux/tree/ilp32-4.16
> >> Linaro toolchain:
> >> http://snapshots.linaro.org/components/toolchain/binaries/7.3-2018.04-rc1/aarch64-linux-gnu_ilp32/
> >> Debian repo:
> >> http://people.linaro.org/~wookey/ilp32/
> >> OpenSUSE repo:
> >> https://build.opensuse.org/project/show/devel:ARM:Factory:Contrib:ILP32
> >> 
> >> Changes:
> >> v3: https://lkml.org/lkml/2014/9/3/704
> >> v4: https://lkml.org/lkml/2015/4/13/691
> >> v5: https://lkml.org/lkml/2015/9/29/911
> >> v6: https://lkml.org/lkml/2016/5/23/661
> >> v7: https://lkml.org/lkml/2017/1/9/213
> >> v8: https://lkml.org/lkml/2017/6/19/624
> >> v9: - rebased on top of v4.16;
> >>- signal subsystem reworked to avoid code duplication, as requested
> >>  by Dave Martin (patches 18 and 20);
> >>- new files introduced in series use SPDX notation for license;
> >>- linux-api and linux-arch CCed as the series changes kernel ABI;
> >>- checkpatch and other minor fixes.
> >>- Zhou Chengming's reported-by for patch 2 and signed-off-by for
> >>  patch 21 removed because his email became invalid. Zhou, please
> >>  share your new email.
> > 
> > This is 4.20-based version.
> > https://github.com/norov/linux/tree/ilp32-4.20
> > 
> > There's no important changes comparing to 4.19, but I would like to remind
> > that this series contains some generic arch patches that ACKed, but still
> > not upstreamed:
> > Yury Norov  asm-generic: Drop getrlimit and setrlimit syscalls from 
> > default list
> > Yury Norov  32-bit userspace ABI: introduce ARCH_32BIT_OFF_T config 
> > option
> > Yury Norov  compat ABI: use non-compat openat and open_by_handle_at 
> > variants
> > James Morse ptrace: Add compat PTRACE_{G,S}ETSIGMASK handlers
> > Dave Martin arm64: signal: Make parse_user_sigframe() independent of 
> > rt_sigframe layout
> > 
> > Please also notice that my email address is changed, from now it's
> > yno...@marvell.com
> > 
> 
> Having spent some time hating x32 lately, here is a cursory review:

I followed the recent discussion on x32 drop, but I don't track the story
from the beginning. Can you summarize your complains here (or maybe in
patch for Documentation)? It will be good for ilp32 because we will check
the series against all possible problems, and it will be good for history
because future compat architectures developers will be aware.

On your following questions, I'll try answer according to my understanding.
If it's wrong or incomplete, I kindly ask others fix me.

> You say “ILP32 has context-related structures different from both aarch32 and
> aarch64/lp64.”  Why?  Why not just use the lp64 context structure?

Because the alternative is nothing better than how we do now. We can make
rt_sigframe for ilp32 look exactly like lp64. But on user side we will
have to add pads therefore. On the kernel side, before passing rt_sigframe
to kernel, we will have to make sure that pads are zeroed, just like we do
with x0-x7 in delouse_pt_regs() before passing syscall arguments to handlers.
So we cannot get rid of get/set_sigframe() converters in this case.

Current approach lets us keep user part looking more natural, and on
kernel side we don't add complexity because we reuse existing aarch32
or existing lp64 handlers, and don't invent something new.

> Do you have some objection to letting greg_t be a u64 on ilp32?

In binfmt_ilp32.c we
#define compat_elf_gregset_telf_gregset_t
So in fact we use 64-bit gregset. Or I misunderstood you?

> Since you can’t tell LP64 and ILP32 syscalls apart by number, you need to add
> and wire up a new AUDIT_ARCH value for ILP32. syscall_get_arch() needs to 
> work correctly.

Thanks for pointing to this. I'll append AUDIT_ARCH patch to this series.

Yury


Re: [PATCH v9 00/24] ILP32 for ARM64

2019-01-07 Thread Yuri Norov
Hi all,

On Wed, May 16, 2018 at 11:18:45AM +0300, Yury Norov wrote:
> This series enables AARCH64 with ILP32 mode.
> 
> As supporting work, it introduces ARCH_32BIT_OFF_T configuration
> option that is enabled for existing 32-bit architectures but disabled
> for new arches (so 64-bit off_t userspace type is used by new userspace).
> Also it deprecates getrlimit and setrlimit syscalls prior to prlimit64.
> 
> Based on kernel v4.16. Tested with LTP, glibc testsuite, trinity, lmbench,
> CPUSpec.
> 
> This series on github: 
> https://github.com/norov/linux/tree/ilp32-4.16
> Linaro toolchain:
> http://snapshots.linaro.org/components/toolchain/binaries/7.3-2018.04-rc1/aarch64-linux-gnu_ilp32/
> Debian repo:
> http://people.linaro.org/~wookey/ilp32/
> OpenSUSE repo:
> https://build.opensuse.org/project/show/devel:ARM:Factory:Contrib:ILP32
> 
> Changes:
> v3: https://lkml.org/lkml/2014/9/3/704
> v4: https://lkml.org/lkml/2015/4/13/691
> v5: https://lkml.org/lkml/2015/9/29/911
> v6: https://lkml.org/lkml/2016/5/23/661
> v7: https://lkml.org/lkml/2017/1/9/213
> v8: https://lkml.org/lkml/2017/6/19/624
> v9: - rebased on top of v4.16;
> - signal subsystem reworked to avoid code duplication, as requested
>   by Dave Martin (patches 18 and 20);
> - new files introduced in series use SPDX notation for license;
> - linux-api and linux-arch CCed as the series changes kernel ABI;
> - checkpatch and other minor fixes.
> - Zhou Chengming's reported-by for patch 2 and signed-off-by for
>   patch 21 removed because his email became invalid. Zhou, please
>   share your new email.

This is 4.20-based version.
https://github.com/norov/linux/tree/ilp32-4.20

There's no important changes comparing to 4.19, but I would like to remind
that this series contains some generic arch patches that ACKed, but still
not upstreamed:
Yury Norov  asm-generic: Drop getrlimit and setrlimit syscalls from default 
list
Yury Norov  32-bit userspace ABI: introduce ARCH_32BIT_OFF_T config option
Yury Norov  compat ABI: use non-compat openat and open_by_handle_at variants
James Morse ptrace: Add compat PTRACE_{G,S}ETSIGMASK handlers
Dave Martin arm64: signal: Make parse_user_sigframe() independent of 
rt_sigframe layout

Please also notice that my email address is changed, from now it's
yno...@marvell.com

Yury


[PATCH 4/4] test_bitmap: add testcases for bitmap_parselist

2018-12-23 Thread Yuri Norov
Add tests for non-number character, empty regions, integer overflow.

Signed-off-by: Yury Norov 
---
 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(0x),
BITMAP_FROM_U64(0xfffe),
BITMAP_FROM_U64(0xULL),
-   BITMAP_FROM_U64(0xULL)
+   BITMAP_FROM_U64(0xULL),
+   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", [9 * step], 32, 0},
{0, "0-31:1/4,32-63:2/4",   [10 * step], 64, 0},
{0, "0-31:3/4,32-63:4/4",   [11 * step], 64, 0},
+   {0, "  ,,  0-31:3/4  ,, 32-63:4/4  ,,  ",   [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, "", [12], 8, 0},
+   {0, "\n",   [12], 8, 0},
+   {0, ",,  ,,  , ,  ,",   [12], 8, 0},
+   {0, " ,  ,,  , ,   ",   [12], 8, 0},
+   {0, " ,  ,,  , ,   \n", [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



[PATCH 0/4] rework bitmap_parselist

2018-12-23 Thread Yuri Norov
bitmap_parselist evolved from a pretty simple idea for long and now lacks
for refactoring. It is not structured, has nested loops and a set of
opaque-named variables. All this leads to extremely hard understanding
of the code. Once during the optimization of it I missed a scenario which
leads to kernel hangup. Tetsuo Handa spotted this and found it simpler to
rewrite the code instead fixing it. (Though, that attempt had some flaws.)
https://lkml.org/lkml/2018/4/1/93

Things are more complicated than they may be because bitmap_parselist()
is part of user interface, and its behavior should not change.

In this patchset __bitmap_parselist() is split to a set of smaller
functions doing only one thing.

Yury Norov (4):
  bitmap_parselist: don't calculate length of the input string
  bitmap_parselist: move part of logic to helpers
  bitmap_parselist: rework input string parser
  test_bitmap: add testcases for bitmap_parselist

 lib/bitmap.c  | 294 ++
 lib/test_bitmap.c |  18 ++-
 2 files changed, 208 insertions(+), 104 deletions(-)

-- 
2.17.1



[PATCH 3/4] bitmap_parselist: rework input string parser

2018-12-23 Thread Yuri Norov
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 
---
 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(, _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(, 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(>start, str, end, is_user);
+   if (IS_ERR(str))
+   return str;
+
+   ret = get_char(, 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(>end, str + 1, end, is_user);
+   if (IS_ERR(str))
+   return str;
+
+   ret = get_char(, 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(>off, str + 1, end, is_user);
+   if (IS_ERR(str))
+   return str;
+
+   ret = get_char(, str, is_user);
+   if (ret)
+   return (char *)ret;
+
+   if (c != '/')
+   return (char *)-EINVAL;
+
+   str = bitmap_getnum(>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;
- 

[PATCH 1/4] bitmap_parselist: don't calculate length of the input string

2018-12-23 Thread Yuri Norov
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 
---
 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



[PATCH 2/4] bitmap_parselist: move part of logic to helpers

2018-12-23 Thread Yuri Norov
Move region checking and setting functionality of __bitmap_parselist()
to helpers.

Signed-off-by: Yury Norov 
---
 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  1218  38
+ * .......
+ * ^  ^ ^   ^
+ *  start  off   grlenend
+ */
+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();
+   if (ret)
+   return ret;
+
+   ret = bitmap_set_region(, maskp, nmaskbits);
+   if (ret)
+   return ret;
+
} while (buflen && c == ',');
return 0;
 }
-- 
2.17.1