Linux-Next Archive on lore.kernel.org
 help / color / Atom feed
* Coverity: blkdev_zone_mgmt(): Memory - illegal accesses
@ 2019-11-12  1:35 coverity-bot
  2019-11-18 20:46 ` Kees Cook
  0 siblings, 1 reply; 3+ messages in thread
From: coverity-bot @ 2019-11-12  1:35 UTC (permalink / raw)
  To: Damien Le Moal
  Cc: Christoph Hellwig, Hannes Reinecke, Jens Axboe,
	Gustavo A. R. Silva, linux-next

Hello!

This is an experimental automated report about issues detected by Coverity
from a scan of next-20191108 as part of the linux-next weekly scan project:
https://scan.coverity.com/projects/linux-next-weekly-scan

You're getting this email because you were associated with the identified
lines of code (noted below) that were touched by recent commits:

a2d6b3a2d390 ("block: Improve zone reset execution")

Coverity reported the following:

*** CID 1487849:  Memory - illegal accesses  (USE_AFTER_FREE)
/block/blk-zoned.c: 293 in blkdev_zone_mgmt()
287
288     		/* This may take a while, so be nice to others */
289     		cond_resched();
290     	}
291
292     	ret = submit_bio_wait(bio);
vvv     CID 1487849:  Memory - illegal accesses  (USE_AFTER_FREE)
vvv     Calling "bio_put" dereferences freed pointer "bio".
293     	bio_put(bio);
294
295     	return ret;
296     }
297     EXPORT_SYMBOL_GPL(blkdev_zone_mgmt);
298

If this is a false positive, please let us know so we can mark it as
such, or teach the Coverity rules to be smarter. If not, please make
sure fixes get into linux-next. :) For patches fixing this, please
include these lines (but double-check the "Fixes" first):

Reported-by: coverity-bot <keescook+coverity-bot@chromium.org>
Addresses-Coverity-ID: 1487849 ("Memory - illegal accesses")
Fixes: a2d6b3a2d390 ("block: Improve zone reset execution")


Thanks for your attention!

-- 
Coverity-bot

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

* Re: Coverity: blkdev_zone_mgmt(): Memory - illegal accesses
  2019-11-12  1:35 Coverity: blkdev_zone_mgmt(): Memory - illegal accesses coverity-bot
@ 2019-11-18 20:46 ` Kees Cook
  2019-11-19  2:57   ` Damien Le Moal
  0 siblings, 1 reply; 3+ messages in thread
From: Kees Cook @ 2019-11-18 20:46 UTC (permalink / raw)
  To: Jens Axboe, Damien Le Moal
  Cc: Christoph Hellwig, Hannes Reinecke, Gustavo A. R. Silva, linux-next

On Mon, Nov 11, 2019 at 05:35:00PM -0800, coverity-bot wrote:
> Hello!
> 
> This is an experimental automated report about issues detected by Coverity
> from a scan of next-20191108 as part of the linux-next weekly scan project:
> https://scan.coverity.com/projects/linux-next-weekly-scan
> 
> You're getting this email because you were associated with the identified
> lines of code (noted below) that were touched by recent commits:
> 
> a2d6b3a2d390 ("block: Improve zone reset execution")
> 
> Coverity reported the following:
> 
> *** CID 1487849:  Memory - illegal accesses  (USE_AFTER_FREE)
> /block/blk-zoned.c: 293 in blkdev_zone_mgmt()
> 287
> 288     		/* This may take a while, so be nice to others */
> 289     		cond_resched();
> 290     	}
> 291
> 292     	ret = submit_bio_wait(bio);
> vvv     CID 1487849:  Memory - illegal accesses  (USE_AFTER_FREE)
> vvv     Calling "bio_put" dereferences freed pointer "bio".
> 293     	bio_put(bio);

I don't know this area of the code very well, but looking through the
helpers here, it does seem like "bio" gets (or can be) freed before
returning from submit_bio_wait() (regardless to what the comment
says):

submit_bio_wait()
  submit_bio()
    generic_make_request()
      generic_make_request_check()
        bio_endio()
          __bio_chain_endio()
            bio_put()

The path passes into some error handling here, but it does seem like it
could be possible to hit both bio_put()s?

Can anyone speak to this?

-Kees

> 294
> 295     	return ret;
> 296     }
> 297     EXPORT_SYMBOL_GPL(blkdev_zone_mgmt);
> 298
> 
> If this is a false positive, please let us know so we can mark it as
> such, or teach the Coverity rules to be smarter. If not, please make
> sure fixes get into linux-next. :) For patches fixing this, please
> include these lines (but double-check the "Fixes" first):
> 
> Reported-by: coverity-bot <keescook+coverity-bot@chromium.org>
> Addresses-Coverity-ID: 1487849 ("Memory - illegal accesses")
> Fixes: a2d6b3a2d390 ("block: Improve zone reset execution")
> 
> 
> Thanks for your attention!
> 
> -- 
> Coverity-bot

-- 
Kees Cook

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

* Re: Coverity: blkdev_zone_mgmt(): Memory - illegal accesses
  2019-11-18 20:46 ` Kees Cook
