All of lore.kernel.org
 help / color / mirror / Atom feed
From: kernel test robot <lkp@intel.com>
To: Qu Wenruo <wqu@suse.com>, linux-btrfs@vger.kernel.org
Cc: kbuild-all@lists.01.org, Filipe Manana <fdmanana@suse.com>
Subject: Re: [PATCH v3] btrfs: trim: fix underflow in trim length to prevent access beyond device boundary
Date: Sat, 1 Aug 2020 04:52:34 +0800	[thread overview]
Message-ID: <202008010403.NVCICIjG%lkp@intel.com> (raw)
In-Reply-To: <20200731094815.104794-1-wqu@suse.com>

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

Hi Qu,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on kdave/for-next]
[also build test WARNING on v5.8-rc7 next-20200731]
[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/0day-ci/linux/commits/Qu-Wenruo/btrfs-trim-fix-underflow-in-trim-length-to-prevent-access-beyond-device-boundary/20200731-174928
base:   https://git.kernel.org/pub/scm/linux/kernel/git/kdave/linux.git for-next
config: x86_64-randconfig-s022-20200731 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-14) 9.3.0
reproduce:
        # apt-get install sparse
        # sparse version: v0.6.2-115-g5fc204f2-dirty
        # save the attached .config to linux build tree
        make W=1 C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' ARCH=x86_64 

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


sparse warnings: (new ones prefixed by >>)

>> fs/btrfs/extent-tree.c:5676:25: sparse: sparse: incompatible types in comparison expression (different address spaces):
>> fs/btrfs/extent-tree.c:5676:25: sparse:    struct rcu_string [noderef] __rcu *
>> fs/btrfs/extent-tree.c:5676:25: sparse:    struct rcu_string *
   fs/btrfs/extent-tree.c:1769:9: sparse: sparse: context imbalance in 'run_and_cleanup_extent_op' - unexpected unlock
   fs/btrfs/extent-tree.c:1842:28: sparse: sparse: context imbalance in 'cleanup_ref_head' - unexpected unlock
   fs/btrfs/extent-tree.c:1918:36: sparse: sparse: context imbalance in 'btrfs_run_delayed_refs_for_head' - unexpected unlock
   fs/btrfs/extent-tree.c:1983:21: sparse: sparse: context imbalance in '__btrfs_run_delayed_refs' - wrong count at exit

vim +5676 fs/btrfs/extent-tree.c

  5619	
  5620	/*
  5621	 * It used to be that old block groups would be left around forever.
  5622	 * Iterating over them would be enough to trim unused space.  Since we
  5623	 * now automatically remove them, we also need to iterate over unallocated
  5624	 * space.
  5625	 *
  5626	 * We don't want a transaction for this since the discard may take a
  5627	 * substantial amount of time.  We don't require that a transaction be
  5628	 * running, but we do need to take a running transaction into account
  5629	 * to ensure that we're not discarding chunks that were released or
  5630	 * allocated in the current transaction.
  5631	 *
  5632	 * Holding the chunks lock will prevent other threads from allocating
  5633	 * or releasing chunks, but it won't prevent a running transaction
  5634	 * from committing and releasing the memory that the pending chunks
  5635	 * list head uses.  For that, we need to take a reference to the
  5636	 * transaction and hold the commit root sem.  We only need to hold
  5637	 * it while performing the free space search since we have already
  5638	 * held back allocations.
  5639	 */
  5640	static int btrfs_trim_free_extents(struct btrfs_device *device, u64 *trimmed)
  5641	{
  5642		u64 start = SZ_1M, len = 0, end = 0;
  5643		int ret;
  5644	
  5645		*trimmed = 0;
  5646	
  5647		/* Discard not supported = nothing to do. */
  5648		if (!blk_queue_discard(bdev_get_queue(device->bdev)))
  5649			return 0;
  5650	
  5651		/* Not writable = nothing to do. */
  5652		if (!test_bit(BTRFS_DEV_STATE_WRITEABLE, &device->dev_state))
  5653			return 0;
  5654	
  5655		/* No free space = nothing to do. */
  5656		if (device->total_bytes <= device->bytes_used)
  5657			return 0;
  5658	
  5659		ret = 0;
  5660	
  5661		while (1) {
  5662			struct btrfs_fs_info *fs_info = device->fs_info;
  5663			u64 bytes;
  5664	
  5665			ret = mutex_lock_interruptible(&fs_info->chunk_mutex);
  5666			if (ret)
  5667				break;
  5668	
  5669			find_first_clear_extent_bit(&device->alloc_state, start,
  5670						    &start, &end,
  5671						    CHUNK_TRIMMED | CHUNK_ALLOCATED);
  5672	
  5673			/* CHUNK_* bits not cleared properly */
  5674			if (start > device->total_bytes) {
  5675				WARN_ON(IS_ENABLED(CONFIG_BTRFS_DEBUG));
> 5676				btrfs_warn_in_rcu(fs_info,
  5677	"ignoring attempt to trim beyond device size: offset %llu length %llu device %s device size %llu",
  5678						  start, end - start + 1,
  5679						  rcu_str_deref(device->name),
  5680						  device->total_bytes);
  5681				mutex_unlock(&fs_info->chunk_mutex);
  5682				ret = 0;
  5683				break;
  5684			}
  5685	
  5686			/* The remaining part has already been trimmed */
  5687			if (start == device->total_bytes) {
  5688				mutex_unlock(&fs_info->chunk_mutex);
  5689				ret = 0;
  5690				break;
  5691			}
  5692	
  5693			/* Ensure we skip the reserved area in the first 1M */
  5694			start = max_t(u64, start, SZ_1M);
  5695	
  5696			/*
  5697			 * If find_first_clear_extent_bit find a range that spans the
  5698			 * end of the device it will set end to -1, in this case it's up
  5699			 * to the caller to trim the value to the size of the device.
  5700			 */
  5701			end = min(end, device->total_bytes - 1);
  5702	
  5703			len = end - start + 1;
  5704	
  5705			/* We didn't find any extents */
  5706			if (!len) {
  5707				mutex_unlock(&fs_info->chunk_mutex);
  5708				ret = 0;
  5709				break;
  5710			}
  5711	
  5712			ret = btrfs_issue_discard(device->bdev, start, len,
  5713						  &bytes);
  5714			if (!ret)
  5715				set_extent_bits(&device->alloc_state, start,
  5716						start + bytes - 1,
  5717						CHUNK_TRIMMED);
  5718			mutex_unlock(&fs_info->chunk_mutex);
  5719	
  5720			if (ret)
  5721				break;
  5722	
  5723			start += len;
  5724			*trimmed += bytes;
  5725	
  5726			if (fatal_signal_pending(current)) {
  5727				ret = -ERESTARTSYS;
  5728				break;
  5729			}
  5730	
  5731			cond_resched();
  5732		}
  5733	
  5734		return ret;
  5735	}
  5736	

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

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

