From 7656aec3b9ef60592c8cf251dfb5cdb6088cd328 Mon Sep 17 00:00:00 2001 From: Dmitry Antipov Date: Thu, 26 Dec 2019 15:25:35 +0300 Subject: Avoid buffer overwrite due to uuid_utoa() misuse Code like: f(..., uuid_utoa(x), uuid_utoa(y)); is not valid (causes undefined behaviour) because uuid_utoa() uses the only static thread-local buffer which will be overwritten by the subsequent call. All such cases should be converted to use uuid_utoa_r() with explicitly specified buffer. Change-Id: I5e72bab806d96a9dd1707c28ed69ca033b9c8d6c Updates: bz#1193929 Signed-off-by: Dmitry Antipov --- xlators/cluster/afr/src/afr-self-heal-entry.c | 5 ++-- xlators/mount/fuse/src/fuse-bridge.c | 36 ++++++++++++++++---------- xlators/storage/posix/src/posix-inode-fd-ops.c | 5 ++-- xlators/storage/posix/src/posix-metadata.c | 7 +++-- 4 files changed, 33 insertions(+), 20 deletions(-) (limited to 'xlators') diff --git a/xlators/cluster/afr/src/afr-self-heal-entry.c b/xlators/cluster/afr/src/afr-self-heal-entry.c index 3ce882e8c4a..e7062289c79 100644 --- a/xlators/cluster/afr/src/afr-self-heal-entry.c +++ b/xlators/cluster/afr/src/afr-self-heal-entry.c @@ -86,6 +86,7 @@ afr_selfheal_recreate_entry(call_frame_t *frame, int dst, int source, 0, }; unsigned char *newentry = NULL; + char dir_uuid_str[64] = {0}, iatt_uuid_str[64] = {0}; priv = this->private; iatt = &replies[source].poststat; @@ -93,8 +94,8 @@ afr_selfheal_recreate_entry(call_frame_t *frame, int dst, int source, gf_msg(this->name, GF_LOG_ERROR, 0, AFR_MSG_SELF_HEAL_FAILED, "Invalid ia_type (%d) or gfid(%s). source brick=%d, " "pargfid=%s, name=%s", - iatt->ia_type, uuid_utoa(iatt->ia_gfid), source, - uuid_utoa(dir->gfid), name); + iatt->ia_type, uuid_utoa_r(iatt->ia_gfid, iatt_uuid_str), source, + uuid_utoa_r(dir->gfid, dir_uuid_str), name); ret = -EINVAL; goto out; } diff --git a/xlators/mount/fuse/src/fuse-bridge.c b/xlators/mount/fuse/src/fuse-bridge.c index 5bfaf2c2ac1..b41a0fcfa22 100644 --- a/xlators/mount/fuse/src/fuse-bridge.c +++ b/xlators/mount/fuse/src/fuse-bridge.c @@ -2354,21 +2354,26 @@ fuse_rename_cbk(call_frame_t *frame, void *cookie, xlator_t *this, { fuse_state_t *state = NULL; fuse_in_header_t *finh = NULL; + char loc_uuid_str[64] = {0}, loc2_uuid_str[64] = {0}; state = frame->root->state; finh = state->finh; - fuse_log_eh(this, - "op_ret: %d, op_errno: %d, %" PRIu64 - ": %s() " - "path: %s parent: %s ==> path: %s parent: %s" - "gfid: %s", - op_ret, op_errno, frame->root->unique, - gf_fop_list[frame->root->op], state->loc.path, - state->loc.parent ? uuid_utoa(state->loc.parent->gfid) : "", - state->loc2.path, - state->loc2.parent ? uuid_utoa(state->loc2.parent->gfid) : "", - state->loc.inode ? uuid_utoa(state->loc.inode->gfid) : ""); + fuse_log_eh( + this, + "op_ret: %d, op_errno: %d, %" PRIu64 + ": %s() " + "path: %s parent: %s ==> path: %s parent: %s" + "gfid: %s", + op_ret, op_errno, frame->root->unique, gf_fop_list[frame->root->op], + state->loc.path, + (state->loc.parent ? uuid_utoa_r(state->loc.parent->gfid, loc_uuid_str) + : ""), + state->loc2.path, + (state->loc2.parent + ? uuid_utoa_r(state->loc2.parent->gfid, loc2_uuid_str) + : ""), + state->loc.inode ? uuid_utoa(state->loc.inode->gfid) : ""); /* need to check for loc->parent to keep clang-scan happy. It gets dereferenced below, and is checked for NULL above. */ @@ -3101,15 +3106,18 @@ fuse_copy_file_range_cbk(call_frame_t *frame, void *cookie, xlator_t *this, void fuse_copy_file_range_resume(fuse_state_t *state) { + char fd_uuid_str[64] = {0}, fd_dst_uuid_str[64] = {0}; + gf_log("glusterfs-fuse", GF_LOG_TRACE, "%" PRIu64 ": COPY_FILE_RANGE " "(input fd: %p (gfid: %s), " "output fd: %p (gfid: %s) size=%zu, " "offset_in=%" PRIu64 ", offset_out=%" PRIu64 ")", - state->finh->unique, state->fd, uuid_utoa(state->fd->inode->gfid), - state->fd_dst, uuid_utoa(state->fd_dst->inode->gfid), state->size, - state->off_in, state->off_out); + state->finh->unique, state->fd, + uuid_utoa_r(state->fd->inode->gfid, fd_uuid_str), state->fd_dst, + uuid_utoa_r(state->fd_dst->inode->gfid, fd_dst_uuid_str), + state->size, state->off_in, state->off_out); FUSE_FOP(state, fuse_copy_file_range_cbk, GF_FOP_COPY_FILE_RANGE, copy_file_range, state->fd, state->off_in, state->fd_dst, diff --git a/xlators/storage/posix/src/posix-inode-fd-ops.c b/xlators/storage/posix/src/posix-inode-fd-ops.c index b8104604ecc..e5c51eee032 100644 --- a/xlators/storage/posix/src/posix-inode-fd-ops.c +++ b/xlators/storage/posix/src/posix-inode-fd-ops.c @@ -2061,6 +2061,7 @@ posix_copy_file_range(call_frame_t *frame, xlator_t *this, fd_t *fd_in, gf_boolean_t locked = _gf_false; gf_boolean_t update_atomic = _gf_false; posix_inode_ctx_t *ctx = NULL; + char in_uuid_str[64] = {0}, out_uuid_str[64] = {0}; VALIDATE_OR_GOTO(frame, out); VALIDATE_OR_GOTO(this, out); @@ -2201,8 +2202,8 @@ posix_copy_file_range(call_frame_t *frame, xlator_t *this, fd_t *fd_in, gf_msg(this->name, GF_LOG_ERROR, op_errno, P_MSG_COPY_FILE_RANGE_FAILED, "copy_file_range failed: fd_in: %p (gfid: %s) ," " fd_out %p (gfid:%s)", - fd_in, uuid_utoa(fd_in->inode->gfid), fd_out, - uuid_utoa(fd_out->inode->gfid)); + fd_in, uuid_utoa_r(fd_in->inode->gfid, in_uuid_str), fd_out, + uuid_utoa_r(fd_out->inode->gfid, out_uuid_str)); goto out; } diff --git a/xlators/storage/posix/src/posix-metadata.c b/xlators/storage/posix/src/posix-metadata.c index 63967cf3ade..872a693f513 100644 --- a/xlators/storage/posix/src/posix-metadata.c +++ b/xlators/storage/posix/src/posix-metadata.c @@ -820,6 +820,7 @@ posix_set_ctime_cfr(call_frame_t *frame, xlator_t *this, }; int ret = 0; struct posix_private *priv = NULL; + char in_uuid_str[64] = {0}, out_uuid_str[64] = {0}; priv = this->private; @@ -834,9 +835,11 @@ posix_set_ctime_cfr(call_frame_t *frame, xlator_t *this, "posix set mdata failed, No ctime : in: %s gfid_in:%s " "out: %s gfid_out:%s", real_path_in, - inode_in ? uuid_utoa(inode_in->gfid) : "No inode", + (inode_in ? uuid_utoa_r(inode_in->gfid, in_uuid_str) + : "No inode"), real_path_out, - inode_out ? uuid_utoa(inode_out->gfid) : "No inode"); + (inode_out ? uuid_utoa_r(inode_out->gfid, out_uuid_str) + : "No inode")); goto out; } -- cgit