Ludovic Courtès <ludo@gnu.org> writes:

> @@ -2245,9 +2249,18 @@ void DerivationGoal::runChild()
>              if (rmdir("real-root") == -1)
>                  throw SysError("cannot remove real-root directory");
>  
> -	    /* Remount root as read-only.  */
> -            if (mount("/", "/", 0, MS_BIND | MS_REMOUNT | MS_RDONLY, 0) == -1)
> -                throw SysError(format("read-only remount of build root '%1%' failed") % chrootRootDir);
> +	    /* Make the root read-only.
> +
> +	       The build process could make it world-accessible, but that's

Strictly speaking, in the case of --build-users-group, it couldn't even
do that.

> +	       OK: since 'chrootRootTop' is *not* world-accessible, a
> +	       world-accessible 'chrootRootDir' cannot be used to grant access
> +	       to the store to external processes.

It may be more general to write "grant access to the build environment",
unless you're using this as a shorthand for "grant access to the build
environment, and thereby a setuid binary, and thereby (in some
configurations) the store".

Looks good to me, hopefully there aren't any major packages further down
the line that rely on chmod("/", ...) failing.

- reepca