* 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@vger.kernel.org
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, other threads:[~2019-11-19 2:57 UTC | newest]
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
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.