On Tue, Nov 12, 2019 at 10:48:19AM +0800, Chao Yu wrote: > On 2019/11/12 9:34, 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: > > > > 0b20fcec8651 ("f2fs: cache global IPU bio") > > > > Coverity reported the following: > > > > *** CID 1487851: Memory - illegal accesses (USE_AFTER_FREE) > > /fs/f2fs/data.c: 604 in add_ipu_page() > > 598 break; > > 599 } > > 600 up_write(&io->bio_list_lock); > > 601 } > > 602 > > 603 if (ret) { > > vvv CID 1487851: Memory - illegal accesses (USE_AFTER_FREE) > > vvv Calling "bio_put" dereferences freed pointer "*bio". > > 604 bio_put(*bio); > > 605 *bio = NULL; > > 606 } > > 607 > > 608 return ret; > > 609 } > > Thanks for the report. > > I double check these related codes: > > static int add_ipu_page(struct f2fs_sb_info *sbi, struct bio **bio, > struct page *page) > { > enum temp_type temp; > bool found = false; > int ret = -EAGAIN; > > for (temp = HOT; temp < NR_TEMP_TYPE && !found; temp++) { > struct f2fs_bio_info *io = sbi->write_io[DATA] + temp; > struct list_head *head = &io->bio_list; > struct bio_entry *be; > > down_write(&io->bio_list_lock); > list_for_each_entry(be, head, list) { > if (be->bio != *bio) > continue; > > found = true; > > if (bio_add_page(*bio, page, PAGE_SIZE, 0) == PAGE_SIZE) { > ret = 0; > break; > } > > /* bio is full */ > del_bio_entry(be); > __submit_bio(sbi, *bio, DATA); > break; > } > up_write(&io->bio_list_lock); > } > > if (ret) { > > If we get here, that means 1) found nothing due to someone has submitted bio for > us, or 2) found target bio, however bio is full, we submitted the bio. For both > conditions, previously, we grab one extra ref on bio, here, we just release the > ref and reset *bio to NULL, then caller can allocate new bio. > > Let me know if I'm missing something. Okay, I've noted it as a false positive. I don't know this code at all, so I can't really comment on the lifetime expectations here. :) Thanks for looking at it! -Kees > > bio_put(*bio); > *bio = NULL; > } > > return ret; > } > > > > > 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@xxxxxxxxxxxx> > > Addresses-Coverity-ID: 1487851 ("Memory - illegal accesses") > > Fixes: 0b20fcec8651 ("f2fs: cache global IPU bio") > > > > > > Thanks for your attention! > > -- Kees Cook
