[PATCH linux-next] kernel: Make taskstats available via genetlink per namespace

[PATCH linux-next] kernel: Make taskstats available via genetlink per namespace

[cgel.zte]

From: xu xin <xu.xin16@zte.com.cn>

Currently, the application getdelays cannot get taskstats in a net
namespace. The returned error is just like the following:
-sh-4.4# ps -ef | tail -5
root       186     2  0 09:23 ?        00:00:00 [kworker/2:1H]
root       187     2  0 09:23 ?        00:00:00 [kworker/0:2-eve]
root       190   183  0 09:23 ?        00:00:00 -sh
root       198   190  0 09:25 ?        00:00:00 ps -ef
root       199   190  0 09:25 ?        00:00:00 tail -5
-sh-4.4#
-sh-4.4# ./getdelays -d -p 186 -v
print delayacct stats ON
debug on
Error getting family id, errno 0

As more and more applications are deployed in containers like Docker,
it is necessary to supoort getdelays to be used in net namespace.
Taskstats is safe for use per namespace as genetlink checks the
capability of namespace message by netlink_ns_capable().

Make taskstats available via genetlink per namespace.

Reported-by: Changcheng Deng <deng.changcheng@zte.com.cn>
Signed-off-by: xu xin <xu.xin16@zte.com.cn>
---
 kernel/taskstats.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/kernel/taskstats.c b/kernel/taskstats.c
index 2b4898b4752e..4d6bcaaf52a0 100644
--- a/kernel/taskstats.c
+++ b/kernel/taskstats.c
@@ -664,6 +664,7 @@ static struct genl_family family __ro_after_init = {
 	.module		= THIS_MODULE,
 	.ops		= taskstats_ops,
 	.n_ops		= ARRAY_SIZE(taskstats_ops),
+	.netnsok	= true,
 };
 
 /* Needed early in initialization */
-- 
2.25.1

[PATCH resend] kernel: Make taskstats available via genetlink per namespace

[xu xin]

Currently, the application getdelays cannot get taskstats in a net
namespace. The returned error is just like the following:
-sh-4.4# ps -ef | tail -5
root       186     2  0 09:23 ?        00:00:00 [kworker/2:1H]
root       187     2  0 09:23 ?        00:00:00 [kworker/0:2-eve]
root       190   183  0 09:23 ?        00:00:00 -sh
root       198   190  0 09:25 ?        00:00:00 ps -ef
root       199   190  0 09:25 ?        00:00:00 tail -5
-sh-4.4#
-sh-4.4# ./getdelays -d -p 186 -v
print delayacct stats ON
debug on
Error getting family id, errno 0

As more and more applications are deployed in containers like Docker,
it is necessary to support getdelays to be used in net namespace.
Taskstats is safe for use per namespace as genetlink checks the
capability of namespace message by netlink_ns_capable().

Make taskstats available via genetlink per namespace.

Reported-by: Changcheng Deng <deng.changcheng@zte.com.cn>
Signed-off-by: xu xin <xu.xin16@zte.com.cn>
---
 kernel/taskstats.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/kernel/taskstats.c b/kernel/taskstats.c
index 2b4898b4752e..4d6bcaaf52a0 100644
--- a/kernel/taskstats.c
+++ b/kernel/taskstats.c
@@ -664,6 +664,7 @@ static struct genl_family family __ro_after_init = {
 	.module		= THIS_MODULE,
 	.ops		= taskstats_ops,
 	.n_ops		= ARRAY_SIZE(taskstats_ops),
+	.netnsok	= true,
 };
 
 /* Needed early in initialization */
-- 
2.25.1

Re: [PATCH resend] kernel: Make taskstats available via genetlink per namespace

[Eric W. Biederman]

xu xin <cgel.zte@gmail.com> writes:

> Currently, the application getdelays cannot get taskstats in a net
> namespace. The returned error is just like the following:
> -sh-4.4# ps -ef | tail -5
> root       186     2  0 09:23 ?        00:00:00 [kworker/2:1H]
> root       187     2  0 09:23 ?        00:00:00 [kworker/0:2-eve]
> root       190   183  0 09:23 ?        00:00:00 -sh
> root       198   190  0 09:25 ?        00:00:00 ps -ef
> root       199   190  0 09:25 ?        00:00:00 tail -5
> -sh-4.4#
> -sh-4.4# ./getdelays -d -p 186 -v
> print delayacct stats ON
> debug on
> Error getting family id, errno 0
>
> As more and more applications are deployed in containers like Docker,
> it is necessary to support getdelays to be used in net namespace.
> Taskstats is safe for use per namespace as genetlink checks the
> capability of namespace message by netlink_ns_capable().
>
> Make taskstats available via genetlink per namespace.

