linux-cifs.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [bug report] cifs: add an smb3_fs_context to cifs_sb
@ 2020-12-15 15:05 Dan Carpenter
  0 siblings, 0 replies; only message in thread
From: Dan Carpenter @ 2020-12-15 15:05 UTC (permalink / raw)
  To: lsahlber; +Cc: linux-cifs

Hello Ronnie Sahlberg,

The patch d17abdf75665: "cifs: add an smb3_fs_context to cifs_sb"
from Nov 10, 2020, leads to the following static checker warning:

	fs/cifs/cifsfs.c:876 cifs_smb3_do_mount()
	error: double free of 'cifs_sb->prepath'

fs/cifs/cifsfs.c
   813          rc = cifs_setup_cifs_sb(cifs_sb);
   814          if (rc) {
   815                  root = ERR_PTR(rc);
   816                  goto out;
   817          }
   818  
   819          rc = cifs_mount(cifs_sb, cifs_sb->ctx);
   820          if (rc) {
   821                  if (!(flags & SB_SILENT))
   822                          cifs_dbg(VFS, "cifs_mount failed w/return code = %d\n",
   823                                   rc);
   824                  root = ERR_PTR(rc);
   825                  goto out;
   826          }
   827  
   828          mnt_data.ctx = cifs_sb->ctx;
   829          mnt_data.cifs_sb = cifs_sb;
   830          mnt_data.flags = flags;
   831  
   832          /* BB should we make this contingent on mount parm? */
   833          flags |= SB_NODIRATIME | SB_NOATIME;
   834  
   835          sb = sget(fs_type, cifs_match_super, cifs_set_super, flags, &mnt_data);
   836          if (IS_ERR(sb)) {
   837                  root = ERR_CAST(sb);
   838                  cifs_umount(cifs_sb);

cifs_umount() frees everything.  Smatch doesn't catch some of it because
it happens in a delayed thread.

   839                  goto out;
   840          }
   841  
   842          if (sb->s_root) {
   843                  cifs_dbg(FYI, "Use existing superblock\n");
   844                  cifs_umount(cifs_sb);
                        ^^^^^^^^^^^^^^^^^^^^
This frees "cifs_sb".

   845          } else {
   846                  rc = cifs_read_super(sb);
   847                  if (rc) {
   848                          root = ERR_PTR(rc);
   849                          goto out_super;
   850                  }
   851  
   852                  sb->s_flags |= SB_ACTIVE;
   853          }
   854  
   855          root = cifs_get_root(cifs_sb->ctx, sb);
                                     ^^^^^^^^^^^^
So this is a use after free.

   856          if (IS_ERR(root))
   857                  goto out_super;
   858  
   859          cifs_dbg(FYI, "dentry root is: %p\n", root);
   860          return root;
   861  
   862  out_super:
   863          deactivate_locked_super(sb);
   864  out:
   865          if (cifs_sb) {
   866                  kfree(cifs_sb->prepath);
   867                  smb3_cleanup_fs_context(cifs_sb->ctx);
   868                  kfree(cifs_sb);

All these three are double frees.

   869          }
   870          return root;
   871  }

regards,
dan carpenter

^ permalink raw reply	[flat|nested] only message in thread

only message in thread, other threads:[~2020-12-15 15:07 UTC | newest]

Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-12-15 15:05 [bug report] cifs: add an smb3_fs_context to cifs_sb Dan Carpenter

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).