[PATCH AUTOSEL 5.14 19/25] connector: send event on write to /proc/[pid]/comm

[PATCH AUTOSEL 5.14 19/25] connector: send event on write to /proc/[pid]/comm

[Sasha Levin]

From: Ohhoon Kwon <ohoono.kwon@samsung.com>

[ Upstream commit c2f273ebd89a79ed87ef1025753343e327b99ac9 ]

While comm change event via prctl has been reported to proc connector by
'commit f786ecba4158 ("connector: add comm change event report to proc
connector")', connector listeners were missing comm changes by explicit
writes on /proc/[pid]/comm.

Let explicit writes on /proc/[pid]/comm report to proc connector.

Link: https://lkml.kernel.org/r/20210701133458epcms1p68e9eb9bd0eee8903ba26679a37d9d960@epcms1p6
Signed-off-by: Ohhoon Kwon <ohoono.kwon@samsung.com>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: David S. Miller <davem@davemloft.net>
Cc: Christian Brauner <christian.brauner@ubuntu.com>
Cc: Eric W. Biederman <ebiederm@xmission.com>
Cc: Alexey Dobriyan <adobriyan@gmail.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
 fs/proc/base.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/fs/proc/base.c b/fs/proc/base.c
index e5b5f7709d48..533d5836eb9a 100644
--- a/fs/proc/base.c
+++ b/fs/proc/base.c
@@ -95,6 +95,7 @@
 #include <linux/posix-timers.h>
 #include <linux/time_namespace.h>
 #include <linux/resctrl.h>
+#include <linux/cn_proc.h>
 #include <trace/events/oom.h>
 #include "internal.h"
 #include "fd.h"
@@ -1674,8 +1675,10 @@ static ssize_t comm_write(struct file *file, const char __user *buf,
 	if (!p)
 		return -ESRCH;
 
-	if (same_thread_group(current, p))
+	if (same_thread_group(current, p)) {
 		set_task_comm(p, buffer);
+		proc_comm_connector(p);
+	}
 	else
 		count = -EINVAL;
 
-- 
2.30.2

Re: [PATCH AUTOSEL 5.14 19/25] connector: send event on write to /proc/[pid]/comm

[Eric W. Biederman]

Sasha Levin <sashal@kernel.org> writes:

> From: Ohhoon Kwon <ohoono.kwon@samsung.com>
>
> [ Upstream commit c2f273ebd89a79ed87ef1025753343e327b99ac9 ]
>
> While comm change event via prctl has been reported to proc connector by
> 'commit f786ecba4158 ("connector: add comm change event report to proc
> connector")', connector listeners were missing comm changes by explicit
> writes on /proc/[pid]/comm.
>
> Let explicit writes on /proc/[pid]/comm report to proc connector.

This is a potential userspace ABI breakage?  Why backport it?

Especially if there is no one asking for the behavior change in
userspace?

Eric


>
> Link: https://lkml.kernel.org/r/20210701133458epcms1p68e9eb9bd0eee8903ba26679a37d9d960@epcms1p6
> Signed-off-by: Ohhoon Kwon <ohoono.kwon@samsung.com>
> Cc: Ingo Molnar <mingo@kernel.org>
> Cc: David S. Miller <davem@davemloft.net>
> Cc: Christian Brauner <christian.brauner@ubuntu.com>
> Cc: Eric W. Biederman <ebiederm@xmission.com>
> Cc: Alexey Dobriyan <adobriyan@gmail.com>
> Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
> Signed-off-by: Sasha Levin <sashal@kernel.org>
> ---
>  fs/proc/base.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/fs/proc/base.c b/fs/proc/base.c
> index e5b5f7709d48..533d5836eb9a 100644
> --- a/fs/proc/base.c
> +++ b/fs/proc/base.c
> @@ -95,6 +95,7 @@
>  #include <linux/posix-timers.h>
>  #include <linux/time_namespace.h>
>  #include <linux/resctrl.h>
> +#include <linux/cn_proc.h>
>  #include <trace/events/oom.h>
>  #include "internal.h"
>  #include "fd.h"
> @@ -1674,8 +1675,10 @@ static ssize_t comm_write(struct file *file, const char __user *buf,
>  	if (!p)
>  		return -ESRCH;
>  
> -	if (same_thread_group(current, p))
> +	if (same_thread_group(current, p)) {
>  		set_task_comm(p, buffer);
> +		proc_comm_connector(p);
> +	}
>  	else
>  		count = -EINVAL;

Re: [PATCH AUTOSEL 5.14 19/25] connector: send event on write to /proc/[pid]/comm

[Sasha Levin]

On Wed, Sep 15, 2021 at 08:45:37AM -0500, Eric W. Biederman wrote:
>Sasha Levin <sashal@kernel.org> writes:
>
>> From: Ohhoon Kwon <ohoono.kwon@samsung.com>
>>
>> [ Upstream commit c2f273ebd89a79ed87ef1025753343e327b99ac9 ]
>>
>> While comm change event via prctl has been reported to proc connector by
>> 'commit f786ecba4158 ("connector: add comm change event report to proc
>> connector")', connector listeners were missing comm changes by explicit
>> writes on /proc/[pid]/comm.
>>
>> Let explicit writes on /proc/[pid]/comm report to proc connector.
>
>This is a potential userspace ABI breakage?  Why backport it?
>
>Especially if there is no one asking for the behavior change in
>userspace?

This sounds like a concern with the patch going upstream rather than
going to stable? stable has the same policy around ABI changes such as
upstream.

-- 
Thanks,
Sasha

Re: [PATCH AUTOSEL 5.14 19/25] connector: send event on write to /proc/[pid]/comm

[Eric W. Biederman]

Sasha Levin <sashal@kernel.org> writes:

> On Wed, Sep 15, 2021 at 08:45:37AM -0500, Eric W. Biederman wrote:
>>Sasha Levin <sashal@kernel.org> writes:
>>
>>> From: Ohhoon Kwon <ohoono.kwon@samsung.com>
>>>
>>> [ Upstream commit c2f273ebd89a79ed87ef1025753343e327b99ac9 ]
>>>
>>> While comm change event via prctl has been reported to proc connector by
>>> 'commit f786ecba4158 ("connector: add comm change event report to proc
>>> connector")', connector listeners were missing comm changes by explicit
>>> writes on /proc/[pid]/comm.
>>>
>>> Let explicit writes on /proc/[pid]/comm report to proc connector.
>>
>>This is a potential userspace ABI breakage?  Why backport it?
>>
>>Especially if there is no one asking for the behavior change in
>>userspace?
>
> This sounds like a concern with the patch going upstream rather than
> going to stable? stable has the same policy around ABI changes such as
> upstream.

Let me say it another way.  This looks more like an evolution of the
functionality rather than a bug fix.

With something like this unless someone cares I don't think it should be
backported.  It is all risk and no benefit.

This is all doubly so because I think there are about 2 connector users
and connector is not especially good at the job it tries to fulfill.
It is for that exact reason that connector does not work in containers.
We couldn't find any users who cared.

After the fiasco with the rlimit/ucount changes getting backported
before they are even stable is that I am tired of saying about backports
meh whatever.

If there is no one who actually cares (which is what I learned about
autosel from the rlimit/ucount fiasco) it makes no sense to backport
things unless they really are bug fixes.

Backporting this just looks like senseless churn.

Eric