| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <01c6db4f-eb47-467a-9bb9-3ca6ee4817d7@moroto.mountain>
Date: Sat, 8 Jun 2024 17:25:20 +0300
From: Dan Carpenter <dan.carpenter@...aro.org>
To: Jan Kara <jack@...e.cz>
Cc: linux-ext4@...r.kernel.org
Subject: [bug report] ext4: Convert data=journal writeback to use
ext4_writepages()
Hello Jan Kara,
Commit 3f079114bf52 ("ext4: Convert data=journal writeback to use
ext4_writepages()") from Feb 28, 2023 (linux-next), leads to the
following Smatch static checker warning:
fs/ext4/inode.c:2478 mpage_prepare_extent_to_map()
error: we previously assumed 'handle' could be null (see line 2417)
fs/ext4/inode.c
2362 static int mpage_prepare_extent_to_map(struct mpage_da_data *mpd)
2363 {
2364 struct address_space *mapping = mpd->inode->i_mapping;
2365 struct folio_batch fbatch;
2366 unsigned int nr_folios;
2367 pgoff_t index = mpd->first_page;
2368 pgoff_t end = mpd->last_page;
2369 xa_mark_t tag;
2370 int i, err = 0;
2371 int blkbits = mpd->inode->i_blkbits;
2372 ext4_lblk_t lblk;
2373 struct buffer_head *head;
2374 handle_t *handle = NULL;
2375 int bpp = ext4_journal_blocks_per_page(mpd->inode);
2376
2377 if (mpd->wbc->sync_mode == WB_SYNC_ALL || mpd->wbc->tagged_writepages)
2378 tag = PAGECACHE_TAG_TOWRITE;
2379 else
2380 tag = PAGECACHE_TAG_DIRTY;
2381
2382 mpd->map.m_len = 0;
2383 mpd->next_page = index;
2384 if (ext4_should_journal_data(mpd->inode)) {
2385 handle = ext4_journal_start(mpd->inode, EXT4_HT_WRITE_PAGE,
2386 bpp);
2387 if (IS_ERR(handle))
2388 return PTR_ERR(handle);
2389 }
2390 folio_batch_init(&fbatch);
2391 while (index <= end) {
2392 nr_folios = filemap_get_folios_tag(mapping, &index, end,
2393 tag, &fbatch);
2394 if (nr_folios == 0)
2395 break;
2396
2397 for (i = 0; i < nr_folios; i++) {
2398 struct folio *folio = fbatch.folios[i];
2399
2400 /*
2401 * Accumulated enough dirty pages? This doesn't apply
2402 * to WB_SYNC_ALL mode. For integrity sync we have to
2403 * keep going because someone may be concurrently
2404 * dirtying pages, and we might have synced a lot of
2405 * newly appeared dirty pages, but have not synced all
2406 * of the old dirty pages.
2407 */
2408 if (mpd->wbc->sync_mode == WB_SYNC_NONE &&
2409 mpd->wbc->nr_to_write <=
2410 mpd->map.m_len >> (PAGE_SHIFT - blkbits))
2411 goto out;
2412
2413 /* If we can't merge this page, we are done. */
2414 if (mpd->map.m_len > 0 && mpd->next_page != folio->index)
2415 goto out;
2416
2417 if (handle) {
^^^^^^
Checked for NULL
2418 err = ext4_journal_ensure_credits(handle, bpp,
2419 0);
2420 if (err < 0)
2421 goto out;
2422 }
2423
2424 folio_lock(folio);
2425 /*
2426 * If the page is no longer dirty, or its mapping no
2427 * longer corresponds to inode we are writing (which
2428 * means it has been truncated or invalidated), or the
2429 * page is already under writeback and we are not doing
2430 * a data integrity writeback, skip the page
2431 */
2432 if (!folio_test_dirty(folio) ||
2433 (folio_test_writeback(folio) &&
2434 (mpd->wbc->sync_mode == WB_SYNC_NONE)) ||
2435 unlikely(folio->mapping != mapping)) {
2436 folio_unlock(folio);
2437 continue;
2438 }
2439
2440 folio_wait_writeback(folio);
2441 BUG_ON(folio_test_writeback(folio));
2442
2443 /*
2444 * Should never happen but for buggy code in
2445 * other subsystems that call
2446 * set_page_dirty() without properly warning
2447 * the file system first. See [1] for more
2448 * information.
2449 *
2450 * [1] https://lore.kernel.org/linux-mm/20180103100430.GE4911@quack2.suse.cz
2451 */
2452 if (!folio_buffers(folio)) {
2453 ext4_warning_inode(mpd->inode, "page %lu does not have buffers attached", folio->index);
2454 folio_clear_dirty(folio);
2455 folio_unlock(folio);
2456 continue;
2457 }
2458
2459 if (mpd->map.m_len == 0)
2460 mpd->first_page = folio->index;
2461 mpd->next_page = folio_next_index(folio);
2462 /*
2463 * Writeout when we cannot modify metadata is simple.
2464 * Just submit the page. For data=journal mode we
2465 * first handle writeout of the page for checkpoint and
2466 * only after that handle delayed page dirtying. This
2467 * makes sure current data is checkpointed to the final
2468 * location before possibly journalling it again which
2469 * is desirable when the page is frequently dirtied
2470 * through a pin.
2471 */
2472 if (!mpd->can_map) {
2473 err = mpage_submit_folio(mpd, folio);
2474 if (err < 0)
2475 goto out;
2476 /* Pending dirtying of journalled data? */
2477 if (folio_test_checked(folio)) {
--> 2478 err = mpage_journal_page_buffers(handle,
^^^^^^
Unchecked dereferenced inside the function.
2479 mpd, folio);
2480 if (err < 0)
2481 goto out;
2482 mpd->journalled_more_data = 1;
2483 }
2484 mpage_folio_done(mpd, folio);
2485 } else {
2486 /* Add all dirty buffers to mpd */
2487 lblk = ((ext4_lblk_t)folio->index) <<
2488 (PAGE_SHIFT - blkbits);
2489 head = folio_buffers(folio);
2490 err = mpage_process_page_bufs(mpd, head, head,
2491 lblk);
2492 if (err <= 0)
2493 goto out;
2494 err = 0;
2495 }
2496 }
2497 folio_batch_release(&fbatch);
2498 cond_resched();
2499 }
2500 mpd->scanned_until_end = 1;
2501 if (handle)
2502 ext4_journal_stop(handle);
2503 return 0;
2504 out:
2505 folio_batch_release(&fbatch);
2506 if (handle)
2507 ext4_journal_stop(handle);
2508 return err;
2509 }
regards,
dan carpenter
Powered by blists - more mailing lists