2021-12-08 08:33:29

by Leo Yan

[permalink] [raw]
Subject: [PATCH v2 0/7] pid: Introduce helper task_is_in_root_ns()

The kernel uses open code to check if a process is in root PID namespace
or not in several places.

Suggested by Suzuki, this patch set is to create a helper function
task_is_in_init_pid_ns() to replace open code.

This patch set has been applied on the mainline kernel and built for
Arm64 kernel with enabling all relevant modules.

Changes from v1:
* Renamed helper function from task_is_in_root_ns() to
task_is_in_init_pid_ns(). (Leon Romanovsky)
* Improved patches' commit logs for more neat.


Leo Yan (7):
pid: Introduce helper task_is_in_init_pid_ns()
coresight: etm3x: Use task_is_in_init_pid_ns()
coresight: etm4x: Use task_is_in_init_pid_ns()
connector/cn_proc: Use task_is_in_init_pid_ns()
coda: Use task_is_in_init_pid_ns()
audit: Use task_is_in_init_pid_ns()
taskstats: Use task_is_in_init_pid_ns()

drivers/connector/cn_proc.c | 2 +-
drivers/hwtracing/coresight/coresight-etm3x-sysfs.c | 8 ++++----
drivers/hwtracing/coresight/coresight-etm4x-sysfs.c | 8 ++++----
fs/coda/inode.c | 2 +-
fs/coda/psdev.c | 2 +-
include/linux/pid_namespace.h | 5 +++++
kernel/audit.c | 2 +-
kernel/taskstats.c | 2 +-
8 files changed, 18 insertions(+), 13 deletions(-)

--
2.25.1



2021-12-08 08:33:33

by Leo Yan

[permalink] [raw]
Subject: [PATCH v2 1/7] pid: Introduce helper task_is_in_init_pid_ns()

Currently the kernel uses open code in multiple places to check if a
task is in the root PID namespace with the kind of format:

if (task_active_pid_ns(current) == &init_pid_ns)
do_something();

This patch creates a new helper function, task_is_in_init_pid_ns(), it
returns true if a passed task is in the root PID namespace, otherwise
returns false. So it will be used to replace open codes.

Suggested-by: Suzuki K Poulose <[email protected]>
Signed-off-by: Leo Yan <[email protected]>
---
include/linux/pid_namespace.h | 5 +++++
1 file changed, 5 insertions(+)

diff --git a/include/linux/pid_namespace.h b/include/linux/pid_namespace.h
index 7c7e627503d2..07481bb87d4e 100644
--- a/include/linux/pid_namespace.h
+++ b/include/linux/pid_namespace.h
@@ -86,4 +86,9 @@ extern struct pid_namespace *task_active_pid_ns(struct task_struct *tsk);
void pidhash_init(void);
void pid_idr_init(void);

+static inline bool task_is_in_init_pid_ns(struct task_struct *tsk)
+{
+ return task_active_pid_ns(tsk) == &init_pid_ns;
+}
+
#endif /* _LINUX_PID_NS_H */
--
2.25.1


2021-12-08 08:33:36

by Leo Yan

[permalink] [raw]
Subject: [PATCH v2 2/7] coresight: etm3x: Use task_is_in_init_pid_ns()

This patch replaces open code with task_is_in_init_pid_ns() to check if
a task is in root PID namespace.

