commit f9c82a4ea89c3 ("Increase size of ucounts to atomic_long_t") changed the data type of ucounts/ucounts_max to long, but missed to adjust a few other places. This is noticeable on big endian platforms from user space because the /proc/sys/user/max_*_names files all contain 0. Fixes: f9c82a4ea89c ("Increase size of ucounts to atomic_long_t") Signed-off-by: Sven Schnelle <svens@linux.ibm.com> --- fs/notify/fanotify/fanotify_user.c | 10 ++++++---- fs/notify/inotify/inotify_user.c | 10 ++++++---- kernel/ucount.c | 16 ++++++++-------- 3 files changed, 20 insertions(+), 16 deletions(-) diff --git a/fs/notify/fanotify/fanotify_user.c b/fs/notify/fanotify/fanotify_user.c index 64864fb40b40..6576657a1a25 100644 --- a/fs/notify/fanotify/fanotify_user.c +++ b/fs/notify/fanotify/fanotify_user.c @@ -58,18 +58,20 @@ struct ctl_table fanotify_table[] = { { .procname = "max_user_groups", .data = &init_user_ns.ucount_max[UCOUNT_FANOTIFY_GROUPS], - .maxlen = sizeof(int), + .maxlen = sizeof(long), .mode = 0644, - .proc_handler = proc_dointvec_minmax, + .proc_handler = proc_doulongvec_minmax, .extra1 = SYSCTL_ZERO, + .extra2 = SYSCTL_INT_MAX, }, { .procname = "max_user_marks", .data = &init_user_ns.ucount_max[UCOUNT_FANOTIFY_MARKS], - .maxlen = sizeof(int), + .maxlen = sizeof(long), .mode = 0644, - .proc_handler = proc_dointvec_minmax, + .proc_handler = proc_doulongvec_minmax, .extra1 = SYSCTL_ZERO, + .extra2 = SYSCTL_INT_MAX, }, { .procname = "max_queued_events", diff --git a/fs/notify/inotify/inotify_user.c b/fs/notify/inotify/inotify_user.c index 98f61b31745a..55fe7cdea2fb 100644 --- a/fs/notify/inotify/inotify_user.c +++ b/fs/notify/inotify/inotify_user.c @@ -59,18 +59,20 @@ struct ctl_table inotify_table[] = { { .procname = "max_user_instances", .data = &init_user_ns.ucount_max[UCOUNT_INOTIFY_INSTANCES], - .maxlen = sizeof(int), + .maxlen = sizeof(long), .mode = 0644, - .proc_handler = proc_dointvec_minmax, + .proc_handler = proc_doulongvec_minmax, .extra1 = SYSCTL_ZERO, + .extra2 = SYSCTL_INT_MAX, }, { .procname = "max_user_watches", .data = &init_user_ns.ucount_max[UCOUNT_INOTIFY_WATCHES], - .maxlen = sizeof(int), + .maxlen = sizeof(long), .mode = 0644, - .proc_handler = proc_dointvec_minmax, + .proc_handler = proc_doulongvec_minmax, .extra1 = SYSCTL_ZERO, + .extra2 = SYSCTL_INT_MAX, }, { .procname = "max_queued_events", diff --git a/kernel/ucount.c b/kernel/ucount.c index 87799e2379bd..f852591e395c 100644 --- a/kernel/ucount.c +++ b/kernel/ucount.c @@ -58,14 +58,14 @@ static struct ctl_table_root set_root = { .permissions = set_permissions, }; -#define UCOUNT_ENTRY(name) \ - { \ - .procname = name, \ - .maxlen = sizeof(int), \ - .mode = 0644, \ - .proc_handler = proc_dointvec_minmax, \ - .extra1 = SYSCTL_ZERO, \ - .extra2 = SYSCTL_INT_MAX, \ +#define UCOUNT_ENTRY(name) \ + { \ + .procname = name, \ + .maxlen = sizeof(long), \ + .mode = 0644, \ + .proc_handler = proc_doulongvec_minmax, \ + .extra1 = SYSCTL_ZERO, \ + .extra2 = SYSCTL_INT_MAX, \ } static struct ctl_table user_table[] = { UCOUNT_ENTRY("max_user_namespaces"), -- 2.25.1
Sven Schnelle <svens@linux.ibm.com> writes: > commit f9c82a4ea89c3 ("Increase size of ucounts to atomic_long_t") > changed the data type of ucounts/ucounts_max to long, but missed to > adjust a few other places. This is noticeable on big endian platforms > from user space because the /proc/sys/user/max_*_names files all > contain 0. Applied. Thank you. > > Fixes: f9c82a4ea89c ("Increase size of ucounts to atomic_long_t") > Signed-off-by: Sven Schnelle <svens@linux.ibm.com> > --- > fs/notify/fanotify/fanotify_user.c | 10 ++++++---- > fs/notify/inotify/inotify_user.c | 10 ++++++---- > kernel/ucount.c | 16 ++++++++-------- > 3 files changed, 20 insertions(+), 16 deletions(-) > > diff --git a/fs/notify/fanotify/fanotify_user.c b/fs/notify/fanotify/fanotify_user.c > index 64864fb40b40..6576657a1a25 100644 > --- a/fs/notify/fanotify/fanotify_user.c > +++ b/fs/notify/fanotify/fanotify_user.c > @@ -58,18 +58,20 @@ struct ctl_table fanotify_table[] = { > { > .procname = "max_user_groups", > .data = &init_user_ns.ucount_max[UCOUNT_FANOTIFY_GROUPS], > - .maxlen = sizeof(int), > + .maxlen = sizeof(long), > .mode = 0644, > - .proc_handler = proc_dointvec_minmax, > + .proc_handler = proc_doulongvec_minmax, > .extra1 = SYSCTL_ZERO, > + .extra2 = SYSCTL_INT_MAX, > }, > { > .procname = "max_user_marks", > .data = &init_user_ns.ucount_max[UCOUNT_FANOTIFY_MARKS], > - .maxlen = sizeof(int), > + .maxlen = sizeof(long), > .mode = 0644, > - .proc_handler = proc_dointvec_minmax, > + .proc_handler = proc_doulongvec_minmax, > .extra1 = SYSCTL_ZERO, > + .extra2 = SYSCTL_INT_MAX, > }, > { > .procname = "max_queued_events", > diff --git a/fs/notify/inotify/inotify_user.c b/fs/notify/inotify/inotify_user.c > index 98f61b31745a..55fe7cdea2fb 100644 > --- a/fs/notify/inotify/inotify_user.c > +++ b/fs/notify/inotify/inotify_user.c > @@ -59,18 +59,20 @@ struct ctl_table inotify_table[] = { > { > .procname = "max_user_instances", > .data = &init_user_ns.ucount_max[UCOUNT_INOTIFY_INSTANCES], > - .maxlen = sizeof(int), > + .maxlen = sizeof(long), > .mode = 0644, > - .proc_handler = proc_dointvec_minmax, > + .proc_handler = proc_doulongvec_minmax, > .extra1 = SYSCTL_ZERO, > + .extra2 = SYSCTL_INT_MAX, > }, > { > .procname = "max_user_watches", > .data = &init_user_ns.ucount_max[UCOUNT_INOTIFY_WATCHES], > - .maxlen = sizeof(int), > + .maxlen = sizeof(long), > .mode = 0644, > - .proc_handler = proc_dointvec_minmax, > + .proc_handler = proc_doulongvec_minmax, > .extra1 = SYSCTL_ZERO, > + .extra2 = SYSCTL_INT_MAX, > }, > { > .procname = "max_queued_events", > diff --git a/kernel/ucount.c b/kernel/ucount.c > index 87799e2379bd..f852591e395c 100644 > --- a/kernel/ucount.c > +++ b/kernel/ucount.c > @@ -58,14 +58,14 @@ static struct ctl_table_root set_root = { > .permissions = set_permissions, > }; > > -#define UCOUNT_ENTRY(name) \ > - { \ > - .procname = name, \ > - .maxlen = sizeof(int), \ > - .mode = 0644, \ > - .proc_handler = proc_dointvec_minmax, \ > - .extra1 = SYSCTL_ZERO, \ > - .extra2 = SYSCTL_INT_MAX, \ > +#define UCOUNT_ENTRY(name) \ > + { \ > + .procname = name, \ > + .maxlen = sizeof(long), \ > + .mode = 0644, \ > + .proc_handler = proc_doulongvec_minmax, \ > + .extra1 = SYSCTL_ZERO, \ > + .extra2 = SYSCTL_INT_MAX, \ > } > static struct ctl_table user_table[] = { > UCOUNT_ENTRY("max_user_namespaces"),
On Fri, Jul 30, 2021 at 08:28:54AM +0200, Sven Schnelle wrote: > commit f9c82a4ea89c3 ("Increase size of ucounts to atomic_long_t") > changed the data type of ucounts/ucounts_max to long, but missed to > adjust a few other places. This is noticeable on big endian platforms > from user space because the /proc/sys/user/max_*_names files all > contain 0. > > Fixes: f9c82a4ea89c ("Increase size of ucounts to atomic_long_t") > Signed-off-by: Sven Schnelle <svens@linux.ibm.com> This patch in -next as commit e43fc41d1f7f ("ucounts: add missing data type changes") causes Windows Subsystem for Linux to fail to start: [error 0x8007010b when launching `wsl.exe -d Arch'] Could not access starting directory "\\wsl$\Arch\home\nathan" Specifically, it is the change to max_user_watches in fs/notify/inotify/inotify_user.c, as the below diff gets me back to working. Unfortunately, I have no additional information to offer beyond that as WSL's init is custom and closed source (as far as I am aware) and there are no real debugging utilities. Cheers, Nathan diff --git a/fs/notify/inotify/inotify_user.c b/fs/notify/inotify/inotify_user.c index 55fe7cdea2fb..32178a95c1b3 100644 --- a/fs/notify/inotify/inotify_user.c +++ b/fs/notify/inotify/inotify_user.c @@ -68,9 +68,9 @@ struct ctl_table inotify_table[] = { { .procname = "max_user_watches", .data = &init_user_ns.ucount_max[UCOUNT_INOTIFY_WATCHES], - .maxlen = sizeof(long), + .maxlen = sizeof(int), .mode = 0644, - .proc_handler = proc_doulongvec_minmax, + .proc_handler = proc_dointvec_minmax, .extra1 = SYSCTL_ZERO, .extra2 = SYSCTL_INT_MAX, },
Nathan Chancellor <nathan@kernel.org> writes: > On Fri, Jul 30, 2021 at 08:28:54AM +0200, Sven Schnelle wrote: >> commit f9c82a4ea89c3 ("Increase size of ucounts to atomic_long_t") >> changed the data type of ucounts/ucounts_max to long, but missed to >> adjust a few other places. This is noticeable on big endian platforms >> from user space because the /proc/sys/user/max_*_names files all >> contain 0. >> >> Fixes: f9c82a4ea89c ("Increase size of ucounts to atomic_long_t") >> Signed-off-by: Sven Schnelle <svens@linux.ibm.com> > > This patch in -next as commit e43fc41d1f7f ("ucounts: add missing data type > changes") causes Windows Subsystem for Linux to fail to start: > > [error 0x8007010b when launching `wsl.exe -d Arch'] Could not access starting > directory "\\wsl$\Arch\home\nathan" > > Specifically, it is the change to max_user_watches in > fs/notify/inotify/inotify_user.c, as the below diff gets me back to working. > Unfortunately, I have no additional information to offer beyond that as WSL's > init is custom and closed source (as far as I am aware) and there are no real > debugging utilities. Could you try this patch and tell us what value is being set? The only think I can imagine is that someone wants unlimited watches and sets the value to a ridiculously large value and the interpretation of that value winds up being different between int and long. This should allow you to read either dmesg or the kernel's log as it boots up and see what value is being written. From there it should be relatively straight forward to figure out what is going on. Thank you, Eric diff --git a/kernel/sysctl.c b/kernel/sysctl.c index 272f4a272f8c..733c4cfa1f60 100644 --- a/kernel/sysctl.c +++ b/kernel/sysctl.c @@ -975,6 +975,14 @@ int proc_dointvec_minmax(struct ctl_table *table, int write, .min = (int *) table->extra1, .max = (int *) table->extra2, }; +#if 1 + size_t lenv = *lenp; + if (write && (lenv > 0) && (lenv < INT_MAX)) { + int len = lenv; + printk(KERN_EMERG "intvec: %s <- %*.*s\n", + table->procname, len, len, (char *)buffer); + } +#endif return do_proc_dointvec(table, write, buffer, lenp, ppos, do_proc_dointvec_minmax_conv, ¶m); }
Hi Eric, On 8/4/2021 12:47 PM, Eric W. Biederman wrote: > Nathan Chancellor <nathan@kernel.org> writes: > >> On Fri, Jul 30, 2021 at 08:28:54AM +0200, Sven Schnelle wrote: >>> commit f9c82a4ea89c3 ("Increase size of ucounts to atomic_long_t") >>> changed the data type of ucounts/ucounts_max to long, but missed to >>> adjust a few other places. This is noticeable on big endian platforms >>> from user space because the /proc/sys/user/max_*_names files all >>> contain 0. >>> >>> Fixes: f9c82a4ea89c ("Increase size of ucounts to atomic_long_t") >>> Signed-off-by: Sven Schnelle <svens@linux.ibm.com> >> >> This patch in -next as commit e43fc41d1f7f ("ucounts: add missing data type >> changes") causes Windows Subsystem for Linux to fail to start: >> >> [error 0x8007010b when launching `wsl.exe -d Arch'] Could not access starting >> directory "\\wsl$\Arch\home\nathan" >> >> Specifically, it is the change to max_user_watches in >> fs/notify/inotify/inotify_user.c, as the below diff gets me back to working. >> Unfortunately, I have no additional information to offer beyond that as WSL's >> init is custom and closed source (as far as I am aware) and there are no real >> debugging utilities. > > Could you try this patch and tell us what value is being set? > > The only think I can imagine is that someone wants unlimited watches and > sets the value to a ridiculously large value and the interpretation of > that value winds up being different between int and long. > > This should allow you to read either dmesg or the kernel's log as it > boots up and see what value is being written. From there it should > be relatively straight forward to figure out what is going on. I applied this diff on top of mine and running 'dmesg |& grep intvec' shows: [ 0.282500] intvec: dmesg_restrict <- 0 [ 0.282510] intvec: max_user_watches <- 524288 This seems much smaller than INT_MAX so I am not sure how the value could be different between int and long but I am not at all familiar with the sysctl code. More than happy to continue to test debug patches or provide any additional information as I can. Cheers, Nathan > diff --git a/kernel/sysctl.c b/kernel/sysctl.c > index 272f4a272f8c..733c4cfa1f60 100644 > --- a/kernel/sysctl.c > +++ b/kernel/sysctl.c > @@ -975,6 +975,14 @@ int proc_dointvec_minmax(struct ctl_table *table, int write, > .min = (int *) table->extra1, > .max = (int *) table->extra2, > }; > +#if 1 > + size_t lenv = *lenp; > + if (write && (lenv > 0) && (lenv < INT_MAX)) { > + int len = lenv; > + printk(KERN_EMERG "intvec: %s <- %*.*s\n", > + table->procname, len, len, (char *)buffer); > + } > +#endif > return do_proc_dointvec(table, write, buffer, lenp, ppos, > do_proc_dointvec_minmax_conv, ¶m); > } >
Nathan Chancellor <nathan@kernel.org> writes: > Hi Eric, > > On 8/4/2021 12:47 PM, Eric W. Biederman wrote: >> Nathan Chancellor <nathan@kernel.org> writes: >> >>> On Fri, Jul 30, 2021 at 08:28:54AM +0200, Sven Schnelle wrote: >>>> commit f9c82a4ea89c3 ("Increase size of ucounts to atomic_long_t") >>>> changed the data type of ucounts/ucounts_max to long, but missed to >>>> adjust a few other places. This is noticeable on big endian platforms >>>> from user space because the /proc/sys/user/max_*_names files all >>>> contain 0. >>>> >>>> Fixes: f9c82a4ea89c ("Increase size of ucounts to atomic_long_t") >>>> Signed-off-by: Sven Schnelle <svens@linux.ibm.com> >>> >>> This patch in -next as commit e43fc41d1f7f ("ucounts: add missing data type >>> changes") causes Windows Subsystem for Linux to fail to start: >>> >>> [error 0x8007010b when launching `wsl.exe -d Arch'] Could not access starting >>> directory "\\wsl$\Arch\home\nathan" >>> >>> Specifically, it is the change to max_user_watches in >>> fs/notify/inotify/inotify_user.c, as the below diff gets me back to working. >>> Unfortunately, I have no additional information to offer beyond that as WSL's >>> init is custom and closed source (as far as I am aware) and there are no real >>> debugging utilities. >> >> Could you try this patch and tell us what value is being set? >> >> The only think I can imagine is that someone wants unlimited watches and >> sets the value to a ridiculously large value and the interpretation of >> that value winds up being different between int and long. >> >> This should allow you to read either dmesg or the kernel's log as it >> boots up and see what value is being written. From there it should >> be relatively straight forward to figure out what is going on. > > I applied this diff on top of mine and running 'dmesg |& grep intvec' shows: > > [ 0.282500] intvec: dmesg_restrict <- 0 > [ 0.282510] intvec: max_user_watches <- 524288 > > This seems much smaller than INT_MAX so I am not sure how the value could be > different between int and long but I am not at all familiar with the sysctl > code. > > More than happy to continue to test debug patches or provide any additional > information as I can. Yes. Very strange. Could you perhaps try the instrumenting proc_doulongvec_minmax the same way and see what is written in the failing case? While looking at the code I did see one other serious bug. The min and max values are int constants intstead of long constants. Could you test the change below and see if it makes a difference? Eric diff --git a/fs/notify/fanotify/fanotify_user.c b/fs/notify/fanotify/fanotify_user.c index 6576657a1a25..28b67cb9458d 100644 --- a/fs/notify/fanotify/fanotify_user.c +++ b/fs/notify/fanotify/fanotify_user.c @@ -54,6 +54,9 @@ static int fanotify_max_queued_events __read_mostly; #include <linux/sysctl.h> +static long ft_zero = 0; +static long ft_int_max = INT_MAX; + struct ctl_table fanotify_table[] = { { .procname = "max_user_groups", @@ -61,8 +64,8 @@ struct ctl_table fanotify_table[] = { .maxlen = sizeof(long), .mode = 0644, .proc_handler = proc_doulongvec_minmax, - .extra1 = SYSCTL_ZERO, - .extra2 = SYSCTL_INT_MAX, + .extra1 = &ft_zero, + .extra2 = &ft_int_max, }, { .procname = "max_user_marks", @@ -70,8 +73,8 @@ struct ctl_table fanotify_table[] = { .maxlen = sizeof(long), .mode = 0644, .proc_handler = proc_doulongvec_minmax, - .extra1 = SYSCTL_ZERO, - .extra2 = SYSCTL_INT_MAX, + .extra1 = &ft_zero, + .extra2 = &ft_int_max, }, { .procname = "max_queued_events", diff --git a/fs/notify/inotify/inotify_user.c b/fs/notify/inotify/inotify_user.c index 55fe7cdea2fb..62051247f6d2 100644 --- a/fs/notify/inotify/inotify_user.c +++ b/fs/notify/inotify/inotify_user.c @@ -55,6 +55,9 @@ struct kmem_cache *inotify_inode_mark_cachep __read_mostly; #include <linux/sysctl.h> +static long it_zero = 0; +static long it_int_max = INT_MAX; + struct ctl_table inotify_table[] = { { .procname = "max_user_instances", @@ -62,8 +65,8 @@ struct ctl_table inotify_table[] = { .maxlen = sizeof(long), .mode = 0644, .proc_handler = proc_doulongvec_minmax, - .extra1 = SYSCTL_ZERO, - .extra2 = SYSCTL_INT_MAX, + .extra1 = &it_zero, + .extra2 = &it_int_max, }, { .procname = "max_user_watches", @@ -71,8 +74,8 @@ struct ctl_table inotify_table[] = { .maxlen = sizeof(long), .mode = 0644, .proc_handler = proc_doulongvec_minmax, - .extra1 = SYSCTL_ZERO, - .extra2 = SYSCTL_INT_MAX, + .extra1 = &it_zero, + .extra2 = &it_int_max, }, { .procname = "max_queued_events", diff --git a/kernel/ucount.c b/kernel/ucount.c index 260ae7da815f..bb51849e6375 100644 --- a/kernel/ucount.c +++ b/kernel/ucount.c @@ -58,14 +58,17 @@ static struct ctl_table_root set_root = { .permissions = set_permissions, }; +static long ue_zero = 0; +static long ue_int_max = INT_MAX; + #define UCOUNT_ENTRY(name) \ { \ .procname = name, \ .maxlen = sizeof(long), \ .mode = 0644, \ .proc_handler = proc_doulongvec_minmax, \ - .extra1 = SYSCTL_ZERO, \ - .extra2 = SYSCTL_INT_MAX, \ + .extra1 = &ue_zero, \ + .extra2 = &ue_int_max, \ } static struct ctl_table user_table[] = { UCOUNT_ENTRY("max_user_namespaces"),
On 8/5/2021 9:48 AM, Eric W. Biederman wrote: > Nathan Chancellor <nathan@kernel.org> writes: > >> Hi Eric, >> >> On 8/4/2021 12:47 PM, Eric W. Biederman wrote: >>> Nathan Chancellor <nathan@kernel.org> writes: >>> >>>> On Fri, Jul 30, 2021 at 08:28:54AM +0200, Sven Schnelle wrote: >>>>> commit f9c82a4ea89c3 ("Increase size of ucounts to atomic_long_t") >>>>> changed the data type of ucounts/ucounts_max to long, but missed to >>>>> adjust a few other places. This is noticeable on big endian platforms >>>>> from user space because the /proc/sys/user/max_*_names files all >>>>> contain 0. >>>>> >>>>> Fixes: f9c82a4ea89c ("Increase size of ucounts to atomic_long_t") >>>>> Signed-off-by: Sven Schnelle <svens@linux.ibm.com> >>>> >>>> This patch in -next as commit e43fc41d1f7f ("ucounts: add missing data type >>>> changes") causes Windows Subsystem for Linux to fail to start: >>>> >>>> [error 0x8007010b when launching `wsl.exe -d Arch'] Could not access starting >>>> directory "\\wsl$\Arch\home\nathan" >>>> >>>> Specifically, it is the change to max_user_watches in >>>> fs/notify/inotify/inotify_user.c, as the below diff gets me back to working. >>>> Unfortunately, I have no additional information to offer beyond that as WSL's >>>> init is custom and closed source (as far as I am aware) and there are no real >>>> debugging utilities. >>> >>> Could you try this patch and tell us what value is being set? >>> >>> The only think I can imagine is that someone wants unlimited watches and >>> sets the value to a ridiculously large value and the interpretation of >>> that value winds up being different between int and long. >>> >>> This should allow you to read either dmesg or the kernel's log as it >>> boots up and see what value is being written. From there it should >>> be relatively straight forward to figure out what is going on. >> >> I applied this diff on top of mine and running 'dmesg |& grep intvec' shows: >> >> [ 0.282500] intvec: dmesg_restrict <- 0 >> [ 0.282510] intvec: max_user_watches <- 524288 >> >> This seems much smaller than INT_MAX so I am not sure how the value could be >> different between int and long but I am not at all familiar with the sysctl >> code. >> >> More than happy to continue to test debug patches or provide any additional >> information as I can. > > Yes. Very strange. > > Could you perhaps try the instrumenting proc_doulongvec_minmax the same > way and see what is written in the failing case? > > While looking at the code I did see one other serious bug. The min and > max values are int constants intstead of long constants. > > Could you test the change below and see if it makes a difference? That indeed fixes the issue! I assume you will squash it into the original commit but if not, feel free to add: Tested-by: Nathan Chancellor <nathan@kernel.org> > Eric > > > diff --git a/fs/notify/fanotify/fanotify_user.c b/fs/notify/fanotify/fanotify_user.c > index 6576657a1a25..28b67cb9458d 100644 > --- a/fs/notify/fanotify/fanotify_user.c > +++ b/fs/notify/fanotify/fanotify_user.c > @@ -54,6 +54,9 @@ static int fanotify_max_queued_events __read_mostly; > > #include <linux/sysctl.h> > > +static long ft_zero = 0; > +static long ft_int_max = INT_MAX; > + > struct ctl_table fanotify_table[] = { > { > .procname = "max_user_groups", > @@ -61,8 +64,8 @@ struct ctl_table fanotify_table[] = { > .maxlen = sizeof(long), > .mode = 0644, > .proc_handler = proc_doulongvec_minmax, > - .extra1 = SYSCTL_ZERO, > - .extra2 = SYSCTL_INT_MAX, > + .extra1 = &ft_zero, > + .extra2 = &ft_int_max, > }, > { > .procname = "max_user_marks", > @@ -70,8 +73,8 @@ struct ctl_table fanotify_table[] = { > .maxlen = sizeof(long), > .mode = 0644, > .proc_handler = proc_doulongvec_minmax, > - .extra1 = SYSCTL_ZERO, > - .extra2 = SYSCTL_INT_MAX, > + .extra1 = &ft_zero, > + .extra2 = &ft_int_max, > }, > { > .procname = "max_queued_events", > diff --git a/fs/notify/inotify/inotify_user.c b/fs/notify/inotify/inotify_user.c > index 55fe7cdea2fb..62051247f6d2 100644 > --- a/fs/notify/inotify/inotify_user.c > +++ b/fs/notify/inotify/inotify_user.c > @@ -55,6 +55,9 @@ struct kmem_cache *inotify_inode_mark_cachep __read_mostly; > > #include <linux/sysctl.h> > > +static long it_zero = 0; > +static long it_int_max = INT_MAX; > + > struct ctl_table inotify_table[] = { > { > .procname = "max_user_instances", > @@ -62,8 +65,8 @@ struct ctl_table inotify_table[] = { > .maxlen = sizeof(long), > .mode = 0644, > .proc_handler = proc_doulongvec_minmax, > - .extra1 = SYSCTL_ZERO, > - .extra2 = SYSCTL_INT_MAX, > + .extra1 = &it_zero, > + .extra2 = &it_int_max, > }, > { > .procname = "max_user_watches", > @@ -71,8 +74,8 @@ struct ctl_table inotify_table[] = { > .maxlen = sizeof(long), > .mode = 0644, > .proc_handler = proc_doulongvec_minmax, > - .extra1 = SYSCTL_ZERO, > - .extra2 = SYSCTL_INT_MAX, > + .extra1 = &it_zero, > + .extra2 = &it_int_max, > }, > { > .procname = "max_queued_events", > diff --git a/kernel/ucount.c b/kernel/ucount.c > index 260ae7da815f..bb51849e6375 100644 > --- a/kernel/ucount.c > +++ b/kernel/ucount.c > @@ -58,14 +58,17 @@ static struct ctl_table_root set_root = { > .permissions = set_permissions, > }; > > +static long ue_zero = 0; > +static long ue_int_max = INT_MAX; > + > #define UCOUNT_ENTRY(name) \ > { \ > .procname = name, \ > .maxlen = sizeof(long), \ > .mode = 0644, \ > .proc_handler = proc_doulongvec_minmax, \ > - .extra1 = SYSCTL_ZERO, \ > - .extra2 = SYSCTL_INT_MAX, \ > + .extra1 = &ue_zero, \ > + .extra2 = &ue_int_max, \ > } > static struct ctl_table user_table[] = { > UCOUNT_ENTRY("max_user_namespaces"), > > >
On Fri, 6 Aug 2021 at 00:56, Nathan Chancellor <nathan@kernel.org> wrote: > > On 8/5/2021 9:48 AM, Eric W. Biederman wrote: > > Nathan Chancellor <nathan@kernel.org> writes: > > > >> Hi Eric, > >> > >> On 8/4/2021 12:47 PM, Eric W. Biederman wrote: > >>> Nathan Chancellor <nathan@kernel.org> writes: > >>> > >>>> On Fri, Jul 30, 2021 at 08:28:54AM +0200, Sven Schnelle wrote: > >>>>> commit f9c82a4ea89c3 ("Increase size of ucounts to atomic_long_t") > >>>>> changed the data type of ucounts/ucounts_max to long, but missed to > >>>>> adjust a few other places. This is noticeable on big endian platforms > >>>>> from user space because the /proc/sys/user/max_*_names files all > >>>>> contain 0. > >>>>> > >>>>> Fixes: f9c82a4ea89c ("Increase size of ucounts to atomic_long_t") > >>>>> Signed-off-by: Sven Schnelle <svens@linux.ibm.com> > >>>> > >>>> This patch in -next as commit e43fc41d1f7f ("ucounts: add missing data type > >>>> changes") causes Windows Subsystem for Linux to fail to start: On Linux next-20210802..next-20210805 we have been noticing that LTP syscalls inotify06 test case getting failed all architectures. BAD: Linux next-20210802 inotify06.c:57: TBROK: Failed to close FILE '/proc/sys/fs/inotify/max_user_instances': EINVAL (22) inotify06.c:107: TWARN: Failed to close FILE '/proc/sys/fs/inotify/max_user_instances': EINVAL (22) GOOD: Linux next-20210730 inotify06.c:92: TPASS: kernel survived inotify beating Investigation: Following changes found between good and bad Linux next tags under fs/notify git log --oneline --stat next-20210730..next-20210802 fs/notify e43fc41d1f7f ucounts: add missing data type changes fs/notify/fanotify/fanotify_user.c | 10 ++++++---- fs/notify/inotify/inotify_user.c | 10 ++++++---- 2 files changed, 12 insertions(+), 8 deletions(-) Conclusion: We have confirmed this patch caused the LTP syscalls inotify06 test failure. After applying your proposed fix patch [1] the reported test getting pass. However, I have to run full test plan to confirm this do not cause regressions. Tested-by: Linux Kernel Functional Testing <lkft@linaro.org> [1] https://lore.kernel.org/lkml/87v94jalck.fsf@disp2133/ > > diff --git a/fs/notify/fanotify/fanotify_user.c b/fs/notify/fanotify/fanotify_user.c > > index 6576657a1a25..28b67cb9458d 100644 > > --- a/fs/notify/fanotify/fanotify_user.c > > +++ b/fs/notify/fanotify/fanotify_user.c > > @@ -54,6 +54,9 @@ static int fanotify_max_queued_events __read_mostly; > > > > #include <linux/sysctl.h> > > > > +static long ft_zero = 0; > > +static long ft_int_max = INT_MAX; > > + > > struct ctl_table fanotify_table[] = { > > { > > .procname = "max_user_groups", > > @@ -61,8 +64,8 @@ struct ctl_table fanotify_table[] = { > > .maxlen = sizeof(long), > > .mode = 0644, > > .proc_handler = proc_doulongvec_minmax, > > - .extra1 = SYSCTL_ZERO, > > - .extra2 = SYSCTL_INT_MAX, > > + .extra1 = &ft_zero, > > + .extra2 = &ft_int_max, > > }, > > { > > .procname = "max_user_marks", > > @@ -70,8 +73,8 @@ struct ctl_table fanotify_table[] = { > > .maxlen = sizeof(long), > > .mode = 0644, > > .proc_handler = proc_doulongvec_minmax, > > - .extra1 = SYSCTL_ZERO, > > - .extra2 = SYSCTL_INT_MAX, > > + .extra1 = &ft_zero, > > + .extra2 = &ft_int_max, > > }, > > { > > .procname = "max_queued_events", > > diff --git a/fs/notify/inotify/inotify_user.c b/fs/notify/inotify/inotify_user.c > > index 55fe7cdea2fb..62051247f6d2 100644 > > --- a/fs/notify/inotify/inotify_user.c > > +++ b/fs/notify/inotify/inotify_user.c > > @@ -55,6 +55,9 @@ struct kmem_cache *inotify_inode_mark_cachep __read_mostly; > > > > #include <linux/sysctl.h> > > > > +static long it_zero = 0; > > +static long it_int_max = INT_MAX; > > + > > struct ctl_table inotify_table[] = { > > { > > .procname = "max_user_instances", > > @@ -62,8 +65,8 @@ struct ctl_table inotify_table[] = { > > .maxlen = sizeof(long), > > .mode = 0644, > > .proc_handler = proc_doulongvec_minmax, > > - .extra1 = SYSCTL_ZERO, > > - .extra2 = SYSCTL_INT_MAX, > > + .extra1 = &it_zero, > > + .extra2 = &it_int_max, > > }, > > { > > .procname = "max_user_watches", > > @@ -71,8 +74,8 @@ struct ctl_table inotify_table[] = { > > .maxlen = sizeof(long), > > .mode = 0644, > > .proc_handler = proc_doulongvec_minmax, > > - .extra1 = SYSCTL_ZERO, > > - .extra2 = SYSCTL_INT_MAX, > > + .extra1 = &it_zero, > > + .extra2 = &it_int_max, > > }, > > { > > .procname = "max_queued_events", > > diff --git a/kernel/ucount.c b/kernel/ucount.c > > index 260ae7da815f..bb51849e6375 100644 > > --- a/kernel/ucount.c > > +++ b/kernel/ucount.c > > @@ -58,14 +58,17 @@ static struct ctl_table_root set_root = { > > .permissions = set_permissions, > > }; > > > > +static long ue_zero = 0; > > +static long ue_int_max = INT_MAX; > > + > > #define UCOUNT_ENTRY(name) \ > > { \ > > .procname = name, \ > > .maxlen = sizeof(long), \ > > .mode = 0644, \ > > .proc_handler = proc_doulongvec_minmax, \ > > - .extra1 = SYSCTL_ZERO, \ > > - .extra2 = SYSCTL_INT_MAX, \ > > + .extra1 = &ue_zero, \ > > + .extra2 = &ue_int_max, \ > > } > > static struct ctl_table user_table[] = { > > UCOUNT_ENTRY("max_user_namespaces"), -- Linaro LKFT https://lkft.linaro.org
commit f9c82a4ea89c3 ("Increase size of ucounts to atomic_long_t") changed the data type of ucounts/ucounts_max to long, but missed to adjust a few other places. This is noticeable on big endian platforms from user space because the /proc/sys/user/max_*_names files all contain 0. v4 - Made the min and max constants long so the sysctl values are actually settable on little endian machines. -- EWB Fixes: f9c82a4ea89c ("Increase size of ucounts to atomic_long_t") Signed-off-by: Sven Schnelle <svens@linux.ibm.com> Tested-by: Nathan Chancellor <nathan@kernel.org> Tested-by: Linux Kernel Functional Testing <lkft@linaro.org> Acked-by: Alexey Gladkov <legion@kernel.org> v1: https://lkml.kernel.org/r/20210721115800.910778-1-svens@linux.ibm.com v2: https://lkml.kernel.org/r/20210721125233.1041429-1-svens@linux.ibm.com v3: https://lkml.kernel.org/r/20210730062854.3601635-1-svens@linux.ibm.com Signed-off-by: Eric W. Biederman <ebiederm@xmission.com> --- Thanks everyone for testing and helping find the cause of this bug. I will push this out to linux-next shortly. fs/notify/fanotify/fanotify_user.c | 17 +++++++++++------ fs/notify/inotify/inotify_user.c | 17 +++++++++++------ kernel/ucount.c | 19 +++++++++++-------- 3 files changed, 33 insertions(+), 20 deletions(-) diff --git a/fs/notify/fanotify/fanotify_user.c b/fs/notify/fanotify/fanotify_user.c index 64864fb40b40..28b67cb9458d 100644 --- a/fs/notify/fanotify/fanotify_user.c +++ b/fs/notify/fanotify/fanotify_user.c @@ -54,22 +54,27 @@ static int fanotify_max_queued_events __read_mostly; #include <linux/sysctl.h> +static long ft_zero = 0; +static long ft_int_max = INT_MAX; + struct ctl_table fanotify_table[] = { { .procname = "max_user_groups", .data = &init_user_ns.ucount_max[UCOUNT_FANOTIFY_GROUPS], - .maxlen = sizeof(int), + .maxlen = sizeof(long), .mode = 0644, - .proc_handler = proc_dointvec_minmax, - .extra1 = SYSCTL_ZERO, + .proc_handler = proc_doulongvec_minmax, + .extra1 = &ft_zero, + .extra2 = &ft_int_max, }, { .procname = "max_user_marks", .data = &init_user_ns.ucount_max[UCOUNT_FANOTIFY_MARKS], - .maxlen = sizeof(int), + .maxlen = sizeof(long), .mode = 0644, - .proc_handler = proc_dointvec_minmax, - .extra1 = SYSCTL_ZERO, + .proc_handler = proc_doulongvec_minmax, + .extra1 = &ft_zero, + .extra2 = &ft_int_max, }, { .procname = "max_queued_events", diff --git a/fs/notify/inotify/inotify_user.c b/fs/notify/inotify/inotify_user.c index 98f61b31745a..62051247f6d2 100644 --- a/fs/notify/inotify/inotify_user.c +++ b/fs/notify/inotify/inotify_user.c @@ -55,22 +55,27 @@ struct kmem_cache *inotify_inode_mark_cachep __read_mostly; #include <linux/sysctl.h> +static long it_zero = 0; +static long it_int_max = INT_MAX; + struct ctl_table inotify_table[] = { { .procname = "max_user_instances", .data = &init_user_ns.ucount_max[UCOUNT_INOTIFY_INSTANCES], - .maxlen = sizeof(int), + .maxlen = sizeof(long), .mode = 0644, - .proc_handler = proc_dointvec_minmax, - .extra1 = SYSCTL_ZERO, + .proc_handler = proc_doulongvec_minmax, + .extra1 = &it_zero, + .extra2 = &it_int_max, }, { .procname = "max_user_watches", .data = &init_user_ns.ucount_max[UCOUNT_INOTIFY_WATCHES], - .maxlen = sizeof(int), + .maxlen = sizeof(long), .mode = 0644, - .proc_handler = proc_dointvec_minmax, - .extra1 = SYSCTL_ZERO, + .proc_handler = proc_doulongvec_minmax, + .extra1 = &it_zero, + .extra2 = &it_int_max, }, { .procname = "max_queued_events", diff --git a/kernel/ucount.c b/kernel/ucount.c index 77be3bbe3cc4..bb51849e6375 100644 --- a/kernel/ucount.c +++ b/kernel/ucount.c @@ -58,14 +58,17 @@ static struct ctl_table_root set_root = { .permissions = set_permissions, }; -#define UCOUNT_ENTRY(name) \ - { \ - .procname = name, \ - .maxlen = sizeof(int), \ - .mode = 0644, \ - .proc_handler = proc_dointvec_minmax, \ - .extra1 = SYSCTL_ZERO, \ - .extra2 = SYSCTL_INT_MAX, \ +static long ue_zero = 0; +static long ue_int_max = INT_MAX; + +#define UCOUNT_ENTRY(name) \ + { \ + .procname = name, \ + .maxlen = sizeof(long), \ + .mode = 0644, \ + .proc_handler = proc_doulongvec_minmax, \ + .extra1 = &ue_zero, \ + .extra2 = &ue_int_max, \ } static struct ctl_table user_table[] = { UCOUNT_ENTRY("max_user_namespaces"), -- 2.20.1
On Mon 09-08-21 15:43:56, Eric W. Biederman wrote: > > commit f9c82a4ea89c3 ("Increase size of ucounts to atomic_long_t") > changed the data type of ucounts/ucounts_max to long, but missed to > adjust a few other places. This is noticeable on big endian platforms > from user space because the /proc/sys/user/max_*_names files all > contain 0. > > v4 - Made the min and max constants long so the sysctl values > are actually settable on little endian machines. > -- EWB > > Fixes: f9c82a4ea89c ("Increase size of ucounts to atomic_long_t") > Signed-off-by: Sven Schnelle <svens@linux.ibm.com> > Tested-by: Nathan Chancellor <nathan@kernel.org> > Tested-by: Linux Kernel Functional Testing <lkft@linaro.org> > Acked-by: Alexey Gladkov <legion@kernel.org> > v1: https://lkml.kernel.org/r/20210721115800.910778-1-svens@linux.ibm.com > v2: https://lkml.kernel.org/r/20210721125233.1041429-1-svens@linux.ibm.com > v3: https://lkml.kernel.org/r/20210730062854.3601635-1-svens@linux.ibm.com > Signed-off-by: Eric W. Biederman <ebiederm@xmission.com> Looks good. Feel free to add: Reviewed-by: Jan Kara <jack@suse.cz> Honza > --- > > Thanks everyone for testing and helping find the cause of this bug. I > will push this out to linux-next shortly. > > fs/notify/fanotify/fanotify_user.c | 17 +++++++++++------ > fs/notify/inotify/inotify_user.c | 17 +++++++++++------ > kernel/ucount.c | 19 +++++++++++-------- > 3 files changed, 33 insertions(+), 20 deletions(-) > > diff --git a/fs/notify/fanotify/fanotify_user.c b/fs/notify/fanotify/fanotify_user.c > index 64864fb40b40..28b67cb9458d 100644 > --- a/fs/notify/fanotify/fanotify_user.c > +++ b/fs/notify/fanotify/fanotify_user.c > @@ -54,22 +54,27 @@ static int fanotify_max_queued_events __read_mostly; > > #include <linux/sysctl.h> > > +static long ft_zero = 0; > +static long ft_int_max = INT_MAX; > + > struct ctl_table fanotify_table[] = { > { > .procname = "max_user_groups", > .data = &init_user_ns.ucount_max[UCOUNT_FANOTIFY_GROUPS], > - .maxlen = sizeof(int), > + .maxlen = sizeof(long), > .mode = 0644, > - .proc_handler = proc_dointvec_minmax, > - .extra1 = SYSCTL_ZERO, > + .proc_handler = proc_doulongvec_minmax, > + .extra1 = &ft_zero, > + .extra2 = &ft_int_max, > }, > { > .procname = "max_user_marks", > .data = &init_user_ns.ucount_max[UCOUNT_FANOTIFY_MARKS], > - .maxlen = sizeof(int), > + .maxlen = sizeof(long), > .mode = 0644, > - .proc_handler = proc_dointvec_minmax, > - .extra1 = SYSCTL_ZERO, > + .proc_handler = proc_doulongvec_minmax, > + .extra1 = &ft_zero, > + .extra2 = &ft_int_max, > }, > { > .procname = "max_queued_events", > diff --git a/fs/notify/inotify/inotify_user.c b/fs/notify/inotify/inotify_user.c > index 98f61b31745a..62051247f6d2 100644 > --- a/fs/notify/inotify/inotify_user.c > +++ b/fs/notify/inotify/inotify_user.c > @@ -55,22 +55,27 @@ struct kmem_cache *inotify_inode_mark_cachep __read_mostly; > > #include <linux/sysctl.h> > > +static long it_zero = 0; > +static long it_int_max = INT_MAX; > + > struct ctl_table inotify_table[] = { > { > .procname = "max_user_instances", > .data = &init_user_ns.ucount_max[UCOUNT_INOTIFY_INSTANCES], > - .maxlen = sizeof(int), > + .maxlen = sizeof(long), > .mode = 0644, > - .proc_handler = proc_dointvec_minmax, > - .extra1 = SYSCTL_ZERO, > + .proc_handler = proc_doulongvec_minmax, > + .extra1 = &it_zero, > + .extra2 = &it_int_max, > }, > { > .procname = "max_user_watches", > .data = &init_user_ns.ucount_max[UCOUNT_INOTIFY_WATCHES], > - .maxlen = sizeof(int), > + .maxlen = sizeof(long), > .mode = 0644, > - .proc_handler = proc_dointvec_minmax, > - .extra1 = SYSCTL_ZERO, > + .proc_handler = proc_doulongvec_minmax, > + .extra1 = &it_zero, > + .extra2 = &it_int_max, > }, > { > .procname = "max_queued_events", > diff --git a/kernel/ucount.c b/kernel/ucount.c > index 77be3bbe3cc4..bb51849e6375 100644 > --- a/kernel/ucount.c > +++ b/kernel/ucount.c > @@ -58,14 +58,17 @@ static struct ctl_table_root set_root = { > .permissions = set_permissions, > }; > > -#define UCOUNT_ENTRY(name) \ > - { \ > - .procname = name, \ > - .maxlen = sizeof(int), \ > - .mode = 0644, \ > - .proc_handler = proc_dointvec_minmax, \ > - .extra1 = SYSCTL_ZERO, \ > - .extra2 = SYSCTL_INT_MAX, \ > +static long ue_zero = 0; > +static long ue_int_max = INT_MAX; > + > +#define UCOUNT_ENTRY(name) \ > + { \ > + .procname = name, \ > + .maxlen = sizeof(long), \ > + .mode = 0644, \ > + .proc_handler = proc_doulongvec_minmax, \ > + .extra1 = &ue_zero, \ > + .extra2 = &ue_int_max, \ > } > static struct ctl_table user_table[] = { > UCOUNT_ENTRY("max_user_namespaces"), > -- > 2.20.1 > -- Jan Kara <jack@suse.com> SUSE Labs, CR