| Message ID | 3d34069ab2d249d866ea1d18a47e4170dbfb5982.1610132102.git.luto@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | x86/vm86/32: Remove VM86_SCREEN_BITMAP support | expand |
On Fri, Jan 8, 2021 at 10:55 AM Andy Lutomirski <luto@kernel.org> wrote: > > I can't find any users at all of this mechanism, so just remove it. Ack, as long as it sits in linux-next for a while. Linus
On Fri, Jan 8, 2021 at 1:59 PM Andy Lutomirski <luto@kernel.org> wrote: > > The implementation was rather buggy. It unconditionally marked PTEs > read-only, even for VM_SHARED mappings. I'm not sure whether this is > actually a problem, but it certainly seems unwise. More importantly, it > released the mmap lock before flushing the TLB, which could allow a racing > CoW operation to falsely believe that the underlying memory was not > writable. > > I can't find any users at all of this mechanism, so just remove it. > > Cc: Andrea Arcangeli <aarcange@redhat.com> > Cc: Linux-MM <linux-mm@kvack.org> > Cc: Jason Gunthorpe <jgg@ziepe.ca> > Cc: x86@kernel.org > Cc: Linus Torvalds <torvalds@linux-foundation.org> > Cc: Matthew Wilcox <willy@infradead.org> > Cc: Jann Horn <jannh@google.com> > Cc: Jan Kara <jack@suse.cz> > Cc: Yu Zhao <yuzhao@google.com> > Cc: Peter Xu <peterx@redhat.com> > Signed-off-by: Andy Lutomirski <luto@kernel.org> > --- > arch/x86/include/uapi/asm/vm86.h | 2 +- > arch/x86/kernel/vm86_32.c | 55 ++++++-------------------------- > 2 files changed, 10 insertions(+), 47 deletions(-) > > diff --git a/arch/x86/include/uapi/asm/vm86.h b/arch/x86/include/uapi/asm/vm86.h > index d2ee4e307ef8..50004fb4590d 100644 > --- a/arch/x86/include/uapi/asm/vm86.h > +++ b/arch/x86/include/uapi/asm/vm86.h > @@ -106,7 +106,7 @@ struct vm86_struct { > /* > * flags masks > */ > -#define VM86_SCREEN_BITMAP 0x0001 > +#define VM86_SCREEN_BITMAP 0x0001 /* no longer supported */ > > struct vm86plus_info_struct { > unsigned long force_return_for_pic:1; > diff --git a/arch/x86/kernel/vm86_32.c b/arch/x86/kernel/vm86_32.c > index 764573de3996..28b9e8d511e1 100644 > --- a/arch/x86/kernel/vm86_32.c > +++ b/arch/x86/kernel/vm86_32.c > @@ -160,49 +160,6 @@ void save_v86_state(struct kernel_vm86_regs *regs, int retval) > do_exit(SIGSEGV); > } > > -static void mark_screen_rdonly(struct mm_struct *mm) > -{ > - struct vm_area_struct *vma; > - spinlock_t *ptl; > - pgd_t *pgd; > - p4d_t *p4d; > - pud_t *pud; > - pmd_t *pmd; > - pte_t *pte; > - int i; > - > - mmap_write_lock(mm); > - pgd = pgd_offset(mm, 0xA0000); > - if (pgd_none_or_clear_bad(pgd)) > - goto out; > - p4d = p4d_offset(pgd, 0xA0000); > - if (p4d_none_or_clear_bad(p4d)) > - goto out; > - pud = pud_offset(p4d, 0xA0000); > - if (pud_none_or_clear_bad(pud)) > - goto out; > - pmd = pmd_offset(pud, 0xA0000); > - > - if (pmd_trans_huge(*pmd)) { > - vma = find_vma(mm, 0xA0000); > - split_huge_pmd(vma, pmd, 0xA0000); > - } > - if (pmd_none_or_clear_bad(pmd)) > - goto out; > - pte = pte_offset_map_lock(mm, pmd, 0xA0000, &ptl); > - for (i = 0; i < 32; i++) { > - if (pte_present(*pte)) > - set_pte(pte, pte_wrprotect(*pte)); > - pte++; > - } > - pte_unmap_unlock(pte, ptl); > -out: > - mmap_write_unlock(mm); > - flush_tlb_mm_range(mm, 0xA0000, 0xA0000 + 32*PAGE_SIZE, PAGE_SHIFT, false); > -} > - > - > - > static int do_vm86_irq_handling(int subfunction, int irqnumber); > static long do_sys_vm86(struct vm86plus_struct __user *user_vm86, bool plus); > > @@ -282,6 +239,15 @@ static long do_sys_vm86(struct vm86plus_struct __user *user_vm86, bool plus) > offsetof(struct vm86_struct, int_revectored))) > return -EFAULT; > > + > + /* VM86_SCREEN_BITMAP had numerous bugs and appears to have no users. */ > + if (v.flags & VM86_SCREEN_BITMAP) { > + char comm[TASK_COMM_LEN]; > + > + pr_info_once("vm86: '%s' uses VM86_SCREEN_BITMAP, which is no longer supported\n", get_task_comm(comm, current); > + return -EINVAL; > + } > + > memset(&vm86regs, 0, sizeof(vm86regs)); > > vm86regs.pt.bx = v.regs.ebx; > @@ -370,9 +336,6 @@ static long do_sys_vm86(struct vm86plus_struct __user *user_vm86, bool plus) > update_task_stack(tsk); > preempt_enable(); > > - if (vm86->flags & VM86_SCREEN_BITMAP) > - mark_screen_rdonly(tsk->mm); > - > memcpy((struct kernel_vm86_regs *)regs, &vm86regs, sizeof(vm86regs)); > return regs->ax; > } You can also remove screen_bitmap from struct vm86 (the kernel internal structure), and the check_v8086_mode() function. -- Brian Gerst
Hi Andy,
I love your patch! Perhaps something to improve:
[auto build test WARNING on tip/master]
[also build test WARNING on linux/master linus/master tip/x86/core v5.11-rc2 next-20210108]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Andy-Lutomirski/x86-vm86-32-Remove-VM86_SCREEN_BITMAP-support/20210109-025703
base: https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git 6c44caf1e694c346a5d9de6277079f097fb78359
config: i386-randconfig-s001-20210108 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-15) 9.3.0
reproduce:
# apt-get install sparse
# sparse version: v0.6.3-208-g46a52ca4-dirty
# https://github.com/0day-ci/linux/commit/616553cd85241d71033c0cdd03655cd50157c46c
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Andy-Lutomirski/x86-vm86-32-Remove-VM86_SCREEN_BITMAP-support/20210109-025703
git checkout 616553cd85241d71033c0cdd03655cd50157c46c
# save the attached .config to linux build tree
make W=1 C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' ARCH=i386
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All warnings (new ones prefixed by >>):
arch/x86/kernel/vm86_32.c: In function 'do_sys_vm86':
arch/x86/kernel/vm86_32.c:829: error: unterminated argument list invoking macro "pr_info_once"
829 |
|
arch/x86/kernel/vm86_32.c:247:3: error: 'pr_info_once' undeclared (first use in this function)
247 | pr_info_once("vm86: '%s' uses VM86_SCREEN_BITMAP, which is no longer supported\n", get_task_comm(comm, current);
| ^~~~~~~~~~~~
arch/x86/kernel/vm86_32.c:247:3: note: each undeclared identifier is reported only once for each function it appears in
arch/x86/kernel/vm86_32.c:247:15: error: expected ';' at end of input
247 | pr_info_once("vm86: '%s' uses VM86_SCREEN_BITMAP, which is no longer supported\n", get_task_comm(comm, current);
| ^
| ;
......
829 |
|
arch/x86/kernel/vm86_32.c:247:3: error: expected declaration or statement at end of input
247 | pr_info_once("vm86: '%s' uses VM86_SCREEN_BITMAP, which is no longer supported\n", get_task_comm(comm, current);
| ^~~~~~~~~~~~
arch/x86/kernel/vm86_32.c:245:8: warning: unused variable 'comm' [-Wunused-variable]
245 | char comm[TASK_COMM_LEN];
| ^~~~
arch/x86/kernel/vm86_32.c:247:3: error: expected declaration or statement at end of input
247 | pr_info_once("vm86: '%s' uses VM86_SCREEN_BITMAP, which is no longer supported\n", get_task_comm(comm, current);
| ^~~~~~~~~~~~
arch/x86/kernel/vm86_32.c:200:18: warning: unused variable 'regs' [-Wunused-variable]
200 | struct pt_regs *regs = current_pt_regs();
| ^~~~
arch/x86/kernel/vm86_32.c:199:26: warning: unused variable 'vm86regs' [-Wunused-variable]
199 | struct kernel_vm86_regs vm86regs;
| ^~~~~~~~
arch/x86/kernel/vm86_32.c: At top level:
>> arch/x86/kernel/vm86_32.c:163:12: warning: 'do_vm86_irq_handling' used but never defined
163 | static int do_vm86_irq_handling(int subfunction, int irqnumber);
| ^~~~~~~~~~~~~~~~~~~~
arch/x86/kernel/vm86_32.c: In function 'do_sys_vm86':
arch/x86/kernel/vm86_32.c:829: error: control reaches end of non-void function [-Werror=return-type]
829 |
|
cc1: some warnings being treated as errors
vim +/do_vm86_irq_handling +163 arch/x86/kernel/vm86_32.c
^1da177e4c3f415 arch/i386/kernel/vm86.c Linus Torvalds 2005-04-16 162
^1da177e4c3f415 arch/i386/kernel/vm86.c Linus Torvalds 2005-04-16 @163 static int do_vm86_irq_handling(int subfunction, int irqnumber);
1342635638cba9b arch/x86/kernel/vm86_32.c Brian Gerst 2015-07-29 164 static long do_sys_vm86(struct vm86plus_struct __user *user_vm86, bool plus);
^1da177e4c3f415 arch/i386/kernel/vm86.c Linus Torvalds 2005-04-16 165
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
On Fri, Jan 08, 2021 at 10:55:05AM -0800, Andy Lutomirski wrote: > The implementation was rather buggy. It unconditionally marked PTEs > released the mmap lock before flushing the TLB, which could allow a racing > CoW operation to falsely believe that the underlying memory was not > writable. > > I can't find any users at all of this mechanism, so just remove it. Reviewed-by: Andrea Arcangeli <aarcange@redhat.com>
Andy Lutomirski <luto@kernel.org> writes: > The implementation was rather buggy. It unconditionally marked PTEs > read-only, even for VM_SHARED mappings. I'm not sure whether this is > actually a problem, but it certainly seems unwise. More importantly, it > released the mmap lock before flushing the TLB, which could allow a racing > CoW operation to falsely believe that the underlying memory was not > writable. > > I can't find any users at all of this mechanism, so just remove it. In another age this was used by dosemu. Have you looked at dosemu to see if it still uses this support (on 32bit where dosemu can use vm86)? It may still be a valid removal target I just wanted to point out what the original user was. Eric > Cc: Andrea Arcangeli <aarcange@redhat.com> > Cc: Linux-MM <linux-mm@kvack.org> > Cc: Jason Gunthorpe <jgg@ziepe.ca> > Cc: x86@kernel.org > Cc: Linus Torvalds <torvalds@linux-foundation.org> > Cc: Matthew Wilcox <willy@infradead.org> > Cc: Jann Horn <jannh@google.com> > Cc: Jan Kara <jack@suse.cz> > Cc: Yu Zhao <yuzhao@google.com> > Cc: Peter Xu <peterx@redhat.com> > Signed-off-by: Andy Lutomirski <luto@kernel.org> > --- > arch/x86/include/uapi/asm/vm86.h | 2 +- > arch/x86/kernel/vm86_32.c | 55 ++++++-------------------------- > 2 files changed, 10 insertions(+), 47 deletions(-) > > diff --git a/arch/x86/include/uapi/asm/vm86.h b/arch/x86/include/uapi/asm/vm86.h > index d2ee4e307ef8..50004fb4590d 100644 > --- a/arch/x86/include/uapi/asm/vm86.h > +++ b/arch/x86/include/uapi/asm/vm86.h > @@ -106,7 +106,7 @@ struct vm86_struct { > /* > * flags masks > */ > -#define VM86_SCREEN_BITMAP 0x0001 > +#define VM86_SCREEN_BITMAP 0x0001 /* no longer supported */ > > struct vm86plus_info_struct { > unsigned long force_return_for_pic:1; > diff --git a/arch/x86/kernel/vm86_32.c b/arch/x86/kernel/vm86_32.c > index 764573de3996..28b9e8d511e1 100644 > --- a/arch/x86/kernel/vm86_32.c > +++ b/arch/x86/kernel/vm86_32.c > @@ -160,49 +160,6 @@ void save_v86_state(struct kernel_vm86_regs *regs, int retval) > do_exit(SIGSEGV); > } > > -static void mark_screen_rdonly(struct mm_struct *mm) > -{ > - struct vm_area_struct *vma; > - spinlock_t *ptl; > - pgd_t *pgd; > - p4d_t *p4d; > - pud_t *pud; > - pmd_t *pmd; > - pte_t *pte; > - int i; > - > - mmap_write_lock(mm); > - pgd = pgd_offset(mm, 0xA0000); > - if (pgd_none_or_clear_bad(pgd)) > - goto out; > - p4d = p4d_offset(pgd, 0xA0000); > - if (p4d_none_or_clear_bad(p4d)) > - goto out; > - pud = pud_offset(p4d, 0xA0000); > - if (pud_none_or_clear_bad(pud)) > - goto out; > - pmd = pmd_offset(pud, 0xA0000); > - > - if (pmd_trans_huge(*pmd)) { > - vma = find_vma(mm, 0xA0000); > - split_huge_pmd(vma, pmd, 0xA0000); > - } > - if (pmd_none_or_clear_bad(pmd)) > - goto out; > - pte = pte_offset_map_lock(mm, pmd, 0xA0000, &ptl); > - for (i = 0; i < 32; i++) { > - if (pte_present(*pte)) > - set_pte(pte, pte_wrprotect(*pte)); > - pte++; > - } > - pte_unmap_unlock(pte, ptl); > -out: > - mmap_write_unlock(mm); > - flush_tlb_mm_range(mm, 0xA0000, 0xA0000 + 32*PAGE_SIZE, PAGE_SHIFT, false); > -} > - > - > - > static int do_vm86_irq_handling(int subfunction, int irqnumber); > static long do_sys_vm86(struct vm86plus_struct __user *user_vm86, bool plus); > > @@ -282,6 +239,15 @@ static long do_sys_vm86(struct vm86plus_struct __user *user_vm86, bool plus) > offsetof(struct vm86_struct, int_revectored))) > return -EFAULT; > > + > + /* VM86_SCREEN_BITMAP had numerous bugs and appears to have no users. */ > + if (v.flags & VM86_SCREEN_BITMAP) { > + char comm[TASK_COMM_LEN]; > + > + pr_info_once("vm86: '%s' uses VM86_SCREEN_BITMAP, which is no longer supported\n", get_task_comm(comm, current); > + return -EINVAL; > + } > + > memset(&vm86regs, 0, sizeof(vm86regs)); > > vm86regs.pt.bx = v.regs.ebx; > @@ -370,9 +336,6 @@ static long do_sys_vm86(struct vm86plus_struct __user *user_vm86, bool plus) > update_task_stack(tsk); > preempt_enable(); > > - if (vm86->flags & VM86_SCREEN_BITMAP) > - mark_screen_rdonly(tsk->mm); > - > memcpy((struct kernel_vm86_regs *)regs, &vm86regs, sizeof(vm86regs)); > return regs->ax; > }
> On Jan 9, 2021, at 12:17 PM, ebiederm@xmission.com wrote: > > Andy Lutomirski <luto@kernel.org> writes: > >> The implementation was rather buggy. It unconditionally marked PTEs >> read-only, even for VM_SHARED mappings. I'm not sure whether this is >> actually a problem, but it certainly seems unwise. More importantly, it >> released the mmap lock before flushing the TLB, which could allow a racing >> CoW operation to falsely believe that the underlying memory was not >> writable. >> >> I can't find any users at all of this mechanism, so just remove it. > > In another age this was used by dosemu. Have you looked at dosemu to > see if it still uses this support (on 32bit where dosemu can use vm86)? > > It may still be a valid removal target I just wanted to point out what > the original user was. I’m pretty sure that dosemu2 does not use this support. I think the original dosemu doesn’t either, but I’m also not convinced it has any users at all. I meant to cc Stas, and I will for v2. > > Eric > >> Cc: Andrea Arcangeli <aarcange@redhat.com> >> Cc: Linux-MM <linux-mm@kvack.org> >> Cc: Jason Gunthorpe <jgg@ziepe.ca> >> Cc: x86@kernel.org >> Cc: Linus Torvalds <torvalds@linux-foundation.org> >> Cc: Matthew Wilcox <willy@infradead.org> >> Cc: Jann Horn <jannh@google.com> >> Cc: Jan Kara <jack@suse.cz> >> Cc: Yu Zhao <yuzhao@google.com> >> Cc: Peter Xu <peterx@redhat.com> >> Signed-off-by: Andy Lutomirski <luto@kernel.org> >> --- >> arch/x86/include/uapi/asm/vm86.h | 2 +- >> arch/x86/kernel/vm86_32.c | 55 ++++++-------------------------- >> 2 files changed, 10 insertions(+), 47 deletions(-) >> >> diff --git a/arch/x86/include/uapi/asm/vm86.h b/arch/x86/include/uapi/asm/vm86.h >> index d2ee4e307ef8..50004fb4590d 100644 >> --- a/arch/x86/include/uapi/asm/vm86.h >> +++ b/arch/x86/include/uapi/asm/vm86.h >> @@ -106,7 +106,7 @@ struct vm86_struct { >> /* >> * flags masks >> */ >> -#define VM86_SCREEN_BITMAP 0x0001 >> +#define VM86_SCREEN_BITMAP 0x0001 /* no longer supported */ >> >> struct vm86plus_info_struct { >> unsigned long force_return_for_pic:1; >> diff --git a/arch/x86/kernel/vm86_32.c b/arch/x86/kernel/vm86_32.c >> index 764573de3996..28b9e8d511e1 100644 >> --- a/arch/x86/kernel/vm86_32.c >> +++ b/arch/x86/kernel/vm86_32.c >> @@ -160,49 +160,6 @@ void save_v86_state(struct kernel_vm86_regs *regs, int retval) >> do_exit(SIGSEGV); >> } >> >> -static void mark_screen_rdonly(struct mm_struct *mm) >> -{ >> - struct vm_area_struct *vma; >> - spinlock_t *ptl; >> - pgd_t *pgd; >> - p4d_t *p4d; >> - pud_t *pud; >> - pmd_t *pmd; >> - pte_t *pte; >> - int i; >> - >> - mmap_write_lock(mm); >> - pgd = pgd_offset(mm, 0xA0000); >> - if (pgd_none_or_clear_bad(pgd)) >> - goto out; >> - p4d = p4d_offset(pgd, 0xA0000); >> - if (p4d_none_or_clear_bad(p4d)) >> - goto out; >> - pud = pud_offset(p4d, 0xA0000); >> - if (pud_none_or_clear_bad(pud)) >> - goto out; >> - pmd = pmd_offset(pud, 0xA0000); >> - >> - if (pmd_trans_huge(*pmd)) { >> - vma = find_vma(mm, 0xA0000); >> - split_huge_pmd(vma, pmd, 0xA0000); >> - } >> - if (pmd_none_or_clear_bad(pmd)) >> - goto out; >> - pte = pte_offset_map_lock(mm, pmd, 0xA0000, &ptl); >> - for (i = 0; i < 32; i++) { >> - if (pte_present(*pte)) >> - set_pte(pte, pte_wrprotect(*pte)); >> - pte++; >> - } >> - pte_unmap_unlock(pte, ptl); >> -out: >> - mmap_write_unlock(mm); >> - flush_tlb_mm_range(mm, 0xA0000, 0xA0000 + 32*PAGE_SIZE, PAGE_SHIFT, false); >> -} >> - >> - >> - >> static int do_vm86_irq_handling(int subfunction, int irqnumber); >> static long do_sys_vm86(struct vm86plus_struct __user *user_vm86, bool plus); >> >> @@ -282,6 +239,15 @@ static long do_sys_vm86(struct vm86plus_struct __user *user_vm86, bool plus) >> offsetof(struct vm86_struct, int_revectored))) >> return -EFAULT; >> >> + >> + /* VM86_SCREEN_BITMAP had numerous bugs and appears to have no users. */ >> + if (v.flags & VM86_SCREEN_BITMAP) { >> + char comm[TASK_COMM_LEN]; >> + >> + pr_info_once("vm86: '%s' uses VM86_SCREEN_BITMAP, which is no longer supported\n", get_task_comm(comm, current); >> + return -EINVAL; >> + } >> + >> memset(&vm86regs, 0, sizeof(vm86regs)); >> >> vm86regs.pt.bx = v.regs.ebx; >> @@ -370,9 +336,6 @@ static long do_sys_vm86(struct vm86plus_struct __user *user_vm86, bool plus) >> update_task_stack(tsk); >> preempt_enable(); >> >> - if (vm86->flags & VM86_SCREEN_BITMAP) >> - mark_screen_rdonly(tsk->mm); >> - >> memcpy((struct kernel_vm86_regs *)regs, &vm86regs, sizeof(vm86regs)); >> return regs->ax; >> }
diff --git a/arch/x86/include/uapi/asm/vm86.h b/arch/x86/include/uapi/asm/vm86.h index d2ee4e307ef8..50004fb4590d 100644 --- a/arch/x86/include/uapi/asm/vm86.h +++ b/arch/x86/include/uapi/asm/vm86.h @@ -106,7 +106,7 @@ struct vm86_struct { /* * flags masks */ -#define VM86_SCREEN_BITMAP 0x0001 +#define VM86_SCREEN_BITMAP 0x0001 /* no longer supported */ struct vm86plus_info_struct { unsigned long force_return_for_pic:1; diff --git a/arch/x86/kernel/vm86_32.c b/arch/x86/kernel/vm86_32.c index 764573de3996..28b9e8d511e1 100644 --- a/arch/x86/kernel/vm86_32.c +++ b/arch/x86/kernel/vm86_32.c @@ -160,49 +160,6 @@ void save_v86_state(struct kernel_vm86_regs *regs, int retval) do_exit(SIGSEGV); } -static void mark_screen_rdonly(struct mm_struct *mm) -{ - struct vm_area_struct *vma; - spinlock_t *ptl; - pgd_t *pgd; - p4d_t *p4d; - pud_t *pud; - pmd_t *pmd; - pte_t *pte; - int i; - - mmap_write_lock(mm); - pgd = pgd_offset(mm, 0xA0000); - if (pgd_none_or_clear_bad(pgd)) - goto out; - p4d = p4d_offset(pgd, 0xA0000); - if (p4d_none_or_clear_bad(p4d)) - goto out; - pud = pud_offset(p4d, 0xA0000); - if (pud_none_or_clear_bad(pud)) - goto out; - pmd = pmd_offset(pud, 0xA0000); - - if (pmd_trans_huge(*pmd)) { - vma = find_vma(mm, 0xA0000); - split_huge_pmd(vma, pmd, 0xA0000); - } - if (pmd_none_or_clear_bad(pmd)) - goto out; - pte = pte_offset_map_lock(mm, pmd, 0xA0000, &ptl); - for (i = 0; i < 32; i++) { - if (pte_present(*pte)) - set_pte(pte, pte_wrprotect(*pte)); - pte++; - } - pte_unmap_unlock(pte, ptl); -out: - mmap_write_unlock(mm); - flush_tlb_mm_range(mm, 0xA0000, 0xA0000 + 32*PAGE_SIZE, PAGE_SHIFT, false); -} - - - static int do_vm86_irq_handling(int subfunction, int irqnumber); static long do_sys_vm86(struct vm86plus_struct __user *user_vm86, bool plus); @@ -282,6 +239,15 @@ static long do_sys_vm86(struct vm86plus_struct __user *user_vm86, bool plus) offsetof(struct vm86_struct, int_revectored))) return -EFAULT; + + /* VM86_SCREEN_BITMAP had numerous bugs and appears to have no users. */ + if (v.flags & VM86_SCREEN_BITMAP) { + char comm[TASK_COMM_LEN]; + + pr_info_once("vm86: '%s' uses VM86_SCREEN_BITMAP, which is no longer supported\n", get_task_comm(comm, current); + return -EINVAL; + } + memset(&vm86regs, 0, sizeof(vm86regs)); vm86regs.pt.bx = v.regs.ebx; @@ -370,9 +336,6 @@ static long do_sys_vm86(struct vm86plus_struct __user *user_vm86, bool plus) update_task_stack(tsk); preempt_enable(); - if (vm86->flags & VM86_SCREEN_BITMAP) - mark_screen_rdonly(tsk->mm); - memcpy((struct kernel_vm86_regs *)regs, &vm86regs, sizeof(vm86regs)); return regs->ax; }
The implementation was rather buggy. It unconditionally marked PTEs read-only, even for VM_SHARED mappings. I'm not sure whether this is actually a problem, but it certainly seems unwise. More importantly, it released the mmap lock before flushing the TLB, which could allow a racing CoW operation to falsely believe that the underlying memory was not writable. I can't find any users at all of this mechanism, so just remove it. Cc: Andrea Arcangeli <aarcange@redhat.com> Cc: Linux-MM <linux-mm@kvack.org> Cc: Jason Gunthorpe <jgg@ziepe.ca> Cc: x86@kernel.org Cc: Linus Torvalds <torvalds@linux-foundation.org> Cc: Matthew Wilcox <willy@infradead.org> Cc: Jann Horn <jannh@google.com> Cc: Jan Kara <jack@suse.cz> Cc: Yu Zhao <yuzhao@google.com> Cc: Peter Xu <peterx@redhat.com> Signed-off-by: Andy Lutomirski <luto@kernel.org> --- arch/x86/include/uapi/asm/vm86.h | 2 +- arch/x86/kernel/vm86_32.c | 55 ++++++-------------------------- 2 files changed, 10 insertions(+), 47 deletions(-)