[PATCH] cgroup: namespace: replace BUG_ON() in copy_cgroup_ns() with WARN_ON_ONCE()

Shaojie Sun posted 1 patch 3 weeks, 2 days ago
kernel/cgroup/namespace.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
[PATCH] cgroup: namespace: replace BUG_ON() in copy_cgroup_ns() with WARN_ON_ONCE()
Posted by Shaojie Sun 3 weeks, 2 days ago
copy_cgroup_ns() uses BUG_ON() to guard against a NULL old_ns. The
condition cannot currently be triggered by any caller, and the sole
caller, create_new_namespaces(), already checks the return value with
IS_ERR() and unwinds correctly.

Replace the BUG_ON() with WARN_ON_ONCE() and return -EINVAL instead,
consistent with the policy of not killing the whole machine for a
recoverable programming error.

Signed-off-by: Shaojie Sun <sunshaojie@kylinos.cn>
---
 kernel/cgroup/namespace.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/kernel/cgroup/namespace.c b/kernel/cgroup/namespace.c
index ea4ee13936be..13930955c254 100644
--- a/kernel/cgroup/namespace.c
+++ b/kernel/cgroup/namespace.c
@@ -53,7 +53,8 @@ struct cgroup_namespace *copy_cgroup_ns(u64 flags,
 	struct ucounts *ucounts;
 	struct css_set *cset;
 
-	BUG_ON(!old_ns);
+	if (WARN_ON_ONCE(!old_ns))
+		return ERR_PTR(-EINVAL);
 
 	if (!(flags & CLONE_NEWCGROUP)) {
 		get_cgroup_ns(old_ns);
-- 
2.50.1
Re: [PATCH] cgroup: namespace: replace BUG_ON() in copy_cgroup_ns() with WARN_ON_ONCE()
Posted by Michal Koutný 3 weeks, 2 days ago
Hi Shaojie.


On Wed, Sep 02, 2026 at 06:18:56PM +0800, Shaojie Sun <sunshaojie@kylinos.cn> wrote:
> copy_cgroup_ns() uses BUG_ON() to guard against a NULL old_ns. The
> condition cannot currently be triggered by any caller, and the sole
> caller, create_new_namespaces(), already checks the return value with
> IS_ERR() and unwinds correctly.
> 
> Replace the BUG_ON() with WARN_ON_ONCE() and return -EINVAL instead,
> consistent with the policy of not killing the whole machine for a
> recoverable programming error.

Do you plan to tackle other namespaces too? (Adding their guys to Cc:)

It'd be good to have some consistency across them if this is going to be
touched. (I see that some simply don't care whereas others have the same
BUG_ON(). I might personally prefer the former)

Regards,
Michal


> 
> Signed-off-by: Shaojie Sun <sunshaojie@kylinos.cn>
> ---
>  kernel/cgroup/namespace.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/kernel/cgroup/namespace.c b/kernel/cgroup/namespace.c
> index ea4ee13936be..13930955c254 100644
> --- a/kernel/cgroup/namespace.c
> +++ b/kernel/cgroup/namespace.c
> @@ -53,7 +53,8 @@ struct cgroup_namespace *copy_cgroup_ns(u64 flags,
>  	struct ucounts *ucounts;
>  	struct css_set *cset;
>  
> -	BUG_ON(!old_ns);
> +	if (WARN_ON_ONCE(!old_ns))
> +		return ERR_PTR(-EINVAL);
>  
>  	if (!(flags & CLONE_NEWCGROUP)) {
>  		get_cgroup_ns(old_ns);
> -- 
> 2.50.1
> 
[PATCH v2 0/3] drop BUG_ON() NULL checks in namespace copy helpers
Posted by Shaojie Sun 3 weeks, 2 days ago
v2:
- Reworked into a 3-patch series that simply drops the BUG_ON()s
  instead of replacing them with WARN_ON_ONCE() + error return,
  following the discussion with Tejun. Now also covers copy_utsname()
  and copy_mnt_ns() so that all copy_*_ns() helpers behave
  consistently.

The copy_*_ns() helpers are inconsistent in how they handle a NULL
namespace pointer. copy_mnt_ns(), copy_utsname() and copy_cgroup_ns()
have a BUG_ON() on the pointer, while copy_time_ns(), copy_pid_ns(),
copy_ipcs() and copy_net_ns() don't check it at all.

The sole caller, create_new_namespaces(), always passes the
corresponding namespace pointer from the task's nsproxy, so the checks
can never trigger and the helpers dereference the pointer right away
anyway.

Drop the BUG_ON()s so that all copy_*_ns() helpers behave consistently.
The maintainers of the respective subsystems are on Cc.

Shaojie Sun (3):
  cgroup: namespace: drop BUG_ON() in copy_cgroup_ns()
  uts: drop BUG_ON() in copy_utsname()
  fs: namespace: drop BUG_ON() in copy_mnt_ns()

 fs/namespace.c            | 2 --
 kernel/cgroup/namespace.c | 2 --
 kernel/utsname.c          | 1 -
 3 files changed, 5 deletions(-)

-- 
2.50.1
Re: [PATCH v2 0/3] drop BUG_ON() NULL checks in namespace copy helpers
Posted by Shaojie Sun 3 weeks, 2 days ago
Apologies, the cover letter of this series says

    "... following the discussion with Tejun."

It should read "... following the discussion with Michal Koutný".
The patches themselves are unaffected.

Thanks,
Shaojie
Re: (subset) [PATCH v2 0/3] drop BUG_ON() NULL checks in namespace copy helpers
Posted by Christian Brauner 3 weeks ago
On Thu, 03 Sep 2026 14:48:25 +0800, Shaojie Sun wrote:
> drop BUG_ON() NULL checks in namespace copy helpers
> 
> v2:
> - Reworked into a 3-patch series that simply drops the BUG_ON()s
>   instead of replacing them with WARN_ON_ONCE() + error return,
>   following the discussion with Tejun. Now also covers copy_utsname()
>   and copy_mnt_ns() so that all copy_*_ns() helpers behave
>   consistently.
> 
> [...]

Applied to the namespace-7.4.misc branch of the vfs/vfs.git tree.
Patches in the namespace-7.4.misc branch should appear in linux-next soon.

Please report any outstanding bugs that were missed during review in a
new review to the original patch series allowing us to drop it.

It's encouraged to provide Acked-bys and Reviewed-bys even though the
patch has now been applied. If possible patch trailers will be updated.

Note that commit hashes shown below are subject to change due to rebase,
trailer updates or similar. If in doubt, please check the listed branch.

tree:   https://git.kernel.org/pub/scm/linux/kernel/git/vfs/vfs.git
branch: master

[2/3] uts: drop BUG_ON() in copy_utsname()
      https://git.kernel.org/vfs/vfs/c/d54cc3a5272c
[3/3] fs: namespace: drop BUG_ON() in copy_mnt_ns()
      https://git.kernel.org/vfs/vfs/c/4141e148e629
[PATCH v2 1/3] cgroup: namespace: drop BUG_ON() in copy_cgroup_ns()
Posted by Shaojie Sun 3 weeks, 2 days ago
The sole caller, create_new_namespaces(), always passes a valid cgroup
namespace pointer from the task's nsproxy. The other copy_*_ns()
helpers, e.g. copy_time_ns() and copy_pid_ns(), don't perform such a
check either. Drop the BUG_ON() for consistency with them.

Signed-off-by: Shaojie Sun <sunshaojie@kylinos.cn>
---
 kernel/cgroup/namespace.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/kernel/cgroup/namespace.c b/kernel/cgroup/namespace.c
index ea4ee13936be..76660332ec64 100644
--- a/kernel/cgroup/namespace.c
+++ b/kernel/cgroup/namespace.c
@@ -53,8 +53,6 @@ struct cgroup_namespace *copy_cgroup_ns(u64 flags,
 	struct ucounts *ucounts;
 	struct css_set *cset;
 
-	BUG_ON(!old_ns);
-
 	if (!(flags & CLONE_NEWCGROUP)) {
 		get_cgroup_ns(old_ns);
 		return old_ns;
-- 
2.50.1
Re: [PATCH v2 1/3] cgroup: namespace: drop BUG_ON() in copy_cgroup_ns()
Posted by Tejun Heo 3 weeks, 1 day ago
Hello,

On Thu, Sep 03, 2026 at 02:48:26PM +0800, Shaojie Sun wrote:
> The sole caller, create_new_namespaces(), always passes a valid cgroup
> namespace pointer from the task's nsproxy. The other copy_*_ns()
> helpers, e.g. copy_time_ns() and copy_pid_ns(), don't perform such a
> check either. Drop the BUG_ON() for consistency with them.
>
> Signed-off-by: Shaojie Sun <sunshaojie@kylinos.cn>

Applied to cgroup/for-7.4 with Tao's Reviewed-by added.

Thanks.

--
tejun
Re: [PATCH v2 1/3] cgroup: namespace: drop BUG_ON() in copy_cgroup_ns()
Posted by Tao Cui 3 weeks, 1 day ago

在 2026/9/3 14:48, Shaojie Sun 写道:
> The sole caller, create_new_namespaces(), always passes a valid cgroup
> namespace pointer from the task's nsproxy. The other copy_*_ns()
> helpers, e.g. copy_time_ns() and copy_pid_ns(), don't perform such a
> check either. Drop the BUG_ON() for consistency with them.
> 
> Signed-off-by: Shaojie Sun <sunshaojie@kylinos.cn>
> ---
>  kernel/cgroup/namespace.c | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/kernel/cgroup/namespace.c b/kernel/cgroup/namespace.c
> index ea4ee13936be..76660332ec64 100644
> --- a/kernel/cgroup/namespace.c
> +++ b/kernel/cgroup/namespace.c
> @@ -53,8 +53,6 @@ struct cgroup_namespace *copy_cgroup_ns(u64 flags,
>  	struct ucounts *ucounts;
>  	struct css_set *cset;
>  
> -	BUG_ON(!old_ns);
> -
>  	if (!(flags & CLONE_NEWCGROUP)) {
>  		get_cgroup_ns(old_ns);
>  		return old_ns;

Reviewed-by: Tao Cui <cuitao@kylinos.cn>
[PATCH v2 2/3] uts: drop BUG_ON() in copy_utsname()
Posted by Shaojie Sun 3 weeks, 2 days ago
The sole caller, create_new_namespaces(), always passes a valid uts
namespace pointer from the task's nsproxy. The other copy_*_ns()
helpers, e.g. copy_time_ns() and copy_pid_ns(), don't perform such a
check either. Drop the BUG_ON() for consistency with them.

Signed-off-by: Shaojie Sun <sunshaojie@kylinos.cn>
---
 kernel/utsname.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/kernel/utsname.c b/kernel/utsname.c
index ebbfc578a9d3..1ebf87e24607 100644
--- a/kernel/utsname.c
+++ b/kernel/utsname.c
@@ -81,7 +81,6 @@ struct uts_namespace *copy_utsname(u64 flags,
 {
 	struct uts_namespace *new_ns;
 
-	BUG_ON(!old_ns);
 	get_uts_ns(old_ns);
 
 	if (!(flags & CLONE_NEWUTS))
-- 
2.50.1
Re: [PATCH v2 2/3] uts: drop BUG_ON() in copy_utsname()
Posted by Bradley Morgan 3 weeks, 2 days ago
On 3 September 2026 07:48:27 BST, Shaojie Sun <sunshaojie@kylinos.cn>
wrote:
>The sole caller, create_new_namespaces(), always passes a valid uts
>namespace pointer from the task's nsproxy. The other copy_*_ns()
>helpers, e.g. copy_time_ns() and copy_pid_ns(), don't perform such a
>check either. Drop the BUG_ON() for consistency with them.
>

Reviewed-by: Bradley Morgan <brads@mainlining.org>


>Signed-off-by: Shaojie Sun <sunshaojie@kylinos.cn>
>---
> kernel/utsname.c | 1 -
> 1 file changed, 1 deletion(-)
>
>diff --git a/kernel/utsname.c b/kernel/utsname.c
>index ebbfc578a9d3..1ebf87e24607 100644
>--- a/kernel/utsname.c
>+++ b/kernel/utsname.c
>@@ -81,7 +81,6 @@ struct uts_namespace *copy_utsname(u64 flags,
> {
> 	struct uts_namespace *new_ns;
> 
>-	BUG_ON(!old_ns);
> 	get_uts_ns(old_ns);
> 
> 	if (!(flags & CLONE_NEWUTS))
>

--- Thanks!
https://lore.kernel.org/all/EE579805-42F2-4C58-B752-F28779EEB717@grrlz.net/
[PATCH v2 3/3] fs: namespace: drop BUG_ON() in copy_mnt_ns()
Posted by Shaojie Sun 3 weeks, 2 days ago
The sole caller, create_new_namespaces(), always passes a valid mount
namespace pointer from the task's nsproxy. The other copy_*_ns()
helpers, e.g. copy_time_ns() and copy_pid_ns(), don't perform such a
check either. Drop the BUG_ON() for consistency with them.

Signed-off-by: Shaojie Sun <sunshaojie@kylinos.cn>
---
 fs/namespace.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/fs/namespace.c b/fs/namespace.c
index 1ecd96c918b3..0f35c8c14027 100644
--- a/fs/namespace.c
+++ b/fs/namespace.c
@@ -4247,8 +4247,6 @@ struct mnt_namespace *copy_mnt_ns(u64 flags, struct mnt_namespace *ns,
 	struct mount *new;
 	int copy_flags;
 
-	BUG_ON(!ns);
-
 	if (likely(!(flags & CLONE_NEWNS))) {
 		get_mnt_ns(ns);
 		return ns;
-- 
2.50.1
Re: [PATCH v2 3/3] fs: namespace: drop BUG_ON() in copy_mnt_ns()
Posted by Jan Kara 3 weeks, 1 day ago
On Thu 03-09-26 14:48:28, Shaojie Sun wrote:
> The sole caller, create_new_namespaces(), always passes a valid mount
> namespace pointer from the task's nsproxy. The other copy_*_ns()
> helpers, e.g. copy_time_ns() and copy_pid_ns(), don't perform such a
> check either. Drop the BUG_ON() for consistency with them.
> 
> Signed-off-by: Shaojie Sun <sunshaojie@kylinos.cn>

It's indeed pointless. Feel free to add:

Reviewed-by: Jan Kara <jack@suse.cz>

								Honza

> ---
>  fs/namespace.c | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/fs/namespace.c b/fs/namespace.c
> index 1ecd96c918b3..0f35c8c14027 100644
> --- a/fs/namespace.c
> +++ b/fs/namespace.c
> @@ -4247,8 +4247,6 @@ struct mnt_namespace *copy_mnt_ns(u64 flags, struct mnt_namespace *ns,
>  	struct mount *new;
>  	int copy_flags;
>  
> -	BUG_ON(!ns);
> -
>  	if (likely(!(flags & CLONE_NEWNS))) {
>  		get_mnt_ns(ns);
>  		return ns;
> -- 
> 2.50.1
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR
Re: [PATCH v2 3/3] fs: namespace: drop BUG_ON() in copy_mnt_ns()
Posted by Tao Cui 3 weeks, 1 day ago

在 2026/9/3 14:48, Shaojie Sun 写道:
> The sole caller, create_new_namespaces(), always passes a valid mount
> namespace pointer from the task's nsproxy. The other copy_*_ns()
> helpers, e.g. copy_time_ns() and copy_pid_ns(), don't perform such a
> check either. Drop the BUG_ON() for consistency with them.
> 
> Signed-off-by: Shaojie Sun <sunshaojie@kylinos.cn>
> ---
>  fs/namespace.c | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/fs/namespace.c b/fs/namespace.c
> index 1ecd96c918b3..0f35c8c14027 100644
> --- a/fs/namespace.c
> +++ b/fs/namespace.c
> @@ -4247,8 +4247,6 @@ struct mnt_namespace *copy_mnt_ns(u64 flags, struct mnt_namespace *ns,
>  	struct mount *new;
>  	int copy_flags;
>  
> -	BUG_ON(!ns);
> -
>  	if (likely(!(flags & CLONE_NEWNS))) {
>  		get_mnt_ns(ns);
>  		return ns;
Reviewed-by: Tao Cui <cuitao@kylinos.cn>