linux-ext4 - Re: [PATCH v3 11/12] ext4: Stop providing .writepage hook

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 6 Dec 2022 11:52:25 +0100
From:   Jan Kara <jack@...e.cz>
To:     Theodore Ts'o <tytso@....edu>
Cc:     Jan Kara <jack@...e.cz>, linux-ext4@...r.kernel.org,
        Christoph Hellwig <hch@...radead.org>,
        "Ritesh Harjani (IBM)" <ritesh.list@...il.com>,
        Christoph Hellwig <hch@....de>,
        Matthew Wilcox <willy@...radead.org>, linux-mm@...ck.org
Subject: Re: [PATCH v3 11/12] ext4: Stop providing .writepage hook

On Mon 05-12-22 22:17:26, Theodore Ts'o wrote:
> On Mon, Dec 05, 2022 at 01:29:25PM +0100, Jan Kara wrote:
> > Now we don't need .writepage hook for anything anymore. Reclaim is fine
> > with relying on .writepages to clean pages and we often couldn't do much
> > from the .writepage callback anyway. We only need to provide
> > .migrate_folio callback for the ext4_journalled_aops - let's use
> > buffer_migrate_page_norefs() there so that buffers cannot be modified
>   ^^^^^^^^^^^^^^^^^^^^^^^^^^  this should be buffer_migrate_folio_norefs, no?
> > under jdb2's hands.
> > 
> > Reviewed-by: Christoph Hellwig <hch@....de>
> > Signed-off-by: Jan Kara <jack@...e.cz>
> 
> Could you clarify in the commit how critical it is to use the
> _norefs() variant?  It's not entirely clear what you mean by "let's
> use...".  I think what is meant is that we need to use ..._noref() or
> we can get in trouble if while the page update is getting committed,
> there is an attempted to migrate the folio containing the page.

Exacly. For example when commit code does writeout of transaction buffers
in jbd2_journal_write_metadata_buffer(), we don't hold page lock or have
page writeback bit set or have buffer locked. So page migration code would
go and happily migrate the page elsewhere while the copy is running thus
corrupting data.

I've added this to the changelog.

> buffer_migrate_folio_norefs() is currently not exported (although
> buffer_migrate_folio is).  So if we need it for ext4, we're going to
> have to EXPORT_SYMBOL buffer_migrate_folio_norefs.
> 
> Any objections from the mm folks?

I don't expect any objection. The only reason we didn't export that
function when I've added it was that only blkdev code was using it and that
cannot be compiled as a module. Should I send a patch to 

I've added a patch to the series to export this function. It is attached.

I can also repost the whole series if these are the only changes that block
the inclusion.


								Honza
-- 
Jan Kara <jack@...e.com>
SUSE Labs, CR

View attachment "0001-mm-Export-buffer_migrate_folio_norefs.patch" of type "text/x-patch" (769 bytes)