| Message ID | 20230207133916.3109147-1-dhowells@redhat.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | iomap, splice: Fix DIO/splice_read race memory corruptor and kill off ITER_PIPE | expand |
On 2/7/23 6:39?AM, David Howells wrote: > [!] Jens: Note that there's a window in the linux-block/for-next branch > with a memory corruptor bug that someone bisecting might hit. These > two patches would be better pushed to the front of my iov-extract > branch to eliminate the window. Would it be possible for you to > replace my branch in your for-next branch at this point? I'll check in on these two patches later today, but just wanted to say that we can definitely just toss the existing branch, and setup a new one based on -rc7 that adds these two first, then pulls in the other branch on top to avoid this. Not a big deal, and warranted in this case.
Hi Jens, > [!] Jens: Note that there's a window in the linux-block/for-next branch > with a memory corruptor bug that someone bisecting might hit. These > two patches would be better pushed to the front of my iov-extract > branch to eliminate the window. Would it be possible for you to > replace my branch in your for-next branch at this point? In case you would prefer to do this, I've updated my iov-extract[*] branch to put these two patches at the bottom and pushed the other patches back on top of it, dropping the pipe extraction bits from the iov_iter_extract_pages() patch in case you want to use that. I've also removed the FOLL_PIN that was in that patch as it's implied by calling pin_user_pages_fast() and is being made private to mm/. I can also repost the full set of patches if that would be useful. David [*] https://git.kernel.org/pub/scm/linux/kernel/git/dhowells/linux-fs.git/log/?h=iov-extract
Hi Jens, Christoph, Al, Syzbot found a bug[1] that my bio/FOLL_PIN code[2] inadvertently introduced. The problem is that with my patches, pages obtained from kernel-backed iterators aren't ref'd or pinned when they're extracted and thus struct bio doesn't retain them. A DIO-read from a file through iomap that races with truncate may in __iomap_dio_rw() call iov_iter_revert() on the iov_iter it was given. Unfortunately, if the iterator is an ITER_PIPE, the reversion has side effects: the pages rolled back get released. Those pages, however, are not retained by the uncompleted bio and may get modified after their release. The first patch fixes this by switching to bulk allocating all the necessary pages up front and adding them to an ITER_BVEC iterator, doing the I/O and only then trimming the excess pages. The remaining pages are then pushed into the pipe. This has the downside (as the code stands) of not handling any partial page lurking in the pipe - though that could be places as the first element in the bvec. OTOH, using the bulk allocation API should be more efficient. As this is the only user of ITER_PIPE, the second patch removes ITER_PIPE and all its associated iov_iter helper functions. Thanks to Hillf Danton for spotting that iov_iter_revert() was involved[3]. [!] Jens: Note that there's a window in the linux-block/for-next branch with a memory corruptor bug that someone bisecting might hit. These two patches would be better pushed to the front of my iov-extract branch to eliminate the window. Would it be possible for you to replace my branch in your for-next branch at this point? I've pushed the patches here also: https://git.kernel.org/pub/scm/linux/kernel/git/dhowells/linux-fs.git/log/?h=iov-fixes David Link: https://lore.kernel.org/r/000000000000b0b3c005f3a09383@google.com/ [1] Link: https://lore.kernel.org/r/20230126141626.2809643-1-dhowells@redhat.com/ [2] Link: https://lore.kernel.org/r/20230207094731.1390-1-hdanton@sina.com/ [3] David Howells (2): vfs, iomap: Fix generic_file_splice_read() to avoid reversion of ITER_PIPE iov_iter: Kill ITER_PIPE fs/cifs/file.c | 8 +- fs/splice.c | 76 ++++++- include/linux/uio.h | 14 -- lib/iov_iter.c | 492 +------------------------------------------- mm/filemap.c | 3 - 5 files changed, 72 insertions(+), 521 deletions(-)