Skip to content

Commit 212bf41

Browse files
ereshetovaamschuma-ntap
ereshetova
authored and
amschuma-ntap
committed
fs, nfs: convert nfs_client.cl_count from atomic_t to refcount_t
atomic_t variables are currently used to implement reference counters with the following properties: - counter is initialized to 1 using atomic_set() - a resource is freed upon counter reaching zero - once counter reaches zero, its further increments aren't allowed - counter schema uses basic atomic operations (set, inc, inc_not_zero, dec_and_test, etc.) Such atomic variables should be converted to a newly provided refcount_t type and API that prevents accidental counter overflows and underflows. This is important since overflows and underflows can lead to use-after-free situation and be exploitable. The variable nfs_client.cl_count is used as pure reference counter. Convert it to refcount_t and fix up the operations. Suggested-by: Kees Cook <keescook@chromium.org> Reviewed-by: David Windsor <dwindsor@gmail.com> Reviewed-by: Hans Liljestrand <ishkamiel@gmail.com> Signed-off-by: Elena Reshetova <elena.reshetova@intel.com> Signed-off-by: Anna Schumaker <Anna.Schumaker@Netapp.com>
1 parent 2f62b5a commit 212bf41

File tree

7 files changed

+33
-32
lines changed

7 files changed

+33
-32
lines changed

‎fs/nfs/client.c

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -163,7 +163,7 @@ struct nfs_client *nfs_alloc_client(const struct nfs_client_initdata *cl_init)
163163

164164
clp->rpc_ops = clp->cl_nfs_mod->rpc_ops;
165165

166-
atomic_set(&clp->cl_count, 1);
166+
refcount_set(&clp->cl_count, 1);
167167
clp->cl_cons_state = NFS_CS_INITING;
168168

169169
memcpy(&clp->cl_addr, cl_init->addr, cl_init->addrlen);
@@ -269,7 +269,7 @@ void nfs_put_client(struct nfs_client *clp)
269269

270270
nn = net_generic(clp->cl_net, nfs_net_id);
271271

272-
if (atomic_dec_and_lock(&clp->cl_count, &nn->nfs_client_lock)) {
272+
if (refcount_dec_and_lock(&clp->cl_count, &nn->nfs_client_lock)) {
273273
list_del(&clp->cl_share_link);
274274
nfs_cb_idr_remove_locked(clp);
275275
spin_unlock(&nn->nfs_client_lock);
@@ -314,7 +314,7 @@ static struct nfs_client *nfs_match_client(const struct nfs_client_initdata *dat
314314
sap))
315315
continue;
316316

317-
atomic_inc(&clp->cl_count);
317+
refcount_inc(&clp->cl_count);
318318
return clp;
319319
}
320320
return NULL;
@@ -1006,7 +1006,7 @@ struct nfs_server *nfs_clone_server(struct nfs_server *source,
10061006
/* Copy data from the source */
10071007
server->nfs_client = source->nfs_client;
10081008
server->destroy = source->destroy;
1009-
atomic_inc(&server->nfs_client->cl_count);
1009+
refcount_inc(&server->nfs_client->cl_count);
10101010
nfs_server_copy_userdata(server, source);
10111011

10121012
server->fsid = fattr->fsid;
@@ -1166,7 +1166,7 @@ static int nfs_server_list_show(struct seq_file *m, void *v)
11661166
clp->rpc_ops->version,
11671167
rpc_peeraddr2str(clp->cl_rpcclient, RPC_DISPLAY_HEX_ADDR),
11681168
rpc_peeraddr2str(clp->cl_rpcclient, RPC_DISPLAY_HEX_PORT),
1169-
atomic_read(&clp->cl_count),
1169+
refcount_read(&clp->cl_count),
11701170
clp->cl_hostname);
11711171
rcu_read_unlock();
11721172

‎fs/nfs/filelayout/filelayout.c

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -471,10 +471,10 @@ filelayout_read_pagelist(struct nfs_pgio_header *hdr)
471471
return PNFS_NOT_ATTEMPTED;
472472

473473
dprintk("%s USE DS: %s cl_count %d\n", __func__,
474-
ds->ds_remotestr, atomic_read(&ds->ds_clp->cl_count));
474+
ds->ds_remotestr, refcount_read(&ds->ds_clp->cl_count));
475475

