Re: [bug report] nfs: make sillyrename an async operation

[NeilBrown <neilb@suse.com>]

[-- Attachment #1: Type: text/plain, Size: 2123 bytes --]

On Wed, Dec 19 2018, Dan Carpenter wrote:

> [ Ancient code.  But NeilB changed get_cred() recently so Smatch thinks
>   it understands it now.  - dan ]
>
> Hello Jeff Layton,
>
> The patch d3d4152a5d59: "nfs: make sillyrename an async operation"
> from Sep 17, 2010, leads to the following static checker warning:
>
> 	fs/nfs/unlink.c:351 nfs_async_rename()
> 	warn: 'data->cred' isn't an ERR_PTR

It would have been impressive to determine that at the time, though I
think it was true.
At the time it was rpc_lookup_cred(), that provided data->cred.
That was
        return rpcauth_lookupcred(&generic_auth, 0);
of which the key line is :
        ret = auth->au_ops->lookup_cred(auth, &acred, flags);

and generic_auth.au_ops->lookup_cred was
        return rpcauth_lookup_credcache(&generic_auth, acred, flags);

which can only return an error if
              cred->cr_ops->cr_init != NULL &&
and
                int res = cred->cr_ops->cr_init(auth, cred);
                if (res < 0) {

There is no generic_auth.au_ops->cr_init, so no error can be return.

It is much easier to determine that now.

Thanks,
NeilBrown

From: NeilBrown <neilb@suse.com>
Date: Thu, 20 Dec 2018 10:29:55 +1100
Subject: [PATCH] NFS: remove unnecessary test for IS_ERR(cred)

As gte_current_cred() cannot return an error,
this test is not necessary.
It hasn't been necessary for years, but it wasn't so obvious
before.

Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: NeilBrown <neilb@suse.com>
---
 fs/nfs/unlink.c | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/fs/nfs/unlink.c b/fs/nfs/unlink.c
index a227ab7d6891..79b97b3c4427 100644
--- a/fs/nfs/unlink.c
+++ b/fs/nfs/unlink.c
@@ -348,11 +348,6 @@ nfs_async_rename(struct inode *old_dir, struct inode *new_dir,
 	task_setup_data.callback_data = data;
 
 	data->cred = get_current_cred();
-	if (IS_ERR(data->cred)) {
-		struct rpc_task *task = ERR_CAST(data->cred);
-		kfree(data);
-		return task;
-	}
 
 	msg.rpc_argp = &data->args;
 	msg.rpc_resp = &data->res;
-- 
2.14.0.rc0.dirty


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]