From: Jeff Hostetler Date: Fri, 05 Mar 2021 21:30:57 GMT Subject: Re: [PATCH v4 09/12] unix-socket: disallow chdir() when creating unix domain sockets Message-ID: In-Reply-To: On 3/4/21 3:34 PM, Junio C Hamano wrote: > Jeff King writes: > >> The feature is definitely useful; I think I did 1eb10f4091 (unix-socket: >> handle long socket pathnames, 2012-01-09) in response to a real problem. >> >> Certainly callers could handle the error themselves. The reason I pushed >> it down into the socket code was to avoid having to implement in >> multiple callers. There are only two, but we'd have needed it in both >> sides (credential-cache--daemon as the listener, and credential-cache as >> the client). >> >> Ironically, the listening side now does a permanent chdir() to the >> socket directory anyway, since 6e61449051 (credential-cache--daemon: >> change to the socket dir on startup, 2016-02-23). So we could just do >> that first, and then feed the basename to the socket code. >> >> The client side would still need to handle it, though. It could probably >> also chdir to the socket directory without any real downside (once >> started, I don't think the helper program needs to access the filesystem >> at all outside of the socket). >> >> So I dunno. I'd be OK to just rip the feature out in favor of doing >> those chdir()s. But that seems like a non-zero amount of work versus >> leaving, and the existing code has the benefit that if another caller >> shows up, it could benefit from the feature. > > I am OK to keep the series as-is, and leave it to a possible future > work to remove the need for chdir even for long paths and not having > to return an error with ENAMETOOLONG; when such an update happens, > the "fail if need to chdir" feature this patch is adding will become > a no-op. > I think I'd like to keep things as I have them now with the "disallow chdir()" option bit and save the "fork() / bind()" solution for a later patch series. Simple IPC is large enough as it is and the new ENAMETOOLONG error will only affect callers who set the bit. A later patch series can easily test and confirm the "fork() / bind() solution in isolation and test it on the other Unix hosts and then remove the bit from those callers (if we want). Jeff