From: Dan Carpenter <dan.carpenter@linaro.org>
To: Xiubo Li <xiubli@redhat.com>
Cc: ceph-devel@vger.kernel.org
Subject: Re: [bug report] ceph: fix potential use-after-free bug when trimming caps
Date: Sat, 6 May 2023 10:13:06 +0300 [thread overview]
Message-ID: <43a81ea0-c24f-4a27-a313-0da1abd41004@kili.mountain> (raw)
In-Reply-To: <937dc7c7-1907-5511-d691-7f531e72bd8f@redhat.com>
On Sat, May 06, 2023 at 09:32:30AM +0800, Xiubo Li wrote:
> Hi Dan,
>
> On 5/5/23 17:21, Dan Carpenter wrote:
> > Hello Xiubo Li,
> >
> > The patch aaf67de78807: "ceph: fix potential use-after-free bug when
> > trimming caps" from Apr 19, 2023, leads to the following Smatch
> > static checker warning:
> >
> > fs/ceph/mds_client.c:3968 reconnect_caps_cb()
> > warn: missing error code here? '__get_cap_for_mds()' failed. 'err' = '0'
> >
> > fs/ceph/mds_client.c
> > 3933 static int reconnect_caps_cb(struct inode *inode, int mds, void *arg)
> > 3934 {
> > 3935 union {
> > 3936 struct ceph_mds_cap_reconnect v2;
> > 3937 struct ceph_mds_cap_reconnect_v1 v1;
> > 3938 } rec;
> > 3939 struct ceph_inode_info *ci = ceph_inode(inode);
> > 3940 struct ceph_reconnect_state *recon_state = arg;
> > 3941 struct ceph_pagelist *pagelist = recon_state->pagelist;
> > 3942 struct dentry *dentry;
> > 3943 struct ceph_cap *cap;
> > 3944 char *path;
> > 3945 int pathlen = 0, err = 0;
> > 3946 u64 pathbase;
> > 3947 u64 snap_follows;
> > 3948
> > 3949 dentry = d_find_primary(inode);
> > 3950 if (dentry) {
> > 3951 /* set pathbase to parent dir when msg_version >= 2 */
> > 3952 path = ceph_mdsc_build_path(dentry, &pathlen, &pathbase,
> > 3953 recon_state->msg_version >= 2);
> > 3954 dput(dentry);
> > 3955 if (IS_ERR(path)) {
> > 3956 err = PTR_ERR(path);
> > 3957 goto out_err;
> > 3958 }
> > 3959 } else {
> > 3960 path = NULL;
> > 3961 pathbase = 0;
> > 3962 }
> > 3963
> > 3964 spin_lock(&ci->i_ceph_lock);
> > 3965 cap = __get_cap_for_mds(ci, mds);
> > 3966 if (!cap) {
> > 3967 spin_unlock(&ci->i_ceph_lock);
> > --> 3968 goto out_err;
> >
> > Set an error code?
>
> This was intended, the 'err' was initialized as '0' in line 3945.
>
> It's no harm to skip this _cb() for current cap, so just succeeds it.
>
Smatch considers it intentional of the "ret = 0;" assignment is within
4 or 5 (I forget) lines of the goto. Otherwise adding a comment would
help reviewers.
regards,
dan carpenter
next prev parent reply other threads:[~2023-05-06 7:13 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-05-05 9:21 [bug report] ceph: fix potential use-after-free bug when trimming caps Dan Carpenter
2023-05-06 1:32 ` Xiubo Li
2023-05-06 7:13 ` Dan Carpenter [this message]
2023-05-08 6:42 ` Xiubo Li
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=43a81ea0-c24f-4a27-a313-0da1abd41004@kili.mountain \
--to=dan.carpenter@linaro.org \
--cc=ceph-devel@vger.kernel.org \
--cc=xiubli@redhat.com \
/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.