All of lore.kernel.org
 help / color / mirror / Atom feed
* [fuse:fs_fuse_split 5/5] fs/fuse/virtio_fs.c:1458 virtio_fs_get_tree() error: uninitialized symbol 'fc'.
@ 2021-02-15 11:30 ` Dan Carpenter
  0 siblings, 0 replies; 5+ messages in thread
From: Dan Carpenter @ 2021-02-15 11:30 UTC (permalink / raw)
  To: kbuild

[-- Attachment #1: Type: text/plain, Size: 5316 bytes --]

tree:   https://git.kernel.org/pub/scm/linux/kernel/git/mszeredi/fuse.git fs_fuse_split
head:   674d5faded4c40245ea02240e731aa82c7ab4c9e
commit: 674d5faded4c40245ea02240e731aa82c7ab4c9e [5/5] fuse: alloc initial fuse_conn and fuse_mount
config: i386-randconfig-m021-20210209 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-15) 9.3.0

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>

New smatch warnings:
fs/fuse/virtio_fs.c:1458 virtio_fs_get_tree() error: uninitialized symbol 'fc'.

Old smatch warnings:
fs/fuse/virtio_fs.c:1444 virtio_fs_get_tree() error: double free of 'fm'

vim +/fc +1458 fs/fuse/virtio_fs.c

a62a8ef9d97da2 Stefan Hajnoczi 2018-06-12  1405  static int virtio_fs_get_tree(struct fs_context *fsc)
a62a8ef9d97da2 Stefan Hajnoczi 2018-06-12  1406  {
a62a8ef9d97da2 Stefan Hajnoczi 2018-06-12  1407  	struct virtio_fs *fs;
a62a8ef9d97da2 Stefan Hajnoczi 2018-06-12  1408  	struct super_block *sb;
a62a8ef9d97da2 Stefan Hajnoczi 2018-06-12  1409  	struct fuse_conn *fc;
fcee216beb9c15 Max Reitz       2020-05-06  1410  	struct fuse_mount *fm;
a62a8ef9d97da2 Stefan Hajnoczi 2018-06-12  1411  	int err;
a62a8ef9d97da2 Stefan Hajnoczi 2018-06-12  1412  
a62a8ef9d97da2 Stefan Hajnoczi 2018-06-12  1413  	/* This gets a reference on virtio_fs object. This ptr gets installed
a62a8ef9d97da2 Stefan Hajnoczi 2018-06-12  1414  	 * in fc->iq->priv. Once fuse_conn is going away, it calls ->put()
a62a8ef9d97da2 Stefan Hajnoczi 2018-06-12  1415  	 * to drop the reference to this object.
a62a8ef9d97da2 Stefan Hajnoczi 2018-06-12  1416  	 */
a62a8ef9d97da2 Stefan Hajnoczi 2018-06-12  1417  	fs = virtio_fs_find_instance(fsc->source);
a62a8ef9d97da2 Stefan Hajnoczi 2018-06-12  1418  	if (!fs) {
a62a8ef9d97da2 Stefan Hajnoczi 2018-06-12  1419  		pr_info("virtio-fs: tag <%s> not found\n", fsc->source);
a62a8ef9d97da2 Stefan Hajnoczi 2018-06-12  1420  		return -EINVAL;
a62a8ef9d97da2 Stefan Hajnoczi 2018-06-12  1421  	}
a62a8ef9d97da2 Stefan Hajnoczi 2018-06-12  1422  
833c5a42e28bee Miklos Szeredi  2020-11-11  1423  	err = -ENOMEM;
674d5faded4c40 Miklos Szeredi  2021-02-11  1424  	fm = fuse_conn_new(get_user_ns(current_user_ns()), &virtio_fs_fiq_ops, fs, NULL, NULL);
833c5a42e28bee Miklos Szeredi  2020-11-11  1425  	if (!fm)
833c5a42e28bee Miklos Szeredi  2020-11-11  1426  		goto out_err;

"fc" not initialized on this path.

674d5faded4c40 Miklos Szeredi  2021-02-11  1427  	fc = fm->fc;
a62a8ef9d97da2 Stefan Hajnoczi 2018-06-12  1428  	fc->delete_stale = true;
bf109c64040f5b Max Reitz       2020-04-21  1429  	fc->auto_submounts = true;
a62a8ef9d97da2 Stefan Hajnoczi 2018-06-12  1430  
fcee216beb9c15 Max Reitz       2020-05-06  1431  	fsc->s_fs_info = fm;
b19d3d00d662cf Miklos Szeredi  2020-11-11  1432  	sb = sget_fc(fsc, virtio_fs_test_super, set_anon_super_fc);
514b5e3ff45e6c Miklos Szeredi  2020-11-11  1433  	if (fsc->s_fs_info) {
514b5e3ff45e6c Miklos Szeredi  2020-11-11  1434  		fuse_conn_put(fc);
514b5e3ff45e6c Miklos Szeredi  2020-11-11  1435  		kfree(fm);

The error handling in this function is very confusing...

514b5e3ff45e6c Miklos Szeredi  2020-11-11  1436  	}
a62a8ef9d97da2 Stefan Hajnoczi 2018-06-12  1437  	if (IS_ERR(sb))
a62a8ef9d97da2 Stefan Hajnoczi 2018-06-12  1438  		return PTR_ERR(sb);
a62a8ef9d97da2 Stefan Hajnoczi 2018-06-12  1439  
a62a8ef9d97da2 Stefan Hajnoczi 2018-06-12  1440  	if (!sb->s_root) {
1dd539577c42b6 Vivek Goyal     2020-08-19  1441  		err = virtio_fs_fill_super(sb, fsc);
a62a8ef9d97da2 Stefan Hajnoczi 2018-06-12  1442  		if (err) {
514b5e3ff45e6c Miklos Szeredi  2020-11-11  1443  			fuse_conn_put(fc);
514b5e3ff45e6c Miklos Szeredi  2020-11-11  1444  			kfree(fm);

Smatch doesn't complain about a double free so presumably the earlier
kfree(fm) is done IFF sb is an error pointer.

66ab33bf6d4341 Miklos Szeredi  2020-11-11  1445  			sb->s_fs_info = NULL;
a62a8ef9d97da2 Stefan Hajnoczi 2018-06-12  1446  			deactivate_locked_super(sb);
a62a8ef9d97da2 Stefan Hajnoczi 2018-06-12  1447  			return err;
a62a8ef9d97da2 Stefan Hajnoczi 2018-06-12  1448  		}
a62a8ef9d97da2 Stefan Hajnoczi 2018-06-12  1449  
a62a8ef9d97da2 Stefan Hajnoczi 2018-06-12  1450  		sb->s_flags |= SB_ACTIVE;
a62a8ef9d97da2 Stefan Hajnoczi 2018-06-12  1451  	}
a62a8ef9d97da2 Stefan Hajnoczi 2018-06-12  1452  
a62a8ef9d97da2 Stefan Hajnoczi 2018-06-12  1453  	WARN_ON(fsc->root);
a62a8ef9d97da2 Stefan Hajnoczi 2018-06-12  1454  	fsc->root = dget(sb->s_root);
a62a8ef9d97da2 Stefan Hajnoczi 2018-06-12  1455  	return 0;
833c5a42e28bee Miklos Szeredi  2020-11-11  1456  
833c5a42e28bee Miklos Szeredi  2020-11-11  1457  out_err:
833c5a42e28bee Miklos Szeredi  2020-11-11 @1458  	kfree(fc);
833c5a42e28bee Miklos Szeredi  2020-11-11  1459  	mutex_lock(&virtio_fs_mutex);
833c5a42e28bee Miklos Szeredi  2020-11-11  1460  	virtio_fs_put(fs);
833c5a42e28bee Miklos Szeredi  2020-11-11  1461  	mutex_unlock(&virtio_fs_mutex);
833c5a42e28bee Miklos Szeredi  2020-11-11  1462  	return err;
a62a8ef9d97da2 Stefan Hajnoczi 2018-06-12  1463  }

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 37179 bytes --]

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [fuse:fs_fuse_split 5/5] fs/fuse/virtio_fs.c:1458 virtio_fs_get_tree() error: uninitialized symbol 'fc'.
@ 2021-02-15 11:30 ` Dan Carpenter
  0 siblings, 0 replies; 5+ messages in thread
