Received-SPF: pass (google.com: domain of linux-nfs-owner@vger.kernel.org designates 23.128.96.32 as permitted sender) client-ip=23.128.96.32;
Date:   Sun, 15 Oct 2023 13:47:06 -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: <ZSwlmqabc2URieJ4@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?1kFjWCdFgOYslRHvCLBBD8rlsX8hPOnLUxhZFpZps/8VkOFzC3Dg4PTBSAbn?=
 =?us-ascii?Q?w/o7cRSLKpy44S4BX3/dNfkPcnqH7uc00vWbiWHtqkiydPYg5XiimWqYPOnb?=
 =?us-ascii?Q?nHvSZQGSmjRQhlSBzbJOn8EtW/of7Sg5SIt6bYL+7d8/g9YAS/L8L2oX/2Z7?=
 =?us-ascii?Q?DY/m4W2Z2qkEH8ZTafuf77DR60zeDVJh9pCHfvyIWMn/5eJk3T6GJOB0ghIT?=
 =?us-ascii?Q?9OYTZUhn0rELu6TzLwfV3wfsbrNu5ioz5WtuHG510TceMHJ4UroaobMrvfrS?=
 =?us-ascii?Q?mWrv4rDjW4co08VC/I8X9nrFXVLLPXOuryt+3WSP7653ysdAfm8ASD/Xw6yF?=
 =?us-ascii?Q?h13+CzMghafcHalBqAWkDtzxBggy8pTfU9BS/oje4LwhnZII0GzTWX6jy+tu?=
 =?us-ascii?Q?xVt7hclqg31MboKOTnYie28VsI1NuUkMtlwXHoX6CR8KZFNMuimGdB4jBlp6?=
 =?us-ascii?Q?lFEMwS+jeu7V3pWwBP8vOUa09Py2Zqo0mYYm2tPzqyClGMtLhluLv0BzjkZA?=
 =?us-ascii?Q?mJ1jF60+2g/9Fz5vj7gqDN2BmVzGrlYhbVzrtnecHKDp6lCPKC/E0ZnCuhlI?=
 =?us-ascii?Q?lGZKuedQkIMbnDotnp9tpaGAZZwKvPp7chYFWVE1pFFd60RUZszzDEf3op1V?=
 =?us-ascii?Q?OT1ZSz7aztK2/Fi0TsEDnvRItfegVtye1Veakyy/BYACHBZub2mOR1irwjkE?=
 =?us-ascii?Q?gHl7+V2bI5xmL+LyMEOPxBvMwg4YJcuX/Yk+4pnRdjcG6UfMQXWxJh5Ch2ae?=
 =?us-ascii?Q?1oE/ya1K/26TSQic0jf8h9GkCsaRP3bcGlnWw9YQV0AJurRnJS3kHrqZ3olF?=
 =?us-ascii?Q?rB1yaEZ2L4LS8geIZ+IoonGmQ+4dNaTEmgM4YeO27g9GLibbY+9HlM6KKVXy?=
 =?us-ascii?Q?PKNrgf1590EB1pGI/aV2mOzjr5RsTFtL8WvfQ9kVsHXJ7NNUeiOn3B6TeUW7?=
 =?us-ascii?Q?oWeBDXPzMR6bmXhs5dLuFH8Z6dXQGhqkTEncy/izNZ/z2/QW6ZYAR1dnqHWu?=
 =?us-ascii?Q?f/JS5tXp61S8q4o3eZciY3of4GNeU+I2Qzt8/g6jcQyN7gn3MuJ7QdWgr9xR?=
 =?us-ascii?Q?+K7Rw2cV5A9G1y/3qkHGIXVTVnWSEuQAaU2jYnDnJg7cUOiAPpANJ9Tn4R0F?=
 =?us-ascii?Q?8WjzXQ159n3jr5GzrhhCTXd//wLrhL2hxPKrXN0polhIdUlR6573CjozqyBg?=
 =?us-ascii?Q?RkBcjD5N7bbNORvcHJTQX+HIDRY1P4xkuuH9ohVP5vM09tfHWiRP5OodHhIx?=
 =?us-ascii?Q?wfbHRJYc4mARUpPutU7rvMwmLsnYbJYFA+BR8H+Y2y7BVbK5lkqUOCTXt6Qg?=
 =?us-ascii?Q?/GAuMww4p2sLyRNH+w4XP8Hncvj4F4wiwSH9XaSwB7yd6qGIDevjdZdF5+o6?=
 =?us-ascii?Q?vc/gfsxnbSH5kEbTDYbCJhW9z4BIbpG54f2GEBOjocyuWHzfz+wkAMU5PC14?=
 =?us-ascii?Q?umS3w/F5NVQ59rHtcUthwAL20MH9kYLcqFnfadU28iPTSvvki80RKVMgiso2?=
 =?us-ascii?Q?q3G+zYP8T2cd9AdgXusukjMHcxcEOp6duP58BIHHhmiSPJR3MHnmbgVP3GOo?=
 =?us-ascii?Q?ZCziZmH35KD6XdJ8QP2pnnvU9ZmhOAKHS5Wg3X0MkqUT/C9ADdmR4ZKevWEG?=
 =?us-ascii?Q?cQ=3D=3D?=
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-0: JNJAAu5uGHSqDQymvT/Rrbs4U5M8DOooHdl7Px/iUHMaPVeFyIxqHL2HEEWkhV/dCVJB9kdhGcpgW+/Yc8N8dCdMI0ODSMAhzFGsqZttLUaS4CkhygHCsGfudjEOIoDGD2oFe/TgoZu4ZQXVBSB55yh8BClh3bq6YUInpEITNyi0fv0B4DtSCFB6wH3+XGi346PYq9D7ntbFRKkHE+EbwiQjZX/0hQNvMullwrM+MplwDwmxGJBRdr1xEl+vCWTCyybVu2Sm1WwdhKnZ31b1PxT3OqgUl3HvFCxSTe11dy/bq5JvT5QPjWZY6JestqWV7WP9Z9mkFqG0FGvRP5D/A0sowvkXCJvrBH8RIsF8acXJ4YskQGWlyjVb1B5uOVuihsEV60SReqd90tMF7q3bYOLV4NtknV9mCykEd7EKo7/EsoOIs7ZB1VfPjEjyCnlPRZe4BDi3/7uWtBTegD3pLlq92Ic8oA1+ZhypJzFRFYek7kTOHU0fXzgXv9kPdVk3lOskHuFParQxAeWzGDMeCKwAULBrVxKaEe1P5vcWJEY4RnnaJzcCOo17rW1/bSPE/IEKS+5uuHcKW4FRyyYXaim/qbEJx+D17Z2ddq0tUOkpUB06PcdQ6GAKmB65yAwFcFqpulFdF0mxGzEKgqA306Aj1d7j/5N3qaSIamBn6J7bHAjFgEuPHf+hjzXUHZfPXtAV+cXfSPCouoWFSZg3CWKY0gK+KXcYYLtr3jDdCJw9z+XzWKDwDcVVDW7paSVN6xJxhxoLr51s8YtkiXR1LElYSdEbJags2eQYSc00vRdyRD9+YrJY1OoKAi98m7AD
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: bd30c345-3058-4d89-b125-08dbcda6c129
X-MS-Exchange-CrossTenant-AuthSource: BN0PR10MB5128.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 15 Oct 2023 17:47:09.3111
 (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: FEhfiB1qpjKmg6ld1tNpudg2kxOTESSl7o9drXvxQ8l2H+k9I/9NUtqOIqR3/bmYeBF+v5iv0jfxHdHQsFzP8A==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BLAPR10MB5027
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-15_03,2023-10-12_01,2023-05-22_02
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 phishscore=0 spamscore=0 malwarescore=0
 adultscore=0 suspectscore=0 mlxscore=0 mlxlogscore=999 bulkscore=0
 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2309180000
 definitions=main-2310150161
X-Proofpoint-GUID: rJbnGApAeO41slXlId1cqP-o-juINRfz
X-Proofpoint-ORIG-GUID: rJbnGApAeO41slXlId1cqP-o-juINRfz
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 agentk.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 (agentk.vger.email [0.0.0.0]); Sun, 15 Oct 2023 10:50: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 ;-/]

I'll assume you will be sending this on to Linus, then:

Acked-by: Chuck Lever <chuck.lever@oracle.com>

Jeff or I will get back to you in a day or two on the results of
regression testing, but I don't expect a problem.


> 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;

-- 
Chuck Lever