Re: [PATCH 0/2] page_count can't be used to decide when wp_page_copy

2021-01-15 Thread Jan Kara
On Sat 09-01-21 11:46:46, Linus Torvalds wrote: > On Sat, Jan 9, 2021 at 11:33 AM Matthew Wilcox wrote: > > > > On Thu, Jan 07, 2021 at 01:05:19PM -0800, Linus Torvalds wrote: > > > Side note, and not really related to UFFD, but the mmap_sem in > > > general: I was at one point actually hoping

Re: [PATCH 0/2] page_count can't be used to decide when wp_page_copy

2021-01-13 Thread Jerome Glisse
On Wed, Jan 13, 2021 at 07:39:36PM -0400, Jason Gunthorpe wrote: > On Wed, Jan 13, 2021 at 04:56:38PM -0500, Jerome Glisse wrote: > > > is a broken model and the way GPU use GUP is less broken then RDMA. In > > GPU driver GUP contract with userspace is that the data the GPU can > > access is a

Re: [PATCH 0/2] page_count can't be used to decide when wp_page_copy

2021-01-13 Thread Jason Gunthorpe
On Wed, Jan 13, 2021 at 04:56:38PM -0500, Jerome Glisse wrote: > is a broken model and the way GPU use GUP is less broken then RDMA. In > GPU driver GUP contract with userspace is that the data the GPU can > access is a snapshot of what the process memory was at the time you > asked for the GUP.

Re: [PATCH 0/2] page_count can't be used to decide when wp_page_copy

2021-01-13 Thread Jerome Glisse
On Fri, Jan 08, 2021 at 08:42:55PM -0400, Jason Gunthorpe wrote: > On Fri, Jan 08, 2021 at 05:43:56PM -0500, Andrea Arcangeli wrote: > > On Fri, Jan 08, 2021 at 02:19:45PM -0400, Jason Gunthorpe wrote: > > > On Fri, Jan 08, 2021 at 12:00:36PM -0500, Andrea Arcangeli wrote: > > > > > The majority

Re: [PATCH 0/2] page_count can't be used to decide when wp_page_copy

2021-01-11 Thread Jason Gunthorpe
On Sat, Jan 09, 2021 at 11:49:58AM +0800, Hillf Danton wrote: > On Fri, 8 Jan 2021 14:19:45 -0400 Jason Gunthorpe wrote: > > > > What I was trying to explain below, is I think we agreed that a page > > under active FOLL_LONGTERM pin *can not* be write protected. > > > > Establishing the

Re: [PATCH 0/2] page_count can't be used to decide when wp_page_copy

2021-01-11 Thread Jason Gunthorpe
On Fri, Jan 08, 2021 at 09:50:08PM -0500, Andrea Arcangeli wrote: > For all those that aren't using mmu notifier and that rely solely on > page pins, they still require privilege, except they do through /dev/ > permissions. It is normal that the dev nodes are a+rw so it doesn't really require

Re: [PATCH 0/2] page_count can't be used to decide when wp_page_copy

2021-01-09 Thread Linus Torvalds
On Sat, Jan 9, 2021 at 11:33 AM Matthew Wilcox wrote: > > On Thu, Jan 07, 2021 at 01:05:19PM -0800, Linus Torvalds wrote: > > Side note, and not really related to UFFD, but the mmap_sem in > > general: I was at one point actually hoping that we could make the > > mmap_sem a spinlock, or at least

Re: [PATCH 0/2] page_count can't be used to decide when wp_page_copy

2021-01-09 Thread Matthew Wilcox
On Thu, Jan 07, 2021 at 01:05:19PM -0800, Linus Torvalds wrote: > Side note, and not really related to UFFD, but the mmap_sem in > general: I was at one point actually hoping that we could make the > mmap_sem a spinlock, or at least make the rule be that we never do any > IO under it. At which

Re: [PATCH 0/2] page_count can't be used to decide when wp_page_copy

2021-01-09 Thread Linus Torvalds
On Sat, Jan 9, 2021 at 11:03 AM Andy Lutomirski wrote: > > > > > Sorry to ask but I'm curious, what also goes wrong if the user > > modifies memory under GUP pin from vmsplice? That's not obvious to > > see. > > It breaks the otherwise true rule that the data in pipe buffers is > immutable. Note

Re: [PATCH 0/2] page_count can't be used to decide when wp_page_copy

