{"thread":{"id":"57115","subject":"[PATCH] daemon: plug memory leak on overlong path","startedAt":"2021-12-18T09:47:19Z","lastAt":"2021-12-18T10:18:22Z","messageCount":2,"participants":["René Scharfe","Jeff King"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"444418","messageId":"5b151dc3-d4c7-29d3-71ed-a79033693d5d@web.de","threadId":"57115","inReplyTo":null,"subject":"[PATCH] daemon: plug memory leak on overlong path","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2021-12-18T09:47:03Z","receivedAt":"2021-12-18T09:47:19Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Release the strbuf containing the interpolated path after copying it to\na stack buffer and before erroring out if it's too long.\n\nSigned-off-by: René Scharfe <l.s.r@web.de>\n---\n daemon.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/daemon.c b/daemon.c\nindex 4a000ee4af..94a5b8a364 100644\n--- a/daemon.c\n+++ b/daemon.c\n@@ -232,13 +232,13 @@ static const char *path_ok(const char *directory, struct hostinfo *hi)\n\n \t\trlen = strlcpy(interp_path, expanded_path.buf,\n \t\t\t       sizeof(interp_path));\n+\t\tstrbuf_release(&expanded_path);\n \t\tif (rlen >= sizeof(interp_path)) {\n \t\t\tlogerror(\"interpolated path too large: %s\",\n \t\t\t\t interp_path);\n \t\t\treturn NULL;\n \t\t}\n\n-\t\tstrbuf_release(&expanded_path);\n \t\tloginfo(\"Interpolated dir '%s'\", interp_path);\n\n \t\tdir = interp_path;\n--\n2.34.0\n"},{"id":"444423","messageId":"Yb21alvEAzBkxwGH@coredump.intra.peff.net","threadId":"57115","inReplyTo":"5b151dc3-d4c7-29d3-71ed-a79033693d5d@web.de","subject":"Re: [PATCH] daemon: plug memory leak on overlong path","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2021-12-18T10:18:18Z","receivedAt":"2021-12-18T10:18:22Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, Dec 18, 2021 at 10:47:03AM +0100, René Scharfe wrote:\n\n> Release the strbuf containing the interpolated path after copying it to\n> a stack buffer and before erroring out if it's too long.\n\nThanks, this looks obviously correct. The problem was introduced by my\n6bdb0083be (daemon: detect and reject too-long paths, 2016-10-22).\n\nI don't think it's that big a deal in practice, since we'd always be in\na worker process handling a single request, and will exit immediately\nafter returning from the function. So you could not, say, convince a\nlong-running git-daemon to leak a bunch of memory over time.\n\nBut definitely still worth fixing.\n\n> diff --git a/daemon.c b/daemon.c\n> index 4a000ee4af..94a5b8a364 100644\n> --- a/daemon.c\n> +++ b/daemon.c\n> @@ -232,13 +232,13 @@ static const char *path_ok(const char *directory, struct hostinfo *hi)\n> \n>  \t\trlen = strlcpy(interp_path, expanded_path.buf,\n>  \t\t\t       sizeof(interp_path));\n> +\t\tstrbuf_release(&expanded_path);\n>  \t\tif (rlen >= sizeof(interp_path)) {\n>  \t\t\tlogerror(\"interpolated path too large: %s\",\n>  \t\t\t\t interp_path);\n>  \t\t\treturn NULL;\n>  \t\t}\n> \n> -\t\tstrbuf_release(&expanded_path);\n>  \t\tloginfo(\"Interpolated dir '%s'\", interp_path);\n\nA common mistake in these kinds of fixes is that the variable to be\nfreed is used to generate an error message in the early-return path.\nHere we put \"interp_path\" in the message instead. That does mean the\nerror message shows the truncated name rather than the full one. That\nmay be a hidden feature, though. :)\n\n-Peff\n"}]}