LU-17076 orr: take a ref after lookup_get_insert_fast()

We need to take a reference on all existing object returned by
nrs_orr_res_get(). Otherwise, the object could be freed before
nrs_orr_res_put().
Therefore, a ref should be taken if
rhashtable_lookup_get_insert_fast() returns an existing object.

This avoids the following use-after-free in sanityn test_77c:

ptlrpc_nrs_req_stop_nolock+0x3b/0x150
ptlrpc_server_finish_active_request+0x2b/0x140 [ptlrpc]
ptlrpc_server_handle_request+0x40e/0xc00 [ptlrpc]
ptlrpc_main+0xc8c/0x1680 [ptlrpc]
kthread+0xd1/0xe0
ret_from_fork_nospec_begin+0x21/0x21

Fixes: 42bf5f7 ("LU-8130 nrs: convert NRS ORR/TRR to rhashtable")
Test-Parameters: fstype=zfs testlist=sanityn env=ONLY=77c,ONLY_REPEAT=100
Test-Parameters: fstype=zfs testlist=sanityn env=ONLY=77c,ONLY_REPEAT=100
Test-Parameters: fstype=zfs testlist=sanityn env=ONLY=77c,ONLY_REPEAT=100
Signed-off-by: Etienne AUJAMES <eaujames@ddn.com>
Change-Id: I267287a1075ee91019d3a4492b57f272a4b0cadd
Reviewed-on: https://review.whamcloud.com/c/fs/lustre-release/+/52295
Tested-by: jenkins <devops@whamcloud.com>
Tested-by: Maloo <maloo@whamcloud.com>
Reviewed-by: James Simmons <jsimmons@infradead.org>
Reviewed-by: Alexander Boyko <alexander.boyko@hpe.com>
Reviewed-by: Oleg Drokin <green@whamcloud.com>

diff --git a/lustre/ptlrpc/nrs_orr.c b/lustre/ptlrpc/nrs_orr.c
index 42e1d62..6daedc1 100644 (file)
--- a/lustre/ptlrpc/nrs_orr.c
+++ b/lustre/ptlrpc/nrs_orr.c
@@ -726,7 +726,7 @@ static int nrs_orr_res_get(struct ptlrpc_nrs_policy *policy,
                           struct ptlrpc_nrs_resource **resp, bool moving_req)
 {
        struct nrs_orr_data *orrd;
-       struct nrs_orr_object *orro, *orro2;
+       struct nrs_orr_object *orro, *new_orro;
        struct nrs_orr_key key = { { { 0 } } };
        u32 opc;
        int rc = 0;
@@ -770,48 +770,53 @@ static int nrs_orr_res_get(struct ptlrpc_nrs_policy *policy,
        rcu_read_lock();
        orro = rhashtable_lookup_fast(&orrd->od_obj_hash, &key,
                                      nrs_orr_hash_params);
-       if (orro && refcount_inc_not_zero(&orro->oo_ref)) {
-               rcu_read_unlock();
+       if (orro && refcount_inc_not_zero(&orro->oo_ref))
                goto found_orro;
-       }
+
        rcu_read_unlock();
 
-       OBD_SLAB_CPT_ALLOC_PTR_GFP(orro, orrd->od_cache,
+       OBD_SLAB_CPT_ALLOC_PTR_GFP(new_orro, orrd->od_cache,
                                   nrs_pol2cptab(policy), nrs_pol2cptid(policy),
                                   moving_req ? GFP_ATOMIC : GFP_NOFS);
-       if (orro == NULL)
+       if (new_orro == NULL)
                RETURN(-ENOMEM);
 
-       orro->oo_key = key;
-       refcount_set(&orro->oo_ref, 1);
+       new_orro->oo_key = key;
+       refcount_set(&new_orro->oo_ref, 1);
 try_again:
        rcu_read_lock();
-       orro2 = rhashtable_lookup_get_insert_fast(&orrd->od_obj_hash,
-                                                 &orro->oo_rhead,
+       orro = rhashtable_lookup_get_insert_fast(&orrd->od_obj_hash,
+                                                 &new_orro->oo_rhead,
                                                  nrs_orr_hash_params);
-       /* A returned non-error orro2 means it already exist */
-       if (!IS_ERR_OR_NULL(orro2)) {
-               OBD_SLAB_FREE_PTR(orro, orrd->od_cache);
-               orro = orro2;
+       /* insertion sucessfull */
+       if (likely(orro == NULL)) {
+               orro = new_orro;
+               goto found_orro;
        }
 
-       /* insertion failed */
-       if (IS_ERR(orro2)) {
-               rc = PTR_ERR(orro2);
+       /* A returned non-error orro means it already exist */
+       rc = IS_ERR(orro) ? PTR_ERR(orro) : 0;
+       if (!rc && refcount_inc_not_zero(&orro->oo_ref)) {
+               OBD_SLAB_FREE_PTR(new_orro, orrd->od_cache);
+               goto found_orro;
+       }
+       rcu_read_unlock();
 
-               /* hash table could be resizing. */
-               if (rc == -ENOMEM || rc == -EBUSY) {
-                       rcu_read_unlock();
-                       mdelay(20);
-                       goto try_again;
-               }
-               OBD_SLAB_FREE_PTR(orro, orrd->od_cache);
-               rcu_read_unlock();
+       /* oo_ref == 0, orro will be freed */
+       if (!rc)
+               goto try_again;
 
-               RETURN(rc);
+       /* hash table could be resizing. */
+       if (rc == -ENOMEM || rc == -EBUSY) {
+               mdelay(20);
+               goto try_again;
        }
-       rcu_read_unlock();
+       OBD_SLAB_FREE_PTR(new_orro, orrd->od_cache);
+
+       RETURN(rc);
+
 found_orro:
+       rcu_read_unlock();
        /**
         * For debugging purposes
         */