Received-SPF: pass (google.com: domain of linux-nfs-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Mon, 31 Jul 2023 18:43:22 -0400
From:   Chuck Lever <chuck.lever@oracle.com>
To:     Jeff Layton <jlayton@kernel.org>
Cc:     Neil Brown <neilb@suse.de>, Olga Kornievskaia <kolga@netapp.com>,
        Dai Ngo <Dai.Ngo@oracle.com>, Tom Talpey <tom@talpey.com>,
        linux-nfs@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH RFC] nfsd: don't hand out write delegations on O_WRONLY
 opens
Message-ID: <ZMg5CrlMPsDj95Ua@tissot.1015granger.net>
References: <20230731-wdeleg-v1-1-f8fe1ce11b36@kernel.org>
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20230731-wdeleg-v1-1-f8fe1ce11b36@kernel.org>
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?j4ykoGBjHY/zhgyFz61uevVj9Ni5woqIwpiN2WfHfC/bcJOtE0KUdEekT0Ef?=
 =?us-ascii?Q?tdd276UuVIuM+ZJxvr7W5NKxLIEdSDBKHdtOipP0pFpvEzM6MaD/ofztKVKQ?=
 =?us-ascii?Q?t8tQqOc+znq/9030rNX7Z01PjGEwfunWMBCTWTQnWS3+1lO8ksz5uxEnuCKt?=
 =?us-ascii?Q?2uLb0NiO8hCzudb7nMCdhKtNyRKn+3+S4Fh3181c4QTtjxH9LN4id/gRCvCW?=
 =?us-ascii?Q?J51R2/j5FngrbSnUfnt3NPTndzOoYRjc7UnGiiptGTCKwkJMe8FmBAVweI/u?=
 =?us-ascii?Q?4+VoZCRzHqNbBsGdTDRwi6vHusHdQswxJCONhjbSgTfZYFwfZ5ObNyLBK3nD?=
 =?us-ascii?Q?g8fupsJ5vokyaZ+SFiowpkmdl3/bt1v6DTGT31J6qAqQdw4u0O1eeDnBa/yf?=
 =?us-ascii?Q?K0Nx7B2rKXa6a7nA1AnxS32XHeE5QPflQCAoA9FowhAGbm/FZmH8zev/83Fb?=
 =?us-ascii?Q?Nm6RCAhMzsWYJjahhPuBrukqqvxk6mjUhBo3ZatNoxFKGjKs+ddFk+UjY6F3?=
 =?us-ascii?Q?J93IKaVON/cCv1a3+AELySoEhgL/Pz86X59Ac33qiRZ6I8HB4AHVCdy1m9sf?=
 =?us-ascii?Q?U6iy76HyKdU/BrT0tGWlkSat1+B8JyLuiGmvkXx6+brfhCQlQOd8DHASXlen?=
 =?us-ascii?Q?JcjI+9QpDAVsPNYwKMRz8+LkRYKtpeKNqmt3QSRu65Ha3/8Ujlo6Gp8RiPJR?=
 =?us-ascii?Q?7v2FDLAWYndi4M1cMzLwRd17SGyNzpPoktVWcbrFEGOUndLSL3ZuVteBpeV6?=
 =?us-ascii?Q?RnmHGj5SsYw77jrEQllWiEvijnXerS4DV+nqpsvEjmeDeFL/DBPuyOGo9v/l?=
 =?us-ascii?Q?AuDsnZlzFXB29tq9iatUwT/YjEynzjDgrsBXadzJrnaLNKWIBtg2AEz5szqd?=
 =?us-ascii?Q?uAidMNEpoCkwFvYfxNxGGk6Hiw67PenEoQv/X8Lw/fKsXNUFAXAdx62c6TWc?=
 =?us-ascii?Q?UykTKeefKGMYvq8is9xzam983TW9vmY+Cz9VctZSxpaeDwq/IzAwW5h22e0/?=
 =?us-ascii?Q?NADRfJOm5m1UerjwCXc0p1nd1xQzUs5izMRPPTYf+fcWJCQclWf+xeOPRnqq?=
 =?us-ascii?Q?T9CZE/YOMxrNvA18gVBUupuajapUtpbJyvW2nSNa1exJr0yFNzTUWyOlK5B+?=
 =?us-ascii?Q?XfUIHOBDYdFj6GVdYvUtU3pVTKnb12QYAWgFzJZiy+8+fZzR1HwY5dLGzGE+?=
 =?us-ascii?Q?33SoH7iVqBF9WJ5F9NkxeCUKVQ5dIelhDVWncmVn5vev5FGIyOixgReySdOR?=
 =?us-ascii?Q?uJQ7PofyohdBLtD6x5LyLCP+0hy4zZGw8u6mtCzU6z6u1WALDC0qe3xouXEa?=
 =?us-ascii?Q?khqR7+Lks2VJ01PNxRORmqYkG+hifKcFoPvPLMvCCONhzHs/LzSUt9FgasGj?=
 =?us-ascii?Q?wDYhSGd7JnXQVEryO4PIrUBq2cLO9dtOtFwEnomepj4zDC+VGIi9jfBXmiZz?=
 =?us-ascii?Q?rti6VFbxBRVP9Sieajca0FaWT55eodCD+ikXU0qHKer/qLoKLNh+x55CNgFt?=
 =?us-ascii?Q?EoYVGntGN7ecxRn1nVbGLJzNR+8cpeMOSO8Gg4lV2r3u5VmUvl2VYUUWsFxk?=
 =?us-ascii?Q?Ra18bpixa8HEZKqhOR0I7JTYyPiPCwucsqOl3u+SvVGrbsI0uNdBfOxCjzKc?=
 =?us-ascii?Q?GQ=3D=3D?=
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-0: 8rRoQpAvKc1/E3WbKh7oOx9HeNlDk8GfFdq1VBYCC1Cj66jOGbJfArI/6lhHGuqFtmxo1SwryAAu76wxdMz5Y40S8dnCBf6lyM/vB+Jtm7KfCmQ2Fiuj/cYSH7FWC8OT1js7nePLvgVpl3+x1rQjCT2cOxsj0OeZnJfIWTpgLJBVkkGpdM9f66Gi9xfw/qZuHyFSapETtmlkLk4cxLjtQeM/uZh/a0GF/HiEbEFL5+RDhtPg3cnNdjr5oxodOQ1q5qX42LCW96GSzSUpP4NV74D0aHo2nQIk6GnQVkMb7CI0M7Z2L2BBc9Gvta7laip/1rOE+U29wfzdLnmu0dclQ+X9CdY/x/wj0r4B3elzXT5/jESS97TnxkPsTvqYK5NdvveMdoOC9cXiQxLa51TltPr/AnLdqvmsHV0bdJzljmYNl5BcCELQt6+ZzFFZMGvQkoYqM7wQM5JEAueS5e5dqa1kh151XUfPTtQq6dELaga5xeLosGT0BBV2TFc/VrieXW+WdaWDvb9fiy0/81Zi6XnsCWAHpybwLv2iTwZcWPY/s/rTVULJJDVPu1KQI6ezGWJ8ubCOkIo+LiuaaHinG7bY7y4OORNbEm6am8qqSjJqn1HWRNOlNqj7vVZVCyteRmP+6NEA9UCZ2+SEP5YH4yZyqhubPwwVm8rntfiABoca0bffKZva1jy07PZcS57JH8AFarqnJLcpTmHU4nik737BsT2ofMS0e8U2YlzvT8MnQGyIGqL7hqwsLmQj9zTWPSceQdiup83dYw0u4JQxj+AoH21qYMoILooGo7EcobQJsU6eMzpCekx1ZHkHwh6vGphIbqnCgtutw/e1KiiiUAiO0KPSwas3LCS50bl9SJKOAhDWsjU157/WujaCdEIF
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 4a424f06-68a7-4040-0e68-08db92178d66
X-MS-Exchange-CrossTenant-AuthSource: BN0PR10MB5128.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 31 Jul 2023 22:43:25.8515
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 4e2c6054-71cb-48f1-bd6c-3a9705aca71b
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: cDcgDPRF6VGctHnru1v1dgJd/xjWU3fa6TzSyOTgFYIWMnbYJI+1FsnP3KkVvER6ncRgUKHHxUQLUmcLl9/JnA==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SN4PR10MB5654
X-Proofpoint-Virus-Version: vendor=baseguard
 engine=ICAP:2.0.254,Aquarius:18.0.957,Hydra:6.0.591,FMLib:17.11.176.26
 definitions=2023-07-31_15,2023-07-31_02,2023-05-22_02
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 phishscore=0 bulkscore=0 suspectscore=0
 adultscore=0 mlxlogscore=681 spamscore=0 malwarescore=0 mlxscore=0
 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2306200000
 definitions=main-2307310206