2021-01-09 Thread Andy Lutomirski
> On Jan 8, 2021, at 3:34 PM, Andrea Arcangeli wrote: > > On Fri, Jan 08, 2021 at 10:31:24AM -0800, Andy Lutomirski wrote: >> Can we just remove vmsplice() support? We could make it do a normal > >> copy, thereby getting rid of a fair amount of nastiness and potential >> attacks. Even ignoring

Re: [PATCH 0/2] page_count can't be used to decide when wp_page_copy

2021-01-08 Thread Andrea Arcangeli
Hello Jason, On Fri, Jan 08, 2021 at 08:42:55PM -0400, Jason Gunthorpe wrote: > There is already a patch series floating about to do exactly that for > FOLL_LONGTERM pins based on the existing code in GUP for CMA migration Sounds great. > The ship sailed on this a decade ago, it is completely

Re: [PATCH 0/2] page_count can't be used to decide when wp_page_copy

2021-01-08 Thread Jason Gunthorpe
On Fri, Jan 08, 2021 at 05:43:56PM -0500, Andrea Arcangeli wrote: > On Fri, Jan 08, 2021 at 02:19:45PM -0400, Jason Gunthorpe wrote: > > On Fri, Jan 08, 2021 at 12:00:36PM -0500, Andrea Arcangeli wrote: > > > > The majority cannot be converted to notifiers because they are DMA > > > > based. Every

Re: [PATCH 0/2] page_count can't be used to decide when wp_page_copy

2021-01-08 Thread Andrea Arcangeli
On Fri, Jan 08, 2021 at 10:31:24AM -0800, Andy Lutomirski wrote: > Can we just remove vmsplice() support? We could make it do a normal The single case I've seen vmsplice used so far, that was really cool is localhost live migration of qemu. However despite really cool, it wasn't merged in the

Re: [PATCH 0/2] page_count can't be used to decide when wp_page_copy

2021-01-08 Thread Andrea Arcangeli
On Fri, Jan 08, 2021 at 02:19:45PM -0400, Jason Gunthorpe wrote: > On Fri, Jan 08, 2021 at 12:00:36PM -0500, Andrea Arcangeli wrote: > > > The majority cannot be converted to notifiers because they are DMA > > > based. Every one of those is an ABI for something, and does not expect > > > extra

Re: [PATCH 0/2] page_count can't be used to decide when wp_page_copy

2021-01-08 Thread Linus Torvalds
On Fri, Jan 8, 2021 at 10:19 AM Jason Gunthorpe wrote: > > Sorry, I missed something, how does mmaping a fresh new page in the > child impact the parent? > > I guess the issue is not to mmap but to GUP a shared page No. It has nothing to do with a shared page. The problem with the COW in the

Re: [PATCH 0/2] page_count can't be used to decide when wp_page_copy

2021-01-08 Thread Linus Torvalds
On Fri, Jan 8, 2021 at 10:31 AM Andy Lutomirski wrote: > > Can we just remove vmsplice() support? We could make it do a normal > copy, thereby getting rid of a fair amount of nastiness and potential > attacks. Even ignoring issues relating to the length of time that the > vmsplice reference is

Re: [PATCH 0/2] page_count can't be used to decide when wp_page_copy

2021-01-08 Thread Andy Lutomirski
On Fri, Jan 8, 2021 at 10:19 AM Jason Gunthorpe wrote: > > On Fri, Jan 08, 2021 at 12:00:36PM -0500, Andrea Arcangeli wrote: > > > The majority cannot be converted to notifiers because they are DMA > > > based. Every one of those is an ABI for something, and does not expect > > > extra privilege

Re: [PATCH 0/2] page_count can't be used to decide when wp_page_copy

2021-01-08 Thread Jason Gunthorpe
On Fri, Jan 08, 2021 at 12:00:36PM -0500, Andrea Arcangeli wrote: > > The majority cannot be converted to notifiers because they are DMA > > based. Every one of those is an ABI for something, and does not expect > > extra privilege to function. It would be a major breaking change to > > have

Re: [PATCH 0/2] page_count can't be used to decide when wp_page_copy

2021-01-08 Thread Andrea Arcangeli
On Fri, Jan 08, 2021 at 09:36:49AM -0400, Jason Gunthorpe wrote: > On Thu, Jan 07, 2021 at 04:45:33PM -0500, Andrea Arcangeli wrote: > > On Thu, Jan 07, 2021 at 04:25:25PM -0400, Jason Gunthorpe wrote: > > > On Thu, Jan 07, 2021 at 03:04:00PM -0500, Andrea Arcangeli wrote: > > > > > > > vmsplice

Re: [PATCH 0/2] page_count can't be used to decide when wp_page_copy

