public inbox for passt-dev@passt.top
 help / color / mirror / code / Atom feed
* [PATCH] util: Make setting uidmap and gidmap errors non-fatal
@ 2026-10-02  7:00 Stefano Brivio
  2026-10-06  2:49 ` David Gibson
  0 siblings, 1 reply; 3+ messages in thread
From: Stefano Brivio @ 2026-10-02  7:00 UTC (permalink / raw)
  To: passt-dev; +Cc: David Gibson

Starting from commit 7bf1595c9242 ("isolation: Don't create our userns
as nobody"), we unconditionally set uidmap and gidmap in the detached
user namespace.

If passt is started from a detached PID namespace, but /proc hasn't
been remounted to reflect this, we'll fail to write those entries.

That's actually fine as uidmap and gidmap are something that, strictly
speaking, we only need to write in pasta mode when a command is
detached (it's now done in all cases for simplicity).

Warn, because it's not the expected behaviour (/proc should probably
be remounted first), but don't fail on that.

Link: https://github.com/containers/crun/issues/2283
Suggested-by: David Gibson <david@gibson.dropbear.id.au>
Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
---
 util.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/util.c b/util.c
index c02aea9..ff1f19f 100644
--- a/util.c
+++ b/util.c
@@ -1147,5 +1147,5 @@ void make_ugid_map(pid_t pid, uid_t uid, gid_t gid)
 	if (write_file(uidmap_path, uidmap) ||
 	    write_file(setgroups_path, "deny") ||
 	    write_file(gidmap_path, gidmap))
-		die("Couldn't configure user mappings");
+		warn("Couldn't configure user mappings");
 }
-- 
2.43.0


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] util: Make setting uidmap and gidmap errors non-fatal
  2026-10-02  7:00 [PATCH] util: Make setting uidmap and gidmap errors non-fatal Stefano Brivio
@ 2026-10-06  2:49 ` David Gibson
  2026-10-07 16:45   ` Stefano Brivio
  0 siblings, 1 reply; 3+ messages in thread
From: David Gibson @ 2026-10-06  2:49 UTC (permalink / raw)
  To: Stefano Brivio; +Cc: passt-dev

[-- Attachment #1: Type: text/plain, Size: 1781 bytes --]

On Fri, Oct 02, 2026 at 09:00:50AM +0200, Stefano Brivio wrote:
> Starting from commit 7bf1595c9242 ("isolation: Don't create our userns
> as nobody"), we unconditionally set uidmap and gidmap in the detached
> user namespace.
> 
> If passt is started from a detached PID namespace, but /proc hasn't
> been remounted to reflect this, we'll fail to write those entries.
> 
> That's actually fine as uidmap and gidmap are something that, strictly
> speaking, we only need to write in pasta mode when a command is
> detached (it's now done in all cases for simplicity).
> 
> Warn, because it's not the expected behaviour (/proc should probably
> be remounted first), but don't fail on that.
> 
> Link: https://github.com/containers/crun/issues/2283
> Suggested-by: David Gibson <david@gibson.dropbear.id.au>
> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>

Since this can now fail non-fatally, I'd suggest adding a return code
to make_ugid_map().  The caller (create_userns()) should probably
die() if it fails when we're actually changing UID/GID.

> ---
>  util.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/util.c b/util.c
> index c02aea9..ff1f19f 100644
> --- a/util.c
> +++ b/util.c
> @@ -1147,5 +1147,5 @@ void make_ugid_map(pid_t pid, uid_t uid, gid_t gid)
>  	if (write_file(uidmap_path, uidmap) ||
>  	    write_file(setgroups_path, "deny") ||
>  	    write_file(gidmap_path, gidmap))
> -		die("Couldn't configure user mappings");
> +		warn("Couldn't configure user mappings");
>  }
> -- 
> 2.43.0
> 

-- 
David Gibson (he or they)	| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you, not the other way
				| around.
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] util: Make setting uidmap and gidmap errors non-fatal
  2026-10-06  2:49 ` David Gibson
@ 2026-10-07 16:45   ` Stefano Brivio
  0 siblings, 0 replies; 3+ messages in thread
From: Stefano Brivio @ 2026-10-07 16:45 UTC (permalink / raw)
  To: David Gibson; +Cc: passt-dev

On Tue, 6 Oct 2026 13:49:08 +1100
David Gibson <david@gibson.dropbear.id.au> wrote:

> On Fri, Oct 02, 2026 at 09:00:50AM +0200, Stefano Brivio wrote:
> > Starting from commit 7bf1595c9242 ("isolation: Don't create our userns
> > as nobody"), we unconditionally set uidmap and gidmap in the detached
> > user namespace.
> > 
> > If passt is started from a detached PID namespace, but /proc hasn't
> > been remounted to reflect this, we'll fail to write those entries.
> > 
> > That's actually fine as uidmap and gidmap are something that, strictly
> > speaking, we only need to write in pasta mode when a command is
> > detached (it's now done in all cases for simplicity).
> > 
> > Warn, because it's not the expected behaviour (/proc should probably
> > be remounted first), but don't fail on that.
> > 
> > Link: https://github.com/containers/crun/issues/2283
> > Suggested-by: David Gibson <david@gibson.dropbear.id.au>
> > Signed-off-by: Stefano Brivio <sbrivio@redhat.com>  
> 
> Since this can now fail non-fatally, I'd suggest adding a return code
> to make_ugid_map().  The caller (create_userns()) should probably
> die() if it fails when we're actually changing UID/GID.

It sounds reasonable, feel free to send a patch, but note that we still
have an issue here:

  https://github.com/containers/crun/issues/2283#issuecomment-6040477454

and my further patch linked below in that webpage might be needed. I
would consider further changes only once that is fixed for good.

-- 
Stefano


^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-10-07 16:45 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-10-02  7:00 [PATCH] util: Make setting uidmap and gidmap errors non-fatal Stefano Brivio
2026-10-06  2:49 ` David Gibson
2026-10-07 16:45   ` Stefano Brivio

Code repositories for project(s) associated with this public inbox

	https://passt.top/passt

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for IMAP folder(s).