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 <[email protected]>
---
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 <[email protected]> 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 <[email protected]>
> ---
> 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 <[email protected]>
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 <[email protected]> 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 <[email protected]>
>
> 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 <[email protected]> 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 <[email protected]>
>>
>> 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);
> }
>
On 8/5/2021 9:48 AM, Eric W. Biederman wrote:
> Nathan Chancellor <[email protected]> writes:
>
>> Hi Eric,
>>
>> On 8/4/2021 12:47 PM, Eric W. Biederman wrote:
>>> Nathan Chancellor <[email protected]> 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 <[email protected]>
>>>>
>>>> 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 <[email protected]>
> 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"),
>
>
>
Nathan Chancellor <[email protected]> writes:
> Hi Eric,
>
> On 8/4/2021 12:47 PM, Eric W. Biederman wrote:
>> Nathan Chancellor <[email protected]> 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 <[email protected]>
>>>
>>> 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 Fri, 6 Aug 2021 at 00:56, Nathan Chancellor <[email protected]> wrote:
>
> On 8/5/2021 9:48 AM, Eric W. Biederman wrote:
> > Nathan Chancellor <[email protected]> writes:
> >
> >> Hi Eric,
> >>
> >> On 8/4/2021 12:47 PM, Eric W. Biederman wrote:
> >>> Nathan Chancellor <[email protected]> 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 <[email protected]>
> >>>>
> >>>> 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 <[email protected]>
[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 <[email protected]>
Tested-by: Nathan Chancellor <[email protected]>
Tested-by: Linux Kernel Functional Testing <[email protected]>
Acked-by: Alexey Gladkov <[email protected]>
v1: https://lkml.kernel.org/r/[email protected]
v2: https://lkml.kernel.org/r/[email protected]
v3: https://lkml.kernel.org/r/[email protected]
Signed-off-by: Eric W. Biederman <[email protected]>
---
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 <[email protected]>
> Tested-by: Nathan Chancellor <[email protected]>
> Tested-by: Linux Kernel Functional Testing <[email protected]>
> Acked-by: Alexey Gladkov <[email protected]>
> v1: https://lkml.kernel.org/r/[email protected]
> v2: https://lkml.kernel.org/r/[email protected]
> v3: https://lkml.kernel.org/r/[email protected]
> Signed-off-by: Eric W. Biederman <[email protected]>
Looks good. Feel free to add:
Reviewed-by: Jan Kara <[email protected]>
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 <[email protected]>
SUSE Labs, CR