Re: [PATCH] credential-cache--daemon: Change to root dir on startup

From: Jeff King <peff@peff.net>
Date: Feb 21, 2016, 23:52 UTC
Message-ID: <20160221235225.GE4094@sigill.intra.peff.net>
In-Reply-To: <1456037497-9895-1-git-send-email-jon_p_griffiths@yahoo.com>

On Sun, Feb 21, 2016 at 07:51:37PM +1300, Jon Griffiths wrote:

Show 25 quoted lines

> Stop the daemon from preventing umount of the directory it
> was started in.
> 
> Without this change the daemon prevents umount because it
> it holds open its cwd. If it starts in a directory we want
> to unmount we have to manually kill the process which is
> undesirable and also uncaches any credentials it is holding.
> 
> Signed-off-by: Jon Griffiths <jon_p_griffiths@yahoo.com>
> ---
>  credential-cache--daemon.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/credential-cache--daemon.c b/credential-cache--daemon.c
> index cc65a9c..d911b40 100644
> --- a/credential-cache--daemon.c
> +++ b/credential-cache--daemon.c
> @@ -270,6 +270,8 @@ int main(int argc, const char **argv)
>  	if (ignore_sighup)
>  		signal(SIGHUP, SIG_IGN);
>  
> +	chdir("/");
> +
>  	serve_cache(socket_path, debug);
>  	delete_tempfile(&socket_file);

What happens if socket_path is relative here?

I don't know how common that would be in practice. In general, the daemon is running out of $GIT_DIR or the top-level of the work-tree, I'd guess, which is inappropriate for any user-level config, and probably simply confusing even for repo-level config. I'm actually tempted to just disallow relative paths entirely.

I think your patch _just_ helps the case where the git repository is being unmounted (and our daemon's cwd happened to be there). It doesn't help the case that the socket path is unmounted (which will still get EBUSY because we have the socket open).

I can't think of any reason that chdir() to "/" would be a bad thing (if you get EPERM going to "/", you probably have bigger problems), but another option would be:

  1. chdir(dirname(socket_path));

  2. serve_cache(basename(socket_path));

That works with relative paths, and it puts all of our open files in the same part of the hierarchy.

-Peff