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: Fri, 10 Nov 2023 18:50:33 +0100 From: Andreas Grünbacher <andreas.gruenbacher@...il.com> To: Matthew Wilcox <willy@...radead.org> Cc: Andreas Gruenbacher <agruenba@...hat.com>, Andrew Morton <akpm@...ux-foundation.org>, linux-ext4@...r.kernel.org, gfs2@...ts.linux.dev, linux-fsdevel@...r.kernel.org, linux-xfs@...r.kernel.org, "Darrick J . Wong" <djwong@...nel.org>, linux-erofs@...ts.ozlabs.org, "Theodore Ts'o" <tytso@....edu>, Andreas Dilger <adilger.kernel@...ger.ca> Subject: Re: [PATCH 2/3] mm: Add folio_fill_tail() and use it in iomap Am Fr., 10. Nov. 2023 um 18:09 Uhr schrieb Matthew Wilcox <willy@...radead.org>: > On Thu, Nov 09, 2023 at 10:50:45PM +0100, Andreas Gruenbacher wrote: > > On Tue, Nov 7, 2023 at 10:27 PM Matthew Wilcox (Oracle) > > <willy@...radead.org> wrote: > > > +static inline void folio_fill_tail(struct folio *folio, size_t offset, > > > + const char *from, size_t len) > > > +{ > > > + char *to = kmap_local_folio(folio, offset); > > > + > > > + VM_BUG_ON(offset + len > folio_size(folio)); > > > + > > > + if (folio_test_highmem(folio)) { > > > + size_t max = PAGE_SIZE - offset_in_page(offset); > > > + > > > + while (len > max) { > > > + memcpy(to, from, max); > > > + kunmap_local(to); > > > + len -= max; > > > + from += max; > > > + offset += max; > > > + max = PAGE_SIZE; > > > + to = kmap_local_folio(folio, offset); > > > + } > > > + } > > > + > > > + memcpy(to, from, len); > > > + to = folio_zero_tail(folio, offset, to); > > > > This needs to be: > > > > to = folio_zero_tail(folio, offset + len, to + len); > > Oh, wow, that was stupid of me. I only ran an xfstests against ext4, > which doesn't exercise this code, not gfs2 or erofs. Thanks for > fixing this up. > > I was wondering about adding the assertion: > > VM_BUG_ON((kaddr - offset) % PAGE_SIZE); > > to catch the possible mistake of calling kmap_local_folio(folio, 0) > instead of kmap_local_folio(folio, offset). But maybe that's > sufficiently unlikely a mistake to bother adding a runtime check for. folio_zero_tail() is a bit of an obscure function, so I'm not sure if there will be additional callers. The parameters are described as: * @offset: The byte offset in the folio to start zeroing at. * @kaddr: The address the folio is currently mapped to. What about changing the @kaddr description to 'the (mapped) address within the folio to start zeroing at' or similar? Andreas
Powered by blists - more mailing lists