476476
/* No multipath support. Use first DS */
477-
atomic_inc(&ds->ds_clp->cl_count);
477+
refcount_inc(&ds->ds_clp->cl_count);
478478
hdr->ds_clp = ds->ds_clp;
479479
hdr->ds_commit_idx = idx;
480480
fh = nfs4_fl_select_ds_fh(lseg, j);
@@ -515,10 +515,10 @@ filelayout_write_pagelist(struct nfs_pgio_header *hdr, int sync)
515515

516516
dprintk("%s ino %lu sync %d req %zu@%llu DS: %s cl_count %d\n",
517517
__func__, hdr->inode->i_ino, sync, (size_t) hdr->args.count,
518-
offset, ds->ds_remotestr, atomic_read(&ds->ds_clp->cl_count));
518+
offset, ds->ds_remotestr, refcount_read(&ds->ds_clp->cl_count));
519519

520520
hdr->pgio_done_cb = filelayout_write_done_cb;
521-
atomic_inc(&ds->ds_clp->cl_count);
521+
refcount_inc(&ds->ds_clp->cl_count);
522522
hdr->ds_clp = ds->ds_clp;
523523
hdr->ds_commit_idx = idx;
524524
fh = nfs4_fl_select_ds_fh(lseg, j);
@@ -1064,9 +1064,9 @@ static int filelayout_initiate_commit(struct nfs_commit_data *data, int how)
10641064
goto out_err;
10651065

10661066
dprintk("%s ino %lu, how %d cl_count %d\n", __func__,
1067-
data->inode->i_ino, how, atomic_read(&ds->ds_clp->cl_count));
1067+
data->inode->i_ino, how, refcount_read(&ds->ds_clp->cl_count));
10681068
data->commit_done_cb = filelayout_commit_done_cb;
1069-
atomic_inc(&ds->ds_clp->cl_count);
1069+
refcount_inc(&ds->ds_clp->cl_count);
10701070
data->ds_clp = ds->ds_clp;
10711071
fh = select_ds_fh_from_commit(lseg, data->ds_commit_index);
10721072
if (fh)

‎fs/nfs/flexfilelayout/flexfilelayout.c

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1726,10 +1726,10 @@ ff_layout_read_pagelist(struct nfs_pgio_header *hdr)
17261726
vers = nfs4_ff_layout_ds_version(lseg, idx);
17271727

17281728
dprintk("%s USE DS: %s cl_count %d vers %d\n", __func__,
1729-
ds->ds_remotestr, atomic_read(&ds->ds_clp->cl_count), vers);
1729+
ds->ds_remotestr, refcount_read(&ds->ds_clp->cl_count), vers);
17301730

17311731
hdr->pgio_done_cb = ff_layout_read_done_cb;
1732-
atomic_inc(&ds->ds_clp->cl_count);
1732+
refcount_inc(&ds->ds_clp->cl_count);
17331733
hdr->ds_clp = ds->ds_clp;
17341734
fh = nfs4_ff_layout_select_ds_fh(lseg, idx);
17351735
if (fh)
@@ -1785,11 +1785,11 @@ ff_layout_write_pagelist(struct nfs_pgio_header *hdr, int sync)
17851785

17861786
dprintk("%s ino %lu sync %d req %zu@%llu DS: %s cl_count %d vers %d\n",
17871787
__func__, hdr->inode->i_ino, sync, (size_t) hdr->args.count,
1788-
offset, ds->ds_remotestr, atomic_read(&ds->ds_clp->cl_count),
1788+
offset, ds->ds_remotestr, refcount_read(&ds->ds_clp->cl_count),
17891789
vers);
17901790

17911791
hdr->pgio_done_cb = ff_layout_write_done_cb;
1792-
atomic_inc(&ds->ds_clp->cl_count);
1792+
refcount_inc(&ds->ds_clp->cl_count);
17931793
hdr->ds_clp = ds->ds_clp;
17941794
hdr->ds_commit_idx = idx;
17951795
fh = nfs4_ff_layout_select_ds_fh(lseg, idx);
@@ -1863,11 +1863,11 @@ static int ff_layout_initiate_commit(struct nfs_commit_data *data, int how)
18631863
vers = nfs4_ff_layout_ds_version(lseg, idx);
18641864

