From patchwork Tue Jul 26 14:19:05 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Attila Kovacs X-Patchwork-Id: 12929335 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 6C180C00140 for ; Tue, 26 Jul 2022 14:20:15 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230223AbiGZOUN (ORCPT ); Tue, 26 Jul 2022 10:20:13 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:58540 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S236011AbiGZOUL (ORCPT ); Tue, 26 Jul 2022 10:20:11 -0400 Received: from mail-qt1-x834.google.com (mail-qt1-x834.google.com [IPv6:2607:f8b0:4864:20::834]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 06519F2C for ; Tue, 26 Jul 2022 07:20:10 -0700 (PDT) Received: by mail-qt1-x834.google.com with SMTP id x11so10471530qts.13 for ; Tue, 26 Jul 2022 07:20:09 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=cfa.harvard.edu; s=google; h=from:to:cc:subject:date:message-id:mime-version :content-transfer-encoding; bh=DBgOCZVjCfLSrGPma3nuECbYEVoJ2GP0xe2GTjQBAmA=; b=BwzXhWoXAPSBeZeN+d+UZn/UKLSsIZKk/LEqzsV8TDgp1nwsQWT/ZsSn54cI40WcjA 80+E4ZYtKV8vb4KfiFTbrdEXTW1PSvUrnysFUDBT6fl8JrL/YZUxMajewoCvG/SGuBkf sbKWSKoeTSGjVVBNPbnfbCViLRbz6EBClJ0J5+8b7zwXl7Oz6q5uDlW2VoAzDdRddh8P BgwZsoFH+yAidGb+3joImUBsW6a3ZR+yTfsmtw8PAFqkx2OrBjfh80/yCq8/bpfld+bn C9mVDC9thsrViJpBDoughaHTq2thRnQn0XWvcgSHfsFCWYQx1wXF/QiTzc/zl4xgXFdu 2IwA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:from:to:cc:subject:date:message-id:mime-version :content-transfer-encoding; bh=DBgOCZVjCfLSrGPma3nuECbYEVoJ2GP0xe2GTjQBAmA=; b=3TEO6k5Y/T4TfiWu5ux+8WWisD0OkkyQ3b1pdb3xG//K5wB7n5M8h9sgrJKyrHJcH7 +NmTNhgr1qw7Sn1xGx8x9W/ZvUFbrGAs0FXgnf9hUdW7pM64VySXPqSqOJa6QJfVWae2 fGvDm/tEQSEfq9VaZ3zC+WDOvVX6K1PDFrQJEn38N4GB1HVkpHHrq1uTtpMBmWMdNGR/ U6/JeuDtm/wQc15ufd4x4zP1Bq9jGGUxsFdMAkdLo6g2k3/oL/IsbKm9/d0NECiIAfuo WKc10CWG8+PX/NE9wLZd6yxgc7TiSzW8NCB+1KcJK1LK6+k9jg8a3NgdGg48QMYQe+Nm QzWg== X-Gm-Message-State: AJIora9C44bMO1yD+baNdwQHFuIfy/ILRqiwQYQBqCVXbOPfpET8gzSr Jp3FAcRfCgYUCXL2dVOOUzh47g== X-Google-Smtp-Source: AGRyM1vEXPaVlHt/FA7+4qgfoqonewzKekB/fbVVdAnNj3c1/9bDnz/bYOu5B/aaEKg+kK+G+/GBsw== X-Received: by 2002:a05:622a:1050:b0:31e:f648:4e24 with SMTP id f16-20020a05622a105000b0031ef6484e24mr14475722qte.484.1658845208792; Tue, 26 Jul 2022 07:20:08 -0700 (PDT) Received: from pihe (dhcp-131-142-152-103.cfa.harvard.edu. [131.142.152.103]) by smtp.gmail.com with ESMTPSA id h11-20020a05620a400b00b006b60c965024sm11674579qko.113.2022.07.26.07.20.07 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 26 Jul 2022 07:20:08 -0700 (PDT) Received: from pihe (localhost [127.0.0.1]) by pihe (8.17.1/8.17.1) with ESMTPS id 26QEK73J069095 (version=TLSv1.3 cipher=TLS_AES_256_GCM_SHA384 bits=256 verify=NOT); Tue, 26 Jul 2022 10:20:07 -0400 Received: (from pumukli@localhost) by pihe (8.17.1/8.17.1/Submit) id 26QEK6S7069033; Tue, 26 Jul 2022 10:20:06 -0400 From: Attila Kovacs To: Libtirpc-devel Mailing List Cc: Linux NFS Mailing list Subject: [PATCH 1/2] clnt_dg_freeres() uncleared set active state may deadlock. Date: Tue, 26 Jul 2022 10:19:05 -0400 Message-Id: <20220726141906.69023-1-attila.kovacs@cfa.harvard.edu> X-Mailer: git-send-email 2.37.1 MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-nfs@vger.kernel.org From: Attila Kovacs In clnt_dg.c in clnt_dg_freeres(), cu_fd_lock->active is set to TRUE, with no corresponding clearing when the operation (*xdr_res() call) is completed. This would leave other waiting operations blocked indefinitely, effectively deadlocking the client. For comparison, clnt_vd_freeres() in clnt_vc.c does not set the active state to TRUE. I believe the vc behavior is correct, while the dg behavior is a bug. Signed-off-by: Attila Kovacs --- src/clnt_dg.c | 1 - 1 file changed, 1 deletion(-) diff --git a/src/clnt_dg.c b/src/clnt_dg.c index 7c5d22e..b2043ac 100644 --- a/src/clnt_dg.c +++ b/src/clnt_dg.c @@ -573,7 +573,6 @@ clnt_dg_freeres(cl, xdr_res, res_ptr) mutex_lock(&clnt_fd_lock); while (cu->cu_fd_lock->active) cond_wait(&cu->cu_fd_lock->cv, &clnt_fd_lock); - cu->cu_fd_lock->active = TRUE; xdrs->x_op = XDR_FREE; dummy = (*xdr_res)(xdrs, res_ptr); thr_sigsetmask(SIG_SETMASK, &mask, NULL); From patchwork Tue Jul 26 14:19:06 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Attila Kovacs X-Patchwork-Id: 12929336 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 4445CC00140 for ; Tue, 26 Jul 2022 14:21:13 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S238863AbiGZOVM (ORCPT ); Tue, 26 Jul 2022 10:21:12 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:59356 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S235886AbiGZOVK (ORCPT ); Tue, 26 Jul 2022 10:21:10 -0400 Received: from mail-qv1-xf32.google.com (mail-qv1-xf32.google.com [IPv6:2607:f8b0:4864:20::f32]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id DD04415FFC for ; Tue, 26 Jul 2022 07:21:09 -0700 (PDT) Received: by mail-qv1-xf32.google.com with SMTP id i13so10701655qvo.3 for ; Tue, 26 Jul 2022 07:21:09 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=cfa.harvard.edu; s=google; h=from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=XUev4aVqJ5kVJbWbmM/RDsUFZ/tOjyhFiv32RH06njk=; b=AeTi8kiIURrOjxL10vvu4A0RfLLpLnN33JH5nkxhk9tRKUn0Gnx75oSBAIo1ir3qT0 GO2YsJcOir+suz56U/Ojn3r9HgP8eiT5n1gk/opNBT/7w4kiYQBbkb6GqSUM0Hf61AUU oQD+XnODoAiJ7idrZ7VYDe1d9syoHPtmuSQDFiVSJY6ClRt2cRu62Fli9p/nm2Rr9pMZ LMOZlPXpIjLDX83AwCZU7reAXdm3WQNb8Wc/yWABf1vTLsAi/glS8LfRvdy4h580cUdQ T2LnE3GY5Y2CeulVQ07geEgvRd+g3fXOXWKv0GJCeHBOBYUZ9FEjAMP5YUurxyCwkV5C EClA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=XUev4aVqJ5kVJbWbmM/RDsUFZ/tOjyhFiv32RH06njk=; b=AcGHHI4D7LtH56x8FYHo0AZrB+q+vBTT3LAriBoMRE6x3wn+SI9ESbLy+/KWLa6UOv KsygaFONFZ0q5d9giA5MCqE2/8KxJdhKmtsV/YWayvGbAURxnqJ6iWJ9jf37O8mzTMrr R5pYsvV2U5ivgXzsn/h4CnS/vh5YDiz8RzlldBKRirU4ROMKasqPvUiP5QFIKDj6cjXG jz4qbESofl9V3KdnrI2I5u8SNnWbQ6p7sQJoPG58VAryzJ6YP5AM6GsYK9pP5muuyws6 IiokZHwYhi4Aai70C40Uv/2KxmuGNettRNNPXzVlKeQn+xS3B7vx8JL9uvKcQRPVhHYb rcjQ== X-Gm-Message-State: AJIora8IyObJfMxxwWzymHyYfAv1Rtdp8nWT7rKstzUIX0uP5/dH6CRw jwIQKFxJZR8DVZhjk2XTX31vZi3ytPOX1g== X-Google-Smtp-Source: AGRyM1t1p4UvfIWlT5OPoMpJczH5HNz9HkPHYHm/HsDcICGS2nVhcNQ6bdNeyZYj3iPtbEBry/tXSw== X-Received: by 2002:a0c:dc0f:0:b0:474:1c35:1fbc with SMTP id s15-20020a0cdc0f000000b004741c351fbcmr15052651qvk.78.1658845268458; Tue, 26 Jul 2022 07:21:08 -0700 (PDT) Received: from pihe (dhcp-131-142-152-103.cfa.harvard.edu. [131.142.152.103]) by smtp.gmail.com with ESMTPSA id i67-20020a375446000000b006a6a6f148e6sm11263873qkb.17.2022.07.26.07.21.07 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 26 Jul 2022 07:21:08 -0700 (PDT) Received: from pihe (localhost [127.0.0.1]) by pihe (8.17.1/8.17.1) with ESMTPS id 26QEL74Q069151 (version=TLSv1.3 cipher=TLS_AES_256_GCM_SHA384 bits=256 verify=NOT); Tue, 26 Jul 2022 10:21:07 -0400 Received: (from pumukli@localhost) by pihe (8.17.1/8.17.1/Submit) id 26QEL7Od069101; Tue, 26 Jul 2022 10:21:07 -0400 From: Attila Kovacs To: Libtirpc-devel Mailing List Cc: Linux NFS Mailing list Subject: [PATCH 2/2] thread safe clnt destruction. Date: Tue, 26 Jul 2022 10:19:06 -0400 Message-Id: <20220726141906.69023-2-attila.kovacs@cfa.harvard.edu> X-Mailer: git-send-email 2.37.1 In-Reply-To: <20220726141906.69023-1-attila.kovacs@cfa.harvard.edu> References: <20220726141906.69023-1-attila.kovacs@cfa.harvard.edu> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-nfs@vger.kernel.org From: Attila Kovacs If clnt_dg_destroy() or clnt_vc_destroy() is awoken with other blocked operations pending (such as clnt_*_call(), clnt_*_control(), or clnt_*_freeres()) but no active operation currently being executed, then the client gets destroyed. Then, as the other blocked operations get subsequently awoken, they will try operate on an invalid client handle, potentially causing unpredictable behavior and stack corruption. Signed-off-by: Attila Kovacs --- src/clnt_dg.c | 13 ++++++++++++- src/clnt_fd_locks.h | 2 ++ src/clnt_vc.c | 13 ++++++++++++- 3 files changed, 26 insertions(+), 2 deletions(-) diff --git a/src/clnt_dg.c b/src/clnt_dg.c index b2043ac..166af63 100644 --- a/src/clnt_dg.c +++ b/src/clnt_dg.c @@ -101,6 +101,7 @@ extern mutex_t clnt_fd_lock; #define release_fd_lock(fd_lock, mask) { \ mutex_lock(&clnt_fd_lock); \ fd_lock->active = FALSE; \ + fd_lock->pending--; \ thr_sigsetmask(SIG_SETMASK, &(mask), NULL); \ cond_signal(&fd_lock->cv); \ mutex_unlock(&clnt_fd_lock); \ @@ -311,6 +312,7 @@ clnt_dg_call(cl, proc, xargs, argsp, xresults, resultsp, utimeout) sigfillset(&newmask); thr_sigsetmask(SIG_SETMASK, &newmask, &mask); mutex_lock(&clnt_fd_lock); + cu->cu_fd_lock->pending++; while (cu->cu_fd_lock->active) cond_wait(&cu->cu_fd_lock->cv, &clnt_fd_lock); cu->cu_fd_lock->active = TRUE; @@ -571,10 +573,12 @@ clnt_dg_freeres(cl, xdr_res, res_ptr) sigfillset(&newmask); thr_sigsetmask(SIG_SETMASK, &newmask, &mask); mutex_lock(&clnt_fd_lock); + cu->cu_fd_lock->pending++; while (cu->cu_fd_lock->active) cond_wait(&cu->cu_fd_lock->cv, &clnt_fd_lock); xdrs->x_op = XDR_FREE; dummy = (*xdr_res)(xdrs, res_ptr); + cu->cu_fd_lock->pending--; thr_sigsetmask(SIG_SETMASK, &mask, NULL); cond_signal(&cu->cu_fd_lock->cv); mutex_unlock(&clnt_fd_lock); @@ -602,6 +606,7 @@ clnt_dg_control(cl, request, info) sigfillset(&newmask); thr_sigsetmask(SIG_SETMASK, &newmask, &mask); mutex_lock(&clnt_fd_lock); + cu->cu_fd_lock->pending++; while (cu->cu_fd_lock->active) cond_wait(&cu->cu_fd_lock->cv, &clnt_fd_lock); cu->cu_fd_lock->active = TRUE; @@ -742,8 +747,14 @@ clnt_dg_destroy(cl) sigfillset(&newmask); thr_sigsetmask(SIG_SETMASK, &newmask, &mask); mutex_lock(&clnt_fd_lock); - while (cu_fd_lock->active) + /* wait until all pending operations on client are completed. */ + while (cu_fd_lock->pending > 0) { + /* If a blocked operation can be awakened, then do it. */ + if (cu_fd_lock->active == FALSE) + cond_signal(&cu_fd_lock->cv); + /* keep waiting... */ cond_wait(&cu_fd_lock->cv, &clnt_fd_lock); + } if (cu->cu_closeit) (void)close(cu_fd); XDR_DESTROY(&(cu->cu_outxdrs)); diff --git a/src/clnt_fd_locks.h b/src/clnt_fd_locks.h index 359f995..6ba62cb 100644 --- a/src/clnt_fd_locks.h +++ b/src/clnt_fd_locks.h @@ -50,6 +50,7 @@ static unsigned int fd_locks_prealloc = 0; /* per-fd lock */ struct fd_lock_t { bool_t active; + int pending; /* Number of pending operations on fd */ cond_t cv; }; typedef struct fd_lock_t fd_lock_t; @@ -180,6 +181,7 @@ fd_lock_t* fd_lock_create(int fd, fd_locks_t *fd_locks) { item->fd = fd; item->refs = 1; item->fd_lock.active = FALSE; + item->fd_lock.pending = 0; cond_init(&item->fd_lock.cv, 0, (void *) 0); TAILQ_INSERT_HEAD(list, item, link); } else { diff --git a/src/clnt_vc.c b/src/clnt_vc.c index 3c73e65..5bbc78b 100644 --- a/src/clnt_vc.c +++ b/src/clnt_vc.c @@ -153,6 +153,7 @@ extern mutex_t clnt_fd_lock; #define release_fd_lock(fd_lock, mask) { \ mutex_lock(&clnt_fd_lock); \ fd_lock->active = FALSE; \ + fd_lock->pending--; \ thr_sigsetmask(SIG_SETMASK, &(mask), (sigset_t *) NULL); \ cond_signal(&fd_lock->cv); \ mutex_unlock(&clnt_fd_lock); \ @@ -357,6 +358,7 @@ clnt_vc_call(cl, proc, xdr_args, args_ptr, xdr_results, results_ptr, timeout) sigfillset(&newmask); thr_sigsetmask(SIG_SETMASK, &newmask, &mask); mutex_lock(&clnt_fd_lock); + ct->ct_fd_lock->pending++; while (ct->ct_fd_lock->active) cond_wait(&ct->ct_fd_lock->cv, &clnt_fd_lock); ct->ct_fd_lock->active = TRUE; @@ -495,10 +497,12 @@ clnt_vc_freeres(cl, xdr_res, res_ptr) sigfillset(&newmask); thr_sigsetmask(SIG_SETMASK, &newmask, &mask); mutex_lock(&clnt_fd_lock); + ct->ct_fd_lock->pending++; while (ct->ct_fd_lock->active) cond_wait(&ct->ct_fd_lock->cv, &clnt_fd_lock); xdrs->x_op = XDR_FREE; dummy = (*xdr_res)(xdrs, res_ptr); + ct->ct_fd_lock->pending--; thr_sigsetmask(SIG_SETMASK, &(mask), NULL); cond_signal(&ct->ct_fd_lock->cv); mutex_unlock(&clnt_fd_lock); @@ -533,6 +537,7 @@ clnt_vc_control(cl, request, info) sigfillset(&newmask); thr_sigsetmask(SIG_SETMASK, &newmask, &mask); mutex_lock(&clnt_fd_lock); + ct->ct_fd_lock->pending++; while (ct->ct_fd_lock->active) cond_wait(&ct->ct_fd_lock->cv, &clnt_fd_lock); ct->ct_fd_lock->active = TRUE; @@ -655,8 +660,14 @@ clnt_vc_destroy(cl) sigfillset(&newmask); thr_sigsetmask(SIG_SETMASK, &newmask, &mask); mutex_lock(&clnt_fd_lock); - while (ct_fd_lock->active) + /* wait until all pending operations on client are completed. */ + while (ct_fd_lock->pending > 0) { + /* If a blocked operation can be awakened, then do it. */ + if (ct_fd_lock->active == FALSE) + cond_signal(&ct_fd_lock->cv); + /* keep waiting... */ cond_wait(&ct_fd_lock->cv, &clnt_fd_lock); + } if (ct->ct_closeit && ct->ct_fd != -1) { (void)close(ct->ct_fd); }