Messages in this thread | | | From | Chuck Lever III <> | Subject | Re: fs/nfsd/filecache.c:1118 nfsd_file_do_acquire() warn: passing freed memory 'nf' | Date | Mon, 7 Nov 2022 14:07:07 +0000 |
| |
> On Nov 7, 2022, at 2:48 AM, Dan Carpenter <error27@gmail.com> 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 <error27@gmail.com> 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 <lkp@intel.com> >>> | Reported-by: Dan Carpenter <error27@gmail.com> >>> >>> 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
| |