From: kernel test robot <lkp@intel.com>
To: Haowen Bai <baihaowen@meizu.com>,
Chris Mason <chris.mason@fusionio.com>,
Josef Bacik <josef@toxicpanda.com>,
David Sterba <dsterba@suse.com>
Cc: llvm@lists.linux.dev, kbuild-all@lists.01.org,
Haowen Bai <baihaowen@meizu.com>,
linux-btrfs@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] btrfs: Fix a memory leak in btrfs_ioctl_balance()
Date: Fri, 22 Apr 2022 01:35:50 +0800 [thread overview]
Message-ID: <202204220132.EBMTHukr-lkp@intel.com> (raw)
In-Reply-To: <1650534677-31554-1-git-send-email-baihaowen@meizu.com>
Hi Haowen,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on kdave/for-next]
[also build test WARNING on v5.18-rc3 next-20220421]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/intel-lab-lkp/linux/commits/Haowen-Bai/btrfs-Fix-a-memory-leak-in-btrfs_ioctl_balance/20220421-175644
base: https://git.kernel.org/pub/scm/linux/kernel/git/kdave/linux.git for-next
config: x86_64-randconfig-a005 (https://download.01.org/0day-ci/archive/20220422/202204220132.EBMTHukr-lkp@intel.com/config)
compiler: clang version 15.0.0 (https://github.com/llvm/llvm-project bac6cd5bf85669e3376610cfc4c4f9ca015e7b9b)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/intel-lab-lkp/linux/commit/9e36507b94d20118a936198b321a3544931217f0
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Haowen-Bai/btrfs-Fix-a-memory-leak-in-btrfs_ioctl_balance/20220421-175644
git checkout 9e36507b94d20118a936198b321a3544931217f0
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=x86_64 SHELL=/bin/bash fs/btrfs/
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All warnings (new ones prefixed by >>):
>> fs/btrfs/ioctl.c:4375:7: warning: variable 'need_unlock' is used uninitialized whenever 'if' condition is false [-Wsometimes-uninitialized]
if (!test_bit(BTRFS_FS_BALANCE_RUNNING, &fs_info->flags)) {
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
fs/btrfs/ioctl.c:4463:6: note: uninitialized use occurs here
if (need_unlock)
^~~~~~~~~~~
fs/btrfs/ioctl.c:4375:3: note: remove the 'if' if its condition is always true
if (!test_bit(BTRFS_FS_BALANCE_RUNNING, &fs_info->flags)) {
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
fs/btrfs/ioctl.c:4373:6: warning: variable 'need_unlock' is used uninitialized whenever 'if' condition is false [-Wsometimes-uninitialized]
if (fs_info->balance_ctl) {
^~~~~~~~~~~~~~~~~~~~
fs/btrfs/ioctl.c:4463:6: note: uninitialized use occurs here
if (need_unlock)
^~~~~~~~~~~
fs/btrfs/ioctl.c:4373:2: note: remove the 'if' if its condition is always true
if (fs_info->balance_ctl) {
^~~~~~~~~~~~~~~~~~~~~~~~~~
fs/btrfs/ioctl.c:4343:18: note: initialize the variable 'need_unlock' to silence this warning
bool need_unlock; /* for mut. excl. ops lock */
^
= 0
2 warnings generated.
vim +4375 fs/btrfs/ioctl.c
c9e9f97bdfb64d Ilya Dryomov 2012-01-16 4336
9ba1f6e44ed7a1 Liu Bo 2012-05-11 4337 static long btrfs_ioctl_balance(struct file *file, void __user *arg)
c9e9f97bdfb64d Ilya Dryomov 2012-01-16 4338 {
496ad9aa8ef448 Al Viro 2013-01-23 4339 struct btrfs_root *root = BTRFS_I(file_inode(file))->root;
c9e9f97bdfb64d Ilya Dryomov 2012-01-16 4340 struct btrfs_fs_info *fs_info = root->fs_info;
c9e9f97bdfb64d Ilya Dryomov 2012-01-16 4341 struct btrfs_ioctl_balance_args *bargs;
c9e9f97bdfb64d Ilya Dryomov 2012-01-16 4342 struct btrfs_balance_control *bctl;
ed0fb78fb6aa29 Ilya Dryomov 2013-01-20 4343 bool need_unlock; /* for mut. excl. ops lock */
c9e9f97bdfb64d Ilya Dryomov 2012-01-16 4344 int ret;
c9e9f97bdfb64d Ilya Dryomov 2012-01-16 4345
c9e9f97bdfb64d Ilya Dryomov 2012-01-16 4346 if (!capable(CAP_SYS_ADMIN))
c9e9f97bdfb64d Ilya Dryomov 2012-01-16 4347 return -EPERM;
c9e9f97bdfb64d Ilya Dryomov 2012-01-16 4348
e54bfa31044d60 Liu Bo 2012-06-29 4349 ret = mnt_want_write_file(file);
9ba1f6e44ed7a1 Liu Bo 2012-05-11 4350 if (ret)
9ba1f6e44ed7a1 Liu Bo 2012-05-11 4351 return ret;
9ba1f6e44ed7a1 Liu Bo 2012-05-11 4352
0cb53767e6f475 Nikolay Borisov 2022-03-30 4353 bargs = memdup_user(arg, sizeof(*bargs));
0cb53767e6f475 Nikolay Borisov 2022-03-30 4354 if (IS_ERR(bargs)) {
0cb53767e6f475 Nikolay Borisov 2022-03-30 4355 ret = PTR_ERR(bargs);
0cb53767e6f475 Nikolay Borisov 2022-03-30 4356 goto out;
0cb53767e6f475 Nikolay Borisov 2022-03-30 4357 }
0cb53767e6f475 Nikolay Borisov 2022-03-30 4358
ed0fb78fb6aa29 Ilya Dryomov 2013-01-20 4359 again:
c3e1f96c37d0f8 Goldwyn Rodrigues 2020-08-25 4360 if (btrfs_exclop_start(fs_info, BTRFS_EXCLOP_BALANCE)) {
c9e9f97bdfb64d Ilya Dryomov 2012-01-16 4361 mutex_lock(&fs_info->balance_mutex);
ed0fb78fb6aa29 Ilya Dryomov 2013-01-20 4362 need_unlock = true;
ed0fb78fb6aa29 Ilya Dryomov 2013-01-20 4363 goto locked;
ed0fb78fb6aa29 Ilya Dryomov 2013-01-20 4364 }
ed0fb78fb6aa29 Ilya Dryomov 2013-01-20 4365
ed0fb78fb6aa29 Ilya Dryomov 2013-01-20 4366 /*
0132761017e012 Nicholas D Steeves 2016-05-19 4367 * mut. excl. ops lock is locked. Three possibilities:
ed0fb78fb6aa29 Ilya Dryomov 2013-01-20 4368 * (1) some other op is running
ed0fb78fb6aa29 Ilya Dryomov 2013-01-20 4369 * (2) balance is running
ed0fb78fb6aa29 Ilya Dryomov 2013-01-20 4370 * (3) balance is paused -- special case (think resume)
ed0fb78fb6aa29 Ilya Dryomov 2013-01-20 4371 */
ed0fb78fb6aa29 Ilya Dryomov 2013-01-20 4372 mutex_lock(&fs_info->balance_mutex);
ed0fb78fb6aa29 Ilya Dryomov 2013-01-20 4373 if (fs_info->balance_ctl) {
ed0fb78fb6aa29 Ilya Dryomov 2013-01-20 4374 /* this is either (2) or (3) */
3009a62f3b1823 David Sterba 2018-03-21 @4375 if (!test_bit(BTRFS_FS_BALANCE_RUNNING, &fs_info->flags)) {
ed0fb78fb6aa29 Ilya Dryomov 2013-01-20 4376 mutex_unlock(&fs_info->balance_mutex);
dccdb07bc996e9 David Sterba 2018-03-21 4377 /*
dccdb07bc996e9 David Sterba 2018-03-21 4378 * Lock released to allow other waiters to continue,
dccdb07bc996e9 David Sterba 2018-03-21 4379 * we'll reexamine the status again.
dccdb07bc996e9 David Sterba 2018-03-21 4380 */
ed0fb78fb6aa29 Ilya Dryomov 2013-01-20 4381 mutex_lock(&fs_info->balance_mutex);
ed0fb78fb6aa29 Ilya Dryomov 2013-01-20 4382
ed0fb78fb6aa29 Ilya Dryomov 2013-01-20 4383 if (fs_info->balance_ctl &&
3009a62f3b1823 David Sterba 2018-03-21 4384 !test_bit(BTRFS_FS_BALANCE_RUNNING, &fs_info->flags)) {
ed0fb78fb6aa29 Ilya Dryomov 2013-01-20 4385 /* this is (3) */
ed0fb78fb6aa29 Ilya Dryomov 2013-01-20 4386 need_unlock = false;
ed0fb78fb6aa29 Ilya Dryomov 2013-01-20 4387 goto locked;
ed0fb78fb6aa29 Ilya Dryomov 2013-01-20 4388 }
ed0fb78fb6aa29 Ilya Dryomov 2013-01-20 4389
ed0fb78fb6aa29 Ilya Dryomov 2013-01-20 4390 mutex_unlock(&fs_info->balance_mutex);
ed0fb78fb6aa29 Ilya Dryomov 2013-01-20 4391 goto again;
ed0fb78fb6aa29 Ilya Dryomov 2013-01-20 4392 } else {
ed0fb78fb6aa29 Ilya Dryomov 2013-01-20 4393 /* this is (2) */
ed0fb78fb6aa29 Ilya Dryomov 2013-01-20 4394 mutex_unlock(&fs_info->balance_mutex);
ed0fb78fb6aa29 Ilya Dryomov 2013-01-20 4395 ret = -EINPROGRESS;
9e36507b94d201 Haowen Bai 2022-04-21 4396 goto out_bargs;
ed0fb78fb6aa29 Ilya Dryomov 2013-01-20 4397 }
ed0fb78fb6aa29 Ilya Dryomov 2013-01-20 4398 } else {
ed0fb78fb6aa29 Ilya Dryomov 2013-01-20 4399 /* this is (1) */
ed0fb78fb6aa29 Ilya Dryomov 2013-01-20 4400 mutex_unlock(&fs_info->balance_mutex);
e57138b3e96e45 Anand Jain 2013-08-21 4401 ret = BTRFS_ERROR_DEV_EXCL_RUN_IN_PROGRESS;
9e36507b94d201 Haowen Bai 2022-04-21 4402 goto out_bargs;
ed0fb78fb6aa29 Ilya Dryomov 2013-01-20 4403 }
ed0fb78fb6aa29 Ilya Dryomov 2013-01-20 4404
ed0fb78fb6aa29 Ilya Dryomov 2013-01-20 4405 locked:
de322263d3a6d4 Ilya Dryomov 2012-01-16 4406 if (bargs->flags & BTRFS_BALANCE_RESUME) {
de322263d3a6d4 Ilya Dryomov 2012-01-16 4407 if (!fs_info->balance_ctl) {
de322263d3a6d4 Ilya Dryomov 2012-01-16 4408 ret = -ENOTCONN;
de322263d3a6d4 Ilya Dryomov 2012-01-16 4409 goto out_bargs;
de322263d3a6d4 Ilya Dryomov 2012-01-16 4410 }
de322263d3a6d4 Ilya Dryomov 2012-01-16 4411
de322263d3a6d4 Ilya Dryomov 2012-01-16 4412 bctl = fs_info->balance_ctl;
de322263d3a6d4 Ilya Dryomov 2012-01-16 4413 spin_lock(&fs_info->balance_lock);
de322263d3a6d4 Ilya Dryomov 2012-01-16 4414 bctl->flags |= BTRFS_BALANCE_RESUME;
de322263d3a6d4 Ilya Dryomov 2012-01-16 4415 spin_unlock(&fs_info->balance_lock);
efc0e69c2feab8 Nikolay Borisov 2021-11-25 4416 btrfs_exclop_balance(fs_info, BTRFS_EXCLOP_BALANCE);
de322263d3a6d4 Ilya Dryomov 2012-01-16 4417
de322263d3a6d4 Ilya Dryomov 2012-01-16 4418 goto do_balance;
de322263d3a6d4 Ilya Dryomov 2012-01-16 4419 }
c9e9f97bdfb64d Ilya Dryomov 2012-01-16 4420
0cb53767e6f475 Nikolay Borisov 2022-03-30 4421 if (bargs->flags & ~(BTRFS_BALANCE_ARGS_MASK | BTRFS_BALANCE_TYPE_MASK)) {
0cb53767e6f475 Nikolay Borisov 2022-03-30 4422 ret = -EINVAL;
0cb53767e6f475 Nikolay Borisov 2022-03-30 4423 goto out_bargs;
0cb53767e6f475 Nikolay Borisov 2022-03-30 4424 }
0cb53767e6f475 Nikolay Borisov 2022-03-30 4425
ed0fb78fb6aa29 Ilya Dryomov 2013-01-20 4426 if (fs_info->balance_ctl) {
837d5b6e46d1a4 Ilya Dryomov 2012-01-16 4427 ret = -EINPROGRESS;
837d5b6e46d1a4 Ilya Dryomov 2012-01-16 4428 goto out_bargs;
837d5b6e46d1a4 Ilya Dryomov 2012-01-16 4429 }
837d5b6e46d1a4 Ilya Dryomov 2012-01-16 4430
8d2db7855e7b65 David Sterba 2015-11-04 4431 bctl = kzalloc(sizeof(*bctl), GFP_KERNEL);
c9e9f97bdfb64d Ilya Dryomov 2012-01-16 4432 if (!bctl) {
c9e9f97bdfb64d Ilya Dryomov 2012-01-16 4433 ret = -ENOMEM;
c9e9f97bdfb64d Ilya Dryomov 2012-01-16 4434 goto out_bargs;
c9e9f97bdfb64d Ilya Dryomov 2012-01-16 4435 }
c9e9f97bdfb64d Ilya Dryomov 2012-01-16 4436
c9e9f97bdfb64d Ilya Dryomov 2012-01-16 4437 memcpy(&bctl->data, &bargs->data, sizeof(bctl->data));
c9e9f97bdfb64d Ilya Dryomov 2012-01-16 4438 memcpy(&bctl->meta, &bargs->meta, sizeof(bctl->meta));
c9e9f97bdfb64d Ilya Dryomov 2012-01-16 4439 memcpy(&bctl->sys, &bargs->sys, sizeof(bctl->sys));
c9e9f97bdfb64d Ilya Dryomov 2012-01-16 4440
c9e9f97bdfb64d Ilya Dryomov 2012-01-16 4441 bctl->flags = bargs->flags;
de322263d3a6d4 Ilya Dryomov 2012-01-16 4442 do_balance:
c9e9f97bdfb64d Ilya Dryomov 2012-01-16 4443 /*
c3e1f96c37d0f8 Goldwyn Rodrigues 2020-08-25 4444 * Ownership of bctl and exclusive operation goes to btrfs_balance.
c3e1f96c37d0f8 Goldwyn Rodrigues 2020-08-25 4445 * bctl is freed in reset_balance_state, or, if restriper was paused
c3e1f96c37d0f8 Goldwyn Rodrigues 2020-08-25 4446 * all the way until unmount, in free_fs_info. The flag should be
c3e1f96c37d0f8 Goldwyn Rodrigues 2020-08-25 4447 * cleared after reset_balance_state.
c9e9f97bdfb64d Ilya Dryomov 2012-01-16 4448 */
ed0fb78fb6aa29 Ilya Dryomov 2013-01-20 4449 need_unlock = false;
ed0fb78fb6aa29 Ilya Dryomov 2013-01-20 4450
6fcf6e2bffb6cf David Sterba 2018-05-07 4451 ret = btrfs_balance(fs_info, bctl, bargs);
0f89abf56abbd0 Christian Engelmayer 2015-10-21 4452 bctl = NULL;
ed0fb78fb6aa29 Ilya Dryomov 2013-01-20 4453
6ad365fd1bfcde Nikolay Borisov 2022-03-30 4454 if (ret == 0 || ret == -ECANCELED) {
c9e9f97bdfb64d Ilya Dryomov 2012-01-16 4455 if (copy_to_user(arg, bargs, sizeof(*bargs)))
c9e9f97bdfb64d Ilya Dryomov 2012-01-16 4456 ret = -EFAULT;
c9e9f97bdfb64d Ilya Dryomov 2012-01-16 4457 }
c9e9f97bdfb64d Ilya Dryomov 2012-01-16 4458
0f89abf56abbd0 Christian Engelmayer 2015-10-21 4459 kfree(bctl);
c9e9f97bdfb64d Ilya Dryomov 2012-01-16 4460 out_bargs:
c9e9f97bdfb64d Ilya Dryomov 2012-01-16 4461 kfree(bargs);
c9e9f97bdfb64d Ilya Dryomov 2012-01-16 4462 mutex_unlock(&fs_info->balance_mutex);
ed0fb78fb6aa29 Ilya Dryomov 2013-01-20 4463 if (need_unlock)
c3e1f96c37d0f8 Goldwyn Rodrigues 2020-08-25 4464 btrfs_exclop_finish(fs_info);
ed0fb78fb6aa29 Ilya Dryomov 2013-01-20 4465 out:
e54bfa31044d60 Liu Bo 2012-06-29 4466 mnt_drop_write_file(file);
c9e9f97bdfb64d Ilya Dryomov 2012-01-16 4467 return ret;
c9e9f97bdfb64d Ilya Dryomov 2012-01-16 4468 }
c9e9f97bdfb64d Ilya Dryomov 2012-01-16 4469
--
0-DAY CI Kernel Test Service
https://01.org/lkp
next prev parent reply other threads:[~2022-04-21 17:36 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-04-21 9:51 [PATCH] btrfs: Fix a memory leak in btrfs_ioctl_balance() Haowen Bai
2022-04-21 10:01 ` Qu Wenruo
2022-04-21 10:47 ` Filipe Manana
2022-04-21 11:21 ` Nikolay Borisov
2022-04-21 11:25 ` Nikolay Borisov
2022-04-21 11:34 ` Nikolay Borisov
2022-04-21 13:30 ` David Sterba
2022-04-21 11:41 ` Qu Wenruo
2022-04-21 17:35 ` kernel test robot [this message]
2022-04-22 20:28 ` David Sterba
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=202204220132.EBMTHukr-lkp@intel.com \
--to=lkp@intel.com \
--cc=baihaowen@meizu.com \
--cc=chris.mason@fusionio.com \
--cc=dsterba@suse.com \
--cc=josef@toxicpanda.com \
--cc=kbuild-all@lists.01.org \
--cc=linux-btrfs@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=llvm@lists.linux.dev \
/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.