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
