All of lore.kernel.org
 help / color / mirror / Atom feed
From: Dan Carpenter <dan.carpenter@linaro.org>
To: Shyam Prasad N <sprasad@microsoft.com>
Cc: linux-cifs@vger.kernel.org
Subject: [bug report] cifs: serialize initialization and cleanup of cfid
Date: Mon, 5 May 2025 17:27:26 +0300	[thread overview]
Message-ID: <aBjKzghG0TmwiiOM@stanley.mountain> (raw)

Hello Shyam Prasad N,

Commit 62adfb82c199 ("cifs: serialize initialization and cleanup of
cfid") from Apr 29, 2025 (linux-next), leads to the following Smatch
static checker warning:

	fs/smb/client/cached_dir.c:474 smb2_close_cached_fid()
	warn: 'cfid' was already freed. (line 473)

fs/smb/client/cached_dir.c
    441 static void
    442 smb2_close_cached_fid(struct kref *ref)
    443 {
    444         struct cached_fid *cfid = container_of(ref, struct cached_fid,
    445                                                refcount);
    446         int rc;
    447 
    448         /* make sure not to race with server open */
                   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

    449         mutex_lock(&cfid->cfid_mutex);
    450 
    451         spin_lock(&cfid->cfids->cfid_list_lock);
    452         if (cfid->on_list) {
    453                 list_del(&cfid->entry);
    454                 cfid->on_list = false;
    455                 cfid->cfids->num_entries--;
    456         }
    457         spin_unlock(&cfid->cfids->cfid_list_lock);
    458 
    459         /* no locking necessary as we're the last user of this cfid */
    460         if (cfid->dentry) {
    461                 dput(cfid->dentry);
    462                 cfid->dentry = NULL;
    463         }
    464 
    465         if (cfid->is_open) {
    466                 rc = SMB2_close(0, cfid->tcon, cfid->fid.persistent_fid,
    467                            cfid->fid.volatile_fid);
    468                 if (rc) /* should we retry on -EBUSY or -EAGAIN? */
    469                         cifs_dbg(VFS, "close cached dir rc %d\n", rc);
    470         }
    471 
    472         free_cached_dir(cfid);
                                ^^^^
This frees "cfid".

    473         mutex_unlock(&cfid->cfid_mutex);

If there is something waiting on this lock then it's a use after free on
the waiter side as well.  Maybe this should be reference counted?

--> 474 }

regards,
dan carpenter

             reply	other threads:[~2025-05-05 14:27 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-05-05 14:27 Dan Carpenter [this message]
  -- strict thread matches above, loose matches on Subject: below --
2025-05-05 14:27 [bug report] cifs: serialize initialization and cleanup of cfid Dan Carpenter
2025-05-16 11:06 ` Shyam Prasad N

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=aBjKzghG0TmwiiOM@stanley.mountain \
    --to=dan.carpenter@linaro.org \
    --cc=linux-cifs@vger.kernel.org \
    --cc=sprasad@microsoft.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.