diff options
author | Pranith Kumar K <pkarampu@redhat.com> | 2020-04-02 15:30:28 +0530 |
---|---|---|
committer | Xavi Hernandez <xhernandez@redhat.com> | 2020-04-03 07:46:44 +0000 |
commit | f55c45537435cc2a1412c80a6c922db63ce65965 (patch) | |
tree | 03e06de94a733d689bf2627a22e1aa556625ad6c | |
parent | a7d7ec066e56ac03bf252c26beb20fdc2c3b6772 (diff) |
features/utime: Don't access frame after stack-wind
Problem:
frame is accessed after stack-wind. This can lead to crash
if the cbk frees the frame.
Fix:
Use new frame for the wind instead.
Updates: #832
Change-Id: I64754609f1114b0bbd4d1336fa81a56f2cca6e03
Signed-off-by: Pranith Kumar K <pkarampu@redhat.com>
-rwxr-xr-x | tests/bugs/ctime/issue-832.t | 32 | ||||
-rw-r--r-- | xlators/features/utime/src/utime.c | 35 |
2 files changed, 52 insertions, 15 deletions
diff --git a/tests/bugs/ctime/issue-832.t b/tests/bugs/ctime/issue-832.t new file mode 100755 index 00000000000..740f731ab73 --- /dev/null +++ b/tests/bugs/ctime/issue-832.t @@ -0,0 +1,32 @@ +#!/bin/bash + +. $(dirname $0)/../../include.rc +. $(dirname $0)/../../volume.rc +. $(dirname $0)/../../traps.rc + +#Trigger trusted.glusterfs.mdata setting codepath and see things work as expected +cleanup + +TEST_USER=test-ctime-user +TEST_UID=27341 + +TEST useradd -o -M -u ${TEST_UID} ${TEST_USER} +push_trapfunc "userdel --force ${TEST_USER}" + +TEST glusterd +TEST pidof glusterd + +TEST $CLI volume create $V0 $H0:$B0/$V0 +TEST $CLI volume start $V0 + +$GFS --volfile-id=/$V0 --volfile-server=$H0 $M0; +echo abc > $M0/test +TEST chmod 755 $M0/ +TEST chmod 744 $M0/test +TEST setfattr -x trusted.glusterfs.mdata $B0/$V0/test +EXPECT_WITHIN $UMOUNT_TIMEOUT "Y" force_umount $M0 +$GFS --volfile-id=/$V0 --volfile-server=$H0 $M0; +su ${TEST_USER} -c "cat $M0/test" +TEST getfattr -n trusted.glusterfs.mdata $B0/$V0/test + +cleanup diff --git a/xlators/features/utime/src/utime.c b/xlators/features/utime/src/utime.c index eb6ee6f76f4..2acc63e6a05 100644 --- a/xlators/features/utime/src/utime.c +++ b/xlators/features/utime/src/utime.c @@ -147,6 +147,7 @@ gf_utime_set_mdata_setxattr_cbk(call_frame_t *frame, void *cookie, } frame->local = NULL; call_resume(stub); + STACK_DESTROY(frame->root); return 0; } @@ -162,6 +163,7 @@ gf_utime_set_mdata_lookup_cbk(call_frame_t *frame, void *cookie, xlator_t *this, loc_t loc = { 0, }; + call_frame_t *new_frame = NULL; if (!op_ret && dict_get(xdata, GF_XATTR_MDATA_KEY) == NULL) { dict = dict_new(); @@ -181,29 +183,32 @@ gf_utime_set_mdata_lookup_cbk(call_frame_t *frame, void *cookie, xlator_t *this, "dict set of key for set-ctime-mdata failed"); goto err; } - frame->local = fop_lookup_cbk_stub(frame, default_lookup_cbk, op_ret, - op_errno, inode, stbuf, xdata, - postparent); - if (!frame->local) { + new_frame = copy_frame(frame); + if (!new_frame) { + op_errno = ENOMEM; + goto stub_err; + } + + new_frame->local = fop_lookup_cbk_stub(frame, default_lookup_cbk, + op_ret, op_errno, inode, stbuf, + xdata, postparent); + if (!new_frame->local) { gf_msg(this->name, GF_LOG_WARNING, ENOMEM, UTIME_MSG_NO_MEMORY, "lookup_cbk stub allocation failed"); + op_errno = ENOMEM; + STACK_DESTROY(new_frame->root); goto stub_err; } loc.inode = inode_ref(inode); gf_uuid_copy(loc.gfid, stbuf->ia_gfid); - pid_t pid = frame->root->pid; - uid_t uid = frame->root->uid; - gid_t gid = frame->root->gid; - frame->root->uid = 0; - frame->root->gid = 0; - frame->root->pid = GF_CLIENT_PID_SET_UTIME; - STACK_WIND(frame, gf_utime_set_mdata_setxattr_cbk, FIRST_CHILD(this), - FIRST_CHILD(this)->fops->setxattr, &loc, dict, 0, NULL); - frame->root->uid = uid; - frame->root->gid = gid; - frame->root->pid = pid; + new_frame->root->uid = 0; + new_frame->root->gid = 0; + new_frame->root->pid = GF_CLIENT_PID_SET_UTIME; + STACK_WIND(new_frame, gf_utime_set_mdata_setxattr_cbk, + FIRST_CHILD(this), FIRST_CHILD(this)->fops->setxattr, &loc, + dict, 0, NULL); dict_unref(dict); inode_unref(loc.inode); |