public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* fs/nfsd/filecache.c:1118 nfsd_file_do_acquire() warn: passing freed memory 'nf'
@ 2022-11-05 11:17 Dan Carpenter
  2022-11-05 13:21 ` Jeff Layton
  2022-11-05 15:39 ` Chuck Lever III
  0 siblings, 2 replies; 5+ messages in thread
From: Dan Carpenter @ 2022-11-05 11:17 UTC (permalink / raw)
  To: oe-kbuild, Jeff Layton; +Cc: lkp, oe-kbuild-all, linux-kernel, Chuck Lever

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.

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


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: fs/nfsd/filecache.c:1118 nfsd_file_do_acquire() warn: passing freed memory 'nf'
  2022-11-05 11:17 fs/nfsd/filecache.c:1118 nfsd_file_do_acquire() warn: passing freed memory 'nf' Dan Carpenter
@ 2022-11-05 13:21 ` Jeff Layton
  2022-11-05 15:39 ` Chuck Lever III
  1 sibling, 0 replies; 5+ messages in thread
From: Jeff Layton @ 2022-11-05 13:21 UTC (permalink / raw)
  To: Dan Carpenter, oe-kbuild; +Cc: lkp, oe-kbuild-all, linux-kernel, Chuck Lever

On Sat, 2022-11-05 at 14:17 +0300, Dan Carpenter 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.
> 

We do care -- that's definitely a bug. I'll plan to spin up a patch to
address it. Nice catch!

-- Jeff

> 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);
> 

-- 
Jeff Layton <jlayton@kernel.org>

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: fs/nfsd/filecache.c:1118 nfsd_file_do_acquire() warn: passing freed memory 'nf'
  2022-11-05 11:17 fs/nfsd/filecache.c:1118 nfsd_file_do_acquire() warn: passing freed memory 'nf' Dan Carpenter
  2022-11-05 13:21 ` Jeff Layton
@ 2022-11-05 15:39 ` Chuck Lever III
  2022-11-07  7:48   ` Dan Carpenter
  1 sibling, 1 reply; 5+ messages in thread
From: Chuck Lever III @ 2022-11-05 15:39 UTC (permalink / raw)
  To: Dan Carpenter
  Cc: oe-kbuild@lists.linux.dev, Jeff Layton, lkp@intel.com,
	oe-kbuild-all@lists.linux.dev, linux-kernel@vger.kernel.org



> 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!


> 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




^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: fs/nfsd/filecache.c:1118 nfsd_file_do_acquire() warn: passing freed memory 'nf'
  2022-11-05 15:39 ` Chuck Lever III
@ 2022-11-07  7:48   ` Dan Carpenter
  2022-11-07 14:07     ` Chuck Lever III
  0 siblings, 1 reply; 5+ messages in thread
From: Dan Carpenter @ 2022-11-07  7:48 UTC (permalink / raw)
  To: Chuck Lever III
  Cc: oe-kbuild@lists.linux.dev, Jeff Layton, lkp@intel.com,
	oe-kbuild-all@lists.linux.dev, linux-kernel@vger.kernel.org

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).

regards,
dan carpenter


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: fs/nfsd/filecache.c:1118 nfsd_file_do_acquire() warn: passing freed memory 'nf'
  2022-11-07  7:48   ` Dan Carpenter
@ 2022-11-07 14:07     ` Chuck Lever III
  0 siblings, 0 replies; 5+ messages in thread
From: Chuck Lever III @ 2022-11-07 14:07 UTC (permalink / raw)
  To: Dan Carpenter
  Cc: oe-kbuild@lists.linux.dev, Jeff Layton, lkp@intel.com,
	oe-kbuild-all@lists.linux.dev, linux-kernel@vger.kernel.org



> 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




^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2022-11-07 14:07 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-11-05 11:17 fs/nfsd/filecache.c:1118 nfsd_file_do_acquire() warn: passing freed memory 'nf' Dan Carpenter
2022-11-05 13:21 ` Jeff Layton
2022-11-05 15:39 ` Chuck Lever III
2022-11-07  7:48   ` Dan Carpenter
2022-11-07 14:07     ` Chuck Lever III

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox