| Message ID | 5b151dc3-d4c7-29d3-71ed-a79033693d5d@web.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | daemon: plug memory leak on overlong path | expand |
On Sat, Dec 18, 2021 at 10:47:03AM +0100, René Scharfe wrote: > Release the strbuf containing the interpolated path after copying it to > a stack buffer and before erroring out if it's too long. Thanks, this looks obviously correct. The problem was introduced by my 6bdb0083be (daemon: detect and reject too-long paths, 2016-10-22). I don't think it's that big a deal in practice, since we'd always be in a worker process handling a single request, and will exit immediately after returning from the function. So you could not, say, convince a long-running git-daemon to leak a bunch of memory over time. But definitely still worth fixing. > diff --git a/daemon.c b/daemon.c > index 4a000ee4af..94a5b8a364 100644 > --- a/daemon.c > +++ b/daemon.c > @@ -232,13 +232,13 @@ static const char *path_ok(const char *directory, struct hostinfo *hi) > > rlen = strlcpy(interp_path, expanded_path.buf, > sizeof(interp_path)); > + strbuf_release(&expanded_path); > if (rlen >= sizeof(interp_path)) { > logerror("interpolated path too large: %s", > interp_path); > return NULL; > } > > - strbuf_release(&expanded_path); > loginfo("Interpolated dir '%s'", interp_path); A common mistake in these kinds of fixes is that the variable to be freed is used to generate an error message in the early-return path. Here we put "interp_path" in the message instead. That does mean the error message shows the truncated name rather than the full one. That may be a hidden feature, though. :) -Peff
diff --git a/daemon.c b/daemon.c index 4a000ee4af..94a5b8a364 100644 --- a/daemon.c +++ b/daemon.c @@ -232,13 +232,13 @@ static const char *path_ok(const char *directory, struct hostinfo *hi) rlen = strlcpy(interp_path, expanded_path.buf, sizeof(interp_path)); + strbuf_release(&expanded_path); if (rlen >= sizeof(interp_path)) { logerror("interpolated path too large: %s", interp_path); return NULL; } - strbuf_release(&expanded_path); loginfo("Interpolated dir '%s'", interp_path); dir = interp_path;
Release the strbuf containing the interpolated path after copying it to a stack buffer and before erroring out if it's too long. Signed-off-by: René Scharfe <l.s.r@web.de> --- daemon.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) -- 2.34.0