filter: Fix Segfault#6303
Conversation
45bc565 to
9c3edca
Compare
|
I'm currently just casting the input and output |
|
Instead of casting a |
Co-authored-by: Edward Thomson <ethomson@github.com>
|
I think that we might also need to swap back after invocation? Can you double-check me on this with your repro cases? commit fdc2f61adda5805232d120369f3d2e10e72cb14b
Author: Edward Thomson <ethomson@edwardthomson.com>
Date: Mon Jun 20 11:51:08 2022 -0400
filter: restore `git_str` values for legacy write
We need to swap the `git_buf` and `git_str` values both before and after
legacy filter invocation to ensure that we free the memory allocated
correctly.
diff --git a/src/libgit2/filter.c b/src/libgit2/filter.c
index 788ed9cb5..80a3cae67 100644
--- a/src/libgit2/filter.c
+++ b/src/libgit2/filter.c
@@ -893,6 +893,17 @@ static int buffered_stream_write(
return git_str_put(&buffered_stream->input, buffer, len);
}
+#ifndef GIT_DEPRECATE_HARD
+# define BUF_TO_STRUCT(b, s) \
+ (b)->ptr = (s)->ptr; \
+ (b)->size = (s)->size; \
+ (b)->reserved = (s)->asize;
+# define STRUCT_TO_BUF(s, b) \
+ (s)->ptr = (b)->ptr; \
+ (s)->size = (b)->size; \
+ (s)->asize = (b)->reserved;
+#endif
+
static int buffered_stream_close(git_writestream *s)
{
struct buffered_stream *buffered_stream = (struct buffered_stream *)s;
@@ -907,12 +918,8 @@ static int buffered_stream_close(git_writestream *s)
git_buf legacy_output = GIT_BUF_INIT,
legacy_input = GIT_BUF_INIT;
- legacy_output.ptr = buffered_stream->output->ptr;
- legacy_output.size = buffered_stream->output->size;
- legacy_output.reserved = buffered_stream->output->asize;
- legacy_input.ptr = buffered_stream->input.ptr;
- legacy_input.size = buffered_stream->input.size;
- legacy_input.reserved = buffered_stream->input.asize;
+ BUF_TO_STRUCT(&legacy_output, buffered_stream->output);
+ BUF_TO_STRUCT(&legacy_input, &buffered_stream->input);
error = buffered_stream->legacy_write_fn(
buffered_stream->filter,
@@ -920,6 +927,9 @@ static int buffered_stream_close(git_writestream *s)
&legacy_output,
&legacy_input,
buffered_stream->source);
+
+ STRUCT_TO_BUF(buffered_stream->output, &legacy_output);
+ STRUCT_TO_BUF(&buffered_stream->input, &legacy_input);
} else
#endif
error = buffered_stream->write_fn( |
f887fd6 to
e0a8b4e
Compare
|
@zawata LMK when this branch is 👍 and I'll merge it. We'll ship a new 1.4 once this lands. 🥳 |
|
sorry I've been fighting my system all morning 😅 you will need to copy back the buf contents. I didn't test it after your first suggestion and my inital PR worked because I was just casting so the git_str got updated anyways. I applied your patch and barring any ci failures(which I don't anticipate) then this should be good to go i think |
|
@ethomson I think that CI failure is unrelated?
|
Yep, that was my oversight. 😓
Definitely. Google Source has been... frustrating on many levels lately. Thanks for this PR @zawata ! |
Merge pull request libgit2#6303 from zawata/legacy_buffer_stream_segfault filter: Fix Segfault
Registering a git_filter without setting the
streamcallback causes the git_filter_list_apply_* functions(which eventually call filter.c:setup_stream) to call buffered_legacy_stream_new which sets the legacy_write_fn callback instead of the write_fn callback. but buffered_stream_close will only ever call write_fn callback...thus the segfault.This appears to have been introduced with the git_buf/git_str split.
Hopefully I've taken the right approach here? I ran this against the nodegit test suite(which hasn't implemented filter streaming yet) and it fixed the crashes I was experiencing.