WARNING: multiple messages have this Message-ID (diff)
From: kernel test robot <lkp@intel.com>
To: kbuild-all@lists.01.org
Subject: Re: [PATCH v3] btrfs: trim: fix underflow in trim length to prevent access beyond device boundary
Date: Sat, 01 Aug 2020 04:52:34 +0800	[thread overview]
Message-ID: <202008010403.NVCICIjG%lkp@intel.com> (raw)
In-Reply-To: <20200731094815.104794-1-wqu@suse.com>

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

Hi Qu,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on kdave/for-next]
[also build test WARNING on v5.8-rc7 next-20200731]
[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/0day-ci/linux/commits/Qu-Wenruo/btrfs-trim-fix-underflow-in-trim-length-to-prevent-access-beyond-device-boundary/20200731-174928
base:   https://git.kernel.org/pub/scm/linux/kernel/git/kdave/linux.git for-next
config: x86_64-randconfig-s022-20200731 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-14) 9.3.0
reproduce:
        # apt-get install sparse
        # sparse version: v0.6.2-115-g5fc204f2-dirty
        # save the attached .config to linux build tree
        make W=1 C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' ARCH=x86_64 

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


sparse warnings: (new ones prefixed by >>)

>> fs/btrfs/extent-tree.c:5676:25: sparse: sparse: incompatible types in comparison expression (different address spaces):
>> fs/btrfs/extent-tree.c:5676:25: sparse:    struct rcu_string [noderef] __rcu *
>> fs/btrfs/extent-tree.c:5676:25: sparse:    struct rcu_string *
   fs/btrfs/extent-tree.c:1769:9: sparse: sparse: context imbalance in 'run_and_cleanup_extent_op' - unexpected unlock
   fs/btrfs/extent-tree.c:1842:28: sparse: sparse: context imbalance in 'cleanup_ref_head' - unexpected unlock
   fs/btrfs/extent-tree.c:1918:36: sparse: sparse: context imbalance in 'btrfs_run_delayed_refs_for_head' - unexpected unlock
   fs/btrfs/extent-tree.c:1983:21: sparse: sparse: context imbalance in '__btrfs_run_delayed_refs' - wrong count at exit