2021-01-08 Thread Jason Gunthorpe
On Thu, Jan 07, 2021 at 04:45:33PM -0500, Andrea Arcangeli wrote: > On Thu, Jan 07, 2021 at 04:25:25PM -0400, Jason Gunthorpe wrote: > > On Thu, Jan 07, 2021 at 03:04:00PM -0500, Andrea Arcangeli wrote: > > > > > vmsplice syscall API is insecure allowing long term GUP PINs without > > >

Re: [PATCH 0/2] page_count can't be used to decide when wp_page_copy

2021-01-07 Thread Andrea Arcangeli
On Thu, Jan 07, 2021 at 02:17:50PM -0800, Linus Torvalds wrote: > So I think we can agree that even that softdirty case we can just > handle by "don't do that then". Absolutely. The question is if somebody was happily running clear_refs with a RDMA attached to the process, by the time they update

Re: [PATCH 0/2] page_count can't be used to decide when wp_page_copy

2021-01-07 Thread Linus Torvalds
On Thu, Jan 7, 2021 at 2:03 PM Andrea Arcangeli wrote: > > If you could stop mentioning UFFDIO_WRITEPROTECT and only focus on > softdirty/clear_refs, maybe you wouldn't think my judgment is biased > towards clear_refs/softdirty too. So I think we can agree that even that softdirty case we can

Re: [PATCH 0/2] page_count can't be used to decide when wp_page_copy

2021-01-07 Thread Andrea Arcangeli
On Thu, Jan 07, 2021 at 01:05:19PM -0800, Linus Torvalds wrote: > I think those would very much be worth fixing, so that if > UFFDIO_WRITEPROTECT taking the mmapo_sem for writing causes problems, > we can _fix_ those problems. > > But I think it's entirely wrong to treat UFFDIO_WRITEPROTECT as >

Re: [PATCH 0/2] page_count can't be used to decide when wp_page_copy

2021-01-07 Thread Andrea Arcangeli
On Thu, Jan 07, 2021 at 12:32:09PM -0800, Linus Torvalds wrote: > I think Andrea is blinded by his own love for UFFDIO: when I do a > debian codesearch for UFFDIO_WRITEPROTECT, all it finds is the kernel > and strace (and the qemu copies of the kernel headers). For the record, I feel obliged to

Re: [PATCH 0/2] page_count can't be used to decide when wp_page_copy

2021-01-07 Thread Andrea Arcangeli
On Thu, Jan 07, 2021 at 04:25:25PM -0400, Jason Gunthorpe wrote: > On Thu, Jan 07, 2021 at 03:04:00PM -0500, Andrea Arcangeli wrote: > > > vmsplice syscall API is insecure allowing long term GUP PINs without > > privilege. > > Lots of places are relying on pin_user_pages long term pins of

Re: [PATCH 0/2] page_count can't be used to decide when wp_page_copy

2021-01-07 Thread Linus Torvalds
On Thu, Jan 7, 2021 at 12:32 PM Linus Torvalds wrote: > > Which is really why I think this needs to be fixed by just fixing UFFD > to take the write lock. Side note, and not really related to UFFD, but the mmap_sem in general: I was at one point actually hoping that we could make the mmap_sem a

Re: [PATCH 0/2] page_count can't be used to decide when wp_page_copy

2021-01-07 Thread Linus Torvalds
On Thu, Jan 7, 2021 at 12:25 PM Jason Gunthorpe wrote: > > Lots of places are relying on pin_user_pages long term pins of memory, > and cannot be converted to notifiers. > > I don't think it is reasonable to just declare that insecure and > requires privileges, it is a huge ABI break. Also, I

Re: [PATCH 0/2] page_count can't be used to decide when wp_page_copy

2021-01-07 Thread Jason Gunthorpe
On Thu, Jan 07, 2021 at 03:04:00PM -0500, Andrea Arcangeli wrote: > vmsplice syscall API is insecure allowing long term GUP PINs without > privilege. Lots of places are relying on pin_user_pages long term pins of memory, and cannot be converted to notifiers. I don't think it is reasonable to

[PATCH 0/2] page_count can't be used to decide when wp_page_copy

2021-01-07 Thread Andrea Arcangeli
Hello, I prepared in 2/2 a fix to make UFFDIO_WRITEPROTECT and clear_refs_write cope with page_count in do_wp_page. It'd stack perfectly on top of 1/2 from will which fixes an orthogonal regression and it'd need to be applied first since its Fixes tag comes first. I hope this patchset shows and