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 PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Thu, 17 Mar 2022 21:16:20 +0000 From: Matthew Wilcox <willy@...radead.org> To: Linus Torvalds <torvalds@...ux-foundation.org> Cc: Brian Foster <bfoster@...hat.com>, Linux-MM <linux-mm@...ck.org>, linux-fsdevel <linux-fsdevel@...r.kernel.org>, linux-xfs <linux-xfs@...r.kernel.org>, Hugh Dickins <hughd@...gle.com>, Namjae Jeon <namjae.jeon@...sung.com>, Ashish Sangwan <a.sangwan@...sung.com>, Theodore Ts'o <tytso@....edu>, Jan Kara <jack@...e.cz>, linux-ext4@...r.kernel.org Subject: Re: writeback completion soft lockup BUG in folio_wake_bit() On Thu, Mar 17, 2022 at 12:26:35PM -0700, Linus Torvalds wrote: > On Thu, Mar 17, 2022 at 8:04 AM Matthew Wilcox <willy@...radead.org> wrote: > > > > So how about we do something like this: > > > > - Make folio_start_writeback() and set_page_writeback() return void, > > fixing up AFS and NFS. > > - Add a folio_wait_start_writeback() to use in the VFS > > - Remove the calls to set_page_writeback() in the filesystems > > That sounds lovely, but it does worry me a bit. Not just the odd > 'keepwrite' thing, but also the whole ordering between the folio bit > and the tagging bits. Does the ordering possibly matter? I wouldn't change the ordering of setting the xarray bits and the writeback flag; they'd just be set a little earlier. It'd all be done while the page was still locked. But you're right, there's lots of subtle interactions here. > That whole "xyz_writeback_keepwrite()" thing seems odd. It's used in > only one place (the folio version isn't used at all): > > ext4_writepage(): > > ext4_walk_page_buffers() fails: > redirty_page_for_writepage(wbc, page); > keep_towrite = true; > ext4_bio_write_page(). > > which just looks odd. Why does it even try to continue to do the > writepage when the page buffer thing has failed? > > In the regular write path (ie ext4_write_begin()), a > ext4_walk_page_buffers() failure is fatal or causes a retry). Why is > ext4_writepage() any different? Particularly since it wants to keep > the page dirty, then trying to do the writeback just seems wrong. > > So this code is all a bit odd, I suspect there are decades of "people > continued to do what they historically did" changes, and it is all > worrisome. I found the commit: 1c8349a17137 ("ext4: fix data integrity sync in ordered mode"). Fortunately, we have a documented test for this, generic/127, so we'll know if we've broken it.
Powered by blists - more mailing lists