From: Dan Carpenter @ 2021-02-15 11:30 UTC (permalink / raw)
  To: kbuild-all

[-- Attachment #1: Type: text/plain, Size: 5316 bytes --]

tree:   https://git.kernel.org/pub/scm/linux/kernel/git/mszeredi/fuse.git fs_fuse_split
head:   674d5faded4c40245ea02240e731aa82c7ab4c9e
commit: 674d5faded4c40245ea02240e731aa82c7ab4c9e [5/5] fuse: alloc initial fuse_conn and fuse_mount
config: i386-randconfig-m021-20210209 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-15) 9.3.0

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>

New smatch warnings:
fs/fuse/virtio_fs.c:1458 virtio_fs_get_tree() error: uninitialized symbol 'fc'.

Old smatch warnings:
fs/fuse/virtio_fs.c:1444 virtio_fs_get_tree() error: double free of 'fm'

vim +/fc +1458 fs/fuse/virtio_fs.c

a62a8ef9d97da2 Stefan Hajnoczi 2018-06-12  1405  static int virtio_fs_get_tree(struct fs_context *fsc)
a62a8ef9d97da2 Stefan Hajnoczi 2018-06-12  1406  {
a62a8ef9d97da2 Stefan Hajnoczi 2018-06-12  1407  	struct virtio_fs *fs;
a62a8ef9d97da2 Stefan Hajnoczi 2018-06-12  1408  	struct super_block *sb;
a62a8ef9d97da2 Stefan Hajnoczi 2018-06-12  1409  	struct fuse_conn *fc;
fcee216beb9c15 Max Reitz       2020-05-06  1410  	struct fuse_mount *fm;
a62a8ef9d97da2 Stefan Hajnoczi 2018-06-12  1411  	int err;
a62a8ef9d97da2 Stefan Hajnoczi 2018-06-12  1412  
a62a8ef9d97da2 Stefan Hajnoczi 2018-06-12  1413  	/* This gets a reference on virtio_fs object. This ptr gets installed
a62a8ef9d97da2 Stefan Hajnoczi 2018-06-12  1414  	 * in fc->iq->priv. Once fuse_conn is going away, it calls ->put()
a62a8ef9d97da2 Stefan Hajnoczi 2018-06-12  1415  	 * to drop the reference to this object.
a62a8ef9d97da2 Stefan Hajnoczi 2018-06-12  1416  	 */
a62a8ef9d97da2 Stefan Hajnoczi 2018-06-12  1417  	fs = virtio_fs_find_instance(fsc->source);
a62a8ef9d97da2 Stefan Hajnoczi 2018-06-12  1418  	if (!fs) {
a62a8ef9d97da2 Stefan Hajnoczi 2018-06-12  1419  		pr_info("virtio-fs: tag <%s> not found\n", fsc->source);
a62a8ef9d97da2 Stefan Hajnoczi 2018-06-12  1420  		return -EINVAL;
a62a8ef9d97da2 Stefan Hajnoczi 2018-06-12  1421  	}
a62a8ef9d97da2 Stefan Hajnoczi 2018-06-12  1422  
833c5a42e28bee Miklos Szeredi  2020-11-11  1423  	err = -ENOMEM;
674d5faded4c40 Miklos Szeredi  2021-02-11  1424  	fm = fuse_conn_new(get_user_ns(current_user_ns()), &virtio_fs_fiq_ops, fs, NULL, NULL);
833c5a42e28bee Miklos Szeredi  2020-11-11  1425  	if (!fm)
833c5a42e28bee Miklos Szeredi  2020-11-11  1426  		goto out_err;

"fc" not initialized on this path.

674d5faded4c40 Miklos Szeredi  2021-02-11  1427  	fc = fm->fc;
a62a8ef9d97da2 Stefan Hajnoczi 2018-06-12  1428  	fc->delete_stale = true;
bf109c64040f5b Max Reitz       2020-04-21  1429  	fc->auto_submounts = true;
a62a8ef9d97da2 Stefan Hajnoczi 2018-06-12  1430  
fcee216beb9c15 Max Reitz       2020-05-06  1431  	fsc->s_fs_info = fm;
b19d3d00d662cf Miklos Szeredi  2020-11-11  1432  	sb = sget_fc(fsc, virtio_fs_test_super, set_anon_super_fc);
514b5e3ff45e6c Miklos Szeredi  2020-11-11  1433  	if (fsc->s_fs_info) {
514b5e3ff45e6c Miklos Szeredi  2020-11-11  1434  		fuse_conn_put(fc);
514b5e3ff45e6c Miklos Szeredi  2020-11-11  1435  		kfree(fm);

The error handling in this function is very confusing...

514b5e3ff45e6c Miklos Szeredi  2020-11-11  1436  	}
a62a8ef9d97da2 Stefan Hajnoczi 2018-06-12  1437  	if (IS_ERR(sb))
a62a8ef9d97da2 Stefan Hajnoczi 2018-06-12  1438  		return PTR_ERR(sb);
a62a8ef9d97da2 Stefan Hajnoczi 2018-06-12  1439  
a62a8ef9d97da2 Stefan Hajnoczi 2018-06-12  1440  	if (!sb->s_root) {
1dd539577c42b6 Vivek Goyal     2020-08-19  1441  		err = virtio_fs_fill_super(sb, fsc);
a62a8ef9d97da2 Stefan Hajnoczi 2018-06-12  1442  		if (err) {
514b5e3ff45e6c Miklos Szeredi  2020-11-11  1443  			fuse_conn_put(fc);
514b5e3ff45e6c Miklos Szeredi  2020-11-11  1444  			kfree(fm);

Smatch doesn't complain about a double free so presumably the earlier
kfree(fm) is done IFF sb is an error pointer.

66ab33bf6d4341 Miklos Szeredi  2020-11-11  1445  			sb->s_fs_info = NULL;
a62a8ef9d97da2 Stefan Hajnoczi 2018-06-12  1446  			deactivate_locked_super(sb);
a62a8ef9d97da2 Stefan Hajnoczi 2018-06-12  1447  			return err;
a62a8ef9d97da2 Stefan Hajnoczi 2018-06-12  1448  		}
a62a8ef9d97da2 Stefan Hajnoczi 2018-06-12  1449  
a62a8ef9d97da2 Stefan Hajnoczi 2018-06-12  1450  		sb->s_flags |= SB_ACTIVE;
a62a8ef9d97da2 Stefan Hajnoczi 2018-06-12  1451  	}
a62a8ef9d97da2 Stefan Hajnoczi 2018-06-12  1452  
a62a8ef9d97da2 Stefan Hajnoczi 2018-06-12  1453  	WARN_ON(fsc->root);
a62a8ef9d97da2 Stefan Hajnoczi 2018-06-12  1454  	fsc->root = dget(sb->s_root);
a62a8ef9d97da2 Stefan Hajnoczi 2018-06-12  1455  	return 0;
833c5a42e28bee Miklos Szeredi  2020-11-11  1456  
833c5a42e28bee Miklos Szeredi  2020-11-11  1457  out_err:
833c5a42e28bee Miklos Szeredi  2020-11-11 @1458  	kfree(fc);
833c5a42e28bee Miklos Szeredi  2020-11-11  1459  	mutex_lock(&virtio_fs_mutex);
833c5a42e28bee Miklos Szeredi  2020-11-11  1460  	virtio_fs_put(fs);
833c5a42e28bee Miklos Szeredi  2020-11-11  1461  	mutex_unlock(&virtio_fs_mutex);
833c5a42e28bee Miklos Szeredi  2020-11-11  1462  	return err;
a62a8ef9d97da2 Stefan Hajnoczi 2018-06-12  1463  }

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 37179 bytes --]

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [fuse:fs_fuse_split 5/5] fs/fuse/virtio_fs.c:1458 virtio_fs_get_tree() error: uninitialized symbol 'fc'.
  2021-02-15 11:30 ` Dan Carpenter
@ 2021-02-15 12:48   ` Miklos Szeredi
  -1 siblings, 0 replies; 5+ messages in thread
