[PATCH] security: commoncap: clarify CAP_FS_SET comment in cap_task_fix_setuid()

[PATCH] security: commoncap: clarify CAP_FS_SET comment in cap_task_fix_setuid()

[Adriano Cordova]

CAP_FS_SET is defined as exactly the capabilities an fsuid of 0
historically granted (see CAP_FS_MASK in include/linux/capability.h),
so dropping it when fsuid leaves 0 is correct by construction, not
"too harsh". Replace the stale FIXME with this explanation.

Signed-off-by: Adriano Cordova <adrianox@gmail.com>
---
 security/commoncap.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/security/commoncap.c b/security/commoncap.c
index 3399535808fe..e0a2f386c556 100644
--- a/security/commoncap.c
+++ b/security/commoncap.c
@@ -1169,11 +1169,13 @@ int cap_task_fix_setuid(struct cred *new, const struct cred *old, int flags)
 		break;
 
 	case LSM_SETID_FS:
-		/* juggle the capabilities to follow FSUID changes, unless
-		 * otherwise suppressed
+		/* Juggle the capabilities to follow FSUID changes, unless
+		 * otherwise suppressed.
 		 *
-		 * FIXME - is fsuser used for all CAP_FS_MASK capabilities?
-		 *          if not, we might be a bit too harsh here.
+		 * CAP_FS_SET is exactly the set of capabilities that an
+		 * fsuid of 0 historically granted (see CAP_FS_MASK in
+		 * <linux/capability.h>), so dropping that set when fsuid
+		 * leaves 0 is intentional rather than too harsh.
 		 */
 		if (!issecure(SECURE_NO_SETUID_FIXUP)) {
 			kuid_t root_uid = make_kuid(old->user_ns, 0);
-- 
2.51.0

Re: [PATCH] security: commoncap: clarify CAP_FS_SET comment in cap_task_fix_setuid()

[sashiko-bot]

> CAP_FS_SET is defined as exactly the capabilities an fsuid of 0
> historically granted (see CAP_FS_MASK in include/linux/capability.h),
> so dropping it when fsuid leaves 0 is correct by construction, not
> "too harsh". Replace the stale FIXME with this explanation.
> 
> Signed-off-by: Adriano Cordova <adrianox@gmail.com>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260919130112.849932-1-adrianox@gmail.com?part=1

Re: [PATCH] security: commoncap: clarify CAP_FS_SET comment in cap_task_fix_setuid()

[Serge E. Hallyn]

On Sat, Sep 19, 2026 at 10:01:12AM -0300, Adriano Cordova wrote:
> CAP_FS_SET is defined as exactly the capabilities an fsuid of 0
> historically granted (see CAP_FS_MASK in include/linux/capability.h),
> so dropping it when fsuid leaves 0 is correct by construction, not
> "too harsh". Replace the stale FIXME with this explanation.
> 
> Signed-off-by: Adriano Cordova <adrianox@gmail.com>

Thanks, but,

> ---
>  security/commoncap.c | 10 ++++++----
>  1 file changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/security/commoncap.c b/security/commoncap.c
> index 3399535808fe..e0a2f386c556 100644
> --- a/security/commoncap.c
> +++ b/security/commoncap.c
> @@ -1169,11 +1169,13 @@ int cap_task_fix_setuid(struct cred *new, const struct cred *old, int flags)
>  		break;
>  
>  	case LSM_SETID_FS:
> -		/* juggle the capabilities to follow FSUID changes, unless
> -		 * otherwise suppressed
> +		/* Juggle the capabilities to follow FSUID changes, unless
> +		 * otherwise suppressed.
>  		 *
> -		 * FIXME - is fsuser used for all CAP_FS_MASK capabilities?
> -		 *          if not, we might be a bit too harsh here.
> +		 * CAP_FS_SET is exactly the set of capabilities that an
> +		 * fsuid of 0 historically granted (see CAP_FS_MASK in
> +		 * <linux/capability.h>), so dropping that set when fsuid
> +		 * leaves 0 is intentional rather than too harsh.

"rather than too harsh" seems like arguing against the past comment.  Just
make it something like:

 +		 * CAP_FS_SET is exactly the set of capabilities that an
 +		 * fsuid of 0 historically granted (see CAP_FS_MASK in
 +		 * <linux/capability.h>), so we drop that set when fsuid
 +		 * leaves 0.

or

 +		/* Juggle the capabilities to follow FSUID changes, unless
 +		 * otherwise suppressed.  We drop the CAP_FS_SET when uid leaves
 +		 * 0, since that is the set of capabilities that an fsuid of 0
 +		 *  historically granted.

or even shorter.

>  		 */
>  		if (!issecure(SECURE_NO_SETUID_FIXUP)) {
>  			kuid_t root_uid = make_kuid(old->user_ns, 0);
> -- 
> 2.51.0