Skip to content

Commit

Permalink
dht: fix asan use-after-free bug (#4242)
Browse files Browse the repository at this point in the history 
The client is throwing below stacktrace while asan is enabled.
The client is facing an issue while application is trying
to call removexattr in 2x1 subvol and non-mds subvol is down.
As we can see in below stacktrace dht_setxattr_mds_cbk is calling
dht_setxattr_non_mds_cbk and dht_setxattr_non_mds_cbk is trying to
wipe local because call_cnt is 0 but dht_setxattr_mds_cbk is trying
to access frame->local that;s why it is crashed.

x621000051c34 is located 1844 bytes inside of 4164-byte region [0x621000051500,0x621000052544)
freed by thread T7 here:
    #0 0x7f916ccb9388 in __interceptor_free.part.0 (/lib64/libasan.so.8+0xb9388)
    #1 0x7f91654af204 in dht_local_wipe /root/glusterfs_new/glusterfs/xlators/cluster/dht/src/dht-helper.c:713
    #2 0x7f91654af204 in dht_setxattr_non_mds_cbk /root/glusterfs_new/glusterfs/xlators/cluster/dht/src/dht-common.c:3900
    #3 0x7f91694c1f42 in client4_0_removexattr_cbk /root/glusterfs_new/glusterfs/xlators/protocol/client/src/client-rpc-fops_v2.c:1061
    #4 0x7f91694ba26f in client_submit_request /root/glusterfs_new/glusterfs/xlators/protocol/client/src/client.c:288
    #5 0x7f91695021bd in client4_0_removexattr /root/glusterfs_new/glusterfs/xlators/protocol/client/src/client-rpc-fops_v2.c:4480
    #6 0x7f91694a5f56 in client_removexattr /root/glusterfs_new/glusterfs/xlators/protocol/client/src/client.c:1439
    #7 0x7f91654a1161 in dht_setxattr_mds_cbk /root/glusterfs_new/glusterfs/xlators/cluster/dht/src/dht-common.c:3979
    #8 0x7f91694c1f42 in client4_0_removexattr_cbk /root/glusterfs_new/glusterfs/xlators/protocol/client/src/client-rpc-fops_v2.c:1061
    #9 0x7f916cbc4340 in rpc_clnt_handle_reply /root/glusterfs_new/glusterfs/rpc/rpc-lib/src/rpc-clnt.c:723
    #10 0x7f916cbc4340 in rpc_clnt_notify /root/glusterfs_new/glusterfs/rpc/rpc-lib/src/rpc-clnt.c:890
    #11 0x7f916cbb7ec5 in rpc_transport_notify /root/glusterfs_new/glusterfs/rpc/rpc-lib/src/rpc-transport.c:504
    #12 0x7f916a1aa5fa in socket_event_poll_in_async /root/glusterfs_new/glusterfs/rpc/rpc-transport/socket/src/socket.c:2358
    #13 0x7f916a1bd7c2 in gf_async ../../../../libglusterfs/src/glusterfs/async.h:187
    #14 0x7f916a1bd7c2 in socket_event_poll_in /root/glusterfs_new/glusterfs/rpc/rpc-transport/socket/src/socket.c:2399
    #15 0x7f916a1bd7c2 in socket_event_handler /root/glusterfs_new/glusterfs/rpc/rpc-transport/socket/src/socket.c:2790
    #16 0x7f916a1bd7c2 in socket_event_handler /root/glusterfs_new/glusterfs/rpc/rpc-transport/socket/src/socket.c:2710
    #17 0x7f916c946d22 in event_dispatch_epoll_handler /root/glusterfs_new/glusterfs/libglusterfs/src/event-epoll.c:614
    #18 0x7f916c946d22 in event_dispatch_epoll_worker /root/glusterfs_new/glusterfs/libglusterfs/src/event-epoll.c:725
    #19 0x7f916be8cdec in start_thread (/lib64/libc.so.6+0x8cdec)

Solution: Use switch instead of using if statement to wind a operation, in case of switch
          the code will not try to access local after wind a operation for last dht
          subvol.

Fixes: #3732
Change-Id: I031bc814d6df98058430ef4de7040e3370d1c677

Signed-off-by: Mohit Agrawal <[email protected]>
  • Loading branch information
@mohit84
mohit84 authored Oct 23, 2023
1 parent c22bf0f commit 650ab3a
Showing 1 changed file with 23 additions and 22 deletions.
45 changes: 23 additions & 22 deletions xlators/cluster/dht/src/dht-common.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3963,28 +3963,29 @@ dht_setxattr_mds_cbk(call_frame_t *frame, void *cookie, xlator_t *this,
for (i = 0; i < conf->subvolume_cnt; i++) {
if (mds_subvol && (mds_subvol == conf->subvolumes[i]))
continue;
if (local->fop == GF_FOP_SETXATTR) {
STACK_WIND(frame, dht_setxattr_non_mds_cbk, conf->subvolumes[i],
conf->subvolumes[i]->fops->setxattr, &local->loc,
local->xattr, local->flags, local->xattr_req);
}

if (local->fop == GF_FOP_FSETXATTR) {
STACK_WIND(frame, dht_setxattr_non_mds_cbk, conf->subvolumes[i],
conf->subvolumes[i]->fops->fsetxattr, local->fd,
local->xattr, local->flags, local->xattr_req);
}

if (local->fop == GF_FOP_REMOVEXATTR) {
STACK_WIND(frame, dht_setxattr_non_mds_cbk, conf->subvolumes[i],
conf->subvolumes[i]->fops->removexattr, &local->loc,
local->key, local->xattr_req);
}

if (local->fop == GF_FOP_FREMOVEXATTR) {
STACK_WIND(frame, dht_setxattr_non_mds_cbk, conf->subvolumes[i],
conf->subvolumes[i]->fops->fremovexattr, local->fd,
local->key, local->xattr_req);
switch (local->fop) {
case GF_FOP_SETXATTR:
STACK_WIND(frame, dht_setxattr_non_mds_cbk, conf->subvolumes[i],
conf->subvolumes[i]->fops->setxattr, &local->loc,
local->xattr, local->flags, local->xattr_req);
break;
case GF_FOP_FSETXATTR:
STACK_WIND(frame, dht_setxattr_non_mds_cbk, conf->subvolumes[i],
conf->subvolumes[i]->fops->fsetxattr, local->fd,
local->xattr, local->flags, local->xattr_req);
break;
case GF_FOP_REMOVEXATTR:
STACK_WIND(frame, dht_setxattr_non_mds_cbk, conf->subvolumes[i],
conf->subvolumes[i]->fops->removexattr, &local->loc,
local->key, local->xattr_req);
break;
case GF_FOP_FREMOVEXATTR:
STACK_WIND(frame, dht_setxattr_non_mds_cbk, conf->subvolumes[i],
conf->subvolumes[i]->fops->fremovexattr, local->fd,
local->key, local->xattr_req);
break;
default:
break;
}
}

Expand Down

0 comments on commit 650ab3a

Please sign in to comment.