Re: [PATCH] ntfs: fix kmap_local_page() usage in compress
Hyunchul Lee <[email protected]> Fri, 17 Jul 2026 07:27:44 +0900
| Newsgroups | dev.linux.lists.ntfs,org.kernel.vger.linux-fsdevel |
|---|---|
| Message-ID | <CANFS6bZbBGuTugd4fNZK4i2JcyODeZGtGPObj-qPZFu62mznyQ@mail.gmail.com> |
2026=EB=85=84 7=EC=9B=94 16=EC=9D=BC (=EB=AA=A9) =EC=98=A4=EC=A0=84 11:47, = Namjae Jeon <[email protected]>=EB=8B=98=EC=9D=B4 =EC=9E=91=EC=84=B1: > > Several compressed I/O paths discard the address returned by > kmap_local_page() and later access or unmap the page using page_address()= . > This is invalid for highmem pages, and local mappings must also be unmapp= ed > using the address returned by kmap_local_page(). > > Map each destination page in ntfs_decompress() only while producing the > current sub-block. Use memcpy_from_page(), memcpy_to_page(), and > memzero_page() for the other page accesses. Remove unnecessary local > mappings from ntfs_write_cb(), where pages are accessed through the vmap(= ) > mapping. > > Fixes: 495e90fa3348 ("ntfs: update attrib operations") > Reported-by: Matthew Wilcox <[email protected]> > Signed-off-by: Namjae Jeon <[email protected]> Looks good to me. Reviewed-by: Hyunchul Lee <[email protected]> > --- > fs/ntfs/compress.c | 38 +++++++++++++++----------------------- > 1 file changed, 15 insertions(+), 23 deletions(-) > > diff --git a/fs/ntfs/compress.c b/fs/ntfs/compress.c > index d83d3e8e06ae..e866f43ca30b 100644 > --- a/fs/ntfs/compress.c > +++ b/fs/ntfs/compress.c > @@ -177,6 +177,7 @@ static int ntfs_decompress(struct page *dest_pages[],= int completed_pages[], > > /* Variables for uncompressed data / destination. */ > struct page *dp; /* Current destination page being worked = on. */ > + u8 *dp_kaddr; /* Local kmap for the current destination= page. */ > u8 *dp_addr; /* Current pointer into dp. */ > u8 *dp_sb_start; /* Start of current sub-block in dp. */ > u8 *dp_sb_end; /* End of current sb in dp (dp_sb_start += NTFS_SB_SIZE). */ > @@ -191,6 +192,7 @@ static int ntfs_decompress(struct page *dest_pages[],= int completed_pages[], > /* Default error code. */ > int err =3D -EOVERFLOW; > > + dp_kaddr =3D NULL; > ntfs_debug("Entering, cb_size =3D 0x%x.", cb_size); > do_next_sb: > ntfs_debug("Beginning sub-block at offset =3D 0x%zx in the cb.", > @@ -223,7 +225,6 @@ static int ntfs_decompress(struct page *dest_pages[],= int completed_pages[], > */ > handle_bounds_compressed_page(dp, i_size, > initialized_size); > - kunmap_local(page_address(dp)); > SetPageUptodate(dp); > unlock_page(dp); > if (di =3D=3D xpage) > @@ -269,7 +270,8 @@ static int ntfs_decompress(struct page *dest_pages[],= int completed_pages[], > } > > /* We have a valid destination page. Setup the destination pointe= rs. */ > - dp_addr =3D (u8 *)page_address(dp) + do_sb_start; > + dp_kaddr =3D kmap_local_page(dp); > + dp_addr =3D dp_kaddr + do_sb_start; > > /* Now, we are ready to process the current sub-block (sb). */ > if (!(le16_to_cpup((__le16 *)cb) & NTFS_SB_IS_COMPRESSED)) { > @@ -290,6 +292,8 @@ static int ntfs_decompress(struct page *dest_pages[],= int completed_pages[], > /* Advance destination position to next sub-block. */ > *dest_ofs +=3D NTFS_SB_SIZE; > *dest_ofs &=3D ~PAGE_MASK; > + kunmap_local(dp_kaddr); > + dp_kaddr =3D NULL; > if (!(*dest_ofs)) { > finalize_page: > /* > @@ -324,6 +328,8 @@ static int ntfs_decompress(struct page *dest_pages[],= int completed_pages[], > } > /* We have finished the current sub-block. */ > *dest_ofs &=3D ~PAGE_MASK; > + kunmap_local(dp_kaddr); > + dp_kaddr =3D NULL; > if (!(*dest_ofs)) > goto finalize_page; > goto do_next_sb; > @@ -429,6 +435,8 @@ static int ntfs_decompress(struct page *dest_pages[],= int completed_pages[], > goto do_next_tag; > > return_overflow: > + if (dp_kaddr) > + kunmap_local(dp_kaddr); > ntfs_error(NULL, "Failed. Returning -EOVERFLOW."); > goto return_error; > } > @@ -557,7 +565,6 @@ int ntfs_read_compressed_block(struct folio *folio) > * least wasting our time. > */ > if (!PageDirty(page) && (!PageUptodate(page))) { > - kmap_local_page(page); > continue; > } > unlock_page(page); > @@ -643,8 +650,7 @@ int ntfs_read_compressed_block(struct folio *folio) > } > > lock_page(lpage); > - memcpy(cb_pos, page_address(lpage) + page_ofs, > - vol->cluster_size); > + memcpy_from_page(cb_pos, lpage, page_ofs, vol->cluster_si= ze); > unlock_page(lpage); > put_page(lpage); > cb_pos +=3D vol->cluster_size; > @@ -683,14 +689,7 @@ int ntfs_read_compressed_block(struct folio *folio) > for (; cur_page < cb_max_page; cur_page++) { > page =3D pages[cur_page]; > if (page) { > - if (likely(!cur_ofs)) > - clear_page(page_address(page)); > - else > - memset(page_address(page) + cur_o= fs, 0, > - PAGE_SIZE - > - cur_ofs); > - flush_dcache_page(page); > - kunmap_local(page_address(page)); > + memzero_page(page, cur_ofs, PAGE_SIZE - c= ur_ofs); > SetPageUptodate(page); > unlock_page(page); > if (cur_page =3D=3D xpage) > @@ -708,8 +707,7 @@ int ntfs_read_compressed_block(struct folio *folio) > if (cb_max_ofs && cb_pos < cb_end) { > page =3D pages[cur_page]; > if (page) > - memset(page_address(page) + cur_ofs, 0, > - cb_max_ofs - cur_ofs); > + memzero_page(page, cur_ofs, cb_max_ofs - = cur_ofs); > /* > * No need to update cb_pos at this stage: > * cb_pos +=3D cb_max_ofs - cur_ofs; > @@ -730,7 +728,7 @@ int ntfs_read_compressed_block(struct folio *folio) > for (; cur_page < cb_max_page; cur_page++) { > page =3D pages[cur_page]; > if (page) > - memcpy(page_address(page) + cur_ofs, cb_p= os, > + memcpy_to_page(page, cur_ofs, cb_pos, > PAGE_SIZE - cur_ofs); > cb_pos +=3D PAGE_SIZE - cur_ofs; > cur_ofs =3D 0; > @@ -741,7 +739,7 @@ int ntfs_read_compressed_block(struct folio *folio) > if (cb_max_ofs && cb_pos < cb_end) { > page =3D pages[cur_page]; > if (page) > - memcpy(page_address(page) + cur_ofs, cb_p= os, > + memcpy_to_page(page, cur_ofs, cb_pos, > cb_max_ofs - cur_ofs); > cb_pos +=3D cb_max_ofs - cur_ofs; > cur_ofs =3D cb_max_ofs; > @@ -758,7 +756,6 @@ int ntfs_read_compressed_block(struct folio *folio) > */ > handle_bounds_compressed_page(page, i_siz= e, > initialized_size); > - kunmap_local(page_address(page)); > SetPageUptodate(page); > unlock_page(page); > if (cur2_page =3D=3D xpage) > @@ -794,7 +791,6 @@ int ntfs_read_compressed_block(struct folio *folio) > page =3D pages[prev_cur_page]; > if (page) { > flush_dcache_page(page); > - kunmap_local(page_address(page)); > unlock_page(page); > if (prev_cur_page !=3D xpage) > put_page(page); > @@ -818,7 +814,6 @@ int ntfs_read_compressed_block(struct folio *folio) > "Still have pages left! Terminating them = with extreme prejudice. Inode 0x%llx, page index 0x%lx.", > ni->mft_no, folio->index); > flush_dcache_folio(folio); > - kunmap_local(page_address(page)); > folio_unlock(folio); > if (cur_page !=3D xpage) > folio_put(folio); > @@ -856,7 +851,6 @@ int ntfs_read_compressed_block(struct folio *folio) > page =3D pages[i]; > if (page) { > flush_dcache_page(page); > - kunmap_local(page_address(page)); > unlock_page(page); > if (i !=3D xpage) > put_page(page); > @@ -1308,7 +1302,6 @@ static int ntfs_write_cb(struct ntfs_inode *ni, lof= f_t pos, struct page **pages, > } > pages_disk[i] =3D pg; > lock_page(pg); > - kmap_local_page(pg); > } > > outbuf =3D vmap(pages_disk, pages_count, VM_MAP, PAGE_KERNEL); > @@ -1443,7 +1436,6 @@ static int ntfs_write_cb(struct ntfs_inode *ni, lof= f_t pos, struct page **pages, > for (i =3D 0; i < pages_count; i++) { > pg =3D pages_disk[i]; > if (pg) { > - kunmap_local(page_address(pg)); > unlock_page(pg); > put_page(pg); > } > -- > 2.25.1 > --=20 Thanks, Hyunchul