From: Miklos Szeredi @ 2021-02-15 12:48 UTC (permalink / raw)
  To: Dan Carpenter; +Cc: kbuild, lkp, kbuild-all, linux-fsdevel, fuse-devel

Hi Dan,

Thanks for the report.

Note that the mailing list for fuse kernel development is
<linux-fsdevel@vger.kernel.org> as indicated in MAINTAINERS, while
<fuse-devel@lists.sourceforge.net> is for userspace development.

On Mon, Feb 15, 2021 at 12:31 PM Dan Carpenter <dan.carpenter@oracle.com> wrote:
>
> tree:   https://git.kernel.org/pub/scm/linux/kernel/git/mszeredi/fuse.git fs_fuse_split
> head:   674d5faded4c40245ea02240e731aa82c7ab4c9e
> commit: 674d5faded4c40245ea02240e731aa82c7ab4c9e [5/5] fuse: alloc initial fuse_conn and fuse_mount
> config: i386-randconfig-m021-20210209 (attached as .config)
> compiler: gcc-9 (Debian 9.3.0-15) 9.3.0
>
> If you fix the issue, kindly add following tag as appropriate
> Reported-by: kernel test robot <lkp@intel.com>
> Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
>
> New smatch warnings:
> fs/fuse/virtio_fs.c:1458 virtio_fs_get_tree() error: uninitialized symbol 'fc'.
>
> Old smatch warnings:
> fs/fuse/virtio_fs.c:1444 virtio_fs_get_tree() error: double free of 'fm'
>
> vim +/fc +1458 fs/fuse/virtio_fs.c
>
> a62a8ef9d97da2 Stefan Hajnoczi 2018-06-12  1405  static int virtio_fs_get_tree(struct fs_context *fsc)
> a62a8ef9d97da2 Stefan Hajnoczi 2018-06-12  1406  {
> a62a8ef9d97da2 Stefan Hajnoczi 2018-06-12  1407         struct virtio_fs *fs;
> a62a8ef9d97da2 Stefan Hajnoczi 2018-06-12  1408         struct super_block *sb;
> a62a8ef9d97da2 Stefan Hajnoczi 2018-06-12  1409         struct fuse_conn *fc;
> fcee216beb9c15 Max Reitz       2020-05-06  1410         struct fuse_mount *fm;
> a62a8ef9d97da2 Stefan Hajnoczi 2018-06-12  1411         int err;
> a62a8ef9d97da2 Stefan Hajnoczi 2018-06-12  1412
> a62a8ef9d97da2 Stefan Hajnoczi 2018-06-12  1413         /* This gets a reference on virtio_fs object. This ptr gets installed
> a62a8ef9d97da2 Stefan Hajnoczi 2018-06-12  1414          * in fc->iq->priv. Once fuse_conn is going away, it calls ->put()
> a62a8ef9d97da2 Stefan Hajnoczi 2018-06-12  1415          * to drop the reference to this object.
> a62a8ef9d97da2 Stefan Hajnoczi 2018-06-12  1416          */
> a62a8ef9d97da2 Stefan Hajnoczi 2018-06-12  1417         fs = virtio_fs_find_instance(fsc->source);
> a62a8ef9d97da2 Stefan Hajnoczi 2018-06-12  1418         if (!fs) {
> a62a8ef9d97da2 Stefan Hajnoczi 2018-06-12  1419                 pr_info("virtio-fs: tag <%s> not found\n", fsc->source);
> a62a8ef9d97da2 Stefan Hajnoczi 2018-06-12  1420                 return -EINVAL;
> a62a8ef9d97da2 Stefan Hajnoczi 2018-06-12  1421         }
> a62a8ef9d97da2 Stefan Hajnoczi 2018-06-12  1422
> 833c5a42e28bee Miklos Szeredi  2020-11-11  1423         err = -ENOMEM;
> 674d5faded4c40 Miklos Szeredi  2021-02-11  1424         fm = fuse_conn_new(get_user_ns(current_user_ns()), &virtio_fs_fiq_ops, fs, NULL, NULL);
> 833c5a42e28bee Miklos Szeredi  2020-11-11  1425         if (!fm)
> 833c5a42e28bee Miklos Szeredi  2020-11-11  1426                 goto out_err;
>
> "fc" not initialized on this path.

Yep, thanks.

>
> 674d5faded4c40 Miklos Szeredi  2021-02-11  1427         fc = fm->fc;
> a62a8ef9d97da2 Stefan Hajnoczi 2018-06-12  1428         fc->delete_stale = true;
> bf109c64040f5b Max Reitz       2020-04-21  1429         fc->auto_submounts = true;
> a62a8ef9d97da2 Stefan Hajnoczi 2018-06-12  1430
> fcee216beb9c15 Max Reitz       2020-05-06  1431         fsc->s_fs_info = fm;
> b19d3d00d662cf Miklos Szeredi  2020-11-11  1432         sb = sget_fc(fsc, virtio_fs_test_super, set_anon_super_fc);
> 514b5e3ff45e6c Miklos Szeredi  2020-11-11  1433         if (fsc->s_fs_info) {
> 514b5e3ff45e6c Miklos Szeredi  2020-11-11  1434                 fuse_conn_put(fc);
> 514b5e3ff45e6c Miklos Szeredi  2020-11-11  1435                 kfree(fm);
>
> The error handling in this function is very confusing...

fsc->s_fs_info is non-NULL if sget_fc() didn't take ownership of the
object.  Which can be an error, or it can be the case of an existing
sb being reused.

Needs a comment.  Will do in a separate patch.

> 514b5e3ff45e6c Miklos Szeredi  2020-11-11  1436         }
> a62a8ef9d97da2 Stefan Hajnoczi 2018-06-12  1437         if (IS_ERR(sb))
> a62a8ef9d97da2 Stefan Hajnoczi 2018-06-12  1438                 return PTR_ERR(sb);
> a62a8ef9d97da2 Stefan Hajnoczi 2018-06-12  1439
> a62a8ef9d97da2 Stefan Hajnoczi 2018-06-12  1440         if (!sb->s_root) {
> 1dd539577c42b6 Vivek Goyal     2020-08-19  1441                 err = virtio_fs_fill_super(sb, fsc);
> a62a8ef9d97da2 Stefan Hajnoczi 2018-06-12  1442                 if (err) {
> 514b5e3ff45e6c Miklos Szeredi  2020-11-11  1443                         fuse_conn_put(fc);
> 514b5e3ff45e6c Miklos Szeredi  2020-11-11  1444                         kfree(fm);
>
> Smatch doesn't complain about a double free so presumably the earlier
> kfree(fm) is done IFF sb is an error pointer.

Correct, !sb->s_root implies that this is a new sb, not a reused one
(in which case the earlier kfree() would trigger).

Thanks,
Miklos


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [fuse:fs_fuse_split 5/5] fs/fuse/virtio_fs.c:1458 virtio_fs_get_tree() error: uninitialized symbol 'fc'.
@ 2021-02-15 12:48   ` Miklos Szeredi
  0 siblings, 0 replies; 5+ messages in thread
From: Miklos Szeredi @ 2021-02-15 12:48 UTC (permalink / raw)
  To: kbuild-all

[-- Attachment #1: Type: text/plain, Size: 5042 bytes --]

Hi Dan,

Thanks for the report.

Note that the mailing list for fuse kernel development is
<linux-fsdevel@vger.kernel.org> as indicated in MAINTAINERS, while
<fuse-devel@lists.sourceforge.net> is for userspace development.

On Mon, Feb 15, 2021 at 12:31 PM Dan Carpenter <dan.carpenter@oracle.com> wrote:
>
> tree:   https://git.kernel.org/pub/scm/linux/kernel/git/mszeredi/fuse.git fs_fuse_split
> head:   674d5faded4c40245ea02240e731aa82c7ab4c9e
> commit: 674d5faded4c40245ea02240e731aa82c7ab4c9e [5/5] fuse: alloc initial fuse_conn and fuse_mount
> config: i386-randconfig-m021-20210209 (attached as .config)
> compiler: gcc-9 (Debian 9.3.0-15) 9.3.0
>
> If you fix the issue, kindly add following tag as appropriate
> Reported-by: kernel test robot <lkp@intel.com>
> Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
>
> New smatch warnings:
> fs/fuse/virtio_fs.c:1458 virtio_fs_get_tree() error: uninitialized symbol 'fc'.
>
> Old smatch warnings:
> fs/fuse/virtio_fs.c:1444 virtio_fs_get_tree() error: double free of 'fm'
>
> vim +/fc +1458 fs/fuse/virtio_fs.c
>
> a62a8ef9d97da2 Stefan Hajnoczi 2018-06-12  1405  static int virtio_fs_get_tree(struct fs_context *fsc)
> a62a8ef9d97da2 Stefan Hajnoczi 2018-06-12  1406  {
> a62a8ef9d97da2 Stefan Hajnoczi 2018-06-12  1407         struct virtio_fs *fs;
> a62a8ef9d97da2 Stefan Hajnoczi 2018-06-12  1408         struct super_block *sb;
> a62a8ef9d97da2 Stefan Hajnoczi 2018-06-12  1409         struct fuse_conn *fc;
> fcee216beb9c15 Max Reitz       2020-05-06  1410         struct fuse_mount *fm;
> a62a8ef9d97da2 Stefan Hajnoczi 2018-06-12  1411         int err;
> a62a8ef9d97da2 Stefan Hajnoczi 2018-06-12  1412
> a62a8ef9d97da2 Stefan Hajnoczi 2018-06-12  1413         /* This gets a reference on virtio_fs object. This ptr gets installed
> a62a8ef9d97da2 Stefan Hajnoczi 2018-06-12  1414          * in fc->iq->priv. Once fuse_conn is going away, it calls ->put()
> a62a8ef9d97da2 Stefan Hajnoczi 2018-06-12  1415          * to drop the reference to this object.
> a62a8ef9d97da2 Stefan Hajnoczi 2018-06-12  1416          */
> a62a8ef9d97da2 Stefan Hajnoczi 2018-06-12  1417         fs = virtio_fs_find_instance(fsc->source);
> a62a8ef9d97da2 Stefan Hajnoczi 2018-06-12  1418         if (!fs) {
> a62a8ef9d97da2 Stefan Hajnoczi 2018-06-12  1419                 pr_info("virtio-fs: tag <%s> not found\n", fsc->source);
> a62a8ef9d97da2 Stefan Hajnoczi 2018-06-12  1420                 return -EINVAL;
> a62a8ef9d97da2 Stefan Hajnoczi 2018-06-12  1421         }
> a62a8ef9d97da2 Stefan Hajnoczi 2018-06-12  1422
> 833c5a42e28bee Miklos Szeredi  2020-11-11  1423         err = -ENOMEM;
> 674d5faded4c40 Miklos Szeredi  2021-02-11  1424         fm = fuse_conn_new(get_user_ns(current_user_ns()), &virtio_fs_fiq_ops, fs, NULL, NULL);
> 833c5a42e28bee Miklos Szeredi  2020-11-11  1425         if (!fm)
> 833c5a42e28bee Miklos Szeredi  2020-11-11  1426                 goto out_err;
>
> "fc" not initialized on this path.

Yep, thanks.

>
> 674d5faded4c40 Miklos Szeredi  2021-02-11  1427         fc = fm->fc;
> a62a8ef9d97da2 Stefan Hajnoczi 2018-06-12  1428         fc->delete_stale = true;
> bf109c64040f5b Max Reitz       2020-04-21  1429         fc->auto_submounts = true;
> a62a8ef9d97da2 Stefan Hajnoczi 2018-06-12  1430
> fcee216beb9c15 Max Reitz       2020-05-06  1431         fsc->s_fs_info = fm;
> b19d3d00d662cf Miklos Szeredi  2020-11-11  1432         sb = sget_fc(fsc, virtio_fs_test_super, set_anon_super_fc);
> 514b5e3ff45e6c Miklos Szeredi  2020-11-11  1433         if (fsc->s_fs_info) {
> 514b5e3ff45e6c Miklos Szeredi  2020-11-11  1434                 fuse_conn_put(fc);
> 514b5e3ff45e6c Miklos Szeredi  2020-11-11  1435                 kfree(fm);
>
> The error handling in this function is very confusing...

fsc->s_fs_info is non-NULL if sget_fc() didn't take ownership of the
object.  Which can be an error, or it can be the case of an existing
sb being reused.

Needs a comment.  Will do in a separate patch.

> 514b5e3ff45e6c Miklos Szeredi  2020-11-11  1436         }
> a62a8ef9d97da2 Stefan Hajnoczi 2018-06-12  1437         if (IS_ERR(sb))
> a62a8ef9d97da2 Stefan Hajnoczi 2018-06-12  1438                 return PTR_ERR(sb);
> a62a8ef9d97da2 Stefan Hajnoczi 2018-06-12  1439
> a62a8ef9d97da2 Stefan Hajnoczi 2018-06-12  1440         if (!sb->s_root) {
> 1dd539577c42b6 Vivek Goyal     2020-08-19  1441                 err = virtio_fs_fill_super(sb, fsc);
> a62a8ef9d97da2 Stefan Hajnoczi 2018-06-12  1442                 if (err) {
> 514b5e3ff45e6c Miklos Szeredi  2020-11-11  1443                         fuse_conn_put(fc);
> 514b5e3ff45e6c Miklos Szeredi  2020-11-11  1444                         kfree(fm);
>
> Smatch doesn't complain about a double free so presumably the earlier
> kfree(fm) is done IFF sb is an error pointer.

Correct, !sb->s_root implies that this is a new sb, not a reused one
(in which case the earlier kfree() would trigger).

Thanks,
Miklos

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [fuse:fs_fuse_split 5/5] fs/fuse/virtio_fs.c:1458 virtio_fs_get_tree() error: uninitialized symbol 'fc'.
@ 2021-02-11 21:34 kernel test robot
  0 siblings, 0 replies; 5+ messages in thread
From: kernel test robot @ 2021-02-11 21:34 UTC (permalink / raw)
  To: kbuild

[-- Attachment #1: Type: text/plain, Size: 5645 bytes --]

CC: kbuild-all(a)lists.01.org
CC: fuse-devel(a)lists.sourceforge.net
TO: Miklos Szeredi <mszeredi@redhat.com>

tree:   https://git.kernel.org/pub/scm/linux/kernel/git/mszeredi/fuse.git fs_fuse_split
head:   674d5faded4c40245ea02240e731aa82c7ab4c9e
commit: 674d5faded4c40245ea02240e731aa82c7ab4c9e [5/5] fuse: alloc initial fuse_conn and fuse_mount
:::::: branch date: 9 hours ago
:::::: commit date: 9 hours ago
config: i386-randconfig-m021-20210209 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-15) 9.3.0

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>

New smatch warnings:
fs/fuse/virtio_fs.c:1458 virtio_fs_get_tree() error: uninitialized symbol 'fc'.

Old smatch warnings:
fs/fuse/virtio_fs.c:1444 virtio_fs_get_tree() error: double free of 'fm'

vim +/fc +1458 fs/fuse/virtio_fs.c

a62a8ef9d97da2 Stefan Hajnoczi 2018-06-12  1404  
a62a8ef9d97da2 Stefan Hajnoczi 2018-06-12  1405  static int virtio_fs_get_tree(struct fs_context *fsc)
a62a8ef9d97da2 Stefan Hajnoczi 2018-06-12  1406  {
a62a8ef9d97da2 Stefan Hajnoczi 2018-06-12  1407  	struct virtio_fs *fs;
a62a8ef9d97da2 Stefan Hajnoczi 2018-06-12  1408  	struct super_block *sb;
a62a8ef9d97da2 Stefan Hajnoczi 2018-06-12  1409  	struct fuse_conn *fc;
fcee216beb9c15 Max Reitz       2020-05-06  1410  	struct fuse_mount *fm;
a62a8ef9d97da2 Stefan Hajnoczi 2018-06-12  1411  	int err;
a62a8ef9d97da2 Stefan Hajnoczi 2018-06-12  1412  
a62a8ef9d97da2 Stefan Hajnoczi 2018-06-12  1413  	/* This gets a reference on virtio_fs object. This ptr gets installed
a62a8ef9d97da2 Stefan Hajnoczi 2018-06-12  1414  	 * in fc->iq->priv. Once fuse_conn is going away, it calls ->put()
a62a8ef9d97da2 Stefan Hajnoczi 2018-06-12  1415  	 * to drop the reference to this object.
a62a8ef9d97da2 Stefan Hajnoczi 2018-06-12  1416  	 */
a62a8ef9d97da2 Stefan Hajnoczi 2018-06-12  1417  	fs = virtio_fs_find_instance(fsc->source);
a62a8ef9d97da2 Stefan Hajnoczi 2018-06-12  1418  	if (!fs) {
a62a8ef9d97da2 Stefan Hajnoczi 2018-06-12  1419  		pr_info("virtio-fs: tag <%s> not found\n", fsc->source);
a62a8ef9d97da2 Stefan Hajnoczi 2018-06-12  1420  		return -EINVAL;
a62a8ef9d97da2 Stefan Hajnoczi 2018-06-12  1421  	}
a62a8ef9d97da2 Stefan Hajnoczi 2018-06-12  1422  
833c5a42e28bee Miklos Szeredi  2020-11-11  1423  	err = -ENOMEM;
674d5faded4c40 Miklos Szeredi  2021-02-11  1424  	fm = fuse_conn_new(get_user_ns(current_user_ns()), &virtio_fs_fiq_ops, fs, NULL, NULL);
833c5a42e28bee Miklos Szeredi  2020-11-11  1425  	if (!fm)
833c5a42e28bee Miklos Szeredi  2020-11-11  1426  		goto out_err;
674d5faded4c40 Miklos Szeredi  2021-02-11  1427  	fc = fm->fc;
a62a8ef9d97da2 Stefan Hajnoczi 2018-06-12  1428  	fc->delete_stale = true;
bf109c64040f5b Max Reitz       2020-04-21  1429  	fc->auto_submounts = true;
a62a8ef9d97da2 Stefan Hajnoczi 2018-06-12  1430  
fcee216beb9c15 Max Reitz       2020-05-06  1431  	fsc->s_fs_info = fm;
b19d3d00d662cf Miklos Szeredi  2020-11-11  1432  	sb = sget_fc(fsc, virtio_fs_test_super, set_anon_super_fc);
514b5e3ff45e6c Miklos Szeredi  2020-11-11  1433  	if (fsc->s_fs_info) {
514b5e3ff45e6c Miklos Szeredi  2020-11-11  1434  		fuse_conn_put(fc);
514b5e3ff45e6c Miklos Szeredi  2020-11-11  1435  		kfree(fm);
514b5e3ff45e6c Miklos Szeredi  2020-11-11  1436  	}
a62a8ef9d97da2 Stefan Hajnoczi 2018-06-12  1437  	if (IS_ERR(sb))
a62a8ef9d97da2 Stefan Hajnoczi 2018-06-12  1438  		return PTR_ERR(sb);
a62a8ef9d97da2 Stefan Hajnoczi 2018-06-12  1439  
a62a8ef9d97da2 Stefan Hajnoczi 2018-06-12  1440  	if (!sb->s_root) {
1dd539577c42b6 Vivek Goyal     2020-08-19  1441  		err = virtio_fs_fill_super(sb, fsc);
a62a8ef9d97da2 Stefan Hajnoczi 2018-06-12  1442  		if (err) {
514b5e3ff45e6c Miklos Szeredi  2020-11-11  1443  			fuse_conn_put(fc);
514b5e3ff45e6c Miklos Szeredi  2020-11-11  1444  			kfree(fm);
66ab33bf6d4341 Miklos Szeredi  2020-11-11  1445  			sb->s_fs_info = NULL;
a62a8ef9d97da2 Stefan Hajnoczi 2018-06-12  1446  			deactivate_locked_super(sb);
a62a8ef9d97da2 Stefan Hajnoczi 2018-06-12  1447  			return err;
a62a8ef9d97da2 Stefan Hajnoczi 2018-06-12  1448  		}
a62a8ef9d97da2 Stefan Hajnoczi 2018-06-12  1449  
a62a8ef9d97da2 Stefan Hajnoczi 2018-06-12  1450  		sb->s_flags |= SB_ACTIVE;
a62a8ef9d97da2 Stefan Hajnoczi 2018-06-12  1451  	}
a62a8ef9d97da2 Stefan Hajnoczi 2018-06-12  1452  
a62a8ef9d97da2 Stefan Hajnoczi 2018-06-12  1453  	WARN_ON(fsc->root);
a62a8ef9d97da2 Stefan Hajnoczi 2018-06-12  1454  	fsc->root = dget(sb->s_root);
a62a8ef9d97da2 Stefan Hajnoczi 2018-06-12  1455  	return 0;
833c5a42e28bee Miklos Szeredi  2020-11-11  1456  
833c5a42e28bee Miklos Szeredi  2020-11-11  1457  out_err:
833c5a42e28bee Miklos Szeredi  2020-11-11 @1458  	kfree(fc);
833c5a42e28bee Miklos Szeredi  2020-11-11  1459  	mutex_lock(&virtio_fs_mutex);
833c5a42e28bee Miklos Szeredi  2020-11-11  1460  	virtio_fs_put(fs);
833c5a42e28bee Miklos Szeredi  2020-11-11  1461  	mutex_unlock(&virtio_fs_mutex);
833c5a42e28bee Miklos Szeredi  2020-11-11  1462  	return err;
a62a8ef9d97da2 Stefan Hajnoczi 2018-06-12  1463  }
a62a8ef9d97da2 Stefan Hajnoczi 2018-06-12  1464  

:::::: The code at line 1458 was first introduced by commit
:::::: 833c5a42e28beeefa1f9bd476a63fe8050c1e8ca virtiofs: clean up error handling in virtio_fs_get_tree()

:::::: TO: Miklos Szeredi <mszeredi@redhat.com>
:::::: CC: Miklos Szeredi <mszeredi@redhat.com>

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 37179 bytes --]

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2021-02-15 12:50 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-02-15 11:30 [fuse:fs_fuse_split 5/5] fs/fuse/virtio_fs.c:1458 virtio_fs_get_tree() error: uninitialized symbol 'fc' Dan Carpenter
2021-02-15 11:30 ` Dan Carpenter
2021-02-15 12:48 ` Miklos Szeredi
2021-02-15 12:48   ` Miklos Szeredi
  -- strict thread matches above, loose matches on Subject: below --
2021-02-11 21:34 kernel test robot

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.