Received-SPF: pass (google.com: domain of linux-nfs+bounces-2397-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) client-ip=2604:1380:45e3:2400::1;
Date: Tue, 19 Mar 2024 11:18:15 -0400
From: Chuck Lever <chuck.lever@oracle.com>
To: Jan Kara <jack@suse.cz>
Cc: Christian Brauner <brauner@kernel.org>, Al Viro <viro@zeniv.linux.org.uk>,
        linux-nfs@vger.kernel.org, Jeff Layton <jlayton@kernel.org>
Subject: Re: [PATCH] nfsd: Fix error cleanup path in nfsd_rename()
Message-ID: <Zfmst9GEpKwVVTaF@manet.1015granger.net>
References: <20240318163209.26493-1-jack@suse.cz>
 <ZfigId5Anil0U3cs@manet.1015granger.net>
 <20240319135354.lyvyc7kvihp3kmt4@quack3>
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20240319135354.lyvyc7kvihp3kmt4@quack3>
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: 
	=?us-ascii?Q?Suikckup1dUDJBz7RsBy8/xB4kbm5DbZqpTwVBDcAyE85pFlc9XIQHrOKqP5?=
 =?us-ascii?Q?90sFsDdf2jMfZocP4XsO9eYE4p6WQwGREu3PW+/19RKPmEJfZg4wVC+ayLQg?=
 =?us-ascii?Q?MA/Txm84z/2kHEX1lOfNtuzzbbaFDA5SF1WaxlHpURXzed2Z8J7lxkxPTvt6?=
 =?us-ascii?Q?gtC5MD0EH9FAa/5ZWUFOYBy+pxtN5bl6/xSD60PobTVERnGJIyEOfmzcbIEJ?=
 =?us-ascii?Q?/I6lAK6oncfMDPai3/ah8aLsH99V/5g9DnwqYcP1sFKcKkYL0zlAk1iZBXJh?=
 =?us-ascii?Q?CA1NGSetzqDJyAsYjyuPqWmMFpPHlHbMyB/JP/gDrEiHJowIjrJ7ExS1eCzB?=
 =?us-ascii?Q?zS5OLmIMGcl0LZRdzvoq5hDKay2qWuNmsDtEe4RGZm/4ILOG9II5mNmgnPw+?=
 =?us-ascii?Q?u1MXMx6zSb1y+T/5ZZJiy6V8x8vf/9M11mHVukOWNp9cfOyGDIdU3W1U6ndH?=
 =?us-ascii?Q?6B9RI6RdhdsroKRwukxv39njOeQZvtCGt5+iVZTmrIYuQ9Uke7mbn0nCkits?=
 =?us-ascii?Q?h5QvqFwqvz/w3vqrYnKBNFbHrJi92RJwH7/TVbpc4L7OQFBgj6rK09CNa5te?=
 =?us-ascii?Q?hsy51b4qNYPIvWuZnxET70qj7CGsIuKHxUg7HQtWfQJwIQX3dmItPv+RNgMA?=
 =?us-ascii?Q?/ZZxEv3eRO3VWAdFG4d+ed6tEF973b/G33t3ewUYxKy1o0uzuA92lFiZSwWk?=
 =?us-ascii?Q?bNp+K1tU8ALbsW1/Lg3kWGWz/eUubOdHOQcAZWWlqkham2VTkHEVAi3nQLWK?=
 =?us-ascii?Q?6eUcFgb4imdSrV3ue2x/gAEJbzwFQAEg2P2FADdU0aHriSEu7lhYPNYOEpAj?=
 =?us-ascii?Q?nUg8E8GThr8QRo0Wggyy1UBlKucTPy18Kx2LnT7oAguKpt7Qngoa6pyobbiJ?=
 =?us-ascii?Q?igZuX5E/5WabIzDUUd9krFW+y0Qe35WbSW4bWmy5WPsAlH8ARPKlOzJO+TGH?=
 =?us-ascii?Q?w2Gy54fwjyH7KGfJQkfhRFQmxFPgxfQe7ew8mInhsAD/BhkQQE1czuyckfhU?=
 =?us-ascii?Q?5enaznf16Anr/t4crCDNrWrUkz+WRvEvgJ3cqizSB7+8jubFndu0S4sEjiS9?=
 =?us-ascii?Q?ANkj/cmDAdxV91EbcKGvSstYSStnNX8nJCChNysQDQHiaszp341oQzEBbaz0?=
 =?us-ascii?Q?MRlMwWpHklVF61IHNYwQA8KPr96bBxDrK0DEG+lpAvnvlu0xxWqkq7O/+3wh?=
 =?us-ascii?Q?FkWRzEvERTdWNhKMG5yv9/tbsmeZrIKml6urGFBeIJrja0DSpOQTXdmBB38K?=
 =?us-ascii?Q?jBk0bxrT6s18r1B77Y6GP/lx16ysD7uvFvwX7ebGIunCO09D+dmfVU3liTim?=
 =?us-ascii?Q?meMUgoBM84IL3f5KRiht8D44wyDSfnMmjpKw+lHGCiNtCKqsdEVmgbUiHHHz?=
 =?us-ascii?Q?5E/q4US0XsH1dpeEnAExaUP+yiJTWZhumzDkGYuo9nHes/ZBzwjY0JqcC2uJ?=
 =?us-ascii?Q?3YyCdnwtCtdUKLdTHtV16uDX+7lp+eTK8ZImu6IY/16dDgWZ/FjREOqfFsGM?=
 =?us-ascii?Q?UJlQ4nSEopsBuUcjtIfAGvHzAhUgY7bc3MeMqYbdbcS4Y/P8ryLTItwwxzcX?=
 =?us-ascii?Q?YyKBpAkzeujC6ZGTPXn8IwLGeIEvjRRvijLX0gYaqz4Z9h39u+XAb65FUC/I?=
 =?us-ascii?Q?+Q=3D=3D?=
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-0: 
	JBmGXob+ZRKhrZnE88jefcxJcIOIljTwE/uClOvJgscZ7sUeuDCf5gfUAz1Sa46dTtrHDwBfM+cri18C56GOsvCy1Kyda9LKtrFPCL6ObQhdxxRF5diWt+I/qv7S0JVJe7poCJahzYe6fH4luxQugt9cP+vb22SGslNayDJ5s06N1rIJdpZ38khDYKyP+Vz3hi1+lpARD9qRoMVQW1nR0WMXX2Q69/AbRHHqNEg5TJtmYa4Au1uaRjSXsswc2XsmfMCbQwdv17s1yuYmvNS56guVJa2E/8V837ICqDEsd5IOkJcdLRp/fu+M6zMJ3YhV/3C3m0pIE2Zh3WSAm9Vi3LrtCb24u9RukI5TSGUcfKe8qTo6YWZWOWvPXE/Gl/eIThahWdKneKExn75g2V1UHZW2Efyb4tVcTN/bn59T9I1HigX9w1oTt682nyC5J0vvHlXi//2rbJdlXAJSHQHsKzjMGgRYfcvkF6P6akNyTIQyQzIFt2K2cezWSrvyHLgaW8zpOyCvoXbVW8HuckTesEq5q+tP+O+vPkFVlJFFyeaqYBtehNGRnY7tKJb99AnI7iEMV5n76eNoBN1W7Sj8PubsolFED4HxE6pD3kZCyw8=
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: ed05490d-1aac-411b-7f5d-08dc4827ce06
X-MS-Exchange-CrossTenant-AuthSource: BN0PR10MB5128.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 19 Mar 2024 15:18:18.4385
 (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: ECF77Xjc9GhaIywrRQ/+QDEgzmu3W3Z4HENP+Y7fyQAVM/psSpG55WrudZ7aZaxPACYHoCV9Hw0T8dP6+iPG4Q==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM6PR10MB4394
X-Proofpoint-Virus-Version: vendor=baseguard
 engine=ICAP:2.0.272,Aquarius:18.0.1011,Hydra:6.0.619,FMLib:17.11.176.26
 definitions=2024-03-19_05,2024-03-18_03,2023-05-22_02
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 malwarescore=0 adultscore=0 mlxscore=0
 bulkscore=0 mlxlogscore=999 suspectscore=0 phishscore=0 spamscore=0
 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2403140000
 definitions=main-2403190115
X-Proofpoint-ORIG-GUID: 9H2hU54y1L5XjY0mOMU4rWC4bN0vSkNp
X-Proofpoint-GUID: 9H2hU54y1L5XjY0mOMU4rWC4bN0vSkNp

On Tue, Mar 19, 2024 at 02:53:54PM +0100, Jan Kara wrote:
> On Mon 18-03-24 16:12:17, Chuck Lever wrote:
> > On Mon, Mar 18, 2024 at 05:32:09PM +0100, Jan Kara wrote:
> > > Commit a8b0026847b8 ("rename(): avoid a deadlock in the case of parents
> > > having no common ancestor") added an error bail out path. However this
> > > path does not drop the remount protection that has been acquired. Fix
> > > the cleanup path to properly drop the remount protection.
> > > 
> > > Fixes: a8b0026847b8 ("rename(): avoid a deadlock in the case of parents having no common ancestor")
> > > Signed-off-by: Jan Kara <jack@suse.cz>
> > 
> > Al, Jan, let me know if you'd like me to take this through the
> > nfsd tree for v6.9-rc. If not:
> > 
> > Acked-by: Chuck Lever <chuck.lever@oracle.com>
> 
> Yeah, I guess taking this through NFS tree is the best.

I've pushed this to my nfsd-fixes branch, and will begin testing.


> 								Honza
> 
> > 
> > 
> > > ---
> > >  fs/nfsd/vfs.c | 3 ++-
> > >  1 file changed, 2 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
> > > index 6a9464262fae..2e41eb4c3cec 100644
> > > --- a/fs/nfsd/vfs.c
> > > +++ b/fs/nfsd/vfs.c
> > > @@ -1852,7 +1852,7 @@ nfsd_rename(struct svc_rqst *rqstp, struct svc_fh *ffhp, char *fname, int flen,
> > >  	trap = lock_rename(tdentry, fdentry);
> > >  	if (IS_ERR(trap)) {
> > >  		err = (rqstp->rq_vers == 2) ? nfserr_acces : nfserr_xdev;
> > > -		goto out;
> > > +		goto out_want_write;
> > >  	}
> > >  	err = fh_fill_pre_attrs(ffhp);
> > >  	if (err != nfs_ok)
> > > @@ -1922,6 +1922,7 @@ nfsd_rename(struct svc_rqst *rqstp, struct svc_fh *ffhp, char *fname, int flen,
> > >  	}
> > >  out_unlock:
> > >  	unlock_rename(tdentry, fdentry);
> > > +out_want_write:
> > >  	fh_drop_write(ffhp);
> > >  
> > >  	/*
> > > -- 
> > > 2.35.3
> > > 
> > 
> > -- 
> > Chuck Lever
> > 
> -- 
> Jan Kara <jack@suse.com>
> SUSE Labs, CR

-- 
Chuck Lever