From: Dan Carpenter <error27@gmail.com>
To: Chuck Lever III <chuck.lever@oracle.com>
Cc: "oe-kbuild@lists.linux.dev" <oe-kbuild@lists.linux.dev>,
Jeff Layton <jlayton@kernel.org>, "lkp@intel.com" <lkp@intel.com>,
"oe-kbuild-all@lists.linux.dev" <oe-kbuild-all@lists.linux.dev>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: fs/nfsd/filecache.c:1118 nfsd_file_do_acquire() warn: passing freed memory 'nf'
Date: Mon, 7 Nov 2022 10:48:10 +0300 [thread overview]
Message-ID: <Y2i4Olz9GHpO02bP@kadam> (raw)
In-Reply-To: <6F00D08C-6D94-486A-A3F2-B11C9FF5E9E2@oracle.com>
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
next prev parent reply other threads:[~2022-11-07 7:48 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-11-05 7:26 fs/nfsd/filecache.c:1118 nfsd_file_do_acquire() warn: passing freed memory 'nf' kernel test robot
2022-11-05 11:17 ` Dan Carpenter
2022-11-05 13:21 ` Jeff Layton
2022-11-05 15:39 ` Chuck Lever III
2022-11-07 7:48 ` Dan Carpenter [this message]
2022-11-07 14:07 ` Chuck Lever III
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=Y2i4Olz9GHpO02bP@kadam \
--to=error27@gmail.com \
--cc=chuck.lever@oracle.com \
--cc=jlayton@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=lkp@intel.com \
--cc=oe-kbuild-all@lists.linux.dev \
--cc=oe-kbuild@lists.linux.dev \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.