linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3] ucounts: add missing data type changes
@ 2021-07-30  6:28 Sven Schnelle
  2021-07-30 17:52 ` Eric W. Biederman
  2021-08-04  2:40 ` Nathan Chancellor
  0 siblings, 2 replies; 10+ messages in thread
From: Sven Schnelle @ 2021-07-30  6:28 UTC (permalink / raw)
  To: Eric W. Biederman; +Cc: Alexey Gladkov, linux-kernel, Sven Schnelle

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


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH v3] ucounts: add missing data type changes
  2021-07-30  6:28 [PATCH v3] ucounts: add missing data type changes Sven Schnelle
@ 2021-07-30 17:52 ` Eric W. Biederman
  2021-08-04  2:40 ` Nathan Chancellor
  1 sibling, 0 replies; 10+ messages in thread
From: Eric W. Biederman @ 2021-07-30 17:52 UTC (permalink / raw)
  To: Sven Schnelle; +Cc: Alexey Gladkov, linux-kernel

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"),

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v3] ucounts: add missing data type changes
  2021-07-30  6:28 [PATCH v3] ucounts: add missing data type changes Sven Schnelle
  2021-07-30 17:52 ` Eric W. Biederman
@ 2021-08-04  2:40 ` Nathan Chancellor
  2021-08-04 19:47   ` Eric W. Biederman
  1 sibling, 1 reply; 10+ messages in thread
From: Nathan Chancellor @ 2021-08-04  2:40 UTC (permalink / raw)
  To: Sven Schnelle; +Cc: Eric W. Biederman, Alexey Gladkov, linux-kernel

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,
        },


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH v3] ucounts: add missing data type changes
  2021-08-04  2:40 ` Nathan Chancellor
@ 2021-08-04 19:47   ` Eric W. Biederman
  2021-08-04 20:27     ` Nathan Chancellor
  0 siblings, 1 reply; 10+ messages in thread
From: Eric W. Biederman @ 2021-08-04 19:47 UTC (permalink / raw)
  To: Nathan Chancellor; +Cc: Sven Schnelle, Alexey Gladkov, linux-kernel

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, &param);
 }

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH v3] ucounts: add missing data type changes
  2021-08-04 19:47   ` Eric W. Biederman
@ 2021-08-04 20:27     ` Nathan Chancellor
  2021-08-05 16:48       ` Eric W. Biederman
  0 siblings, 1 reply; 10+ messages in thread
From: Nathan Chancellor @ 2021-08-04 20:27 UTC (permalink / raw)
  To: Eric W. Biederman; +Cc: Sven Schnelle, Alexey Gladkov, linux-kernel

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, &param);
>   }
> 

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v3] ucounts: add missing data type changes
  2021-08-04 20:27     ` Nathan Chancellor
@ 2021-08-05 16:48       ` Eric W. Biederman
  2021-08-05 19:26         ` Nathan Chancellor
  0 siblings, 1 reply; 10+ messages in thread
From: Eric W. Biederman @ 2021-08-05 16:48 UTC (permalink / raw)
  To: Nathan Chancellor; +Cc: Sven Schnelle, Alexey Gladkov, linux-kernel

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"),




^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH v3] ucounts: add missing data type changes
  2021-08-05 16:48       ` Eric W. Biederman
@ 2021-08-05 19:26         ` Nathan Chancellor
  2021-08-06 10:36           ` Naresh Kamboju
  0 siblings, 1 reply; 10+ messages in thread
From: Nathan Chancellor @ 2021-08-05 19:26 UTC (permalink / raw)
  To: Eric W. Biederman; +Cc: Sven Schnelle, Alexey Gladkov, linux-kernel

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"),
> 
> 
> 

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v3] ucounts: add missing data type changes
  2021-08-05 19:26         ` Nathan Chancellor
@ 2021-08-06 10:36           ` Naresh Kamboju
  2021-08-09 20:43             ` [PATCH v4] " Eric W. Biederman
  0 siblings, 1 reply; 10+ messages in thread
From: Naresh Kamboju @ 2021-08-06 10:36 UTC (permalink / raw)
  To: Eric W. Biederman
  Cc: Sven Schnelle, Nathan Chancellor, Alexey Gladkov, open list,
	LTP List, linux-fsdevel, Jan Kara

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v4] ucounts: add missing data type changes
  2021-08-06 10:36           ` Naresh Kamboju
@ 2021-08-09 20:43             ` Eric W. Biederman
  2021-08-10  9:01               ` Jan Kara
  0 siblings, 1 reply; 10+ messages in thread
From: Eric W. Biederman @ 2021-08-09 20:43 UTC (permalink / raw)
  To: Naresh Kamboju
  Cc: Sven Schnelle, Nathan Chancellor, Alexey Gladkov, open list,
	LTP List, linux-fsdevel, Jan Kara


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


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH v4] ucounts: add missing data type changes
  2021-08-09 20:43             ` [PATCH v4] " Eric W. Biederman
@ 2021-08-10  9:01               ` Jan Kara
  0 siblings, 0 replies; 10+ messages in thread
From: Jan Kara @ 2021-08-10  9:01 UTC (permalink / raw)
  To: Eric W. Biederman
  Cc: Naresh Kamboju, Sven Schnelle, Nathan Chancellor, Alexey Gladkov,
	open list, LTP List, linux-fsdevel, Jan Kara

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2021-08-10  9:02 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-07-30  6:28 [PATCH v3] ucounts: add missing data type changes Sven Schnelle
2021-07-30 17:52 ` Eric W. Biederman
2021-08-04  2:40 ` Nathan Chancellor
2021-08-04 19:47   ` Eric W. Biederman
2021-08-04 20:27     ` Nathan Chancellor
2021-08-05 16:48       ` Eric W. Biederman
2021-08-05 19:26         ` Nathan Chancellor
2021-08-06 10:36           ` Naresh Kamboju
2021-08-09 20:43             ` [PATCH v4] " Eric W. Biederman
2021-08-10  9:01               ` Jan Kara

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).