From 650ab3a0cf8ecec597974f64f62d81f59c4e3161 Mon Sep 17 00:00:00 2001 From: mohit84 Date: Mon, 23 Oct 2023 13:51:45 +0530 Subject: [PATCH] dht: fix asan use-after-free bug (#4242) 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 --- xlators/cluster/dht/src/dht-common.c | 45 ++++++++++++++-------------- 1 file changed, 23 insertions(+), 22 deletions(-) diff --git a/xlators/cluster/dht/src/dht-common.c b/xlators/cluster/dht/src/dht-common.c index 50458c1b569..59715ebc00b 100644 --- a/xlators/cluster/dht/src/dht-common.c +++ b/xlators/cluster/dht/src/dht-common.c @@ -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; } }