Let me add a polite nack to this patch.

Taskstats is completely senseless in a network namespace.  There is no
translation of identifiers into the context of the receiver of the
message.

As such taskstats can not be meaningfully used in a container.

To make this work requires updating the taskstats code to do something
sensible when in a pid namespace, as well as when in a network
namespace.

I would like to give a suggest on how to do something sensible but
I don't have any idea at this point.  The code would have been converted
long ago on general principles if this was a straight forward thing to
do.

The taskstats interface only makes sense when you are within all of the
initial namespaces.

Eric

> Reported-by: Changcheng Deng <deng.changcheng@zte.com.cn>
> Signed-off-by: xu xin <xu.xin16@zte.com.cn>
> ---
>  kernel/taskstats.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/kernel/taskstats.c b/kernel/taskstats.c
> index 2b4898b4752e..4d6bcaaf52a0 100644
> --- a/kernel/taskstats.c
> +++ b/kernel/taskstats.c
> @@ -664,6 +664,7 @@ static struct genl_family family __ro_after_init = {
>  	.module		= THIS_MODULE,
>  	.ops		= taskstats_ops,
>  	.n_ops		= ARRAY_SIZE(taskstats_ops),
> +	.netnsok	= true,
>  };
>  
>  /* Needed early in initialization */

Re: [PATCH resend] kernel: Make taskstats available via genetlink per namespace

[cgel.zte]

>> -sh-4.4# ./getdelays -d -p 186 -v
>> print delayacct stats ON
>> debug on
>> Error getting family id, errno 0
>>
>> As more and more applications are deployed in containers like Docker,
>> it is necessary to support getdelays to be used in net namespace.
>> Taskstats is safe for use per namespace as genetlink checks the
>> capability of namespace message by netlink_ns_capable().
>>
>> Make taskstats available via genetlink per namespace.
>
> Let me add a polite nack to this patch.

> Taskstats is completely senseless in a network namespace.  There is no
> translation of identifiers into the context of the receiver of the
> message.

The interface of taskstats is genetlink that is sensible in net namsespace.

> To make this work requires updating the taskstats code to do something
> sensible when in a pid namespace, as well as when in a network
> namespace.

Yes. Taskstats already does convert the input process ID into the task in the
correspoding pid namsespace. Do you mean to add some check of current user's
capability like SYS_ADMIN or else?

Re:Re: [PATCH resend] kernel: Make taskstats available via genetlink per namespace

[cgel.zte]

>>> -sh-4.4# ./getdelays -d -p 186 -v
>>> print delayacct stats ON
>>> debug on
>>> Error getting family id, errno 0
>>>
>>> As more and more applications are deployed in containers like Docker,
>>> it is necessary to support getdelays to be used in net namespace.
>>> Taskstats is safe for use per namespace as genetlink checks the
>>> capability of namespace message by netlink_ns_capable().
>>>
>>> Make taskstats available via genetlink per namespace.
>>
>> Let me add a polite nack to this patch.
>
>> Taskstats is completely senseless in a network namespace.  There is no
>> translation of identifiers into the context of the receiver of the
>> message.
>
>The interface of taskstats is genetlink that is sensible in net namsespace.
>
>> To make this work requires updating the taskstats code to do something
>> sensible when in a pid namespace, as well as when in a network
>> namespace.
>
> Yes. Taskstats already does convert the input process ID into the task in the
> correspoding pid namsespace. Do you mean to add some check of current user's
> capability like SYS_ADMIN or else?

Actually, here, I think it's meaningful to set the genl_family's netnsok of Taskstat
as true. As you said, Taskstats itself is senseless in a network namespace. So, we
don't have to limit it to the only init_net_ns, it is basically okay to make it
available in all network namespace. Certainly, maybe taskstats itself also needs to
updated, because it does seem to be missing something to just use CAP_NET_ADMIN as the
acquisition restriction of taskstat.

Thanks,
xu xin