From patchwork Thu Jun 2 19:48:58 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Olga Kornievskaia X-Patchwork-Id: 9151107 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 8F58B60221 for ; Thu, 2 Jun 2016 19:49:02 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 82B5C2780C for ; Thu, 2 Jun 2016 19:49:02 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 773CE282EE; Thu, 2 Jun 2016 19:49:02 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.8 required=2.0 tests=BAYES_00,DKIM_SIGNED, RCVD_IN_DNSWL_HI,T_DKIM_INVALID autolearn=ham version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id C026A2780C for ; Thu, 2 Jun 2016 19:49:01 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932637AbcFBTtA (ORCPT ); Thu, 2 Jun 2016 15:49:00 -0400 Received: from mail-io0-f193.google.com ([209.85.223.193]:33872 "EHLO mail-io0-f193.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932446AbcFBTs7 (ORCPT ); Thu, 2 Jun 2016 15:48:59 -0400 Received: by mail-io0-f193.google.com with SMTP id l9so7847430ioe.1 for ; Thu, 02 Jun 2016 12:48:59 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=mime-version:sender:in-reply-to:references:date:message-id:subject :from:to:cc; bh=ByyzNfntN4L1uGNXbdYLPTeYiZESRt42ZnB/WpRRwGk=; b=bzofh7V2x+Vkjr0z6Lo0b6dzMwJ2GK4wLUuWNFxZcu+EBeW0liOurwK6N0RM+PHDPG LOpGAyCiMoR9uvEbngAQjZsGC49wtnQCbUNnmezqScUx03M9g3liZKpi8I87OZrydPDJ MA+336LH6jOUzVvdet/psswBEjsu10NA525Frx3H2HHAZglIbXJasKZlTXloHgppHk3v OspdIzWB8rxcWIMheUzJjyJ0ErGa7W634LLWR8ySU6pA7z5BJx1KuyoHPDcDIL5RUztS ZPHuJJiQl2q1XGZONeMBBue9FnaqCTEs7g1/8m2YpFnP+EvF3vUUpAhInQ7Ho0QSb5aA mgjg== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=umich.edu; s=google; h=mime-version:sender:in-reply-to:references:date:message-id:subject :from:to:cc; bh=ByyzNfntN4L1uGNXbdYLPTeYiZESRt42ZnB/WpRRwGk=; b=Ykxcpt1/UehdY86b9XMX0SrmZkx0xTzBb4fcvhQe628R4hSLYNWI+gl+j2ynok+XKU Px911xvLu1O1ApjZesAkyCIvjrQZwEnFH/VBUzDB2D4kFZu9Kn0BSR+OTv50QLVfB8DE qqsKtAfxCcyvWPk9rjlOaALZlaw2i0AgqiAQk= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:sender:in-reply-to:references:date :message-id:subject:from:to:cc; bh=ByyzNfntN4L1uGNXbdYLPTeYiZESRt42ZnB/WpRRwGk=; b=LIoP0bepxPGCtgeWt9PNP+7WVYuscXCbIpP+/ltcff1Qqv+7vlEOg4QXvFEvhSOQdR 4LcAkAuAVBH+v0ZuulHsZgFYDIT5q2apPgyMfyW238nJKlKt2gHp3IoiiZZP+0NkF7Yp ofUz1F7izblFMXfXENMyN9F5wOd+JQwd5F9r3PKSMO+IGpDVymg+o08nd8sb0prqIi03 Vn8C5dKYWp+JC8xmR/36Go8P37lI1LdAwLiL/NiLQ0Np7zPt9aW4iY8Zgo8IBolfgdv4 AusorykGBDf1sBEpJ/ABbyjN30PxIeDQArqFALbt1L7+yVXIbTzsNJkAbqv4IZAWiokk QPvQ== X-Gm-Message-State: ALyK8tJTr/ZE4RnaWbA7pQSiV72dB00lU568XIttlonezoFDprB1d38+jqvjpe57oxBrBg4LvWrpYaHDoHuNfw== MIME-Version: 1.0 X-Received: by 10.107.184.214 with SMTP id i205mr625233iof.26.1464896938733; Thu, 02 Jun 2016 12:48:58 -0700 (PDT) Received: by 10.107.5.16 with HTTP; Thu, 2 Jun 2016 12:48:58 -0700 (PDT) In-Reply-To: References: Date: Thu, 2 Jun 2016 15:48:58 -0400 X-Google-Sender-Auth: VJQkyoSMrL1OSxHAub5TJPTkUbk Message-ID: Subject: Re: open_downgrade use From: Olga Kornievskaia To: Trond Myklebust Cc: linux-nfs Sender: linux-nfs-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-nfs@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP On Wed, Jun 1, 2016 at 4:59 PM, Olga Kornievskaia wrote: > On Wed, Jun 1, 2016 at 4:41 PM, Trond Myklebust wrote: >> You are misreading what I wrote. Your test should indeed give rise to an >> OPEN_DOWNGRADE (unless there is a delegation involved). The code that was >> misbehaving and that was fixed by the patch was triggering an OPEN_DOWNGRADE >> from a stateid that had only been opened for RW. > > I see. With this patch, the upstream code no longer sends an > OPEN_DOWNGRADE. I will investigate why then as it seems like a bug. Can you please help explain the logic of this commit as my solution is to negate this: commit aee7af356e151494d5014f57b33460b162f181b5 Author: Trond Myklebust Date: Mon Aug 25 22:33:12 2014 -0400 NFSv4: Fix problems with close in the presence of a delegation In the presence of delegations, we can no longer assume that the state->n_rdwr, state->n_rdonly, state->n_wronly reflect the open stateid share mode, and so we need to calculate the initial value for calldata->arg.fmode using the state->flags. Reported-by: James Drews Fixes: 88069f77e1ac5 (NFSv41: Fix a potential state leakage when...) Cc: stable@vger.kernel.org # 2.6.33+ Signed-off-by: Trond Myklebust When close(fd0) come which suppose to translate into OPEN_DOWNGRADE: nfs4_close_prepare is_rdwr=1 is_rdonly=1 is_wronly=0 n_rdwr=0 n_rdonly=1 n_wonly=0 if (state->n_rdwr == 0) { if (state->n_rdonly == 0) call_close |= is_rdonly; else if (is_rdonly) calldata->arg.fmode |= FMODE_READ; <** this is set but call_close ends up being 0 **> if (state->n_wronly == 0) call_close |= is_wronly; else if (is_wronly) calldata->arg.fmode |= FMODE_WRITE; } else if (is_rdwr) calldata->arg.fmode |= FMODE_READ|FMODE_WRITE; so then the check for !call_close later sends it to no_action and nothing is sent. Here's what I propose to fix it: But then I really don't understand why not sent call_close for if (is_rdonly) case? Thank you. > >> >> >> On 6/1/16, 16:31, "linux-nfs-owner@vger.kernel.org on behalf of Olga >> Kornievskaia" >> wrote: >> >>>I'm failing to think of what can trigger an open_downgrade? >>>I thought the following example should trigger an open downgrade: >>> >>>fd0 = open(foo, RDRW) -- should be open on the wire for "both" >>>fd1 = open(foo, RDONLY) -- should be open on the wire for "read" >>>close(fd0) -- should trigger an open_downgrade >>>read(fd1) >>>close(fd1) >>> >>>However this commit says that it's not allowed by the spec. >>> >>>commit cd9288ffaea4359d5cfe2b8d264911506aed26a4 >>>Author: Trond Myklebust >>>Date: Thu Sep 18 11:51:32 2014 -0400 >>> >>> NFSv4: Fix another bug in the close/open_downgrade code >>> >>> James Drew reports another bug whereby the NFS client is now sending >>> an OPEN_DOWNGRADE in a situation where it should really have sent a >>> CLOSE: the client is opening the file for O_RDWR, but then trying to >>> do a downgrade to O_RDONLY, which is not allowed by the NFSv4 spec. >>> >>> Reported-by: James Drews >>> Link: http://lkml.kernel.org/r/541AD7E5.8020409@engr.wisc.edu >>> Fixes: aee7af356e15 (NFSv4: Fix problems with close in the presence...) >>> Cc: stable@vger.kernel.org # 2.6.33+ >>> Signed-off-by: Trond Myklebust >>> >>>If RDWR to RDONLY isn't allowed then why do we have OPEN_DOWNGRADE at all? >>>-- >>>To unsubscribe from this list: send the line "unsubscribe linux-nfs" in >>>the body of a message to majordomo@vger.kernel.org >>>More majordomo info at http://vger.kernel.org/majordomo-info.html >>> >> >> >> Disclaimer >> >> The information contained in this communication from the sender is >> confidential. It is intended solely for use by the recipient and others >> authorized to receive it. If you are not the recipient, you are hereby >> notified that any disclosure, copying, distribution or taking action in >> relation of the contents of this information is strictly prohibited and may >> be unlawful. --- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c index 327b8c3..1db2e31 100644 --- a/fs/nfs/nfs4proc.c +++ b/fs/nfs/nfs4proc.c @@ -2870,7 +2870,7 @@ static void nfs4_close_prepare(struct rpc_task *task, void call_close = 0; spin_unlock(&state->owner->so_lock); - if (!call_close) { + if (!call_close && !calldata->arg.fmode) { /* Note: exit _without_ calling nfs4_close_done */ goto out_no_action; }