Received-SPF: pass (google.com: domain of linux-nfs-owner@vger.kernel.org designates 23.128.96.36 as permitted sender) client-ip=23.128.96.36;
Date:   Mon, 16 Oct 2023 16:32:33 -0400
From:   Chuck Lever <chuck.lever@oracle.com>
To:     Al Viro <viro@zeniv.linux.org.uk>
Cc:     linux-nfs@vger.kernel.org
Subject: Re: [PATCH] nfsd: lock_rename() needs both directories to live on
 the same fs
Message-ID: <ZS2d4aRUxY0LA9S4@tissot.1015granger.net>
References: <20231015172927.GE800259@ZenIV>
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20231015172927.GE800259@ZenIV>
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?dPSddSWaH+NlkfzXltiQ+3vRS5EWNpRqTjj1170pkApO3wFr5fyqiObkGtj6?=
 =?us-ascii?Q?ysMFRizb/ojyMc07eMaQMYh1j1TRSbBkRQB+9jP8XmzN7SDQv3tojYsApatM?=
 =?us-ascii?Q?OjX39OUlcxiz1lBL1QtIQbsLCAZFw9B1MwzLTY6QTNKkh3r/AhN0m5YiAg2V?=
 =?us-ascii?Q?TOXIoxbqP1SMdYqRm7WCPCp/eSm23q2wdCyamplzkEgmPl3s/154oi4scncF?=
 =?us-ascii?Q?LYMcB69jc5gVxLqACnzPyPBYvPApiWlWL4Fhz5U1O1qDb2033qOCryYGEM/t?=
 =?us-ascii?Q?v8VsEXd7DAemy0eLsXvudDLyNkPDe093rW0tCSTkHCZ8EIaHkWL7XBoxmMbO?=
 =?us-ascii?Q?DHwhbuP3ywnJYOhI7bkw9BhwLWTztU4swFoVKZhccQKi03Lq8rVapT+wqMDV?=
 =?us-ascii?Q?4A9bBaxaAcCOXPDUVxaVyceC+35r7FrSs6L81cVBaeAa6xNmTtIifquiOyrQ?=
 =?us-ascii?Q?znMsRoWxABnEUTsMXNyYXglZ6DJKWu9nn89d4RMJ8mN9v3quqoVlo3w/TF4i?=
 =?us-ascii?Q?0xceQ04vxiQKwaBwXgT/o9MAeH5mhHw5+FDSLqjMrApnd1jyjrLas35jfPyA?=
 =?us-ascii?Q?5rB9C2rL6/W5RTSORan3uMvqcUQ/QBvGAAdfRLjbxgUwYTQWt9AzNux1epTl?=
 =?us-ascii?Q?mUcPySAJ5v+c5/I4EhdkUBzGJ3VaEVCwd/N3ZlAmb/WXPOBYn95ZGsyyOsvm?=
 =?us-ascii?Q?XUSlFh0hXMZlHrblAPzy7+N06fsJHnn6yznetCJTfJs9vwfpWUeAIZIXewb3?=
 =?us-ascii?Q?KRYp0reHeIEOWTUGyuql5IKOwBV+fdQO7lxLkHVA5x9HfjKArM/A83F47G0S?=
 =?us-ascii?Q?sAnG2LA9gZvq6FaUMMPapnwoyV5Y4ojuySJmQnGSpuhq5ssOYlQ/jx6DfmDB?=
 =?us-ascii?Q?LIrbqMd4AuXBktqArBSsNV5kIQaskuriYk5D/hMLBBGRk9GGa+oSb+NQKmkk?=
 =?us-ascii?Q?XSBgdwKOYEtfMhmHlegUztzOKJNvuGs4L/zSAwWjt8ZQTmmqGP/k1ukK1RIF?=
 =?us-ascii?Q?tgEwvoEYKL4YpxIAEXwysXpsFIBdgQRivLhgqfnY2EyKQMvgDPLbB8HBEO7z?=
 =?us-ascii?Q?fy81ly0+p4XDdza4xiqLKnSo+OiuMlVvnJgxH2wrOZnlI1t9OhzpnnpUtRHh?=
 =?us-ascii?Q?k66LQqx7D8T9Vc8GuO1ovkMzbWe9K47XnEQFR/S2R6wXfdSk6wHWX2Hn9Xk1?=
 =?us-ascii?Q?IiO4ux2/r4RTYcNAFkIN7AQSt13omCBfTkbmeCNZ+F9/Lhey3cIcWVDUX58L?=
 =?us-ascii?Q?fiZfZkYXWLmuZ/eHqBTGd/LrzJxLtZJ88rzOrNC2xnx/JuYyPOyiiFA83LUV?=
 =?us-ascii?Q?gK3e4AHC13uVvMuli6nlCFO0A+M/HbK/HeqfG/PVkPz6TJObxDStrVaJcqcq?=
 =?us-ascii?Q?ymfxFORiy5bG+88TskLtnYLfb5jfzlDvMTjNEweSZbPPwTQNy/3Dj7rtQ6eX?=
 =?us-ascii?Q?6KwgCH9UQLLtXOhaIgGQSjgSHgrA5RP+w7cREtSDbg11liQTFVnFnEFnY4dd?=
 =?us-ascii?Q?z851tlEdwzfmBOCmbY5yZDZZHUsi2mgywzvmF+FROiIWHCC/I6Lc2WKm6YfS?=
 =?us-ascii?Q?KbGhrql9g5Q9Lx4sCEAAw4qZ4TQz77IqaYyR7CaM7Lv7d7O8hPlDN6YyRtu5?=
 =?us-ascii?Q?Jg=3D=3D?=
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-0: maEG0aKTI+efXBl5K4qwqADgauCWcbP1QSdMdAwV6Yxhh7LCiIHQBQP1RfTbCgL15pXd0JR9shkrUukDkeBzBTPkniHxv0y0UTjYfvSyaCR+P6r1Gb1zfetrjVvP5f7vzrcXF8w5dQzgZJQmMcnWCEfHAZ/Nl+TslSi70t3FWXkDU25sxnrnXrr34IqXdawA3+JoN0zWFSAU92/5PFM+FQUcUY5XXEYCWmyhkXg6ajf8GI7/oReQVTTg1zM2zU4+Yk9z6KUMCFEOrogYQvVaqfXcuyk0mDaRK/kZqeF4TdCfUrVMX9Oa/nyLNmZjbc7OTJz3nW0UudW50vj/RIdWO3DcE+stXWH7yj/OHa8Oj7SCutf3RGIPZNg4RdapytmcWRg5R5l0U5rWMRskT5tp+svyLtHx4OXxMLa8Kvar0ZMVV7n8xD+km8+/+lNJto6zl2zOpO9JULy7Hximtd1bZCXJ8L3BmwhfILeHHcmKFI9UnTU13MO5SMcodoCzOxtgYSUVwiX4PG7ANPwU/JM8BJQ6fMdfG9mTCys05n9eZX3Z/7kk+6y4B5c+s5Mg70PZsKHVxE2dQdUtff0j0Ak93S25ZkC20ggrdVzqDN0EPNZVojiMAWLKfLnVy/WscTuTZPak93BszbJftOXtt3BfNllcVUazdg+GSb16xdhi1clMoocQ8hTjiv9p7YeaAspIE2tSws5hpfitBOtdPCVe4wf2heA71+mhx/jmusykYm1wA1ZldtSPrWY6szyxmUriQ+boTATvP5zI2oAAHDJRUSLtC67N+c0CJVH0x0ri/OoLnlKyNqDuHbFEWwMWsyWc
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: a4493b93-4944-48fd-70ff-08dbce870832
X-MS-Exchange-CrossTenant-AuthSource: BN0PR10MB5128.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 16 Oct 2023 20:32:35.8114
 (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: aK1BNFRIispLB8kirBKnFRjD9hx4CZMbYr7MEUvjLCM28D4fQWwvOJzqImTQmu1SzB5o5cLojPE0dPDBEpfTKA==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SJ0PR10MB6374
X-Proofpoint-Virus-Version: vendor=baseguard
 engine=ICAP:2.0.272,Aquarius:18.0.980,Hydra:6.0.619,FMLib:17.11.176.26
 definitions=2023-10-16_10,2023-10-12_01,2023-05-22_02
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 malwarescore=0 adultscore=0 spamscore=0
 mlxscore=0 phishscore=0 bulkscore=0 suspectscore=0 mlxlogscore=939
 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2309180000
 definitions=main-2310160178
X-Proofpoint-ORIG-GUID: O7TnsIL902mN7v40qrx8FjSakNO3CaLH
X-Proofpoint-GUID: O7TnsIL902mN7v40qrx8FjSakNO3CaLH
X-Spam-Status: No, score=-0.8 required=5.0 tests=DKIM_SIGNED,DKIM_VALID,
	DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,
	SPF_HELO_NONE,SPF_PASS autolearn=unavailable autolearn_force=no
	version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on pete.vger.email
Precedence: bulk
List-ID: <linux-nfs.vger.kernel.org>
X-Mailing-List: linux-nfs@vger.kernel.org
X-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.6.4 (pete.vger.email [0.0.0.0]); Mon, 16 Oct 2023 13:32:52 -0700 (PDT)

On Sun, Oct 15, 2023 at 06:29:27PM +0100, Al Viro wrote:
> ... checking that after lock_rename() is too late.  Incidentally,
> NFSv2 had no nfserr_xdev...
> 
> Fixes: aa387d6ce153 "nfsd: fix EXDEV checking in rename"
> Cc: stable@vger.kernel.org # v3.9+
> Reviewed-by: Jeff Layton <jlayton@kernel.org>
> Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
> ---
> 
> [in git://git.kernel.org/pub/scm/linux/kernel/git/viro/vfs.git #nfsd-fix;
> it's an immutable branch, please either pull from it or put that thing
> into an immutable branch in your tree - there's a lock_rename-related
> series in the making and I'd rather avoid mixing unrelated nfsd stuff
> into it ;-/]
> 
> diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
> index 48260cf68fde..02f5fcaad03f 100644
> --- a/fs/nfsd/vfs.c
> +++ b/fs/nfsd/vfs.c
> @@ -1788,6 +1788,12 @@ nfsd_rename(struct svc_rqst *rqstp, struct svc_fh *ffhp, char *fname, int flen,
>  	if (!flen || isdotent(fname, flen) || !tlen || isdotent(tname, tlen))
>  		goto out;
>  
> +	err = (rqstp->rq_vers == 2) ? nfserr_acces : nfserr_xdev;
> +	if (ffhp->fh_export->ex_path.mnt != tfhp->fh_export->ex_path.mnt)
> +		goto out;
> +	if (ffhp->fh_export->ex_path.dentry != tfhp->fh_export->ex_path.dentry)
> +		goto out;
> +
>  retry:
>  	host_err = fh_want_write(ffhp);
>  	if (host_err) {
> @@ -1823,12 +1829,6 @@ nfsd_rename(struct svc_rqst *rqstp, struct svc_fh *ffhp, char *fname, int flen,
>  	if (ndentry == trap)
>  		goto out_dput_new;
>  
> -	host_err = -EXDEV;
> -	if (ffhp->fh_export->ex_path.mnt != tfhp->fh_export->ex_path.mnt)
> -		goto out_dput_new;
> -	if (ffhp->fh_export->ex_path.dentry != tfhp->fh_export->ex_path.dentry)
> -		goto out_dput_new;
> -
>  	if ((ndentry->d_sb->s_export_op->flags & EXPORT_OP_CLOSE_BEFORE_UNLINK) &&
>  	    nfsd_has_cached_files(ndentry)) {
>  		close_cached = true;

I ran the git regression suite with 12 threads on NFS/RDMA mounts
using a matrix of NFSv3, NFSv4.0, NFSv4.1, and NFSv4.2 on exports
of tmpfs, btrfs, xfs, and ext4. All runs completed successfully
and in a timely manner.

I think the patch is good to go.


-- 
Chuck Lever