[PATCH 4/4] ubifs: Convert do_writepage() to take a folio
Matthew Wilcox
willy at infradead.org
Mon Jun 5 20:22:51 PDT 2023
On Mon, Jun 05, 2023 at 11:37:00PM +0200, Richard Weinberger wrote:
> > - addr = kmap(page);
> > - block = page->index << UBIFS_BLOCKS_PER_PAGE_SHIFT;
> > + addr = kmap_local_folio(folio, offset);
> > + block = folio->index << UBIFS_BLOCKS_PER_PAGE_SHIFT;
> > i = 0;
> > - while (len) {
> > - blen = min_t(int, len, UBIFS_BLOCK_SIZE);
> > + for (;;) {
>
> This change will cause a file system corruption.
> If len is zero (it can be) then a zero length data node will be written.
> The while(len) made sure that upon zero length nothing is written.
I don't see how 'len' can be 0. len is modified each time around the
loop, and if it's decremented to 0, we break. So you must be referring
to a case where the caller of do_writepage passes 0.
There are three callers of do_writepage, two in ubifs_writepage():
int err, len = folio_size(folio);
...
if (folio_pos(folio) + len < i_size) {
...
return do_writepage(folio, len);
len is folio_size(), which is not 0.
len = offset_in_folio(folio, i_size);
Here, we know that len is not 0. We already tested earlier:
if (folio_pos(folio) >= i_size) {
so we know that i_size > folio_pos() and i_size < folio_pos() +
folio_size(). Actually, I should make this more explicit:
len = i_size - folio_pos(folio);
Now it should be clear that len cannot be zero.
The third caller is do_truncation():
loff_t old_size = inode->i_size, new_size = attr->ia_size;
int offset = new_size & (UBIFS_BLOCK_SIZE - 1), budgeted = 1;
if (offset) {
pgoff_t index = new_size >> PAGE_SHIFT;
offset = offset_in_folio(folio,
new_size);
err = do_writepage(folio, offset);
It's not large-folio-safe, but it's definitely not 0.
Did I miss something?
More information about the linux-mtd
mailing list