[PATCH] Clarify the rootid_owns_currentns

[PATCH] Clarify the rootid_owns_currentns

[Serge E. Hallyn]

Split most of the rootid_owns_currentns() functionality
into a more generic rootid_owns_ns() function which
will be easier to write tests for.

Rename the functions and variables to make clear that
the ids being tested could be any uid.

Signed-off-by: Serge Hallyn <serge@hallyn.com>
CC: Ryan Foster <foster.ryan.r@gmail.com>
CC: Christian Brauner <brauner@kernel.org>
---
 security/commoncap.c | 35 +++++++++++++++++++++++------------
 1 file changed, 23 insertions(+), 12 deletions(-)

diff --git a/security/commoncap.c b/security/commoncap.c
index 6bd4adeb4795..8a81fdc12cbe 100644
--- a/security/commoncap.c
+++ b/security/commoncap.c
@@ -358,17 +358,18 @@ int cap_inode_killpriv(struct mnt_idmap *idmap, struct dentry *dentry)
 	return error;
 }
 
-static bool rootid_owns_currentns(vfsuid_t rootvfsuid)
+/**
+ * kuid_root_in_ns - check whether the given kuid is root in the given ns
+ *
+ * @kuid - the kuid to be tested
+ * @ns - the user namespace to test against
+ *
+ * Returns true if @kuid represents the root user in @ns, false otherwise.
+ */
+static bool kuid_root_in_ns(kuid_t kuid, struct user_namespace *ns)
 {
-	struct user_namespace *ns;
-	kuid_t kroot;
-
-	if (!vfsuid_valid(rootvfsuid))
-		return false;
-
-	kroot = vfsuid_into_kuid(rootvfsuid);
-	for (ns = current_user_ns();; ns = ns->parent) {
-		if (from_kuid(ns, kroot) == 0)
+	for (;; ns = ns->parent) {
+		if (from_kuid(ns, kuid) == 0)
 			return true;
 		if (ns == &init_user_ns)
 			break;
@@ -377,6 +378,16 @@ static bool rootid_owns_currentns(vfsuid_t rootvfsuid)
 	return false;
 }
 
+static bool vfsuid_root_in_currentns(vfsuid_t vfsuid)
+{
+	kuid_t kuid;
+
+	if (!vfsuid_valid(vfsuid))
+		return false;
+	kuid = vfsuid_into_kuid(vfsuid);
+	return kuid_root_in_ns(kuid, current_user_ns());
+}
+
 static __u32 sansflags(__u32 m)
 {
 	return m & ~VFS_CAP_FLAGS_EFFECTIVE;
@@ -481,7 +492,7 @@ int cap_inode_getsecurity(struct mnt_idmap *idmap,
 		goto out_free;
 	}
 
-	if (!rootid_owns_currentns(vfsroot)) {
+	if (!vfsuid_root_in_currentns(vfsroot)) {
 		size = -EOVERFLOW;
 		goto out_free;
 	}
@@ -722,7 +733,7 @@ int get_vfs_caps_from_disk(struct mnt_idmap *idmap,
 	/* Limit the caps to the mounter of the filesystem
 	 * or the more limited uid specified in the xattr.
 	 */
-	if (!rootid_owns_currentns(rootvfsuid))
+	if (!vfsuid_root_in_currentns(rootvfsuid))
 		return -ENODATA;
 
 	cpu_caps->permitted.val = le32_to_cpu(caps->data[0].permitted);
-- 
2.34.1

Re: [PATCH] Clarify the rootid_owns_currentns

[kernel test robot]

Hi Serge,

kernel test robot noticed the following build warnings:

[auto build test WARNING on linus/master]
[also build test WARNING on brauner-vfs/vfs.all v6.18-rc5 next-20251114]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Serge-E-Hallyn/Clarify-the-rootid_owns_currentns/20251115-053655
base:   linus/master
patch link:    https://lore.kernel.org/r/aRegH8P4cPlzzlX9%40mail.hallyn.com
patch subject: [PATCH] Clarify the rootid_owns_currentns
config: alpha-allnoconfig (https://download.01.org/0day-ci/archive/20251115/202511150644.EXaXOsVc-lkp@intel.com/config)
compiler: alpha-linux-gcc (GCC) 15.1.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20251115/202511150644.EXaXOsVc-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202511150644.EXaXOsVc-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> Warning: security/commoncap.c:369 function parameter 'kuid' not described in 'kuid_root_in_ns'
>> Warning: security/commoncap.c:369 function parameter 'ns' not described in 'kuid_root_in_ns'

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH] Clarify the rootid_owns_currentns

[Serge E. Hallyn]

On Fri, Nov 14, 2025 at 03:33:19PM -0600, Serge E. Hallyn wrote:
> Split most of the rootid_owns_currentns() functionality
> into a more generic rootid_owns_ns() function which
> will be easier to write tests for.
> 
> Rename the functions and variables to make clear that
> the ids being tested could be any uid.
> 
> Signed-off-by: Serge Hallyn <serge@hallyn.com>
> CC: Ryan Foster <foster.ryan.r@gmail.com>
> CC: Christian Brauner <brauner@kernel.org>

Paul, Christian, let me know if you have any objections, else I will
queue this up in caps-next.

Ryan, based on this you would be able to do more useful unit ktests:
you could create some simple user namespaces with mappings which do
or do not have uid 0 in ns mapped to the kuid you are querying.

> ---
>  security/commoncap.c | 35 +++++++++++++++++++++++------------
>  1 file changed, 23 insertions(+), 12 deletions(-)
> 
> diff --git a/security/commoncap.c b/security/commoncap.c
> index 6bd4adeb4795..8a81fdc12cbe 100644
> --- a/security/commoncap.c
> +++ b/security/commoncap.c
> @@ -358,17 +358,18 @@ int cap_inode_killpriv(struct mnt_idmap *idmap, struct dentry *dentry)
>  	return error;
>  }
>  
> -static bool rootid_owns_currentns(vfsuid_t rootvfsuid)
> +/**
> + * kuid_root_in_ns - check whether the given kuid is root in the given ns
> + *
> + * @kuid - the kuid to be tested
> + * @ns - the user namespace to test against
> + *
> + * Returns true if @kuid represents the root user in @ns, false otherwise.
> + */
> +static bool kuid_root_in_ns(kuid_t kuid, struct user_namespace *ns)
>  {
> -	struct user_namespace *ns;
> -	kuid_t kroot;
> -
> -	if (!vfsuid_valid(rootvfsuid))
> -		return false;
> -
> -	kroot = vfsuid_into_kuid(rootvfsuid);
> -	for (ns = current_user_ns();; ns = ns->parent) {
> -		if (from_kuid(ns, kroot) == 0)
> +	for (;; ns = ns->parent) {
> +		if (from_kuid(ns, kuid) == 0)
>  			return true;
>  		if (ns == &init_user_ns)
>  			break;
> @@ -377,6 +378,16 @@ static bool rootid_owns_currentns(vfsuid_t rootvfsuid)
>  	return false;
>  }
>  
> +static bool vfsuid_root_in_currentns(vfsuid_t vfsuid)
> +{
> +	kuid_t kuid;
> +
> +	if (!vfsuid_valid(vfsuid))
> +		return false;
> +	kuid = vfsuid_into_kuid(vfsuid);
> +	return kuid_root_in_ns(kuid, current_user_ns());
> +}
> +
>  static __u32 sansflags(__u32 m)
>  {
>  	return m & ~VFS_CAP_FLAGS_EFFECTIVE;
> @@ -481,7 +492,7 @@ int cap_inode_getsecurity(struct mnt_idmap *idmap,
>  		goto out_free;
>  	}
>  
> -	if (!rootid_owns_currentns(vfsroot)) {
> +	if (!vfsuid_root_in_currentns(vfsroot)) {
>  		size = -EOVERFLOW;
>  		goto out_free;
>  	}
> @@ -722,7 +733,7 @@ int get_vfs_caps_from_disk(struct mnt_idmap *idmap,
>  	/* Limit the caps to the mounter of the filesystem
>  	 * or the more limited uid specified in the xattr.
>  	 */
> -	if (!rootid_owns_currentns(rootvfsuid))
> +	if (!vfsuid_root_in_currentns(rootvfsuid))
>  		return -ENODATA;
>  
>  	cpu_caps->permitted.val = le32_to_cpu(caps->data[0].permitted);
> -- 
> 2.34.1
> 

Re: [PATCH] Clarify the rootid_owns_currentns

[Paul Moore]

On Tue, Nov 18, 2025 at 9:16 AM Serge E. Hallyn <serge@hallyn.com> wrote:
> On Fri, Nov 14, 2025 at 03:33:19PM -0600, Serge E. Hallyn wrote:
> > Split most of the rootid_owns_currentns() functionality
> > into a more generic rootid_owns_ns() function which
> > will be easier to write tests for.
> >
> > Rename the functions and variables to make clear that
> > the ids being tested could be any uid.
> >
> > Signed-off-by: Serge Hallyn <serge@hallyn.com>
> > CC: Ryan Foster <foster.ryan.r@gmail.com>
> > CC: Christian Brauner <brauner@kernel.org>
>
> Paul, Christian, let me know if you have any objections, else I will
> queue this up in caps-next.

Seems reasonable to me, but it would be good to fix the parameter doc
bug that the kernel test robot identified.  I suspect it is just the
extra vertical comment space between the top one line summary and the
parameter list.

-- 
paul-moore.com

Re: [PATCH] Clarify the rootid_owns_currentns

[Serge E. Hallyn]

On Tue, Nov 18, 2025 at 10:47:06AM -0500, Paul Moore wrote:
> On Tue, Nov 18, 2025 at 9:16 AM Serge E. Hallyn <serge@hallyn.com> wrote:
> > On Fri, Nov 14, 2025 at 03:33:19PM -0600, Serge E. Hallyn wrote:
> > > Split most of the rootid_owns_currentns() functionality
> > > into a more generic rootid_owns_ns() function which
> > > will be easier to write tests for.
> > >
> > > Rename the functions and variables to make clear that
> > > the ids being tested could be any uid.
> > >
> > > Signed-off-by: Serge Hallyn <serge@hallyn.com>
> > > CC: Ryan Foster <foster.ryan.r@gmail.com>
> > > CC: Christian Brauner <brauner@kernel.org>
> >
> > Paul, Christian, let me know if you have any objections, else I will
> > queue this up in caps-next.
> 
> Seems reasonable to me, but it would be good to fix the parameter doc
> bug that the kernel test robot identified.  I suspect it is just the
> extra vertical comment space between the top one line summary and the
> parameter list.

ooh, is that it.  I couldn't for the life of me figure out what it
was complaining about.  Will try that, thanks.

Re: [PATCH] Clarify the rootid_owns_currentns

[Serge E. Hallyn]

On Tue, Nov 18, 2025 at 10:47:06AM -0500, Paul Moore wrote:
> On Tue, Nov 18, 2025 at 9:16 AM Serge E. Hallyn <serge@hallyn.com> wrote:
> > On Fri, Nov 14, 2025 at 03:33:19PM -0600, Serge E. Hallyn wrote:
> > > Split most of the rootid_owns_currentns() functionality
> > > into a more generic rootid_owns_ns() function which
> > > will be easier to write tests for.
> > >
> > > Rename the functions and variables to make clear that
> > > the ids being tested could be any uid.
> > >
> > > Signed-off-by: Serge Hallyn <serge@hallyn.com>
> > > CC: Ryan Foster <foster.ryan.r@gmail.com>
> > > CC: Christian Brauner <brauner@kernel.org>
> >
> > Paul, Christian, let me know if you have any objections, else I will
> > queue this up in caps-next.
> 
> Seems reasonable to me, but it would be good to fix the parameter doc
> bug that the kernel test robot identified.  I suspect it is just the
> extra vertical comment space between the top one line summary and the
> parameter list.

Actually I think it was probably the use of - instead of : after the
parameter name, but I went ahead and changed both, thanks.

Pushed the below patch for linux-next.

Subject: [PATCH 1/1] Clarify the rootid_owns_currentns

Split most of the rootid_owns_currentns() functionality
into a more generic rootid_owns_ns() function which
will be easier to write tests for.

Rename the functions and variables to make clear that
the ids being tested could be any uid.

Signed-off-by: Serge Hallyn <serge@hallyn.com>
CC: Ryan Foster <foster.ryan.r@gmail.com>
CC: Christian Brauner <brauner@kernel.org>

---
v2: change the function parameter documentation to mollify the bot.
---
 security/commoncap.c | 34 ++++++++++++++++++++++------------
 1 file changed, 22 insertions(+), 12 deletions(-)

diff --git a/security/commoncap.c b/security/commoncap.c
index 6bd4adeb4795..496e054c5d37 100644
--- a/security/commoncap.c
+++ b/security/commoncap.c
@@ -358,17 +358,17 @@ int cap_inode_killpriv(struct mnt_idmap *idmap, struct dentry *dentry)
 	return error;
 }
 
-static bool rootid_owns_currentns(vfsuid_t rootvfsuid)
+/**
+ * kuid_root_in_ns - check whether the given kuid is root in the given ns
+ * @kuid: the kuid to be tested
+ * @ns: the user namespace to test against
+ *
+ * Returns true if @kuid represents the root user in @ns, false otherwise.
+ */
+static bool kuid_root_in_ns(kuid_t kuid, struct user_namespace *ns)
 {
-	struct user_namespace *ns;
-	kuid_t kroot;
-
-	if (!vfsuid_valid(rootvfsuid))
-		return false;
-
-	kroot = vfsuid_into_kuid(rootvfsuid);
-	for (ns = current_user_ns();; ns = ns->parent) {
-		if (from_kuid(ns, kroot) == 0)
+	for (;; ns = ns->parent) {
+		if (from_kuid(ns, kuid) == 0)
 			return true;
 		if (ns == &init_user_ns)
 			break;
@@ -377,6 +377,16 @@ static bool rootid_owns_currentns(vfsuid_t rootvfsuid)
 	return false;
 }
 
+static bool vfsuid_root_in_currentns(vfsuid_t vfsuid)
+{
+	kuid_t kuid;
+
+	if (!vfsuid_valid(vfsuid))
+		return false;
+	kuid = vfsuid_into_kuid(vfsuid);
+	return kuid_root_in_ns(kuid, current_user_ns());
+}
+
 static __u32 sansflags(__u32 m)
 {
 	return m & ~VFS_CAP_FLAGS_EFFECTIVE;
@@ -481,7 +491,7 @@ int cap_inode_getsecurity(struct mnt_idmap *idmap,
 		goto out_free;
 	}
 
-	if (!rootid_owns_currentns(vfsroot)) {
+	if (!vfsuid_root_in_currentns(vfsroot)) {
 		size = -EOVERFLOW;
 		goto out_free;
 	}
@@ -722,7 +732,7 @@ int get_vfs_caps_from_disk(struct mnt_idmap *idmap,
 	/* Limit the caps to the mounter of the filesystem
 	 * or the more limited uid specified in the xattr.
 	 */
-	if (!rootid_owns_currentns(rootvfsuid))
+	if (!vfsuid_root_in_currentns(rootvfsuid))
 		return -ENODATA;
 
 	cpu_caps->permitted.val = le32_to_cpu(caps->data[0].permitted);
-- 
2.34.1

Re: [PATCH] Clarify the rootid_owns_currentns

[Paul Moore]

On Tue, Nov 18, 2025 at 7:05 PM Serge E. Hallyn <serge@hallyn.com> wrote:
> On Tue, Nov 18, 2025 at 10:47:06AM -0500, Paul Moore wrote:
> > On Tue, Nov 18, 2025 at 9:16 AM Serge E. Hallyn <serge@hallyn.com> wrote:
> > > On Fri, Nov 14, 2025 at 03:33:19PM -0600, Serge E. Hallyn wrote:
> > > > Split most of the rootid_owns_currentns() functionality
> > > > into a more generic rootid_owns_ns() function which
> > > > will be easier to write tests for.
> > > >
> > > > Rename the functions and variables to make clear that
> > > > the ids being tested could be any uid.
> > > >
> > > > Signed-off-by: Serge Hallyn <serge@hallyn.com>
> > > > CC: Ryan Foster <foster.ryan.r@gmail.com>
> > > > CC: Christian Brauner <brauner@kernel.org>
> > >
> > > Paul, Christian, let me know if you have any objections, else I will
> > > queue this up in caps-next.
> >
> > Seems reasonable to me, but it would be good to fix the parameter doc
> > bug that the kernel test robot identified.  I suspect it is just the
> > extra vertical comment space between the top one line summary and the
> > parameter list.
>
> Actually I think it was probably the use of - instead of : after the
> parameter name, but I went ahead and changed both, thanks.

Ah, yeah, at least one of those was it I'm sure.

> Pushed the below patch for linux-next.

Thanks.

-- 
paul-moore.com