| Message ID | 20230301134641.11819-1-jack@suse.cz (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | udf: Fix handling of in-ICB files | expand |
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@suse.cz> 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@infradead.org> 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;
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@suse.cz> > > 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@infradead.org> 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;
diff --git a/fs/udf/inode.c b/fs/udf/inode.c index f7a9607c2b95..facaf3a20625 100644 --- a/fs/udf/inode.c +++ b/fs/udf/inode.c @@ -193,7 +193,7 @@ static int udf_adinicb_writepage(struct folio *folio, struct udf_inode_info *iinfo = UDF_I(inode); BUG_ON(!PageLocked(page)); - memcpy_to_page(page, 0, iinfo->i_data + iinfo->i_lenEAttr, + memcpy_from_page(iinfo->i_data + iinfo->i_lenEAttr, page, 0, i_size_read(inode)); unlock_page(page); mark_inode_dirty(inode);
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@suse.cz> --- fs/udf/inode.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)