vim +5676 fs/btrfs/extent-tree.c

  5619	
  5620	/*
  5621	 * It used to be that old block groups would be left around forever.
  5622	 * Iterating over them would be enough to trim unused space.  Since we
  5623	 * now automatically remove them, we also need to iterate over unallocated
  5624	 * space.
  5625	 *
  5626	 * We don't want a transaction for this since the discard may take a
  5627	 * substantial amount of time.  We don't require that a transaction be
  5628	 * running, but we do need to take a running transaction into account
  5629	 * to ensure that we're not discarding chunks that were released or
  5630	 * allocated in the current transaction.
  5631	 *
  5632	 * Holding the chunks lock will prevent other threads from allocating
  5633	 * or releasing chunks, but it won't prevent a running transaction
  5634	 * from committing and releasing the memory that the pending chunks
  5635	 * list head uses.  For that, we need to take a reference to the
  5636	 * transaction and hold the commit root sem.  We only need to hold
  5637	 * it while performing the free space search since we have already
  5638	 * held back allocations.
  5639	 */
  5640	static int btrfs_trim_free_extents(struct btrfs_device *device, u64 *trimmed)
  5641	{
  5642		u64 start = SZ_1M, len = 0, end = 0;
  5643		int ret;
  5644	
  5645		*trimmed = 0;
  5646	
  5647		/* Discard not supported = nothing to do. */
  5648		if (!blk_queue_discard(bdev_get_queue(device->bdev)))
  5649			return 0;
  5650	
  5651		/* Not writable = nothing to do. */
  5652		if (!test_bit(BTRFS_DEV_STATE_WRITEABLE, &device->dev_state))
  5653			return 0;
  5654	
  5655		/* No free space = nothing to do. */
  5656		if (device->total_bytes <= device->bytes_used)
  5657			return 0;
  5658	
  5659		ret = 0;
  5660	
  5661		while (1) {
  5662			struct btrfs_fs_info *fs_info = device->fs_info;
  5663			u64 bytes;
  5664	
  5665			ret = mutex_lock_interruptible(&fs_info->chunk_mutex);
  5666			if (ret)
  5667				break;
  5668	
  5669			find_first_clear_extent_bit(&device->alloc_state, start,
  5670						    &start, &end,
  5671						    CHUNK_TRIMMED | CHUNK_ALLOCATED);
  5672	
  5673			/* CHUNK_* bits not cleared properly */
  5674			if (start > device->total_bytes) {
  5675				WARN_ON(IS_ENABLED(CONFIG_BTRFS_DEBUG));
> 5676				btrfs_warn_in_rcu(fs_info,
  5677	"ignoring attempt to trim beyond device size: offset %llu length %llu device %s device size %llu",
  5678						  start, end - start + 1,
  5679						  rcu_str_deref(device->name),
  5680						  device->total_bytes);
  5681				mutex_unlock(&fs_info->chunk_mutex);
  5682				ret = 0;
  5683				break;
  5684			}
  5685	
  5686			/* The remaining part has already been trimmed */
  5687			if (start == device->total_bytes) {
  5688				mutex_unlock(&fs_info->chunk_mutex);
  5689				ret = 0;
  5690				break;
  5691			}
  5692	
  5693			/* Ensure we skip the reserved area in the first 1M */
  5694			start = max_t(u64, start, SZ_1M);
  5695	
  5696			/*
  5697			 * If find_first_clear_extent_bit find a range that spans the
  5698			 * end of the device it will set end to -1, in this case it's up
  5699			 * to the caller to trim the value to the size of the device.
  5700			 */
  5701			end = min(end, device->total_bytes - 1);
  5702	
  5703			len = end - start + 1;
  5704	
  5705			/* We didn't find any extents */
  5706			if (!len) {
  5707				mutex_unlock(&fs_info->chunk_mutex);
  5708				ret = 0;
  5709				break;
  5710			}
  5711	
  5712			ret = btrfs_issue_discard(device->bdev, start, len,
  5713						  &bytes);
  5714			if (!ret)
  5715				set_extent_bits(&device->alloc_state, start,
  5716						start + bytes - 1,
  5717						CHUNK_TRIMMED);
  5718			mutex_unlock(&fs_info->chunk_mutex);
  5719	
  5720			if (ret)
  5721				break;
  5722	
  5723			start += len;
  5724			*trimmed += bytes;
  5725	
  5726			if (fatal_signal_pending(current)) {
  5727				ret = -ERESTARTSYS;
  5728				break;
  5729			}
  5730	
  5731			cond_resched();
  5732		}
  5733	
  5734		return ret;
  5735	}
  5736	

---
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: 33364 bytes --]

  parent reply	other threads:[~2020-07-31 20:53 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-07-31  9:48 [PATCH v3] btrfs: trim: fix underflow in trim length to prevent access beyond device boundary Qu Wenruo
2020-07-31 10:05 ` Filipe Manana
2020-07-31 10:20   ` Qu Wenruo
2020-07-31 10:38     ` Qu Wenruo
2020-07-31 10:42       ` Filipe Manana
2020-07-31 10:40     ` Filipe Manana
2020-07-31 20:52 ` kernel test robot [this message]
2020-07-31 20:52   ` kernel test robot

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=202008010403.NVCICIjG%lkp@intel.com \
    --to=lkp@intel.com \
    --cc=fdmanana@suse.com \
    --cc=kbuild-all@lists.01.org \
    --cc=linux-btrfs@vger.kernel.org \
    --cc=wqu@suse.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.