Signed-off-by: Leo Yan <[email protected]>
---
drivers/hwtracing/coresight/coresight-etm3x-sysfs.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-etm3x-sysfs.c b/drivers/hwtracing/coresight/coresight-etm3x-sysfs.c
index e8c7649f123e..ff76cb56b727 100644
--- a/drivers/hwtracing/coresight/coresight-etm3x-sysfs.c
+++ b/drivers/hwtracing/coresight/coresight-etm3x-sysfs.c
@@ -1030,7 +1030,7 @@ static ssize_t ctxid_pid_show(struct device *dev,
* Don't use contextID tracing if coming from a PID namespace. See
* comment in ctxid_pid_store().
*/
- if (task_active_pid_ns(current) != &init_pid_ns)
+ if (!task_is_in_init_pid_ns(current))
return -EINVAL;

spin_lock(&drvdata->spinlock);
@@ -1058,7 +1058,7 @@ static ssize_t ctxid_pid_store(struct device *dev,
* As such refuse to use the feature if @current is not in the initial
* PID namespace.
*/
- if (task_active_pid_ns(current) != &init_pid_ns)
+ if (!task_is_in_init_pid_ns(current))
return -EINVAL;

ret = kstrtoul(buf, 16, &pid);
@@ -1084,7 +1084,7 @@ static ssize_t ctxid_mask_show(struct device *dev,
* Don't use contextID tracing if coming from a PID namespace. See
* comment in ctxid_pid_store().
*/
- if (task_active_pid_ns(current) != &init_pid_ns)
+ if (!task_is_in_init_pid_ns(current))
return -EINVAL;

val = config->ctxid_mask;
@@ -1104,7 +1104,7 @@ static ssize_t ctxid_mask_store(struct device *dev,
* Don't use contextID tracing if coming from a PID namespace. See
* comment in ctxid_pid_store().
*/
- if (task_active_pid_ns(current) != &init_pid_ns)
+ if (!task_is_in_init_pid_ns(current))
return -EINVAL;

ret = kstrtoul(buf, 16, &val);
--
2.25.1


2021-12-08 08:33:40

by Leo Yan

[permalink] [raw]
Subject: [PATCH v2 3/7] coresight: etm4x: Use task_is_in_init_pid_ns()

This patch replaces open code with task_is_in_init_pid_ns() to check if
a task is in root PID namespace.

Signed-off-by: Leo Yan <[email protected]>
---
drivers/hwtracing/coresight/coresight-etm4x-sysfs.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c b/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c
index a0640fa5c55b..10ef2a29006e 100644
--- a/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c
+++ b/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c
@@ -1890,7 +1890,7 @@ static ssize_t ctxid_pid_show(struct device *dev,
* Don't use contextID tracing if coming from a PID namespace. See
* comment in ctxid_pid_store().
*/
- if (task_active_pid_ns(current) != &init_pid_ns)
+ if (!task_is_in_init_pid_ns(current))
return -EINVAL;

spin_lock(&drvdata->spinlock);
@@ -1918,7 +1918,7 @@ static ssize_t ctxid_pid_store(struct device *dev,
* As such refuse to use the feature if @current is not in the initial
* PID namespace.
*/
- if (task_active_pid_ns(current) != &init_pid_ns)
+ if (!task_is_in_init_pid_ns(current))
return -EINVAL;

/*
@@ -1951,7 +1951,7 @@ static ssize_t ctxid_masks_show(struct device *dev,
* Don't use contextID tracing if coming from a PID namespace. See
* comment in ctxid_pid_store().
*/
- if (task_active_pid_ns(current) != &init_pid_ns)
+ if (!task_is_in_init_pid_ns(current))
return -EINVAL;

spin_lock(&drvdata->spinlock);
@@ -1975,7 +1975,7 @@ static ssize_t ctxid_masks_store(struct device *dev,
* Don't use contextID tracing if coming from a PID namespace. See
* comment in ctxid_pid_store().
*/
- if (task_active_pid_ns(current) != &init_pid_ns)
+ if (!task_is_in_init_pid_ns(current))
return -EINVAL;

/*
--
2.25.1


2021-12-08 08:33:47

by Leo Yan

[permalink] [raw]
Subject: [PATCH v2 4/7] connector/cn_proc: Use task_is_in_init_pid_ns()

This patch replaces open code with task_is_in_init_pid_ns() to check if
a task is in root PID namespace.

Signed-off-by: Leo Yan <[email protected]>
---
drivers/connector/cn_proc.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/connector/cn_proc.c b/drivers/connector/cn_proc.c
index 646ad385e490..ccac1c453080 100644
--- a/drivers/connector/cn_proc.c
+++ b/drivers/connector/cn_proc.c
@@ -358,7 +358,7 @@ static void cn_proc_mcast_ctl(struct cn_msg *msg,
* other namespaces.
*/
if ((current_user_ns() != &init_user_ns) ||
- (task_active_pid_ns(current) != &init_pid_ns))
+ !task_is_in_init_pid_ns(current))
return;

/* Can only change if privileged. */
--
2.25.1


2021-12-08 08:33:52

by Leo Yan

[permalink] [raw]
Subject: [PATCH v2 7/7] taskstats: Use task_is_in_init_pid_ns()

Replace open code with task_is_in_init_pid_ns() for checking root PID
namespace.

Signed-off-by: Leo Yan <[email protected]>
---
kernel/taskstats.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/taskstats.c b/kernel/taskstats.c
index 2b4898b4752e..f570d8e1f001 100644
--- a/kernel/taskstats.c
+++ b/kernel/taskstats.c
@@ -284,7 +284,7 @@ static int add_del_listener(pid_t pid, const struct cpumask *mask, int isadd)
if (current_user_ns() != &init_user_ns)
return -EINVAL;

- if (task_active_pid_ns(current) != &init_pid_ns)
+ if (!task_is_in_init_pid_ns(current))
return -EINVAL;

if (isadd == REGISTER) {
--
2.25.1


2021-12-08 08:33:54

by Leo Yan

[permalink] [raw]
Subject: [PATCH v2 6/7] audit: Use task_is_in_init_pid_ns()

Replace open code with task_is_in_init_pid_ns() for checking root PID
namespace.

Signed-off-by: Leo Yan <[email protected]>
---
kernel/audit.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/audit.c b/kernel/audit.c
index 121d37e700a6..56ea91014180 100644
--- a/kernel/audit.c
+++ b/kernel/audit.c
@@ -1034,7 +1034,7 @@ static int audit_netlink_ok(struct sk_buff *skb, u16 msg_type)
case AUDIT_MAKE_EQUIV:
/* Only support auditd and auditctl in initial pid namespace
* for now. */
- if (task_active_pid_ns(current) != &init_pid_ns)
+ if (!task_is_in_init_pid_ns(current))
return -EPERM;

if (!netlink_capable(skb, CAP_AUDIT_CONTROL))
--
2.25.1


2021-12-08 08:33:58

by Leo Yan

[permalink] [raw]
Subject: [PATCH v2 5/7] coda: Use task_is_in_init_pid_ns()

Replace open code with task_is_in_init_pid_ns() for checking root PID
namespace.

Signed-off-by: Leo Yan <[email protected]>
---
fs/coda/inode.c | 2 +-
fs/coda/psdev.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/fs/coda/inode.c b/fs/coda/inode.c
index d9f1bd7153df..931f4560fdd0 100644
--- a/fs/coda/inode.c
+++ b/fs/coda/inode.c
@@ -152,7 +152,7 @@ static int coda_fill_super(struct super_block *sb, void *data, int silent)
int error;
int idx;

- if (task_active_pid_ns(current) != &init_pid_ns)
+ if (!task_is_in_init_pid_ns(current))
return -EINVAL;

idx = get_device_index((struct coda_mount_data *) data);
diff --git a/fs/coda/psdev.c b/fs/coda/psdev.c
index b39580ad4ce5..73457661fbe8 100644
--- a/fs/coda/psdev.c
+++ b/fs/coda/psdev.c
@@ -270,7 +270,7 @@ static int coda_psdev_open(struct inode * inode, struct file * file)
struct venus_comm *vcp;
int idx, err;

- if (task_active_pid_ns(current) != &init_pid_ns)
+ if (!task_is_in_init_pid_ns(current))
return -EINVAL;

if (current_user_ns() != &init_user_ns)
--
2.25.1


2021-12-08 09:28:44

by Leon Romanovsky

[permalink] [raw]
Subject: Re: [PATCH v2 1/7] pid: Introduce helper task_is_in_init_pid_ns()

On Wed, Dec 08, 2021 at 04:33:14PM +0800, Leo Yan wrote:
> Currently the kernel uses open code in multiple places to check if a
> task is in the root PID namespace with the kind of format:
>
> if (task_active_pid_ns(current) == &init_pid_ns)
> do_something();
>
> This patch creates a new helper function, task_is_in_init_pid_ns(), it
> returns true if a passed task is in the root PID namespace, otherwise
> returns false. So it will be used to replace open codes.
>
> Suggested-by: Suzuki K Poulose <[email protected]>
> Signed-off-by: Leo Yan <[email protected]>
> ---
> include/linux/pid_namespace.h | 5 +++++
> 1 file changed, 5 insertions(+)
>

Thanks,
Reviewed-by: Leon Romanovsky <[email protected]>

2021-12-08 10:25:13

by Suzuki K Poulose

[permalink] [raw]
Subject: Re: [PATCH v2 1/7] pid: Introduce helper task_is_in_init_pid_ns()

On 08/12/2021 08:33, Leo Yan wrote:
> Currently the kernel uses open code in multiple places to check if a
> task is in the root PID namespace with the kind of format:
>
> if (task_active_pid_ns(current) == &init_pid_ns)
> do_something();
>
> This patch creates a new helper function, task_is_in_init_pid_ns(), it
> returns true if a passed task is in the root PID namespace, otherwise
> returns false. So it will be used to replace open codes.
>
> Suggested-by: Suzuki K Poulose <[email protected]>
> Signed-off-by: Leo Yan <[email protected]>
> ---
> include/linux/pid_namespace.h | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/include/linux/pid_namespace.h b/include/linux/pid_namespace.h
> index 7c7e627503d2..07481bb87d4e 100644
> --- a/include/linux/pid_namespace.h
> +++ b/include/linux/pid_namespace.h
> @@ -86,4 +86,9 @@ extern struct pid_namespace *task_active_pid_ns(struct task_struct *tsk);
> void pidhash_init(void);
> void pid_idr_init(void);
>
> +static inline bool task_is_in_init_pid_ns(struct task_struct *tsk)
> +{
> + return task_active_pid_ns(tsk) == &init_pid_ns;
> +}
> +

Looks good to me,

Acked-by: Suzuki K Poulose <[email protected]>

2021-12-08 10:26:17

by Suzuki K Poulose

[permalink] [raw]
Subject: Re: [PATCH v2 2/7] coresight: etm3x: Use task_is_in_init_pid_ns()

On 08/12/2021 08:33, Leo Yan wrote:
> This patch replaces open code with task_is_in_init_pid_ns() to check if
> a task is in root PID namespace.
>
> Signed-off-by: Leo Yan <[email protected]>
> ---

Reviewed-by: Suzuki K Poulose <[email protected]>

2021-12-08 10:27:02

by Suzuki K Poulose

[permalink] [raw]
Subject: Re: [PATCH v2 3/7] coresight: etm4x: Use task_is_in_init_pid_ns()

On 08/12/2021 08:33, Leo Yan wrote:
> This patch replaces open code with task_is_in_init_pid_ns() to check if
> a task is in root PID namespace.
>
> Signed-off-by: Leo Yan <[email protected]>
> ---


Reviewed-by: Suzuki K Poulose <[email protected]>


2021-12-14 05:54:41

by Balbir Singh

[permalink] [raw]
Subject: Re: [PATCH v2 3/7] coresight: etm4x: Use task_is_in_init_pid_ns()

On Wed, Dec 08, 2021 at 04:33:16PM +0800, Leo Yan wrote:
> This patch replaces open code with task_is_in_init_pid_ns() to check if
> a task is in root PID namespace.
>
> Signed-off-by: Leo Yan <[email protected]>
> ---
> drivers/hwtracing/coresight/coresight-etm4x-sysfs.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c b/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c
> index a0640fa5c55b..10ef2a29006e 100644
> --- a/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c
> +++ b/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c
> @@ -1890,7 +1890,7 @@ static ssize_t ctxid_pid_show(struct device *dev,
> * Don't use contextID tracing if coming from a PID namespace. See
> * comment in ctxid_pid_store().
> */
> - if (task_active_pid_ns(current) != &init_pid_ns)
> + if (!task_is_in_init_pid_ns(current))
> return -EINVAL;
>
> spin_lock(&drvdata->spinlock);
> @@ -1918,7 +1918,7 @@ static ssize_t ctxid_pid_store(struct device *dev,
> * As such refuse to use the feature if @current is not in the initial
> * PID namespace.
> */
> - if (task_active_pid_ns(current) != &init_pid_ns)
> + if (!task_is_in_init_pid_ns(current))
> return -EINVAL;
>
> /*
> @@ -1951,7 +1951,7 @@ static ssize_t ctxid_masks_show(struct device *dev,
> * Don't use contextID tracing if coming from a PID namespace. See
> * comment in ctxid_pid_store().
> */
> - if (task_active_pid_ns(current) != &init_pid_ns)
> + if (!task_is_in_init_pid_ns(current))
> return -EINVAL;
>
> spin_lock(&drvdata->spinlock);
> @@ -1975,7 +1975,7 @@ static ssize_t ctxid_masks_store(struct device *dev,
> * Don't use contextID tracing if coming from a PID namespace. See
> * comment in ctxid_pid_store().
> */
> - if (task_active_pid_ns(current) != &init_pid_ns)
> + if (!task_is_in_init_pid_ns(current))
> return -EINVAL;
>
> /*

Acked-by: Balbir Singh <[email protected]>

2021-12-14 05:55:10

by Balbir Singh

[permalink] [raw]
Subject: Re: [PATCH v2 2/7] coresight: etm3x: Use task_is_in_init_pid_ns()

On Wed, Dec 08, 2021 at 04:33:15PM +0800, Leo Yan wrote:
> This patch replaces open code with task_is_in_init_pid_ns() to check if
> a task is in root PID namespace.
>
> Signed-off-by: Leo Yan <[email protected]>
> ---
> drivers/hwtracing/coresight/coresight-etm3x-sysfs.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/hwtracing/coresight/coresight-etm3x-sysfs.c b/drivers/hwtracing/coresight/coresight-etm3x-sysfs.c
> index e8c7649f123e..ff76cb56b727 100644
> --- a/drivers/hwtracing/coresight/coresight-etm3x-sysfs.c
> +++ b/drivers/hwtracing/coresight/coresight-etm3x-sysfs.c
> @@ -1030,7 +1030,7 @@ static ssize_t ctxid_pid_show(struct device *dev,
> * Don't use contextID tracing if coming from a PID namespace. See
> * comment in ctxid_pid_store().
> */
> - if (task_active_pid_ns(current) != &init_pid_ns)
> + if (!task_is_in_init_pid_ns(current))
> return -EINVAL;
>
> spin_lock(&drvdata->spinlock);
> @@ -1058,7 +1058,7 @@ static ssize_t ctxid_pid_store(struct device *dev,
> * As such refuse to use the feature if @current is not in the initial
> * PID namespace.
> */
> - if (task_active_pid_ns(current) != &init_pid_ns)
> + if (!task_is_in_init_pid_ns(current))
> return -EINVAL;
>
> ret = kstrtoul(buf, 16, &pid);
> @@ -1084,7 +1084,7 @@ static ssize_t ctxid_mask_show(struct device *dev,
> * Don't use contextID tracing if coming from a PID namespace. See
> * comment in ctxid_pid_store().
> */
> - if (task_active_pid_ns(current) != &init_pid_ns)
> + if (!task_is_in_init_pid_ns(current))
> return -EINVAL;
>
> val = config->ctxid_mask;
> @@ -1104,7 +1104,7 @@ static ssize_t ctxid_mask_store(struct device *dev,
> * Don't use contextID tracing if coming from a PID namespace. See
> * comment in ctxid_pid_store().
> */
> - if (task_active_pid_ns(current) != &init_pid_ns)
> + if (!task_is_in_init_pid_ns(current))
> return -EINVAL;
>
> ret = kstrtoul(buf, 16, &val);
> --
> 2.25.1
>

Acked-by: Balbir Singh <[email protected]>


2021-12-14 05:55:44

by Balbir Singh

[permalink] [raw]
Subject: Re: [PATCH v2 1/7] pid: Introduce helper task_is_in_init_pid_ns()

On Wed, Dec 08, 2021 at 04:33:14PM +0800, Leo Yan wrote:
> Currently the kernel uses open code in multiple places to check if a
> task is in the root PID namespace with the kind of format:
>
> if (task_active_pid_ns(current) == &init_pid_ns)
> do_something();
>
> This patch creates a new helper function, task_is_in_init_pid_ns(), it
> returns true if a passed task is in the root PID namespace, otherwise
> returns false. So it will be used to replace open codes.
>
> Suggested-by: Suzuki K Poulose <[email protected]>
> Signed-off-by: Leo Yan <[email protected]>
> ---
> include/linux/pid_namespace.h | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/include/linux/pid_namespace.h b/include/linux/pid_namespace.h
> index 7c7e627503d2..07481bb87d4e 100644
> --- a/include/linux/pid_namespace.h
> +++ b/include/linux/pid_namespace.h
> @@ -86,4 +86,9 @@ extern struct pid_namespace *task_active_pid_ns(struct task_struct *tsk);
> void pidhash_init(void);
> void pid_idr_init(void);
>
> +static inline bool task_is_in_init_pid_ns(struct task_struct *tsk)
> +{
> + return task_active_pid_ns(tsk) == &init_pid_ns;
> +}
> +
> #endif /* _LINUX_PID_NS_H */
> --
> 2.25.1
>

Acked-by: Balbir Singh <[email protected]>


2021-12-14 05:56:38

by Balbir Singh

[permalink] [raw]
Subject: Re: [PATCH v2 7/7] taskstats: Use task_is_in_init_pid_ns()

On Wed, Dec 08, 2021 at 04:33:20PM +0800, Leo Yan wrote:
> Replace open code with task_is_in_init_pid_ns() for checking root PID
> namespace.
>
> Signed-off-by: Leo Yan <[email protected]>
> ---
> kernel/taskstats.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/kernel/taskstats.c b/kernel/taskstats.c
> index 2b4898b4752e..f570d8e1f001 100644
> --- a/kernel/taskstats.c
> +++ b/kernel/taskstats.c
> @@ -284,7 +284,7 @@ static int add_del_listener(pid_t pid, const struct cpumask *mask, int isadd)
> if (current_user_ns() != &init_user_ns)
> return -EINVAL;
>
> - if (task_active_pid_ns(current) != &init_pid_ns)
> + if (!task_is_in_init_pid_ns(current))
> return -EINVAL;
>
> if (isadd == REGISTER) {
> --
> 2.25.1
>

Acked-by: Balbir Singh <[email protected]>


2021-12-14 05:57:05

by Balbir Singh

[permalink] [raw]
Subject: Re: [PATCH v2 6/7] audit: Use task_is_in_init_pid_ns()

On Wed, Dec 08, 2021 at 04:33:19PM +0800, Leo Yan wrote:
> Replace open code with task_is_in_init_pid_ns() for checking root PID
> namespace.
>
> Signed-off-by: Leo Yan <[email protected]>
> ---
> kernel/audit.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/kernel/audit.c b/kernel/audit.c
> index 121d37e700a6..56ea91014180 100644
> --- a/kernel/audit.c
> +++ b/kernel/audit.c
> @@ -1034,7 +1034,7 @@ static int audit_netlink_ok(struct sk_buff *skb, u16 msg_type)
> case AUDIT_MAKE_EQUIV:
> /* Only support auditd and auditctl in initial pid namespace
> * for now. */
> - if (task_active_pid_ns(current) != &init_pid_ns)
> + if (!task_is_in_init_pid_ns(current))
> return -EPERM;
>
> if (!netlink_capable(skb, CAP_AUDIT_CONTROL))
> --
> 2.25.1
>

Acked-by: Balbir Singh <[email protected]>


2021-12-14 05:58:39

by Balbir Singh

[permalink] [raw]
Subject: Re: [PATCH v2 4/7] connector/cn_proc: Use task_is_in_init_pid_ns()

On Wed, Dec 08, 2021 at 04:33:17PM +0800, Leo Yan wrote:
> This patch replaces open code with task_is_in_init_pid_ns() to check if
> a task is in root PID namespace.
>
> Signed-off-by: Leo Yan <[email protected]>
> ---
> drivers/connector/cn_proc.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/connector/cn_proc.c b/drivers/connector/cn_proc.c
> index 646ad385e490..ccac1c453080 100644
> --- a/drivers/connector/cn_proc.c
> +++ b/drivers/connector/cn_proc.c
> @@ -358,7 +358,7 @@ static void cn_proc_mcast_ctl(struct cn_msg *msg,
> * other namespaces.
> */
> if ((current_user_ns() != &init_user_ns) ||
> - (task_active_pid_ns(current) != &init_pid_ns))
> + !task_is_in_init_pid_ns(current))
> return;
>

Sounds like there might scope for other wrappers - is_current_in_user_init_ns()

Acked-by: Balbir Singh <[email protected]>


2021-12-14 07:05:50

by Leo Yan

[permalink] [raw]
Subject: Re: [PATCH v2 4/7] connector/cn_proc: Use task_is_in_init_pid_ns()

On Tue, Dec 14, 2021 at 04:58:32PM +1100, Balbir Singh wrote:
> On Wed, Dec 08, 2021 at 04:33:17PM +0800, Leo Yan wrote:
> > This patch replaces open code with task_is_in_init_pid_ns() to check if
> > a task is in root PID namespace.
> >
> > Signed-off-by: Leo Yan <[email protected]>
> > ---
> > drivers/connector/cn_proc.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/connector/cn_proc.c b/drivers/connector/cn_proc.c
> > index 646ad385e490..ccac1c453080 100644
> > --- a/drivers/connector/cn_proc.c
> > +++ b/drivers/connector/cn_proc.c
> > @@ -358,7 +358,7 @@ static void cn_proc_mcast_ctl(struct cn_msg *msg,
> > * other namespaces.
> > */
> > if ((current_user_ns() != &init_user_ns) ||
> > - (task_active_pid_ns(current) != &init_pid_ns))
> > + !task_is_in_init_pid_ns(current))
> > return;
> >
>
> Sounds like there might scope for other wrappers - is_current_in_user_init_ns()

Indeed, for new wrapper is_current_in_user_init_ns(), I searched kernel
and located there have multiple places use open code. If no one works
on this refactoring, I will send a new patchset for it separately.

> Acked-by: Balbir Singh <[email protected]>

Thank you for review, Balbir! Also thanks review from Leon and
Suzuki.

Leo

2021-12-14 22:35:28

by Paul Moore

[permalink] [raw]
Subject: Re: [PATCH v2 6/7] audit: Use task_is_in_init_pid_ns()

On Wed, Dec 8, 2021 at 3:33 AM Leo Yan <[email protected]> wrote:
>
> Replace open code with task_is_in_init_pid_ns() for checking root PID
> namespace.
>
> Signed-off-by: Leo Yan <[email protected]>
> ---
> kernel/audit.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)

I'm not sure how necessary this is, but it looks correct to me.

Acked-by: Paul Moore <[email protected]>

> diff --git a/kernel/audit.c b/kernel/audit.c
> index 121d37e700a6..56ea91014180 100644
> --- a/kernel/audit.c
> +++ b/kernel/audit.c
> @@ -1034,7 +1034,7 @@ static int audit_netlink_ok(struct sk_buff *skb, u16 msg_type)
> case AUDIT_MAKE_EQUIV:
> /* Only support auditd and auditctl in initial pid namespace
> * for now. */
> - if (task_active_pid_ns(current) != &init_pid_ns)
> + if (!task_is_in_init_pid_ns(current))
> return -EPERM;
>
> if (!netlink_capable(skb, CAP_AUDIT_CONTROL))
> --
> 2.25.1

--
paul moore
http://www.paul-moore.com

2021-12-15 19:09:51

by Richard Guy Briggs

[permalink] [raw]
Subject: Re: [PATCH v2 6/7] audit: Use task_is_in_init_pid_ns()

On 2021-12-14 17:35, Paul Moore wrote:
> On Wed, Dec 8, 2021 at 3:33 AM Leo Yan <[email protected]> wrote:
> >
> > Replace open code with task_is_in_init_pid_ns() for checking root PID
> > namespace.
> >
> > Signed-off-by: Leo Yan <[email protected]>
> > ---
> > kernel/audit.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
>
> I'm not sure how necessary this is, but it looks correct to me.

I had the same thought. I looks correct to me. I could see the value
if it permitted init_pid_ns to not be global.

> Acked-by: Paul Moore <[email protected]>

Reviewed-by: Richard Guy Briggs <[email protected]>

> > diff --git a/kernel/audit.c b/kernel/audit.c
> > index 121d37e700a6..56ea91014180 100644
> > --- a/kernel/audit.c
> > +++ b/kernel/audit.c
> > @@ -1034,7 +1034,7 @@ static int audit_netlink_ok(struct sk_buff *skb, u16 msg_type)
> > case AUDIT_MAKE_EQUIV:
> > /* Only support auditd and auditctl in initial pid namespace
> > * for now. */
> > - if (task_active_pid_ns(current) != &init_pid_ns)
> > + if (!task_is_in_init_pid_ns(current))
> > return -EPERM;
> >
> > if (!netlink_capable(skb, CAP_AUDIT_CONTROL))
> > --
> > 2.25.1
>
> paul moore

- RGB

--
Richard Guy Briggs <[email protected]>
Sr. S/W Engineer, Kernel Security, Base Operating Systems
Remote, Ottawa, Red Hat Canada
IRC: rgb, SunRaycer
Voice: +1.647.777.2635, Internal: (81) 32635


2021-12-16 01:09:45

by Leo Yan

[permalink] [raw]
Subject: Re: [PATCH v2 6/7] audit: Use task_is_in_init_pid_ns()

On Wed, Dec 15, 2021 at 02:09:12PM -0500, Richard Guy Briggs wrote:
> On 2021-12-14 17:35, Paul Moore wrote:
> > On Wed, Dec 8, 2021 at 3:33 AM Leo Yan <[email protected]> wrote:
> > >
> > > Replace open code with task_is_in_init_pid_ns() for checking root PID
> > > namespace.
> > >
> > > Signed-off-by: Leo Yan <[email protected]>
> > > ---
> > > kernel/audit.c | 2 +-
> > > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > I'm not sure how necessary this is, but it looks correct to me.
>
> I had the same thought. I looks correct to me. I could see the value
> if it permitted init_pid_ns to not be global.

Just for a background info, we need to check root PID namespace in some
drivers [1], to avoid introducing more open codes, we decided to refactor
with helper task_is_in_init_pid_ns().

[1] https://lore.kernel.org/lkml/[email protected]/

> > Acked-by: Paul Moore <[email protected]>
>
> Reviewed-by: Richard Guy Briggs <[email protected]>

Thanks for review, Paul and Richard.

Leo

2022-01-12 06:40:58

by Leo Yan

[permalink] [raw]
Subject: Re: [PATCH v2 0/7] pid: Introduce helper task_is_in_root_ns()

Hi David,

On Wed, Dec 08, 2021 at 04:33:13PM +0800, Leo Yan wrote:
> The kernel uses open code to check if a process is in root PID namespace
> or not in several places.
>
> Suggested by Suzuki, this patch set is to create a helper function
> task_is_in_init_pid_ns() to replace open code.
>
> This patch set has been applied on the mainline kernel and built for
> Arm64 kernel with enabling all relevant modules.

I'd like sync for how to merging this patch set. Except patch 05/07,
all of other patches in this patch set have been received the reviewed
or acked tags. So could you pick up this patch set?

Furthermore, we have another patch set "coresight: etm: Correct PID
tracing for non-root namespace" [1], which is dependent on the current
patch set and it has been Acked by Suzuki.

I'd like to get opinions from David and CoreSight maintainers Mathieu
and Suzuki, should we merge the patch set [1] via David's tree as well
to avoid dependency issue, or prefer to merge it via CoreSight tree?
If David prefers the prior option, I can resend the patch set [1] with
looping David.

Thanks,
Leo

[1] https://lore.kernel.org/lkml/[email protected]/

2022-01-24 18:58:18

by Leo Yan

[permalink] [raw]
Subject: Re: [PATCH v2 0/7] pid: Introduce helper task_is_in_root_ns()

On Wed, Jan 12, 2022 at 02:40:47PM +0800, Leo Yan wrote:
> Hi David,
>
> On Wed, Dec 08, 2021 at 04:33:13PM +0800, Leo Yan wrote:
> > The kernel uses open code to check if a process is in root PID namespace
> > or not in several places.
> >
> > Suggested by Suzuki, this patch set is to create a helper function
> > task_is_in_init_pid_ns() to replace open code.
> >
> > This patch set has been applied on the mainline kernel and built for
> > Arm64 kernel with enabling all relevant modules.
>
> I'd like sync for how to merging this patch set. Except patch 05/07,
> all of other patches in this patch set have been received the reviewed
> or acked tags. So could you pick up this patch set?
>
> Furthermore, we have another patch set "coresight: etm: Correct PID
> tracing for non-root namespace" [1], which is dependent on the current
> patch set and it has been Acked by Suzuki.
>
> I'd like to get opinions from David and CoreSight maintainers Mathieu
> and Suzuki, should we merge the patch set [1] via David's tree as well
> to avoid dependency issue, or prefer to merge it via CoreSight tree?
> If David prefers the prior option, I can resend the patch set [1] with
> looping David.

Gentle ping, Dave.

I verified the current patch set and CoreSight patch set, both can apply
clearly on the latest mainline kernel (with last commit
dd81e1c7d5fb "Merge tag 'powerpc-5.17-2' of
git://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux").

Thanks,
Leo

> [1] https://lore.kernel.org/lkml/[email protected]/