From 7a0e64d2e0e6a01673365a4062067a79ff690712 Mon Sep 17 00:00:00 2001 From: cjihrig Date: Fri, 25 Aug 2017 09:58:32 -0400 Subject: [PATCH] unix,windows: init all requests in fs calls MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Prior to this change, several of the fs functions checked for invalid arguments before initializing the fs request. If a consumer received a UV_EINVAL from one of these functions, and then called uv_fs_req_cleanup(), the application would crash, as the pointer being freed was not allocated. This commit makes sure that all fs functions initialize the request before returning. Fixes: https://github.com/libuv/libuv/issues/1508 PR-URL: https://github.com/libuv/libuv/pull/1509 Reviewed-By: Bartosz Sosnowski Reviewed-By: Saúl Ibarra Corretgé --- src/unix/fs.c | 9 ++++++--- src/win/fs.c | 17 +++++++++-------- test/test-fs-copyfile.c | 1 + test/test-fs.c | 12 ++++++++---- 4 files changed, 24 insertions(+), 15 deletions(-) diff --git a/src/unix/fs.c b/src/unix/fs.c index 296e9b15..5400d0c7 100644 --- a/src/unix/fs.c +++ b/src/unix/fs.c @@ -1287,10 +1287,11 @@ int uv_fs_read(uv_loop_t* loop, uv_fs_t* req, unsigned int nbufs, int64_t off, uv_fs_cb cb) { + INIT(READ); + if (bufs == NULL || nbufs == 0) return -EINVAL; - INIT(READ); req->file = file; req->nbufs = nbufs; @@ -1425,10 +1426,11 @@ int uv_fs_write(uv_loop_t* loop, unsigned int nbufs, int64_t off, uv_fs_cb cb) { + INIT(WRITE); + if (bufs == NULL || nbufs == 0) return -EINVAL; - INIT(WRITE); req->file = file; req->nbufs = nbufs; @@ -1476,10 +1478,11 @@ int uv_fs_copyfile(uv_loop_t* loop, const char* new_path, int flags, uv_fs_cb cb) { + INIT(COPYFILE); + if (flags & ~UV_FS_COPYFILE_EXCL) return -EINVAL; - INIT(COPYFILE); PATH2; req->flags = flags; POST; diff --git a/src/win/fs.c b/src/win/fs.c index dee34cd3..56706996 100644 --- a/src/win/fs.c +++ b/src/win/fs.c @@ -2010,11 +2010,11 @@ int uv_fs_read(uv_loop_t* loop, unsigned int nbufs, int64_t offset, uv_fs_cb cb) { + uv_fs_req_init(loop, req, UV_FS_READ, cb); + if (bufs == NULL || nbufs == 0) return UV_EINVAL; - uv_fs_req_init(loop, req, UV_FS_READ, cb); - req->file.fd = fd; req->fs.info.nbufs = nbufs; @@ -2046,11 +2046,11 @@ int uv_fs_write(uv_loop_t* loop, unsigned int nbufs, int64_t offset, uv_fs_cb cb) { + uv_fs_req_init(loop, req, UV_FS_WRITE, cb); + if (bufs == NULL || nbufs == 0) return UV_EINVAL; - uv_fs_req_init(loop, req, UV_FS_WRITE, cb); - req->file.fd = fd; req->fs.info.nbufs = nbufs; @@ -2251,12 +2251,12 @@ int uv_fs_realpath(uv_loop_t* loop, uv_fs_t* req, const char* path, uv_fs_cb cb) { int err; - if (!req || !path) { + uv_fs_req_init(loop, req, UV_FS_REALPATH, cb); + + if (!path) { return UV_EINVAL; } - uv_fs_req_init(loop, req, UV_FS_REALPATH, cb); - err = fs__capture_path(req, path, NULL, cb != NULL); if (err) { return uv_translate_sys_error(err); @@ -2435,10 +2435,11 @@ int uv_fs_copyfile(uv_loop_t* loop, uv_fs_cb cb) { int err; + uv_fs_req_init(loop, req, UV_FS_COPYFILE, cb); + if (flags & ~UV_FS_COPYFILE_EXCL) return UV_EINVAL; - uv_fs_req_init(loop, req, UV_FS_COPYFILE, cb); err = fs__capture_path(req, path, new_path, cb != NULL); if (err) diff --git a/test/test-fs-copyfile.c b/test/test-fs-copyfile.c index a973e86a..2d1f9079 100644 --- a/test/test-fs-copyfile.c +++ b/test/test-fs-copyfile.c @@ -99,6 +99,7 @@ TEST_IMPL(fs_copyfile) { /* Fails with EINVAL if bad flags are passed. */ r = uv_fs_copyfile(NULL, &req, src, dst, -1, NULL); ASSERT(r == UV_EINVAL); + uv_fs_req_cleanup(&req); /* Fails with ENOENT if source does not exist. */ unlink(src); diff --git a/test/test-fs.c b/test/test-fs.c index cf05a6c6..660d22cc 100644 --- a/test/test-fs.c +++ b/test/test-fs.c @@ -2751,19 +2751,23 @@ TEST_IMPL(fs_write_alotof_bufs_with_offset) { TEST_IMPL(fs_read_write_null_arguments) { int r; - r = uv_fs_read(NULL, NULL, 0, NULL, 0, -1, NULL); + r = uv_fs_read(NULL, &read_req, 0, NULL, 0, -1, NULL); ASSERT(r == UV_EINVAL); + uv_fs_req_cleanup(&read_req); - r = uv_fs_write(NULL, NULL, 0, NULL, 0, -1, NULL); + r = uv_fs_write(NULL, &write_req, 0, NULL, 0, -1, NULL); ASSERT(r == UV_EINVAL); + uv_fs_req_cleanup(&write_req); iov = uv_buf_init(NULL, 0); - r = uv_fs_read(NULL, NULL, 0, &iov, 0, -1, NULL); + r = uv_fs_read(NULL, &read_req, 0, &iov, 0, -1, NULL); ASSERT(r == UV_EINVAL); + uv_fs_req_cleanup(&read_req); iov = uv_buf_init(NULL, 0); - r = uv_fs_write(NULL, NULL, 0, &iov, 0, -1, NULL); + r = uv_fs_write(NULL, &write_req, 0, &iov, 0, -1, NULL); ASSERT(r == UV_EINVAL); + uv_fs_req_cleanup(&write_req); return 0; }