@ 2019-11-19  2:57   ` Damien Le Moal
  0 siblings, 0 replies; 3+ messages in thread
From: Damien Le Moal @ 2019-11-19  2:57 UTC (permalink / raw)
  To: Kees Cook, Jens Axboe
  Cc: Christoph Hellwig, Hannes Reinecke, Gustavo A. R. Silva, linux-next

On 2019/11/19 5:46, Kees Cook wrote:
> On Mon, Nov 11, 2019 at 05:35:00PM -0800, coverity-bot wrote:
>> Hello!
>>
>> This is an experimental automated report about issues detected by Coverity
>> from a scan of next-20191108 as part of the linux-next weekly scan project:
>> https://scan.coverity.com/projects/linux-next-weekly-scan
>>
>> You're getting this email because you were associated with the identified
>> lines of code (noted below) that were touched by recent commits:
>>
>> a2d6b3a2d390 ("block: Improve zone reset execution")
>>
>> Coverity reported the following:
>>
>> *** CID 1487849:  Memory - illegal accesses  (USE_AFTER_FREE)
>> /block/blk-zoned.c: 293 in blkdev_zone_mgmt()
>> 287
>> 288     		/* This may take a while, so be nice to others */
>> 289     		cond_resched();
>> 290     	}
>> 291
>> 292     	ret = submit_bio_wait(bio);
>> vvv     CID 1487849:  Memory - illegal accesses  (USE_AFTER_FREE)
>> vvv     Calling "bio_put" dereferences freed pointer "bio".
>> 293     	bio_put(bio);
> 
> I don't know this area of the code very well, but looking through the
> helpers here, it does seem like "bio" gets (or can be) freed before
> returning from submit_bio_wait() (regardless to what the comment
> says):
> 
> submit_bio_wait()
>   submit_bio()
>     generic_make_request()
>       generic_make_request_check()
>         bio_endio()
>           __bio_chain_endio()
>             bio_put()
> 
> The path passes into some error handling here, but it does seem like it
> could be possible to hit both bio_put()s?
> 
> Can anyone speak to this?

Kees,

This is a false positive: the end callback for a BIO submitted with
submit_bio_wait() is set to the submit_bio_wait_endio() function which
only does a complete() call for a completion on stack. There is no
bio_put() in the completion path in that case, hence the extra bio_put()
called after the submit_bio_wait().

This is all similar to the functions blkdev_issue_discard() or
blkdev_issue_write_same() in block/blk-lib.c.

> 
> -Kees
> 
>> 294
>> 295     	return ret;
>> 296     }
>> 297     EXPORT_SYMBOL_GPL(blkdev_zone_mgmt);
>> 298
>>
>> If this is a false positive, please let us know so we can mark it as
>> such, or teach the Coverity rules to be smarter. If not, please make
>> sure fixes get into linux-next. :) For patches fixing this, please
>> include these lines (but double-check the "Fixes" first):
>>
>> Reported-by: coverity-bot <keescook+coverity-bot@chromium.org>
>> Addresses-Coverity-ID: 1487849 ("Memory - illegal accesses")
>> Fixes: a2d6b3a2d390 ("block: Improve zone reset execution")
>>
>>
>> Thanks for your attention!
>>
>> -- 
>> Coverity-bot
> 


-- 
Damien Le Moal
Western Digital Research

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

end of thread, back to index

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-11-12  1:35 Coverity: blkdev_zone_mgmt(): Memory - illegal accesses coverity-bot
2019-11-18 20:46 ` Kees Cook
2019-11-19  2:57   ` Damien Le Moal

Linux-Next Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-next/0 linux-next/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-next linux-next/ https://lore.kernel.org/linux-next \
		linux-next@vger.kernel.org
	public-inbox-index linux-next

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-next


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git