X-Proofpoint-ORIG-GUID: 76MT4lR_YYtyvYE_wAug9HFIGHx8qiHC
X-Proofpoint-GUID: 76MT4lR_YYtyvYE_wAug9HFIGHx8qiHC
X-Spam-Status: No, score=-3.1 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_BLOCKED,
        RCVD_IN_MSPIKE_H5,RCVD_IN_MSPIKE_WL,SPF_HELO_NONE,SPF_NONE,
        T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-nfs.vger.kernel.org>
X-Mailing-List: linux-nfs@vger.kernel.org

On Mon, Jul 31, 2023 at 04:27:30PM -0400, Jeff Layton wrote:
> I noticed that xfstests generic/001 was failing against linux-next nfsd.

Only on NFSv4.2 mounts, I presume?


> The client would request a OPEN4_SHARE_ACCESS_WRITE open, and the server
> would hand out a write delegation. The client would then try to use that
> write delegation as the source stateid in a COPY or CLONE operation, and
> the server would respond with NFS4ERR_STALE.
> 
> The problem is that the struct file associated with the delegation does
> not necessarily have read permissions. It's handing out a write
> delegation on what is effectively an O_WRONLY open. RFC 8881 states:
> 
>  "An OPEN_DELEGATE_WRITE delegation allows the client to handle, on its
>   own, all opens."
> 
> Given that the client didn't request any read permissions, and that nfsd
> didn't check for any, it seems wrong to give out a write delegation.

