On Wed 01-03-23 16:10:04, Matthew Wilcox wrote:
> On Wed, Mar 01, 2023 at 02:46:35PM +0100, Jan Kara wrote:
> > The patch converting udf_adinicb_writepage() to avoid manually kmapping
> > the page used memcpy_to_page() however that copies in the wrong
> > direction (effectively overwriting file data with the old contents).
> > What we should be using is memcpy_from_page() to copy data from the page
> > into the inode and then mark inode dirty to store the data.
> >
> > Fixes: 5cfc45321a6d ("udf: Convert udf_adinicb_writepage() to memcpy_to_page()")
> > Signed-off-by: Jan Kara <jack@xxxxxxx>
>
> Oops. Now you're copying in the right direction, we have a folio
> function for that, so we could just folio-ise the entire function?
> Maybe you'd rather keep the fix minimal and apply this later.
>
> Signed-off-by: Matthew Wilcox (Oracle) <willy@xxxxxxxxxxxxx>
Thanks! Yes, I had this in mind but wanted to fix the corruption first so
that I can push it quickly to rc2... But I'll queue your patch for later.
Honza
>
> diff --git a/fs/udf/inode.c b/fs/udf/inode.c
> index f7a9607c2b95..890be63ddd02 100644
> --- a/fs/udf/inode.c
> +++ b/fs/udf/inode.c
> @@ -188,14 +188,14 @@ static void udf_write_failed(struct address_space *mapping, loff_t to)
> static int udf_adinicb_writepage(struct folio *folio,
> struct writeback_control *wbc, void *data)
> {
> - struct page *page = &folio->page;
> - struct inode *inode = page->mapping->host;
> + struct inode *inode = folio->mapping->host;
> struct udf_inode_info *iinfo = UDF_I(inode);
>
> - BUG_ON(!PageLocked(page));
> - memcpy_to_page(page, 0, iinfo->i_data + iinfo->i_lenEAttr,
> + BUG_ON(!folio_test_locked(folio));
> + BUG_ON(folio->index != 0);
> + memcpy_from_file_folio(iinfo->i_data + iinfo->i_lenEAttr, folio, 0,
> i_size_read(inode));
> - unlock_page(page);
> + folio_unlock(folio);
> mark_inode_dirty(inode);
>
> return 0;
--
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR
