Re: [PATCH 10/11] ksmbd: remove setattr preparations in set_file_basic_info()

[Christian Brauner <christian.brauner@ubuntu.com>]

On Wed, Sep 01, 2021 at 09:47:17PM +0900, Namjae Jeon wrote:
> 2021-08-24 0:13 GMT+09:00, Christian Brauner <brauner@kernel.org>:
> > From: Christian Brauner <christian.brauner@ubuntu.com>
> >
> > Permission checking and copying over ownership information is the task
> > of the underlying filesystem not ksmbd. The order is also wrong here.
> > This modifies the inode before notify_change(). If notify_change() fails
> > this will have changed ownership nonetheless. All of this is unnecessary
> > though since the underlying filesystem's ->setattr handler will do all
> > this (if required) by itself.
> >
> > Cc: Steve French <stfrench@microsoft.com>
> > Cc: Christoph Hellwig <hch@infradead.org>
> > Cc: Namjae Jeon <namjae.jeon@samsung.com>
> > Cc: Hyunchul Lee <hyc.lee@gmail.com>
> > Cc: Sergey Senozhatsky <senozhatsky@chromium.org>
> > Cc: linux-cifs@vger.kernel.org
> > Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>
> > ---
> >  fs/ksmbd/smb2pdu.c | 5 -----
> >  1 file changed, 5 deletions(-)
> >
> > diff --git a/fs/ksmbd/smb2pdu.c b/fs/ksmbd/smb2pdu.c
> > index 1148e52a4037..059764753aaa 100644
> > --- a/fs/ksmbd/smb2pdu.c
> > +++ b/fs/ksmbd/smb2pdu.c
> > @@ -5521,12 +5521,7 @@ static int set_file_basic_info(struct ksmbd_file *fp,
> > char *buf,
> >  		if (IS_IMMUTABLE(inode) || IS_APPEND(inode))
> >  			return -EACCES;
> >
> > -		rc = setattr_prepare(user_ns, dentry, &attrs);
> > -		if (rc)
> > -			return -EINVAL;
> > -
> >  		inode_lock(inode);
> > -		setattr_copy(user_ns, inode, &attrs);
> >  		attrs.ia_valid &= ~ATTR_CTIME;
> >  		rc = notify_change(user_ns, dentry, &attrs, NULL);
> setattr_prepare() was used for updating ->ctime to ChangeTime in set
> file basic info request. but notify_change() have just updated it to
> current time. So some of smbtorture tests failed.
> Could you please review the below change ?
> https://github.com/namjaejeon/smb3-kernel/commit/831bcdeaa5231a8d8125f6155833f1cb5dc0f8ca

Ok, looks good.

Christian