A client is, in fact, permitted to use a write delegation stateid
in an otw READ operation. So, this makes sense to me.


> Don't hand out a delegation if the client didn't request
> OPEN4_SHARE_ACCESS_BOTH.
> 
> This fixes xfstest generic/001.
> 
> Closes: https://bugzilla.linux-nfs.org/show_bug.cgi?id=412
> Signed-off-by: Jeff Layton <jlayton@kernel.org>

I'm thinking this should be squashed into commit
68a593f24a35 ("NFSD: Enable write delegation support").


> ---
>  fs/nfsd/nfs4state.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> index ef7118ebee00..9f1c90afed72 100644
> --- a/fs/nfsd/nfs4state.c
> +++ b/fs/nfsd/nfs4state.c
> @@ -5462,6 +5462,8 @@ nfs4_set_delegation(struct nfsd4_open *open, struct nfs4_ol_stateid *stp,
>  		return ERR_PTR(-EAGAIN);
>  
>  	if (open->op_share_access & NFS4_SHARE_ACCESS_WRITE) {
> +		if (!(open->op_share_access & NFS4_SHARE_ACCESS_READ))
> +			return ERR_PTR(-EBADF);

			return ERR_PTR(-EAGAIN);

might be more consistent with the other failure returns in this
function.


>  		nf = find_writeable_file(fp);
>  		dl_type = NFS4_OPEN_DELEGATE_WRITE;
>  	} else {
> 
> ---
> base-commit: ec89391563792edd11d138a853901bce76d11f44
> change-id: 20230731-wdeleg-bbdb6b25a3c6
> 
> Best regards,
> -- 
> Jeff Layton <jlayton@kernel.org>
> 

-- 
Chuck Lever