LinuxLists.cc - fs/nfsd/filecache.c:1118 nfsd_file_do_acquire() warn: passing freed memory 'nf'

2022-11-05 11:54:43

Subject: fs/nfsd/filecache.c:1118 nfsd_file_do_acquire() warn: passing freed memory 'nf'

tree: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master
head: b208b9fbbcba743fb269d15cb46a4036b01936b1
commit: 243a5263014a30436c93ed3f1f864c1da845455e nfsd: rework hashtable handling in nfsd_do_file_acquire
config: alpha-randconfig-m041-20221030
compiler: alpha-linux-gcc (GCC) 12.1.0

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <[email protected]>
| Reported-by: Dan Carpenter <[email protected]>

smatch warnings:
fs/nfsd/filecache.c:1118 nfsd_file_do_acquire() warn: passing freed memory 'nf'

vim +/nf +1118 fs/nfsd/filecache.c

fb70bf124b051d Chuck Lever 2022-03-30 1036 static __be32
be0230069fcbf7 Chuck Lever 2022-07-08 1037 nfsd_file_do_acquire(struct svc_rqst *rqstp, struct svc_fh *fhp,
fb70bf124b051d Chuck Lever 2022-03-30 1038 unsigned int may_flags, struct nfsd_file **pnf, bool open)
65294c1f2c5e72 Jeff Layton 2019-08-18 1039 {
ce502f81ba884c Chuck Lever 2022-07-08 1040 struct nfsd_file_lookup_key key = {
ce502f81ba884c Chuck Lever 2022-07-08 1041 .type = NFSD_FILE_KEY_FULL,
ce502f81ba884c Chuck Lever 2022-07-08 1042 .need = may_flags & NFSD_FILE_MAY_MASK,
ce502f81ba884c Chuck Lever 2022-07-08 1043 .net = SVC_NET(rqstp),
ce502f81ba884c Chuck Lever 2022-07-08 1044 };
243a5263014a30 Jeff Layton 2022-10-04 1045 bool open_retry = true;
243a5263014a30 Jeff Layton 2022-10-04 1046 struct nfsd_file *nf;
ce502f81ba884c Chuck Lever 2022-07-08 1047 __be32 status;
243a5263014a30 Jeff Layton 2022-10-04 1048 int ret;
65294c1f2c5e72 Jeff Layton 2019-08-18 1049
65294c1f2c5e72 Jeff Layton 2019-08-18 1050 status = fh_verify(rqstp, fhp, S_IFREG,
65294c1f2c5e72 Jeff Layton 2019-08-18 1051 may_flags|NFSD_MAY_OWNER_OVERRIDE);
65294c1f2c5e72 Jeff Layton 2019-08-18 1052 if (status != nfs_ok)
65294c1f2c5e72 Jeff Layton 2019-08-18 1053 return status;
ce502f81ba884c Chuck Lever 2022-07-08 1054 key.inode = d_inode(fhp->fh_dentry);
ce502f81ba884c Chuck Lever 2022-07-08 1055 key.cred = get_current_cred();
65294c1f2c5e72 Jeff Layton 2019-08-18 1056
65294c1f2c5e72 Jeff Layton 2019-08-18 1057 retry:
243a5263014a30 Jeff Layton 2022-10-04 1058 rcu_read_lock();
243a5263014a30 Jeff Layton 2022-10-04 1059 nf = rhashtable_lookup(&nfsd_file_rhash_tbl, &key,
ce502f81ba884c Chuck Lever 2022-07-08 1060 nfsd_file_rhash_params);
ce502f81ba884c Chuck Lever 2022-07-08 1061 if (nf)
ce502f81ba884c Chuck Lever 2022-07-08 1062 nf = nfsd_file_get(nf);
243a5263014a30 Jeff Layton 2022-10-04 1063 rcu_read_unlock();
65294c1f2c5e72 Jeff Layton 2019-08-18 1064 if (nf)
65294c1f2c5e72 Jeff Layton 2019-08-18 1065 goto wait_for_construction;
65294c1f2c5e72 Jeff Layton 2019-08-18 1066
243a5263014a30 Jeff Layton 2022-10-04 1067 nf = nfsd_file_alloc(&key, may_flags);
243a5263014a30 Jeff Layton 2022-10-04 1068 if (!nf) {
54f7df7094b329 Chuck Lever 2022-07-08 1069 status = nfserr_jukebox;
54f7df7094b329 Chuck Lever 2022-07-08 1070 goto out_status;
65294c1f2c5e72 Jeff Layton 2019-08-18 1071 }
65294c1f2c5e72 Jeff Layton 2019-08-18 1072
243a5263014a30 Jeff Layton 2022-10-04 1073 ret = rhashtable_lookup_insert_key(&nfsd_file_rhash_tbl,
243a5263014a30 Jeff Layton 2022-10-04 1074 &key, &nf->nf_rhash,
ce502f81ba884c Chuck Lever 2022-07-08 1075 nfsd_file_rhash_params);
243a5263014a30 Jeff Layton 2022-10-04 1076 if (likely(ret == 0))
65294c1f2c5e72 Jeff Layton 2019-08-18 1077 goto open_file;
243a5263014a30 Jeff Layton 2022-10-04 1078
243a5263014a30 Jeff Layton 2022-10-04 1079 nfsd_file_slab_free(&nf->nf_rcu);
243a5263014a30 Jeff Layton 2022-10-04 1080 if (ret == -EEXIST)
243a5263014a30 Jeff Layton 2022-10-04 1081 goto retry;
243a5263014a30 Jeff Layton 2022-10-04 1082 trace_nfsd_file_insert_err(rqstp, key.inode, may_flags, ret);
243a5263014a30 Jeff Layton 2022-10-04 1083 status = nfserr_jukebox;
243a5263014a30 Jeff Layton 2022-10-04 1084 goto out_status;
65294c1f2c5e72 Jeff Layton 2019-08-18 1085
65294c1f2c5e72 Jeff Layton 2019-08-18 1086 wait_for_construction:
65294c1f2c5e72 Jeff Layton 2019-08-18 1087 wait_on_bit(&nf->nf_flags, NFSD_FILE_PENDING, TASK_UNINTERRUPTIBLE);
65294c1f2c5e72 Jeff Layton 2019-08-18 1088
65294c1f2c5e72 Jeff Layton 2019-08-18 1089 /* Did construction of this file fail? */
65294c1f2c5e72 Jeff Layton 2019-08-18 1090 if (!test_bit(NFSD_FILE_HASHED, &nf->nf_flags)) {
ce502f81ba884c Chuck Lever 2022-07-08 1091 trace_nfsd_file_cons_err(rqstp, key.inode, may_flags, nf);
243a5263014a30 Jeff Layton 2022-10-04 1092 if (!open_retry) {
28c7d86bb6172f Trond Myklebust 2020-01-06 1093 status = nfserr_jukebox;
28c7d86bb6172f Trond Myklebust 2020-01-06 1094 goto out;
28c7d86bb6172f Trond Myklebust 2020-01-06 1095 }
243a5263014a30 Jeff Layton 2022-10-04 1096 open_retry = false;
65294c1f2c5e72 Jeff Layton 2019-08-18 1097 nfsd_file_put_noref(nf);
65294c1f2c5e72 Jeff Layton 2019-08-18 1098 goto retry;
65294c1f2c5e72 Jeff Layton 2019-08-18 1099 }
65294c1f2c5e72 Jeff Layton 2019-08-18 1100
4a0e73e635e3f3 Chuck Lever 2022-07-08 1101 nfsd_file_lru_remove(nf);
65294c1f2c5e72 Jeff Layton 2019-08-18 1102 this_cpu_inc(nfsd_file_cache_hits);
65294c1f2c5e72 Jeff Layton 2019-08-18 1103
23ba98de6dcec6 Jeff Layton 2022-07-29 1104 status = nfserrno(nfsd_open_break_lease(file_inode(nf->nf_file), may_flags));
65294c1f2c5e72 Jeff Layton 2019-08-18 1105 out:
65294c1f2c5e72 Jeff Layton 2019-08-18 1106 if (status == nfs_ok) {
29d4bdbbb910f3 Chuck Lever 2022-07-08 1107 if (open)
29d4bdbbb910f3 Chuck Lever 2022-07-08 1108 this_cpu_inc(nfsd_file_acquisitions);
65294c1f2c5e72 Jeff Layton 2019-08-18 1109 *pnf = nf;
65294c1f2c5e72 Jeff Layton 2019-08-18 1110 } else {
65294c1f2c5e72 Jeff Layton 2019-08-18 1111 nfsd_file_put(nf);
65294c1f2c5e72 Jeff Layton 2019-08-18 1112 nf = NULL;
65294c1f2c5e72 Jeff Layton 2019-08-18 1113 }
65294c1f2c5e72 Jeff Layton 2019-08-18 1114
54f7df7094b329 Chuck Lever 2022-07-08 1115 out_status:
ce502f81ba884c Chuck Lever 2022-07-08 1116 put_cred(key.cred);
be0230069fcbf7 Chuck Lever 2022-07-08 1117 if (open)
ce502f81ba884c Chuck Lever 2022-07-08 @1118 trace_nfsd_file_acquire(rqstp, key.inode, may_flags, nf, status);

Passing freed pointers to the trace code is a border line case... Not
sure if we care.

65294c1f2c5e72 Jeff Layton 2019-08-18 1119 return status;
65294c1f2c5e72 Jeff Layton 2019-08-18 1120
65294c1f2c5e72 Jeff Layton 2019-08-18 1121 open_file:
b40a2839470cd6 Chuck Lever 2022-07-08 1122 trace_nfsd_file_alloc(nf);
427f5f83a3191c Chuck Lever 2022-07-08 1123 nf->nf_mark = nfsd_file_mark_find_or_create(nf, key.inode);
fb70bf124b051d Chuck Lever 2022-03-30 1124 if (nf->nf_mark) {
0122e882119ddb Chuck Lever 2022-03-27 1125 if (open) {
f4d84c52643ae1 Chuck Lever 2022-03-27 1126 status = nfsd_open_verified(rqstp, fhp, may_flags,
f4d84c52643ae1 Chuck Lever 2022-03-27 1127 &nf->nf_file);
0122e882119ddb Chuck Lever 2022-03-27 1128 trace_nfsd_file_open(nf, status);

--
0-DAY CI Kernel Test Service
https://01.org/lkp

2022-11-05 13:33:33

by Jeff Layton

[permalink] [raw]

Subject: Re: fs/nfsd/filecache.c:1118 nfsd_file_do_acquire() warn: passing freed memory 'nf'

On Sat, 2022-11-05 at > tree: 14:17 +0300, Dan Carpenter wrote:
git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git">https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master
69d15cb46a4036b01936b1
93ed3f1f864c1da845455e nfsd: rework hashtable handling in nfsd_do_file_acquire
041-20221030
(GCC) 12.1.0
kindly add following tag where applicable
kernel test robot <[email protected]>
Dan Carpenter <[email protected]>
nfsd_file_do_acquire() warn: passing freed memory 'nf'
/> 2022-03-30 1036 static __be32
2022-07-08 1037 nfsd_file_do_acquire(struct svc_rqst *rqstp, struct svc_fh *fhp,
2022-03-30 1038 unsigned int may_flags, struct nfsd_file **pnf, bool open)
2019-08-18 1039 {
2022-07-08 1040 struct nfsd_file_lookup_key key = {
2022-07-08 1041 .type = NFSD_FILE_KEY_FULL,
2022-07-08 1042 .need = may_flags & NFSD_FILE_MAY_MASK,
2022-07-08 1043 .net = SVC_NET(rqstp),
2022-07-08 1044 };
2022-10-04 1045 bool open_retry = true;
2022-10-04 1046 struct nfsd_file *nf;
2022-07-08 1047 __be32 status;
2022-10-04 1048 int ret;
2019-08-18 1049
2019-08-18 1050 status = fh_verify(rqstp, fhp, S_IFREG,
2019-08-18 1051 may_flags|NFSD_MAY_OWNER_OVERRIDE);
2019-08-18 1052 if (status != nfs_ok)
2019-08-18 1053 return status;
2022-07-08 1054 key.inode = d_inode(fhp->fh_dentry);
2022-07-08 1055 key.cred = get_current_cred();
2019-08-18 1056
2019-08-18 1057 retry:
2022-10-04 1058 rcu_read_lock();
2022-10-04 1059 nf = rhashtable_lookup(&nfsd_file_rhash_tbl, &key,
2022-07-08 1060 nfsd_file_rhash_params);
2022-07-08 1061 if (nf)
2022-07-08 1062 nf = nfsd_file_get(nf);
2022-10-04 1063 rcu_read_unlock();
2019-08-18 1064 if (nf)
2019-08-18 1065 goto wait_for_construction;
2019-08-18 1066
2022-10-04 1067 nf = nfsd_file_alloc(&key, may_flags);
2022-10-04 1068 if (!nf) {
2022-07-08 1069 status = nfserr_jukebox;
2022-07-08 1070 goto out_status;
2019-08-18 1071 }
2019-08-18 1072
2022-10-04 1073 ret = rhashtable_lookup_insert_key(&nfsd_file_rhash_tbl,
2022-10-04 1074 &key, &nf->nf_rhash,
2022-07-08 1075 nfsd_file_rhash_params);
2022-10-04 1076 if (likely(ret == 0))
2019-08-18 1077 goto open_file;
2022-10-04 1078
2022-10-04 1079 nfsd_file_slab_free(&nf->nf_rcu);
2022-10-04 1080 if (ret == -EEXIST)
2022-10-04 1081 goto retry;
2022-10-04 1082 trace_nfsd_file_insert_err(rqstp, key.inode, may_flags, ret);
2022-10-04 1083 status = nfserr_jukebox;
2022-10-04 1084 goto out_status;
2019-08-18 1085
2019-08-18 1086 wait_for_construction:
2019-08-18 1087 wait_on_bit(&nf->nf_flags, NFSD_FILE_PENDING, TASK_UNINTERRUPTIBLE);
2019-08-18 1088
2019-08-18 1089 /* Did construction of this file fail? */
2019-08-18 1090 if (!test_bit(NFSD_FILE_HASHED, &nf->nf_flags)) {
2022-07-08 1091 trace_nfsd_file_cons_err(rqstp, key.inode, may_flags, nf);
2022-10-04 1092 if (!open_retry) {
Myklebust 2020-01-06 1093 status = nfserr_jukebox;
Myklebust 2020-01-06 1094 goto out;
Myklebust 2020-01-06 1095 }
2022-10-04 1096 open_retry = false;
2019-08-18 1097 nfsd_file_put_noref(nf);
2019-08-18 1098 goto retry;
2019-08-18 1099 }
2019-08-18 1100
2022-07-08 1101 nfsd_file_lru_remove(nf);
2019-08-18 1102 this_cpu_inc(nfsd_file_cache_hits);
2019-08-18 1103
2022-07-29 1104 status = nfserrno(nfsd_open_break_lease(file_inode(nf->nf_file), may_flags));
2019-08-18 1105 out:
2019-08-18 1106 if (status == nfs_ok) {
2022-07-08 1107 if (open)
2022-07-08 1108 this_cpu_inc(nfsd_file_acquisitions);
2019-08-18 1109 *pnf = nf;
2019-08-18 1110 } else {
2019-08-18 1111 nfsd_file_put(nf);
2019-08-18 1112 nf = NULL;
2019-08-18 1113 }
2019-08-18 1114
2022-07-08 1115 out_status:
2022-07-08 1116 put_cred(key.cred);
2022-07-08 1117 if (open)
2022-07-08 @1118 trace_nfsd_file_acquire(rqstp, key.inode, may_flags, nf, status);
to the trace code is a border line case... Not
a bug. I'll plan to spin up a patch to
2019-08-18 1119 return status;
2019-08-18 1120
2019-08-18 1121 open_file:
2022-07-08 1122 trace_nfsd_file_alloc(nf);
2022-07-08 1123 nf->nf_mark = nfsd_file_mark_find_or_create(nf, key.inode);
2022-03-30 1124 if (nf->nf_mark) {
2022-03-27 1125 if (open) {
2022-03-27 1126 status = nfsd_open_verified(rqstp, fhp, may_flags,
2022-03-27 1127 &nf->nf_file);
2022-03-27 1128 trace_nfsd_file_open(nf, status);
/email-protection#1c3a70682776707d656873725c77796e72797032736e7b3a7b6827"><[email protected]>

2022-11-05 16:23:43

by Chuck Lever III

[permalink] [raw]

Subject: Re: fs/nfsd/filecache.c:1118 nfsd_file_do_acquire() warn: passing freed memory 'nf'

> On Nov 5, 2022, at 7:17 AM, Dan Carpenter <[email protected]> wrote:
>
> tree: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master
> head: b208b9fbbcba743fb269d15cb46a4036b01936b1
> commit: 243a5263014a30436c93ed3f1f864c1da845455e nfsd: rework hashtable handling in nfsd_do_file_acquire
> config: alpha-randconfig-m041-20221030
> compiler: alpha-linux-gcc (GCC) 12.1.0
>
> If you fix the issue, kindly add following tag where applicable
> | Reported-by: kernel test robot <[email protected]>
> | Reported-by: Dan Carpenter <[email protected]>
>
> smatch warnings:
> fs/nfsd/filecache.c:1118 nfsd_file_do_acquire() warn: passing freed memory 'nf'
>
> vim +/nf +1118 fs/nfsd/filecache.c
>
> fb70bf124b051d Chuck Lever 2022-03-30 1036 static __be32
> be0230069fcbf7 Chuck Lever 2022-07-08 1037 nfsd_file_do_acquire(struct svc_rqst *rqstp, struct svc_fh *fhp,
> fb70bf124b051d Chuck Lever 2022-03-30 1038 unsigned int may_flags, struct nfsd_file **pnf, bool open)
> 65294c1f2c5e72 Jeff Layton 2019-08-18 1039 {
> ce502f81ba884c Chuck Lever 2022-07-08 1040 struct nfsd_file_lookup_key key = {
> ce502f81ba884c Chuck Lever 2022-07-08 1041 .type = NFSD_FILE_KEY_FULL,
> ce502f81ba884c Chuck Lever 2022-07-08 1042 .need = may_flags & NFSD_FILE_MAY_MASK,
> ce502f81ba884c Chuck Lever 2022-07-08 1043 .net = SVC_NET(rqstp),
> ce502f81ba884c Chuck Lever 2022-07-08 1044 };
> 243a5263014a30 Jeff Layton 2022-10-04 1045 bool open_retry = true;
> 243a5263014a30 Jeff Layton 2022-10-04 1046 struct nfsd_file *nf;
> ce502f81ba884c Chuck Lever 2022-07-08 1047 __be32 status;
> 243a5263014a30 Jeff Layton 2022-10-04 1048 int ret;
> 65294c1f2c5e72 Jeff Layton 2019-08-18 1049
> 65294c1f2c5e72 Jeff Layton 2019-08-18 1050 status = fh_verify(rqstp, fhp, S_IFREG,
> 65294c1f2c5e72 Jeff Layton 2019-08-18 1051 may_flags|NFSD_MAY_OWNER_OVERRIDE);
> 65294c1f2c5e72 Jeff Layton 2019-08-18 1052 if (status != nfs_ok)
> 65294c1f2c5e72 Jeff Layton 2019-08-18 1053 return status;
> ce502f81ba884c Chuck Lever 2022-07-08 1054 key.inode = d_inode(fhp->fh_dentry);
> ce502f81ba884c Chuck Lever 2022-07-08 1055 key.cred = get_current_cred();
> 65294c1f2c5e72 Jeff Layton 2019-08-18 1056
> 65294c1f2c5e72 Jeff Layton 2019-08-18 1057 retry:
> 243a5263014a30 Jeff Layton 2022-10-04 1058 rcu_read_lock();
> 243a5263014a30 Jeff Layton 2022-10-04 1059 nf = rhashtable_lookup(&nfsd_file_rhash_tbl, &key,
> ce502f81ba884c Chuck Lever 2022-07-08 1060 nfsd_file_rhash_params);
> ce502f81ba884c Chuck Lever 2022-07-08 1061 if (nf)
> ce502f81ba884c Chuck Lever 2022-07-08 1062 nf = nfsd_file_get(nf);
> 243a5263014a30 Jeff Layton 2022-10-04 1063 rcu_read_unlock();
> 65294c1f2c5e72 Jeff Layton 2019-08-18 1064 if (nf)
> 65294c1f2c5e72 Jeff Layton 2019-08-18 1065 goto wait_for_construction;
> 65294c1f2c5e72 Jeff Layton 2019-08-18 1066
> 243a5263014a30 Jeff Layton 2022-10-04 1067 nf = nfsd_file_alloc(&key, may_flags);
> 243a5263014a30 Jeff Layton 2022-10-04 1068 if (!nf) {
> 54f7df7094b329 Chuck Lever 2022-07-08 1069 status = nfserr_jukebox;
> 54f7df7094b329 Chuck Lever 2022-07-08 1070 goto out_status;
> 65294c1f2c5e72 Jeff Layton 2019-08-18 1071 }
> 65294c1f2c5e72 Jeff Layton 2019-08-18 1072
> 243a5263014a30 Jeff Layton 2022-10-04 1073 ret = rhashtable_lookup_insert_key(&nfsd_file_rhash_tbl,
> 243a5263014a30 Jeff Layton 2022-10-04 1074 &key, &nf->nf_rhash,
> ce502f81ba884c Chuck Lever 2022-07-08 1075 nfsd_file_rhash_params);
> 243a5263014a30 Jeff Layton 2022-10-04 1076 if (likely(ret == 0))
> 65294c1f2c5e72 Jeff Layton 2019-08-18 1077 goto open_file;
> 243a5263014a30 Jeff Layton 2022-10-04 1078
> 243a5263014a30 Jeff Layton 2022-10-04 1079 nfsd_file_slab_free(&nf->nf_rcu);
> 243a5263014a30 Jeff Layton 2022-10-04 1080 if (ret == -EEXIST)
> 243a5263014a30 Jeff Layton 2022-10-04 1081 goto retry;
> 243a5263014a30 Jeff Layton 2022-10-04 1082 trace_nfsd_file_insert_err(rqstp, key.inode, may_flags, ret);
> 243a5263014a30 Jeff Layton 2022-10-04 1083 status = nfserr_jukebox;
> 243a5263014a30 Jeff Layton 2022-10-04 1084 goto out_status;
> 65294c1f2c5e72 Jeff Layton 2019-08-18 1085
> 65294c1f2c5e72 Jeff Layton 2019-08-18 1086 wait_for_construction:
> 65294c1f2c5e72 Jeff Layton 2019-08-18 1087 wait_on_bit(&nf->nf_flags, NFSD_FILE_PENDING, TASK_UNINTERRUPTIBLE);
> 65294c1f2c5e72 Jeff Layton 2019-08-18 1088
> 65294c1f2c5e72 Jeff Layton 2019-08-18 1089 /* Did construction of this file fail? */
> 65294c1f2c5e72 Jeff Layton 2019-08-18 1090 if (!test_bit(NFSD_FILE_HASHED, &nf->nf_flags)) {
> ce502f81ba884c Chuck Lever 2022-07-08 1091 trace_nfsd_file_cons_err(rqstp, key.inode, may_flags, nf);
> 243a5263014a30 Jeff Layton 2022-10-04 1092 if (!open_retry) {
> 28c7d86bb6172f Trond Myklebust 2020-01-06 1093 status = nfserr_jukebox;
> 28c7d86bb6172f Trond Myklebust 2020-01-06 1094 goto out;
> 28c7d86bb6172f Trond Myklebust 2020-01-06 1095 }
> 243a5263014a30 Jeff Layton 2022-10-04 1096 open_retry = false;
> 65294c1f2c5e72 Jeff Layton 2019-08-18 1097 nfsd_file_put_noref(nf);
> 65294c1f2c5e72 Jeff Layton 2019-08-18 1098 goto retry;
> 65294c1f2c5e72 Jeff Layton 2019-08-18 1099 }
> 65294c1f2c5e72 Jeff Layton 2019-08-18 1100
> 4a0e73e635e3f3 Chuck Lever 2022-07-08 1101 nfsd_file_lru_remove(nf);
> 65294c1f2c5e72 Jeff Layton 2019-08-18 1102 this_cpu_inc(nfsd_file_cache_hits);
> 65294c1f2c5e72 Jeff Layton 2019-08-18 1103
> 23ba98de6dcec6 Jeff Layton 2022-07-29 1104 status = nfserrno(nfsd_open_break_lease(file_inode(nf->nf_file), may_flags));
> 65294c1f2c5e72 Jeff Layton 2019-08-18 1105 out:
> 65294c1f2c5e72 Jeff Layton 2019-08-18 1106 if (status == nfs_ok) {
> 29d4bdbbb910f3 Chuck Lever 2022-07-08 1107 if (open)
> 29d4bdbbb910f3 Chuck Lever 2022-07-08 1108 this_cpu_inc(nfsd_file_acquisitions);
> 65294c1f2c5e72 Jeff Layton 2019-08-18 1109 *pnf = nf;
> 65294c1f2c5e72 Jeff Layton 2019-08-18 1110 } else {
> 65294c1f2c5e72 Jeff Layton 2019-08-18 1111 nfsd_file_put(nf);
> 65294c1f2c5e72 Jeff Layton 2019-08-18 1112 nf = NULL;
> 65294c1f2c5e72 Jeff Layton 2019-08-18 1113 }
> 65294c1f2c5e72 Jeff Layton 2019-08-18 1114
> 54f7df7094b329 Chuck Lever 2022-07-08 1115 out_status:
> ce502f81ba884c Chuck Lever 2022-07-08 1116 put_cred(key.cred);
> be0230069fcbf7 Chuck Lever 2022-07-08 1117 if (open)
> ce502f81ba884c Chuck Lever 2022-07-08 @1118 trace_nfsd_file_acquire(rqstp, key.inode, may_flags, nf, status);
>
> Passing freed pointers to the trace code is a border line case... Not
> sure if we care.

Speaking only for NFSD, let me echo Jeff and say yes, we care.

In general, I think Linux users/customers and of course our fellow
sustaining engineers want to feel confident that enabling tracing
on a production kernel isn't going to result in less stability.

Thanks for the report!

> 65294c1f2c5e72 Jeff Layton 2019-08-18 1119 return status;
> 65294c1f2c5e72 Jeff Layton 2019-08-18 1120
> 65294c1f2c5e72 Jeff Layton 2019-08-18 1121 open_file:
> b40a2839470cd6 Chuck Lever 2022-07-08 1122 trace_nfsd_file_alloc(nf);
> 427f5f83a3191c Chuck Lever 2022-07-08 1123 nf->nf_mark = nfsd_file_mark_find_or_create(nf, key.inode);
> fb70bf124b051d Chuck Lever 2022-03-30 1124 if (nf->nf_mark) {
> 0122e882119ddb Chuck Lever 2022-03-27 1125 if (open) {
> f4d84c52643ae1 Chuck Lever 2022-03-27 1126 status = nfsd_open_verified(rqstp, fhp, may_flags,
> f4d84c52643ae1 Chuck Lever 2022-03-27 1127 &nf->nf_file);
> 0122e882119ddb Chuck Lever 2022-03-27 1128 trace_nfsd_file_open(nf, status);
>
> --
> 0-DAY CI Kernel Test Service
> https://01.org/lkp
>

--
Chuck Lever

2022-11-07 07:57:38

by Dan Carpenter

[permalink] [raw]

Subject: Re: fs/nfsd/filecache.c:1118 nfsd_file_do_acquire() warn: passing freed memory 'nf'

On Sat, Nov 05, 2022 at 03:39:59PM +0000, Chuck Lever III wrote:
>
>
> > On Nov 5, 2022, at 7:17 AM, Dan Carpenter <[email protected]> wrote:
> >
> > tree: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master
> > head: b208b9fbbcba743fb269d15cb46a4036b01936b1
> > commit: 243a5263014a30436c93ed3f1f864c1da845455e nfsd: rework hashtable handling in nfsd_do_file_acquire
> > config: alpha-randconfig-m041-20221030
> > compiler: alpha-linux-gcc (GCC) 12.1.0
> >
> > If you fix the issue, kindly add following tag where applicable
> > | Reported-by: kernel test robot <[email protected]>
> > | Reported-by: Dan Carpenter <[email protected]>
> >
> > smatch warnings:
> > fs/nfsd/filecache.c:1118 nfsd_file_do_acquire() warn: passing freed memory 'nf'
> >
> > vim +/nf +1118 fs/nfsd/filecache.c
> >
> > fb70bf124b051d Chuck Lever 2022-03-30 1036 static __be32
> > be0230069fcbf7 Chuck Lever 2022-07-08 1037 nfsd_file_do_acquire(struct svc_rqst *rqstp, struct svc_fh *fhp,
> > fb70bf124b051d Chuck Lever 2022-03-30 1038 unsigned int may_flags, struct nfsd_file **pnf, bool open)
> > 65294c1f2c5e72 Jeff Layton 2019-08-18 1039 {
> > ce502f81ba884c Chuck Lever 2022-07-08 1040 struct nfsd_file_lookup_key key = {
> > ce502f81ba884c Chuck Lever 2022-07-08 1041 .type = NFSD_FILE_KEY_FULL,
> > ce502f81ba884c Chuck Lever 2022-07-08 1042 .need = may_flags & NFSD_FILE_MAY_MASK,
> > ce502f81ba884c Chuck Lever 2022-07-08 1043 .net = SVC_NET(rqstp),
> > ce502f81ba884c Chuck Lever 2022-07-08 1044 };
> > 243a5263014a30 Jeff Layton 2022-10-04 1045 bool open_retry = true;
> > 243a5263014a30 Jeff Layton 2022-10-04 1046 struct nfsd_file *nf;
> > ce502f81ba884c Chuck Lever 2022-07-08 1047 __be32 status;
> > 243a5263014a30 Jeff Layton 2022-10-04 1048 int ret;
> > 65294c1f2c5e72 Jeff Layton 2019-08-18 1049
> > 65294c1f2c5e72 Jeff Layton 2019-08-18 1050 status = fh_verify(rqstp, fhp, S_IFREG,
> > 65294c1f2c5e72 Jeff Layton 2019-08-18 1051 may_flags|NFSD_MAY_OWNER_OVERRIDE);
> > 65294c1f2c5e72 Jeff Layton 2019-08-18 1052 if (status != nfs_ok)
> > 65294c1f2c5e72 Jeff Layton 2019-08-18 1053 return status;
> > ce502f81ba884c Chuck Lever 2022-07-08 1054 key.inode = d_inode(fhp->fh_dentry);
> > ce502f81ba884c Chuck Lever 2022-07-08 1055 key.cred = get_current_cred();
> > 65294c1f2c5e72 Jeff Layton 2019-08-18 1056
> > 65294c1f2c5e72 Jeff Layton 2019-08-18 1057 retry:
> > 243a5263014a30 Jeff Layton 2022-10-04 1058 rcu_read_lock();
> > 243a5263014a30 Jeff Layton 2022-10-04 1059 nf = rhashtable_lookup(&nfsd_file_rhash_tbl, &key,
> > ce502f81ba884c Chuck Lever 2022-07-08 1060 nfsd_file_rhash_params);
> > ce502f81ba884c Chuck Lever 2022-07-08 1061 if (nf)
> > ce502f81ba884c Chuck Lever 2022-07-08 1062 nf = nfsd_file_get(nf);
> > 243a5263014a30 Jeff Layton 2022-10-04 1063 rcu_read_unlock();
> > 65294c1f2c5e72 Jeff Layton 2019-08-18 1064 if (nf)
> > 65294c1f2c5e72 Jeff Layton 2019-08-18 1065 goto wait_for_construction;
> > 65294c1f2c5e72 Jeff Layton 2019-08-18 1066
> > 243a5263014a30 Jeff Layton 2022-10-04 1067 nf = nfsd_file_alloc(&key, may_flags);
> > 243a5263014a30 Jeff Layton 2022-10-04 1068 if (!nf) {
> > 54f7df7094b329 Chuck Lever 2022-07-08 1069 status = nfserr_jukebox;
> > 54f7df7094b329 Chuck Lever 2022-07-08 1070 goto out_status;
> > 65294c1f2c5e72 Jeff Layton 2019-08-18 1071 }
> > 65294c1f2c5e72 Jeff Layton 2019-08-18 1072
> > 243a5263014a30 Jeff Layton 2022-10-04 1073 ret = rhashtable_lookup_insert_key(&nfsd_file_rhash_tbl,
> > 243a5263014a30 Jeff Layton 2022-10-04 1074 &key, &nf->nf_rhash,
> > ce502f81ba884c Chuck Lever 2022-07-08 1075 nfsd_file_rhash_params);
> > 243a5263014a30 Jeff Layton 2022-10-04 1076 if (likely(ret == 0))
> > 65294c1f2c5e72 Jeff Layton 2019-08-18 1077 goto open_file;
> > 243a5263014a30 Jeff Layton 2022-10-04 1078
> > 243a5263014a30 Jeff Layton 2022-10-04 1079 nfsd_file_slab_free(&nf->nf_rcu);
> > 243a5263014a30 Jeff Layton 2022-10-04 1080 if (ret == -EEXIST)
> > 243a5263014a30 Jeff Layton 2022-10-04 1081 goto retry;
> > 243a5263014a30 Jeff Layton 2022-10-04 1082 trace_nfsd_file_insert_err(rqstp, key.inode, may_flags, ret);
> > 243a5263014a30 Jeff Layton 2022-10-04 1083 status = nfserr_jukebox;
> > 243a5263014a30 Jeff Layton 2022-10-04 1084 goto out_status;
> > 65294c1f2c5e72 Jeff Layton 2019-08-18 1085
> > 65294c1f2c5e72 Jeff Layton 2019-08-18 1086 wait_for_construction:
> > 65294c1f2c5e72 Jeff Layton 2019-08-18 1087 wait_on_bit(&nf->nf_flags, NFSD_FILE_PENDING, TASK_UNINTERRUPTIBLE);
> > 65294c1f2c5e72 Jeff Layton 2019-08-18 1088
> > 65294c1f2c5e72 Jeff Layton 2019-08-18 1089 /* Did construction of this file fail? */
> > 65294c1f2c5e72 Jeff Layton 2019-08-18 1090 if (!test_bit(NFSD_FILE_HASHED, &nf->nf_flags)) {
> > ce502f81ba884c Chuck Lever 2022-07-08 1091 trace_nfsd_file_cons_err(rqstp, key.inode, may_flags, nf);
> > 243a5263014a30 Jeff Layton 2022-10-04 1092 if (!open_retry) {
> > 28c7d86bb6172f Trond Myklebust 2020-01-06 1093 status = nfserr_jukebox;
> > 28c7d86bb6172f Trond Myklebust 2020-01-06 1094 goto out;
> > 28c7d86bb6172f Trond Myklebust 2020-01-06 1095 }
> > 243a5263014a30 Jeff Layton 2022-10-04 1096 open_retry = false;
> > 65294c1f2c5e72 Jeff Layton 2019-08-18 1097 nfsd_file_put_noref(nf);
> > 65294c1f2c5e72 Jeff Layton 2019-08-18 1098 goto retry;
> > 65294c1f2c5e72 Jeff Layton 2019-08-18 1099 }
> > 65294c1f2c5e72 Jeff Layton 2019-08-18 1100
> > 4a0e73e635e3f3 Chuck Lever 2022-07-08 1101 nfsd_file_lru_remove(nf);
> > 65294c1f2c5e72 Jeff Layton 2019-08-18 1102 this_cpu_inc(nfsd_file_cache_hits);
> > 65294c1f2c5e72 Jeff Layton 2019-08-18 1103
> > 23ba98de6dcec6 Jeff Layton 2022-07-29 1104 status = nfserrno(nfsd_open_break_lease(file_inode(nf->nf_file), may_flags));
> > 65294c1f2c5e72 Jeff Layton 2019-08-18 1105 out:
> > 65294c1f2c5e72 Jeff Layton 2019-08-18 1106 if (status == nfs_ok) {
> > 29d4bdbbb910f3 Chuck Lever 2022-07-08 1107 if (open)
> > 29d4bdbbb910f3 Chuck Lever 2022-07-08 1108 this_cpu_inc(nfsd_file_acquisitions);
> > 65294c1f2c5e72 Jeff Layton 2019-08-18 1109 *pnf = nf;
> > 65294c1f2c5e72 Jeff Layton 2019-08-18 1110 } else {
> > 65294c1f2c5e72 Jeff Layton 2019-08-18 1111 nfsd_file_put(nf);
> > 65294c1f2c5e72 Jeff Layton 2019-08-18 1112 nf = NULL;
> > 65294c1f2c5e72 Jeff Layton 2019-08-18 1113 }
> > 65294c1f2c5e72 Jeff Layton 2019-08-18 1114
> > 54f7df7094b329 Chuck Lever 2022-07-08 1115 out_status:
> > ce502f81ba884c Chuck Lever 2022-07-08 1116 put_cred(key.cred);
> > be0230069fcbf7 Chuck Lever 2022-07-08 1117 if (open)
> > ce502f81ba884c Chuck Lever 2022-07-08 @1118 trace_nfsd_file_acquire(rqstp, key.inode, may_flags, nf, status);
> >
> > Passing freed pointers to the trace code is a border line case... Not
> > sure if we care.
>
> Speaking only for NFSD, let me echo Jeff and say yes, we care.
>
> In general, I think Linux users/customers and of course our fellow
> sustaining engineers want to feel confident that enabling tracing
> on a production kernel isn't going to result in less stability.
>
> Thanks for the report!

Your welcome. What I meant was that it's only a problem if we
dereference the pointer... (I haven't used tracing output so I wasn't
sure on this).

regards,
dan carpenter

2022-11-07 14:42:19

by Chuck Lever III

[permalink] [raw]

Subject: Re: fs/nfsd/filecache.c:1118 nfsd_file_do_acquire() warn: passing freed memory 'nf'

> On Nov 7, 2022, at 2:48 AM, Dan Carpenter <[email protected]> wrote:
>
> On Sat, Nov 05, 2022 at 03:39:59PM +0000, Chuck Lever III wrote:
>>
>>
>>> On Nov 5, 2022, at 7:17 AM, Dan Carpenter <[email protected]> wrote:
>>>
>>> tree: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master
>>> head: b208b9fbbcba743fb269d15cb46a4036b01936b1
>>> commit: 243a5263014a30436c93ed3f1f864c1da845455e nfsd: rework hashtable handling in nfsd_do_file_acquire
>>> config: alpha-randconfig-m041-20221030
>>> compiler: alpha-linux-gcc (GCC) 12.1.0
>>>
>>> If you fix the issue, kindly add following tag where applicable
>>> | Reported-by: kernel test robot <[email protected]>
>>> | Reported-by: Dan Carpenter <[email protected]>
>>>
>>> smatch warnings:
>>> fs/nfsd/filecache.c:1118 nfsd_file_do_acquire() warn: passing freed memory 'nf'
>>>
>>> vim +/nf +1118 fs/nfsd/filecache.c
>>>
>>> fb70bf124b051d Chuck Lever 2022-03-30 1036 static __be32
>>> be0230069fcbf7 Chuck Lever 2022-07-08 1037 nfsd_file_do_acquire(struct svc_rqst *rqstp, struct svc_fh *fhp,
>>> fb70bf124b051d Chuck Lever 2022-03-30 1038 unsigned int may_flags, struct nfsd_file **pnf, bool open)
>>> 65294c1f2c5e72 Jeff Layton 2019-08-18 1039 {
>>> ce502f81ba884c Chuck Lever 2022-07-08 1040 struct nfsd_file_lookup_key key = {
>>> ce502f81ba884c Chuck Lever 2022-07-08 1041 .type = NFSD_FILE_KEY_FULL,
>>> ce502f81ba884c Chuck Lever 2022-07-08 1042 .need = may_flags & NFSD_FILE_MAY_MASK,
>>> ce502f81ba884c Chuck Lever 2022-07-08 1043 .net = SVC_NET(rqstp),
>>> ce502f81ba884c Chuck Lever 2022-07-08 1044 };
>>> 243a5263014a30 Jeff Layton 2022-10-04 1045 bool open_retry = true;
>>> 243a5263014a30 Jeff Layton 2022-10-04 1046 struct nfsd_file *nf;
>>> ce502f81ba884c Chuck Lever 2022-07-08 1047 __be32 status;
>>> 243a5263014a30 Jeff Layton 2022-10-04 1048 int ret;
>>> 65294c1f2c5e72 Jeff Layton 2019-08-18 1049
>>> 65294c1f2c5e72 Jeff Layton 2019-08-18 1050 status = fh_verify(rqstp, fhp, S_IFREG,
>>> 65294c1f2c5e72 Jeff Layton 2019-08-18 1051 may_flags|NFSD_MAY_OWNER_OVERRIDE);
>>> 65294c1f2c5e72 Jeff Layton 2019-08-18 1052 if (status != nfs_ok)
>>> 65294c1f2c5e72 Jeff Layton 2019-08-18 1053 return status;
>>> ce502f81ba884c Chuck Lever 2022-07-08 1054 key.inode = d_inode(fhp->fh_dentry);
>>> ce502f81ba884c Chuck Lever 2022-07-08 1055 key.cred = get_current_cred();
>>> 65294c1f2c5e72 Jeff Layton 2019-08-18 1056
>>> 65294c1f2c5e72 Jeff Layton 2019-08-18 1057 retry:
>>> 243a5263014a30 Jeff Layton 2022-10-04 1058 rcu_read_lock();
>>> 243a5263014a30 Jeff Layton 2022-10-04 1059 nf = rhashtable_lookup(&nfsd_file_rhash_tbl, &key,
>>> ce502f81ba884c Chuck Lever 2022-07-08 1060 nfsd_file_rhash_params);
>>> ce502f81ba884c Chuck Lever 2022-07-08 1061 if (nf)
>>> ce502f81ba884c Chuck Lever 2022-07-08 1062 nf = nfsd_file_get(nf);
>>> 243a5263014a30 Jeff Layton 2022-10-04 1063 rcu_read_unlock();
>>> 65294c1f2c5e72 Jeff Layton 2019-08-18 1064 if (nf)
>>> 65294c1f2c5e72 Jeff Layton 2019-08-18 1065 goto wait_for_construction;
>>> 65294c1f2c5e72 Jeff Layton 2019-08-18 1066
>>> 243a5263014a30 Jeff Layton 2022-10-04 1067 nf = nfsd_file_alloc(&key, may_flags);
>>> 243a5263014a30 Jeff Layton 2022-10-04 1068 if (!nf) {
>>> 54f7df7094b329 Chuck Lever 2022-07-08 1069 status = nfserr_jukebox;
>>> 54f7df7094b329 Chuck Lever 2022-07-08 1070 goto out_status;
>>> 65294c1f2c5e72 Jeff Layton 2019-08-18 1071 }
>>> 65294c1f2c5e72 Jeff Layton 2019-08-18 1072
>>> 243a5263014a30 Jeff Layton 2022-10-04 1073 ret = rhashtable_lookup_insert_key(&nfsd_file_rhash_tbl,
>>> 243a5263014a30 Jeff Layton 2022-10-04 1074 &key, &nf->nf_rhash,
>>> ce502f81ba884c Chuck Lever 2022-07-08 1075 nfsd_file_rhash_params);
>>> 243a5263014a30 Jeff Layton 2022-10-04 1076 if (likely(ret == 0))
>>> 65294c1f2c5e72 Jeff Layton 2019-08-18 1077 goto open_file;
>>> 243a5263014a30 Jeff Layton 2022-10-04 1078
>>> 243a5263014a30 Jeff Layton 2022-10-04 1079 nfsd_file_slab_free(&nf->nf_rcu);
>>> 243a5263014a30 Jeff Layton 2022-10-04 1080 if (ret == -EEXIST)
>>> 243a5263014a30 Jeff Layton 2022-10-04 1081 goto retry;
>>> 243a5263014a30 Jeff Layton 2022-10-04 1082 trace_nfsd_file_insert_err(rqstp, key.inode, may_flags, ret);
>>> 243a5263014a30 Jeff Layton 2022-10-04 1083 status = nfserr_jukebox;
>>> 243a5263014a30 Jeff Layton 2022-10-04 1084 goto out_status;
>>> 65294c1f2c5e72 Jeff Layton 2019-08-18 1085
>>> 65294c1f2c5e72 Jeff Layton 2019-08-18 1086 wait_for_construction:
>>> 65294c1f2c5e72 Jeff Layton 2019-08-18 1087 wait_on_bit(&nf->nf_flags, NFSD_FILE_PENDING, TASK_UNINTERRUPTIBLE);
>>> 65294c1f2c5e72 Jeff Layton 2019-08-18 1088
>>> 65294c1f2c5e72 Jeff Layton 2019-08-18 1089 /* Did construction of this file fail? */
>>> 65294c1f2c5e72 Jeff Layton 2019-08-18 1090 if (!test_bit(NFSD_FILE_HASHED, &nf->nf_flags)) {
>>> ce502f81ba884c Chuck Lever 2022-07-08 1091 trace_nfsd_file_cons_err(rqstp, key.inode, may_flags, nf);
>>> 243a5263014a30 Jeff Layton 2022-10-04 1092 if (!open_retry) {
>>> 28c7d86bb6172f Trond Myklebust 2020-01-06 1093 status = nfserr_jukebox;
>>> 28c7d86bb6172f Trond Myklebust 2020-01-06 1094 goto out;
>>> 28c7d86bb6172f Trond Myklebust 2020-01-06 1095 }
>>> 243a5263014a30 Jeff Layton 2022-10-04 1096 open_retry = false;
>>> 65294c1f2c5e72 Jeff Layton 2019-08-18 1097 nfsd_file_put_noref(nf);
>>> 65294c1f2c5e72 Jeff Layton 2019-08-18 1098 goto retry;
>>> 65294c1f2c5e72 Jeff Layton 2019-08-18 1099 }
>>> 65294c1f2c5e72 Jeff Layton 2019-08-18 1100
>>> 4a0e73e635e3f3 Chuck Lever 2022-07-08 1101 nfsd_file_lru_remove(nf);
>>> 65294c1f2c5e72 Jeff Layton 2019-08-18 1102 this_cpu_inc(nfsd_file_cache_hits);
>>> 65294c1f2c5e72 Jeff Layton 2019-08-18 1103
>>> 23ba98de6dcec6 Jeff Layton 2022-07-29 1104 status = nfserrno(nfsd_open_break_lease(file_inode(nf->nf_file), may_flags));
>>> 65294c1f2c5e72 Jeff Layton 2019-08-18 1105 out:
>>> 65294c1f2c5e72 Jeff Layton 2019-08-18 1106 if (status == nfs_ok) {
>>> 29d4bdbbb910f3 Chuck Lever 2022-07-08 1107 if (open)
>>> 29d4bdbbb910f3 Chuck Lever 2022-07-08 1108 this_cpu_inc(nfsd_file_acquisitions);
>>> 65294c1f2c5e72 Jeff Layton 2019-08-18 1109 *pnf = nf;
>>> 65294c1f2c5e72 Jeff Layton 2019-08-18 1110 } else {
>>> 65294c1f2c5e72 Jeff Layton 2019-08-18 1111 nfsd_file_put(nf);
>>> 65294c1f2c5e72 Jeff Layton 2019-08-18 1112 nf = NULL;
>>> 65294c1f2c5e72 Jeff Layton 2019-08-18 1113 }
>>> 65294c1f2c5e72 Jeff Layton 2019-08-18 1114
>>> 54f7df7094b329 Chuck Lever 2022-07-08 1115 out_status:
>>> ce502f81ba884c Chuck Lever 2022-07-08 1116 put_cred(key.cred);
>>> be0230069fcbf7 Chuck Lever 2022-07-08 1117 if (open)
>>> ce502f81ba884c Chuck Lever 2022-07-08 @1118 trace_nfsd_file_acquire(rqstp, key.inode, may_flags, nf, status);
>>>
>>> Passing freed pointers to the trace code is a border line case... Not
>>> sure if we care.
>>
>> Speaking only for NFSD, let me echo Jeff and say yes, we care.
>>
>> In general, I think Linux users/customers and of course our fellow
>> sustaining engineers want to feel confident that enabling tracing
>> on a production kernel isn't going to result in less stability.
>>
>> Thanks for the report!
>
> Your welcome. What I meant was that it's only a problem if we
> dereference the pointer... (I haven't used tracing output so I wasn't
> sure on this).

Following up: yes, the @nf parameter happens to be dereferenced inside
the nfsd_file_acquire tracepoint (the @inode parameter is not, though).

--
Chuck Lever