Patrick Steinhardt <ps@xxxxxx> writes:
> When trying to create a Unix socket in a path that exceeds the maximum
> socket name length we try to first change the directory into the parent
> folder before creating the socket to reduce the length of the name. When
> this fails we error out of `unix_sockaddr_init()` with an error code,
> which indicates to the caller that the context has not been initialized.
> Consequently, they don't release that context.
>
> This leads to a memory leak: when we have already populated the context
> with the original directory that we need to chdir(3p) back into, but
> then the chdir(3p) into the socket's parent directory fails, then we
> won't release the original directory's path. The leak is exposed by
> t0301, but only via Meson with `meson setup -Dsanitize=leak`:
Did you mean
$ meson configure -Db_sanitize=leak
$ meson test t0301-credential-cache
I'll need to figure out how to make various tweaks at runtime
working with meson based build tree. The next thing I need to
figure out is to see how to get verbose error output from the tests,
as I cannot just go back to the source tree and say "cd t && sh
t0301-credential-cache -v -i -x" because the build is out of tree.
> Direct leak of 129 byte(s) in 1 object(s) allocated from:
> #0 0x5555555e85c6 in realloc.part.0 lsan_interceptors.cpp.o
> #1 0x55555590e3d6 in xrealloc ../wrapper.c:140:8
> #2 0x5555558c8fc6 in strbuf_grow ../strbuf.c:114:2
> #3 0x5555558cacab in strbuf_getcwd ../strbuf.c:605:3
> #4 0x555555923ff6 in unix_sockaddr_init ../unix-socket.c:65:7
> #5 0x555555923e42 in unix_stream_connect ../unix-socket.c:84:6
> #6 0x55555562a984 in send_request ../builtin/credential-cache.c:46:11
> #7 0x55555562a89e in do_cache ../builtin/credential-cache.c:108:6
> #8 0x55555562a655 in cmd_credential_cache ../builtin/credential-cache.c:178:3
> #9 0x555555700547 in run_builtin ../git.c:480:11
> #10 0x5555556ff0e0 in handle_builtin ../git.c:740:9
> #11 0x5555556ffee8 in run_argv ../git.c:807:4
> #12 0x5555556fee6b in cmd_main ../git.c:947:19
> #13 0x55555593f689 in main ../common-main.c:64:11
> #14 0x7ffff7a2a1fb in __libc_start_call_main (/nix/store/h7zcxabfxa7v5xdna45y2hplj31ncf8a-glibc-2.40-36/lib/libc.so.6+0x2a1fb) (BuildId: 0a855678aa0cb573cecbb2bcc73ab8239ec472d0)
> #15 0x7ffff7a2a2b8 in __libc_start_main@GLIBC_2.2.5 (/nix/store/h7zcxabfxa7v5xdna45y2hplj31ncf8a-glibc-2.40-36/lib/libc.so.6+0x2a2b8) (BuildId: 0a855678aa0cb573cecbb2bcc73ab8239ec472d0)
> #16 0x5555555ad1d4 in _start (git+0x591d4)
>
> DEDUP_TOKEN: ___interceptor_realloc.part.0--xrealloc--strbuf_grow--strbuf_getcwd--unix_sockaddr_init--unix_stream_connect--send_request--do_cache--cmd_credential_cache--run_builtin--handle_builtin--run_argv--cmd_main--main--__libc_start_call_main--__libc_start_main@GLIBC_2.2.5--_start
> SUMMARY: LeakSanitizer: 129 byte(s) leaked in 1 allocation(s).
>
> Fix this leak.
>
> Signed-off-by: Patrick Steinhardt <ps@xxxxxx>
> ---
> unix-socket.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
Thanks. The analysis and the fix looked superbly clear.
Will queue.
> diff --git a/unix-socket.c b/unix-socket.c
> index 483c9c448c..8860203c3f 100644
> --- a/unix-socket.c
> +++ b/unix-socket.c
> @@ -65,8 +65,10 @@ static int unix_sockaddr_init(struct sockaddr_un *sa, const char *path,
> if (strbuf_getcwd(&cwd))
> return -1;
> ctx->orig_dir = strbuf_detach(&cwd, NULL);
> - if (chdir_len(dir, slash - dir) < 0)
> + if (chdir_len(dir, slash - dir) < 0) {
> + FREE_AND_NULL(ctx->orig_dir);
> return -1;
> + }
> }
>
> memset(sa, 0, sizeof(*sa));
