Skip to content

Commit

Permalink
Fix leak in evbuffer_add_file() on empty files
Browse files Browse the repository at this point in the history
Found by oss-fuzz, after coverage had been improved in google/oss-fuzz#11257
v2: adjust test
  • Loading branch information
azat committed Mar 11, 2024
1 parent b9e1fe7 commit caf57bb
Show file tree
Hide file tree
Showing 3 changed files with 39 additions and 4 deletions.
3 changes: 1 addition & 2 deletions buffer.c
Original file line number Diff line number Diff line change
Expand Up @@ -3266,8 +3266,7 @@ evbuffer_add_file_segment(struct evbuffer *buf,
} else {
if (evbuffer_file_segment_materialize(seg)<0) {
EVLOCK_UNLOCK(seg->lock, 0);
EVBUFFER_UNLOCK(buf);
return -1;
goto err;
}
}
EVLOCK_UNLOCK(seg->lock, 0);
Expand Down
4 changes: 2 additions & 2 deletions include/event2/buffer.h
Original file line number Diff line number Diff line change
Expand Up @@ -555,8 +555,8 @@ int evbuffer_add_reference_with_offset(struct evbuffer *outbuf, const void *data
flag is set, it uses those functions. Otherwise, it tries to use
mmap (or CreateFileMapping on Windows).
The function owns the resulting file descriptor and will close it
when finished transferring data.
The function owns the resulting file descriptor and will close (even in case
of error) it when finished transferring data.
The results of using evbuffer_remove() or evbuffer_pullup() on
evbuffers whose data was added using this function are undefined.
Expand Down
36 changes: 36 additions & 0 deletions test/regress_buffer.c
Original file line number Diff line number Diff line change
Expand Up @@ -1107,6 +1107,41 @@ addfile_test_readcb(evutil_socket_t fd, short what, void *arg)
}
}

static void
test_evbuffer_add_file_leak1(void *ptr)
{
struct basic_test_data *testdata = ptr;
struct evbuffer *buf = NULL;
char *tmpfilename = NULL;
int fd;

(void)testdata;

fd = regress_make_tmpfile("", 0, &tmpfilename);
/* On Windows, if TMP environment variable is corrupted, we may not be
* able create temporary file, just skip it */
if (fd < 0)
tt_skip();

/* close fd before usage, so that the fallback with pread() will fail (in
* evbuffer_file_segment_materialize()) */
close(fd);

/* mmap(offset=0, length=0) will fail, this is enough */
buf = evbuffer_new();
tt_assert(evbuffer_add_file(buf, fd, 0, 0) == -1);
evbuffer_validate(buf);

end:
if (tmpfilename) {
unlink(tmpfilename);
free(tmpfilename);
}
if (buf)
evbuffer_free(buf);
/* NOTE: file will be closed in evbuffer_add_file() */
}

static void
test_evbuffer_add_file(void *ptr)
{
Expand Down Expand Up @@ -2922,6 +2957,7 @@ struct testcase_t evbuffer_testcases[] = {
{ "copyout", test_evbuffer_copyout, 0, NULL, NULL},
{ "file_segment_add_cleanup_cb", test_evbuffer_file_segment_add_cleanup_cb, 0, NULL, NULL },
{ "pullup_with_empty", test_evbuffer_pullup_with_empty, 0, NULL, NULL },
{ "add_file_leak1", test_evbuffer_add_file_leak1, 0, NULL, NULL },

#define ADDFILE_TEST(name, parameters) \
{ name, test_evbuffer_add_file, TT_FORK|TT_NEED_BASE, \
Expand Down

0 comments on commit caf57bb

Please sign in to comment.