On Sun, Jul 20, 2014 at 10:00:41AM +0200, René Scharfe wrote: > -- >8 -- > Subject: [PATCH] unix-socket: remove stale socket before calling chdir() > > unix_stream_listen() is given a path. It calls unix_sockaddr_init(), > which in turn can call chdir(). After that a relative path doesn't > mean the same as before. Any use of the original path should thus > happen before that call. For that reason, unlink the given path > (to get rid of a possibly existing stale socket) right at the > beginning of the function.
Thanks, I think this ordering problem was just missed in 1eb10f4 (unix-socket: handle long socket pathnames, 2012-01-09). Your solution looks OK, though I think also just using: unlink(sa.sun_path); would work, too (that is the path we are feeding to bind(), whether we have chdir'd or not, so perhaps it is a little more obviously correct?). I'm OK with either. > diff --git a/unix-socket.c b/unix-socket.c > index 01f119f..91bd6b8 100644 > --- a/unix-socket.c > +++ b/unix-socket.c > @@ -99,11 +99,12 @@ int unix_stream_listen(const char *path) > struct sockaddr_un sa; > struct unix_sockaddr_context ctx; > > + unlink(path); > + > if (unix_sockaddr_init(&sa, path, &ctx) < 0) > return -1; > fd = unix_stream_socket(); > > - unlink(path); I briefly wondered if this should be unlinking only when we get EEXIST, but I don't think it is worth caring about. The only caller is credential-cache, and it always wants to unconditionally replace whatever is there (it will already have tried to contact any existing socket). -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
