From patchwork Thu Jul 20 18:23:21 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jeff Layton X-Patchwork-Id: 13320980 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 B61C1C001DC for ; Thu, 20 Jul 2023 18:23:41 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231405AbjGTSXj (ORCPT ); Thu, 20 Jul 2023 14:23:39 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:58336 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231383AbjGTSXh (ORCPT ); Thu, 20 Jul 2023 14:23:37 -0400 Received: from dfw.source.kernel.org (dfw.source.kernel.org [139.178.84.217]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 51612272A; Thu, 20 Jul 2023 11:23:34 -0700 (PDT) Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by dfw.source.kernel.org (Postfix) with ESMTPS id C5DB561BD6; Thu, 20 Jul 2023 18:23:33 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 774AEC433C9; Thu, 20 Jul 2023 18:23:32 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1689877413; bh=jKKeU03qB6wSaY/9QeYm5xXkIiRbAsA5WZOfWdz1d20=; h=From:Date:Subject:References:In-Reply-To:To:Cc:From; b=hdmDeHyOF6rP5yExA2dI53Zgug7c4I+C7VzuO/2/wvdlJxUg6eKfw6MWKux+kCKjr lucoK2ctTkb69VEFqUvFb2zLm3I4ut0luBjAIVxgdICnIvHXI3ilJZ0nstGbx3i3yJ gydyR1NamB1a55v9lyp/9Qov08Yda0bwZ2ZQKjeYyc4UTHTSI9eFdKkzRSH1YBFe7C QUM2NDoUf9XVAwsTHVRC5oKNTvJopcGoj3xLgDWAUFvmHfd1oG603mL2D53Tc60945 Gj4V9AIzFJfDEYWcHpnmjn6YL8/Hhxy7M7YpuKiTbsWsx9xgQUVhEUJs7LX3g3FMy3 bcTMJgDJhtwAw== From: Jeff Layton Date: Thu, 20 Jul 2023 14:23:21 -0400 Subject: [PATCH v2 2/2] nfsd: remove unsafe BUG_ON from set_change_info MIME-Version: 1.0 Message-Id: <20230720-bz2223560-v2-2-070aaf2660b7@kernel.org> References: <20230720-bz2223560-v2-0-070aaf2660b7@kernel.org> In-Reply-To: <20230720-bz2223560-v2-0-070aaf2660b7@kernel.org> To: Chuck Lever , Neil Brown , Olga Kornievskaia , Dai Ngo , Tom Talpey Cc: Boyang Xue , linux-nfs@vger.kernel.org, linux-kernel@vger.kernel.org, Jeff Layton X-Mailer: b4 0.12.3 X-Developer-Signature: v=1; a=openpgp-sha256; l=3438; i=jlayton@kernel.org; h=from:subject:message-id; bh=jKKeU03qB6wSaY/9QeYm5xXkIiRbAsA5WZOfWdz1d20=; b=owEBbQKS/ZANAwAIAQAOaEEZVoIVAcsmYgBkuXuiMsg2S8SgxdqtY3Xh7fzaNBidHZ3LVKQWw 8REgPndYKSJAjMEAAEIAB0WIQRLwNeyRHGyoYTq9dMADmhBGVaCFQUCZLl7ogAKCRAADmhBGVaC FVVqEADN0pNpC9pl2UcTsyIvtNwBpTIRk98fqUkcuK6JLmrwjtjFFWpaxrMcCqaPKqpae68reAf q5gInG7ZzJnDdcBmsWAbb+YfzuZw5vuL1KPYXFDsSkQ+I7ftIggjJu8bp8hC5Zb7fnG8UEBk2bV ljKhKSja5qIKo8Pf28vEPbj6UEX2/LGHC8XxNcrcOrLgsg3TBdvuv2PCHgz6Oasnimn/Q7pqIaU oB7ErmpPrBqS/cJxgxK/6JhYV/Bnhxq5qecp1lZ/e0sFkmirftqXzVbX9Ehm6zC3XUBX2kr9eEe fRTXqGhtFqtt9RU+feNY3bV6Cp4kc+YuprG7L8Wc6pcvgdRGWWBMVu9G4HZGTyQWsDwI/MJycQ2 EMImx1P76f7nMh0mD/M+23iHm9FXuUQ1jEFbKk0KfgCfnPaJ6PFUAb/WNGS+sduLtsvnucHnBO/ NTMPp2+7XRa+GktVrQRFKrsZYRu8reZYh0Xxq2VLRdBinZ1rc4UU03THl7KgoFbsWPb58gTv3HO 8CNDIDdGEP1ouvzfC+xGrhTS49+O2wRkX56n5PvWokFPcOELT29iIAF20uhQqJtg0X1RKnkgHib cGGxt2zjFFMFw6WxrYJ95GeGZjUvAxQj7t+k96rjreoJAmYO/caa9BNRzopz1ZXn2nl+IqX7gTm 4urwpua1Euy3Pew== X-Developer-Key: i=jlayton@kernel.org; a=openpgp; fpr=4BC0D7B24471B2A184EAF5D3000E684119568215 Precedence: bulk List-ID: X-Mailing-List: linux-nfs@vger.kernel.org At one time, nfsd would scrape inode information directly out of struct inode in order to populate the change_info4. At that time, the BUG_ON in set_change_info made some sense, since having it unset meant a coding error. More recently, it calls vfs_getattr to get this information, which can fail. If that fails, fh_pre_saved can end up not being set. While this situation is unfortunate, we don't need to crash the box. Move set_change_info to nfs4proc.c since all of the callers are there. Revise the condition for setting "atomic" to also check for fh_pre_saved, and rework the rest to try and handle either flag being missing when this occurs. Reported-by: Boyang Xue Closes: https://bugzilla.redhat.com/show_bug.cgi?id=2223560 Signed-off-by: Jeff Layton --- fs/nfsd/nfs4proc.c | 31 +++++++++++++++++++++++++++++++ fs/nfsd/xdr4.h | 11 ----------- 2 files changed, 31 insertions(+), 11 deletions(-) diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c index 9285e1eab4d5..4467be7d9c2a 100644 --- a/fs/nfsd/nfs4proc.c +++ b/fs/nfsd/nfs4proc.c @@ -382,6 +382,37 @@ nfsd4_create_file(struct svc_rqst *rqstp, struct svc_fh *fhp, return status; } +/** + * set_change_info - set up the change_info4 for a reply + * @cinfo: pointer to nfsd4_change_info to be populated + * @fhp: pointer to svc_fh to use as source + * + * Many operations in NFSv4 require change_info4 in the reply. This function + * populates that from the info that we (should!) have already collected. In + * the event that we didn't get any pre-attrs, just zero out both. + */ +static void +set_change_info(struct nfsd4_change_info *cinfo, struct svc_fh *fhp) +{ + cinfo->atomic = (u32)(fhp->fh_pre_saved && fhp->fh_post_saved && !fhp->fh_no_atomic_attr); + cinfo->before_change = fhp->fh_pre_change; + cinfo->after_change = fhp->fh_post_change; + + /* + * If fetching the pre-change attributes failed, then we should + * have already failed the whole operation. We could have still + * failed to fetch post-change attributes however. + * + * The pre field should be set at this point. WARN if it's + * that's ever not the case. If either value is unset, then just + * zero out the field since we don't have any other recourse. + */ + if (WARN_ON_ONCE(!fhp->fh_pre_saved)) + cinfo->before_change = 0; + if (!fhp->fh_post_saved) + cinfo->after_change = 0; +} + static __be32 do_open_lookup(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate, struct nfsd4_open *open, struct svc_fh **resfh) { diff --git a/fs/nfsd/xdr4.h b/fs/nfsd/xdr4.h index b2931fdf53be..9e67f63c5f4d 100644 --- a/fs/nfsd/xdr4.h +++ b/fs/nfsd/xdr4.h @@ -775,17 +775,6 @@ void warn_on_nonidempotent_op(struct nfsd4_op *op); #define NFS4_SVC_XDRSIZE sizeof(struct nfsd4_compoundargs) -static inline void -set_change_info(struct nfsd4_change_info *cinfo, struct svc_fh *fhp) -{ - BUG_ON(!fhp->fh_pre_saved); - cinfo->atomic = (u32)(fhp->fh_post_saved && !fhp->fh_no_atomic_attr); - - cinfo->before_change = fhp->fh_pre_change; - cinfo->after_change = fhp->fh_post_change; -} - - bool nfsd4_mach_creds_match(struct nfs4_client *cl, struct svc_rqst *rqstp); bool nfs4svc_decode_compoundargs(struct svc_rqst *rqstp, struct xdr_stream *xdr); bool nfs4svc_encode_compoundres(struct svc_rqst *rqstp, struct xdr_stream *xdr);