[PATCH] NFS: idmap: fix race between key revocation and payload access

[mdshahid03@gmail.com]

From: Mohammad Shahid <mdshahid03@gmail.com>

NFS idmapping retrieves the user key payload after validating the
key, but does not hold key semaphore while accessing the
payload.

The key revocation path acquires the key semaphore for writing
and clears the payload pointer through user_revoke().

Without holding the semaphore ,revocation can race with payload
access.In this case,user_key_payload_rcu can return NULL.

The existing IS_ERR_OR_NULL() check passes this NULL pointer to
PTR_ERR(),which returns zero and incoorectly reports scuccess.

Protect key validation and payload access with key semaphore
and use user_key_payload_locked() while holding the read lock.
Handle a NULL payload explicitly as revoked key.

This prevents concurrent revocation from changing the payload
while NFS is accessing it.

This issue is reported by smatch as possible NULL argunments to
PTR_ERR().

Signed-off-by: Mohammad Shahid <mdshahid03@gmail.com>
---
 fs/nfs/nfs4idmap.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/fs/nfs/nfs4idmap.c b/fs/nfs/nfs4idmap.c
index bc397110d977..06b193722660 100644
--- a/fs/nfs/nfs4idmap.c
+++ b/fs/nfs/nfs4idmap.c
@@ -317,16 +317,16 @@ static ssize_t nfs_idmap_get_key(const char *name, size_t namelen,
 		goto out;
 	}
 
-	rcu_read_lock();
 	rkey->perm |= KEY_USR_VIEW;
 
+	down_read(&rkey->sem);
 	ret = key_validate(rkey);
 	if (ret < 0)
 		goto out_up;
 
-	payload = user_key_payload_rcu(rkey);
-	if (IS_ERR_OR_NULL(payload)) {
-		ret = PTR_ERR(payload);
+	payload = user_key_payload_locked(rkey);
+	if (!payload) {
+		ret = -EKEYREVOKED;
 		goto out_up;
 	}
 
@@ -337,7 +337,7 @@ static ssize_t nfs_idmap_get_key(const char *name, size_t namelen,
 		ret = -EINVAL;
 
 out_up:
-	rcu_read_unlock();
+	up_read(&rkey->sem);
 	key_put(rkey);
 out:
 	return ret;
-- 
2.43.0