search for: get_user_pages_fast

From: Nicholas Bellinger <nab at linux-iscsi.org> This patch addresses a long-standing bug where the get_user_pages_fast() write parameter used for setting the underlying page table entry permission bits was incorrectly set to write=1 for data_direction=DMA_TO_DEVICE, and passed into get_user_pages_fast() via vhost_scsi_map_iov_to_sgl(). However, this parameter is intended to signal WRITEs to pinned userspace PTEs f...

From: Nicholas Bellinger <nab at linux-iscsi.org> This patch addresses a long-standing bug where the get_user_pages_fast() write parameter used for setting the underlying page table entry permission bits was incorrectly set to write=1 for data_direction=DMA_TO_DEVICE, and passed into get_user_pages_fast() via vhost_scsi_map_iov_to_sgl(). However, this parameter is intended to signal WRITEs to pinned userspace PTEs f...

...>> +static int set_bit_to_user(int nr, u32 __user *addr) >> { >> unsigned long log = (unsigned long)addr; >> struct page *page; >> - void *base; >> - int bit = nr + (log % PAGE_SIZE) * 8; >> + u32 old; >> int r; >> >> r = get_user_pages_fast(log, 1, 1, &page); > OK so the trick is that page is pinned so you don't expect > arch_futex_atomic_op_inuser below to fail. get_user_pages_fast > guarantees page is not going away but does it guarantee PTE won't be > invaidated or write protected? Good point, then I thin...

...>> +static int set_bit_to_user(int nr, u32 __user *addr) >> { >> unsigned long log = (unsigned long)addr; >> struct page *page; >> - void *base; >> - int bit = nr + (log % PAGE_SIZE) * 8; >> + u32 old; >> int r; >> >> r = get_user_pages_fast(log, 1, 1, &page); > OK so the trick is that page is pinned so you don't expect > arch_futex_atomic_op_inuser below to fail. get_user_pages_fast > guarantees page is not going away but does it guarantee PTE won't be > invaidated or write protected? Good point, then I thin...

...related to mapping the notify boolean from user space into kernel space: - the actual UVA was rounded up to the next page boundary - resulting in memory corruption in the calling process whenever notifications would be signalled. This has been fixed by just removing the PAGE_ALIGN part, since get_user_pages_fast can figure this out on its own - the mapped page wasn't stored anywhere, so it wasn't unmapped and put back when a VMCI context was destroyed. Fixed this by remembering the page. Acked-by: Andy King <acking at vmware.com> Acked-by: Darius Davis <darius at vmware.com> Sign...

...related to mapping the notify boolean from user space into kernel space: - the actual UVA was rounded up to the next page boundary - resulting in memory corruption in the calling process whenever notifications would be signalled. This has been fixed by just removing the PAGE_ALIGN part, since get_user_pages_fast can figure this out on its own - the mapped page wasn't stored anywhere, so it wasn't unmapped and put back when a VMCI context was destroyed. Fixed this by remembering the page. Acked-by: Andy King <acking at vmware.com> Acked-by: Darius Davis <darius at vmware.com> Sign...

...l issue reported by Sridhar Samudrala which I'm testing various fixes for, that patch is contained to vhost/net though, so there's no conflict, patch will be posted separately. Michael S. Tsirkin (3): vhost: logging thinko fix vhost: initialize log eventfd context pointer vhost: fix get_user_pages_fast error handling drivers/vhost/vhost.c | 14 +++++++++----- 1 files changed, 9 insertions(+), 5 deletions(-)

...l issue reported by Sridhar Samudrala which I'm testing various fixes for, that patch is contained to vhost/net though, so there's no conflict, patch will be posted separately. Michael S. Tsirkin (3): vhost: logging thinko fix vhost: initialize log eventfd context pointer vhost: fix get_user_pages_fast error handling drivers/vhost/vhost.c | 14 +++++++++----- 1 files changed, 9 insertions(+), 5 deletions(-)

...tation/x86/exception-tables.txt. - */ -static int set_bit_to_user(int nr, void __user *addr) +static int set_bit_to_user(int nr, u32 __user *addr) { unsigned long log = (unsigned long)addr; struct page *page; - void *base; - int bit = nr + (log % PAGE_SIZE) * 8; + u32 old_log; int r; r = get_user_pages_fast(log, 1, 1, &page); if (r < 0) return r; BUG_ON(r != 1); - base = kmap_atomic(page); - set_bit(bit, base); - kunmap_atomic(base); + + r = futex_atomic_cmpxchg_inatomic(&old_log, addr, 0, 0); + if (r < 0) + return r; + + old_log |= 1 << nr; + r = put_user(old_log, addr); +...

...tation/x86/exception-tables.txt. - */ -static int set_bit_to_user(int nr, void __user *addr) +static int set_bit_to_user(int nr, u32 __user *addr) { unsigned long log = (unsigned long)addr; struct page *page; - void *base; - int bit = nr + (log % PAGE_SIZE) * 8; + u32 old_log; int r; r = get_user_pages_fast(log, 1, 1, &page); if (r < 0) return r; BUG_ON(r != 1); - base = kmap_atomic(page); - set_bit(bit, base); - kunmap_atomic(base); + + r = futex_atomic_cmpxchg_inatomic(&old_log, addr, 0, 0); + if (r < 0) + return r; + + old_log |= 1 << nr; + r = put_user(old_log, addr); +...

...umentation/x86/exception-tables.txt. - */ -static int set_bit_to_user(int nr, void __user *addr) +static int set_bit_to_user(int nr, u32 __user *addr) { unsigned long log = (unsigned long)addr; struct page *page; - void *base; - int bit = nr + (log % PAGE_SIZE) * 8; + u32 old; int r; r = get_user_pages_fast(log, 1, 1, &page); if (r < 0) return r; BUG_ON(r != 1); - base = kmap_atomic(page); - set_bit(bit, base); - kunmap_atomic(base); + + r = arch_futex_atomic_op_inuser(FUTEX_OP_ADD, 1 << nr, &old, addr); + /* TODO: fallback to kmap() when -ENOSYS? */ + set_page_dirty_lock(pag...

...umentation/x86/exception-tables.txt. - */ -static int set_bit_to_user(int nr, void __user *addr) +static int set_bit_to_user(int nr, u32 __user *addr) { unsigned long log = (unsigned long)addr; struct page *page; - void *base; - int bit = nr + (log % PAGE_SIZE) * 8; + u32 old; int r; r = get_user_pages_fast(log, 1, 1, &page); if (r < 0) return r; BUG_ON(r != 1); - base = kmap_atomic(page); - set_bit(bit, base); - kunmap_atomic(base); + + r = arch_futex_atomic_op_inuser(FUTEX_OP_ADD, 1 << nr, &old, addr); + /* TODO: fallback to kmap() when -ENOSYS? */ + set_page_dirty_lock(pag...

From: "Michael S. Tsirkin" <mst at redhat.com> Date: Mon, 11 Mar 2019 09:59:28 -0400 > On Mon, Mar 11, 2019 at 03:13:17PM +0800, Jason Wang wrote: >> >> On 2019/3/8 ??10:12, Christoph Hellwig wrote: >> > On Wed, Mar 06, 2019 at 02:18:07AM -0500, Jason Wang wrote: >> > > This series tries to access virtqueue metadata through kernel virtual

From: "Michael S. Tsirkin" <mst at redhat.com> Date: Mon, 11 Mar 2019 09:59:28 -0400 > On Mon, Mar 11, 2019 at 03:13:17PM +0800, Jason Wang wrote: >> >> On 2019/3/8 ??10:12, Christoph Hellwig wrote: >> > On Wed, Mar 06, 2019 at 02:18:07AM -0500, Jason Wang wrote: >> > > This series tries to access virtqueue metadata through kernel virtual

...ee as they > depend on a cleanup patch there and the IB changes are very minor. But they > could just as well go through the IB tree. > > NOTE: This series depends on my clean up patch to remove the write parameter > from gup_fast_permitted()[1] > > HFI1, qib, and mthca, use get_user_pages_fast() due to it performance > advantages. These pages can be held for a significant time. But > get_user_pages_fast() does not protect against mapping of FS DAX pages. This I don't get - if you do lock down long term mappings performance of the actual get_user_pages call shouldn't matt...

On 2019/5/10 ??10:59, Jason Wang wrote: >>> >>> ? ????? r = get_user_pages_fast(log, 1, 1, &page); >> OK so the trick is that page is pinned so you don't expect >> arch_futex_atomic_op_inuser below to fail.? get_user_pages_fast >> guarantees page is not going away but does it guarantee PTE won't be >> invaidated or write protected? > >...

On Mon, Mar 19, 2018 at 4:29 PM, David Sterba <dsterba at suse.cz> wrote: > On Mon, Mar 19, 2018 at 05:09:28PM +0200, Michael S. Tsirkin wrote: >> Hello! >> The following code triggered by syzbot >> >> r = get_user_pages_fast(log, 1, 1, &page); >> if (r < 0) >> return r; >> BUG_ON(r != 1); >> >> Just looking at get_user_pages_fast's documentation this seems >> impossible - it is supposed to only ever return # of pages >> pinned or errno...

...size_t size, int write) > +{ > + struct page **pages; > + int npages = DIV_ROUND_UP(size, PAGE_SIZE); > + int npinned; > + void *vaddr; > + > + pages = kmalloc_array(npages, sizeof(struct page *), GFP_KERNEL); > + if (!pages) > + return -ENOMEM; > + > + npinned = get_user_pages_fast(uaddr, npages, write, pages); > + if (npinned != npages) > + goto err; > + As I said I have doubts about the whole approach, but this implementation in particular isn't a good idea as it keeps the page around forever. So no THP, no NUMA rebalancing, userspace-controlled amount of mem...

...size_t size, int write) > +{ > + struct page **pages; > + int npages = DIV_ROUND_UP(size, PAGE_SIZE); > + int npinned; > + void *vaddr; > + > + pages = kmalloc_array(npages, sizeof(struct page *), GFP_KERNEL); > + if (!pages) > + return -ENOMEM; > + > + npinned = get_user_pages_fast(uaddr, npages, write, pages); > + if (npinned != npages) > + goto err; > + As I said I have doubts about the whole approach, but this implementation in particular isn't a good idea as it keeps the page around forever. So no THP, no NUMA rebalancing, userspace-controlled amount of mem...

...int set_bit_to_user(int nr, void __user *addr) > +static int set_bit_to_user(int nr, u32 __user *addr) > { > unsigned long log = (unsigned long)addr; > struct page *page; > - void *base; > - int bit = nr + (log % PAGE_SIZE) * 8; > + u32 old; > int r; > > r = get_user_pages_fast(log, 1, 1, &page); OK so the trick is that page is pinned so you don't expect arch_futex_atomic_op_inuser below to fail. get_user_pages_fast guarantees page is not going away but does it guarantee PTE won't be invaidated or write protected? > if (r < 0) > return r; >...