18651865
dprintk("%s ino %lu, how %d cl_count %d vers %d\n", __func__,
1866-
data->inode->i_ino, how, atomic_read(&ds->ds_clp->cl_count),
1866+
data->inode->i_ino, how, refcount_read(&ds->ds_clp->cl_count),
18671867
vers);
18681868
data->commit_done_cb = ff_layout_commit_done_cb;
18691869
data->cred = ds_cred;
1870-
atomic_inc(&ds->ds_clp->cl_count);
1870+
refcount_inc(&ds->ds_clp->cl_count);
18711871
data->ds_clp = ds->ds_clp;
18721872
fh = select_ds_fh_from_commit(lseg, data->ds_commit_index);
18731873
if (fh)

‎fs/nfs/nfs4client.c

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -483,7 +483,7 @@ static int nfs4_match_client(struct nfs_client *pos, struct nfs_client *new,
483483
* ID and serverowner fields. Wait for CREATE_SESSION
484484
* to finish. */
485485
if (pos->cl_cons_state > NFS_CS_READY) {
486-
atomic_inc(&pos->cl_count);
486+
refcount_inc(&pos->cl_count);
487487
spin_unlock(&nn->nfs_client_lock);
488488

489489
nfs_put_client(*prev);
@@ -559,7 +559,7 @@ int nfs40_walk_client_list(struct nfs_client *new,
559559
* way that a SETCLIENTID_CONFIRM to pos can succeed is
560560
* if new and pos point to the same server:
561561
*/
562-
atomic_inc(&pos->cl_count);
562+
refcount_inc(&pos->cl_count);
563563
spin_unlock(&nn->nfs_client_lock);
564564

565565
nfs_put_client(prev);
@@ -715,7 +715,7 @@ int nfs41_walk_client_list(struct nfs_client *new,
715715
continue;
716716

717717
found:
718-
atomic_inc(&pos->cl_count);
718+
refcount_inc(&pos->cl_count);
719719
*result = pos;
720720
status = 0;
721721
break;
@@ -749,7 +749,7 @@ nfs4_find_client_ident(struct net *net, int cb_ident)
749749
spin_lock(&nn->nfs_client_lock);
750750
clp = idr_find(&nn->cb_ident_idr, cb_ident);
751751
if (clp)
752-
atomic_inc(&clp->cl_count);
752+
refcount_inc(&clp->cl_count);
753753
spin_unlock(&nn->nfs_client_lock);
754754
return clp;
755755
}
@@ -804,7 +804,7 @@ nfs4_find_client_sessionid(struct net *net, const struct sockaddr *addr,
804804
sid->data, NFS4_MAX_SESSIONID_LEN) != 0)
805805
continue;
806806

807-
atomic_inc(&clp->cl_count);
807+
refcount_inc(&clp->cl_count);
808808
spin_unlock(&nn->nfs_client_lock);
809809
return clp;
810810
}

‎fs/nfs/nfs4proc.c

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -4870,7 +4870,7 @@ static void nfs4_renew_release(void *calldata)
48704870
struct nfs4_renewdata *data = calldata;
48714871
struct nfs_client *clp = data->client;
48724872

4873-
if (atomic_read(&clp->cl_count) > 1)
4873+
if (refcount_read(&clp->cl_count) > 1)
48744874
nfs4_schedule_state_renewal(clp);
48754875
nfs_put_client(clp);
48764876
kfree(data);
@@ -4918,7 +4918,7 @@ static int nfs4_proc_async_renew(struct nfs_client *clp, struct rpc_cred *cred,
49184918

49194919
if (renew_flags == 0)
49204920
return 0;
4921-
if (!atomic_inc_not_zero(&clp->cl_count))
4921+
if (!refcount_inc_not_zero(&clp->cl_count))
49224922
return -EIO;
49234923
data = kmalloc(sizeof(*data), GFP_NOFS);
49244924
if (data == NULL) {
@@ -7499,7 +7499,7 @@ nfs4_run_exchange_id(struct nfs_client *clp, struct rpc_cred *cred,
74997499
struct nfs41_exchange_id_data *calldata;
75007500
int status;
75017501

7502-
if (!atomic_inc_not_zero(&clp->cl_count))
7502+
if (!refcount_inc_not_zero(&clp->cl_count))
75037503
return ERR_PTR(-EIO);
75047504

75057505
status = -ENOMEM;
@@ -8099,7 +8099,7 @@ static void nfs41_sequence_release(void *data)
80998099
struct nfs4_sequence_data *calldata = data;
81008100
struct nfs_client *clp = calldata->clp;
81018101

8102-
if (atomic_read(&clp->cl_count) > 1)
8102+
if (refcount_read(&clp->cl_count) > 1)
81038103
nfs4_schedule_state_renewal(clp);
81048104
nfs_put_client(clp);
81058105
kfree(calldata);
@@ -8128,7 +8128,7 @@ static void nfs41_sequence_call_done(struct rpc_task *task, void *data)
81288128
trace_nfs4_sequence(clp, task->tk_status);
81298129
if (task->tk_status < 0) {
81308130
dprintk("%s ERROR %d\n", __func__, task->tk_status);
8131-
if (atomic_read(&clp->cl_count) == 1)
8131+
if (refcount_read(&clp->cl_count) == 1)
81328132
goto out;
81338133

81348134
if (nfs41_sequence_handle_errors(task, clp) == -EAGAIN) {
@@ -8179,7 +8179,7 @@ static struct rpc_task *_nfs41_proc_sequence(struct nfs_client *clp,
81798179
struct rpc_task *ret;
81808180

81818181
ret = ERR_PTR(-EIO);
8182-
if (!atomic_inc_not_zero(&clp->cl_count))
8182+
if (!refcount_inc_not_zero(&clp->cl_count))
81838183
goto out_err;
81848184

81858185
ret = ERR_PTR(-ENOMEM);

‎fs/nfs/nfs4state.c

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1177,7 +1177,7 @@ void nfs4_schedule_state_manager(struct nfs_client *clp)
11771177
if (test_and_set_bit(NFS4CLNT_MANAGER_RUNNING, &clp->cl_state) != 0)
11781178
return;
11791179
__module_get(THIS_MODULE);
1180-
atomic_inc(&clp->cl_count);
1180+
refcount_inc(&clp->cl_count);
11811181

11821182
/* The rcu_read_lock() is not strictly necessary, as the state
11831183
* manager is the only thread that ever changes the rpc_xprt
@@ -1269,7 +1269,7 @@ int nfs4_wait_clnt_recover(struct nfs_client *clp)
12691269

12701270
might_sleep();
12711271

1272-
atomic_inc(&clp->cl_count);
1272+
refcount_inc(&clp->cl_count);
12731273
res = wait_on_bit_action(&clp->cl_state, NFS4CLNT_MANAGER_RUNNING,
12741274
nfs_wait_bit_killable, TASK_KILLABLE);
12751275
if (res)
@@ -2510,7 +2510,7 @@ static void nfs4_state_manager(struct nfs_client *clp)
25102510
break;
25112511
if (test_and_set_bit(NFS4CLNT_MANAGER_RUNNING, &clp->cl_state) != 0)
25122512
break;
2513-
} while (atomic_read(&clp->cl_count) > 1);
2513+
} while (refcount_read(&clp->cl_count) > 1);
25142514
return;
25152515
out_error:
25162516
if (strlen(section))

‎include/linux/nfs_fs_sb.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
#include <linux/sunrpc/xprt.h>
1010

1111
#include <linux/atomic.h>
12+
#include <linux/refcount.h>
1213

1314
struct nfs4_session;
1415
struct nfs_iostats;
@@ -24,7 +25,7 @@ struct nfs41_impl_id;
2425
* The nfs_client identifies our client state to the server.
2526
*/
2627
struct nfs_client {
27-
atomic_t cl_count;
28+
refcount_t cl_count;
2829
atomic_t cl_mds_count;
2930
int cl_cons_state; /* current construction state (-ve: init error) */
3031
#define NFS_CS_READY 0 /* ready to be used */

0 commit comments

Comments
 (0)