All of lore.kernel.org
 help / color / mirror / Atom feed
From: Dan Carpenter <dan.carpenter@oracle.com>
To: zyan@redhat.com
Cc: ceph-devel@vger.kernel.org
Subject: [bug report] ceph: encode inodes' parent/d_name in cap reconnect message
Date: Tue, 30 Nov 2021 13:39:47 +0300	[thread overview]
Message-ID: <20211130103947.GA5827@kili> (raw)

Hello Yan, Zheng,

The patch a33f6432b3a6: "ceph: encode inodes' parent/d_name in cap
reconnect message" from Aug 11, 2020, leads to the following Smatch
static checker warning:

	fs/ceph/mds_client.c:3848 reconnect_caps_cb()
	error: uninitialized symbol 'pathlen'.

fs/ceph/mds_client.c
    3674 static int reconnect_caps_cb(struct inode *inode, struct ceph_cap *cap,
    3675                           void *arg)
    3676 {
    3677         union {
    3678                 struct ceph_mds_cap_reconnect v2;
    3679                 struct ceph_mds_cap_reconnect_v1 v1;
    3680         } rec;
    3681         struct ceph_inode_info *ci = cap->ci;
    3682         struct ceph_reconnect_state *recon_state = arg;
    3683         struct ceph_pagelist *pagelist = recon_state->pagelist;
    3684         struct dentry *dentry;
    3685         char *path;
    3686         int pathlen, err;
    3687         u64 pathbase;
    3688         u64 snap_follows;
    3689 
    3690         dout(" adding %p ino %llx.%llx cap %p %lld %s\n",
    3691              inode, ceph_vinop(inode), cap, cap->cap_id,
    3692              ceph_cap_string(cap->issued));
    3693 
    3694         dentry = d_find_primary(inode);
    3695         if (dentry) {
    3696                 /* set pathbase to parent dir when msg_version >= 2 */
    3697                 path = ceph_mdsc_build_path(dentry, &pathlen, &pathbase,
    3698                                             recon_state->msg_version >= 2);
    3699                 dput(dentry);
    3700                 if (IS_ERR(path)) {
    3701                         err = PTR_ERR(path);
    3702                         goto out_err;
                                 ^^^^^^^^^^^^^
"pathlen" is uninitialized on this goto.  It doesn't cause a bug.  It
just looks weird.

    3703                 }
    3704         } else {
    3705                 path = NULL;
    3706                 pathlen = 0;
    3707                 pathbase = 0;
    3708         }
    3709 
    3710         spin_lock(&ci->i_ceph_lock);
    3711         cap->seq = 0;        /* reset cap seq */
    3712         cap->issue_seq = 0;  /* and issue_seq */
    3713         cap->mseq = 0;       /* and migrate_seq */
    3714         cap->cap_gen = atomic_read(&cap->session->s_cap_gen);
    3715 
    3716         /* These are lost when the session goes away */
    3717         if (S_ISDIR(inode->i_mode)) {
    3718                 if (cap->issued & CEPH_CAP_DIR_CREATE) {
    3719                         ceph_put_string(rcu_dereference_raw(ci->i_cached_layout.pool_ns));
    3720                         memset(&ci->i_cached_layout, 0, sizeof(ci->i_cached_layout));
    3721                 }
    3722                 cap->issued &= ~CEPH_CAP_ANY_DIR_OPS;
    3723         }
    3724 
    3725         if (recon_state->msg_version >= 2) {
    3726                 rec.v2.cap_id = cpu_to_le64(cap->cap_id);
    3727                 rec.v2.wanted = cpu_to_le32(__ceph_caps_wanted(ci));
    3728                 rec.v2.issued = cpu_to_le32(cap->issued);
    3729                 rec.v2.snaprealm = cpu_to_le64(ci->i_snap_realm->ino);
    3730                 rec.v2.pathbase = cpu_to_le64(pathbase);
    3731                 rec.v2.flock_len = (__force __le32)
    3732                         ((ci->i_ceph_flags & CEPH_I_ERROR_FILELOCK) ? 0 : 1);
    3733         } else {
    3734                 rec.v1.cap_id = cpu_to_le64(cap->cap_id);
    3735                 rec.v1.wanted = cpu_to_le32(__ceph_caps_wanted(ci));
    3736                 rec.v1.issued = cpu_to_le32(cap->issued);
    3737                 rec.v1.size = cpu_to_le64(i_size_read(inode));
    3738                 ceph_encode_timespec64(&rec.v1.mtime, &inode->i_mtime);
    3739                 ceph_encode_timespec64(&rec.v1.atime, &inode->i_atime);
    3740                 rec.v1.snaprealm = cpu_to_le64(ci->i_snap_realm->ino);
    3741                 rec.v1.pathbase = cpu_to_le64(pathbase);
    3742         }
    3743 
    3744         if (list_empty(&ci->i_cap_snaps)) {
    3745                 snap_follows = ci->i_head_snapc ? ci->i_head_snapc->seq : 0;
    3746         } else {
    3747                 struct ceph_cap_snap *capsnap =
    3748                         list_first_entry(&ci->i_cap_snaps,
    3749                                          struct ceph_cap_snap, ci_item);
    3750                 snap_follows = capsnap->follows;
    3751         }
    3752         spin_unlock(&ci->i_ceph_lock);
    3753 
    3754         if (recon_state->msg_version >= 2) {
    3755                 int num_fcntl_locks, num_flock_locks;
    3756                 struct ceph_filelock *flocks = NULL;
    3757                 size_t struct_len, total_len = sizeof(u64);
    3758                 u8 struct_v = 0;
    3759 
    3760 encode_again:
    3761                 if (rec.v2.flock_len) {
    3762                         ceph_count_locks(inode, &num_fcntl_locks, &num_flock_locks);
    3763                 } else {
    3764                         num_fcntl_locks = 0;
    3765                         num_flock_locks = 0;
    3766                 }
    3767                 if (num_fcntl_locks + num_flock_locks > 0) {
    3768                         flocks = kmalloc_array(num_fcntl_locks + num_flock_locks,
    3769                                                sizeof(struct ceph_filelock),
    3770                                                GFP_NOFS);
    3771                         if (!flocks) {
    3772                                 err = -ENOMEM;
    3773                                 goto out_err;
    3774                         }
    3775                         err = ceph_encode_locks_to_buffer(inode, flocks,
    3776                                                           num_fcntl_locks,
    3777                                                           num_flock_locks);
    3778                         if (err) {
    3779                                 kfree(flocks);
    3780                                 flocks = NULL;
    3781                                 if (err == -ENOSPC)
    3782                                         goto encode_again;
    3783                                 goto out_err;
    3784                         }
    3785                 } else {
    3786                         kfree(flocks);
    3787                         flocks = NULL;
    3788                 }
    3789 
    3790                 if (recon_state->msg_version >= 3) {
    3791                         /* version, compat_version and struct_len */
    3792                         total_len += 2 * sizeof(u8) + sizeof(u32);
    3793                         struct_v = 2;
    3794                 }
    3795                 /*
    3796                  * number of encoded locks is stable, so copy to pagelist
    3797                  */
    3798                 struct_len = 2 * sizeof(u32) +
    3799                             (num_fcntl_locks + num_flock_locks) *
    3800                             sizeof(struct ceph_filelock);
    3801                 rec.v2.flock_len = cpu_to_le32(struct_len);
    3802 
    3803                 struct_len += sizeof(u32) + pathlen + sizeof(rec.v2);
    3804 
    3805                 if (struct_v >= 2)
    3806                         struct_len += sizeof(u64); /* snap_follows */
    3807 
    3808                 total_len += struct_len;
    3809 
    3810                 if (pagelist->length + total_len > RECONNECT_MAX_SIZE) {
    3811                         err = send_reconnect_partial(recon_state);
    3812                         if (err)
    3813                                 goto out_freeflocks;
    3814                         pagelist = recon_state->pagelist;
    3815                 }
    3816 
    3817                 err = ceph_pagelist_reserve(pagelist, total_len);
    3818                 if (err)
    3819                         goto out_freeflocks;
    3820 
    3821                 ceph_pagelist_encode_64(pagelist, ceph_ino(inode));
    3822                 if (recon_state->msg_version >= 3) {
    3823                         ceph_pagelist_encode_8(pagelist, struct_v);
    3824                         ceph_pagelist_encode_8(pagelist, 1);
    3825                         ceph_pagelist_encode_32(pagelist, struct_len);
    3826                 }
    3827                 ceph_pagelist_encode_string(pagelist, path, pathlen);
    3828                 ceph_pagelist_append(pagelist, &rec, sizeof(rec.v2));
    3829                 ceph_locks_to_pagelist(flocks, pagelist,
    3830                                        num_fcntl_locks, num_flock_locks);
    3831                 if (struct_v >= 2)
    3832                         ceph_pagelist_encode_64(pagelist, snap_follows);
    3833 out_freeflocks:
    3834                 kfree(flocks);
    3835         } else {
    3836                 err = ceph_pagelist_reserve(pagelist,
    3837                                             sizeof(u64) + sizeof(u32) +
    3838                                             pathlen + sizeof(rec.v1));
    3839                 if (err)
    3840                         goto out_err;
    3841 
    3842                 ceph_pagelist_encode_64(pagelist, ceph_ino(inode));
    3843                 ceph_pagelist_encode_string(pagelist, path, pathlen);
    3844                 ceph_pagelist_append(pagelist, &rec, sizeof(rec.v1));
    3845         }
    3846 
    3847 out_err:
--> 3848         ceph_mdsc_free_path(path, pathlen);
    3849         if (!err)
    3850                 recon_state->nr_caps++;
    3851         return err;
    3852 }

regards,
dan carpenter

             reply	other threads:[~2021-11-30 10:40 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-11-30 10:39 Dan Carpenter [this message]
2021-11-30 11:22 ` [bug report] ceph: encode inodes' parent/d_name in cap reconnect message 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=20211130103947.GA5827@kili \
    --to=dan.carpenter@oracle.com \
    --cc=ceph-devel@vger.kernel.org \
    --cc=zyan@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.