Date: Mon, 30 Jul 2018 01:26:51 +0200 From: Oliver Pinter <oliver.pinter@hardenedbsd.org> To: Konstantin Belousov <kostikbel@gmail.com> Cc: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: Re: svn commit: r336876 - in head/sys: amd64/amd64 amd64/ia32 amd64/include conf dev/hyperv/vmbus/amd64 Message-ID: <CAPQ4ffsWKQG9vAna9ORefNW1DYBv4=32s0TDLTe%2Bi6BB96vrdw@mail.gmail.com> In-Reply-To: <20180729223158.GL40119@kib.kiev.ua> References: <201807292047.w6TKl0hV004691@repo.freebsd.org> <CAPQ4ffsHqf8OiqUF-WBJ_0KbZGgAOs-6npyLjqWYFN%2BoCq=8Pw@mail.gmail.com> <20180729223158.GL40119@kib.kiev.ua>
next in thread | previous in thread | raw e-mail | index | archive | help
On 7/30/18, Konstantin Belousov <kostikbel@gmail.com> wrote: > Please trim useless content. > Did I missed anything interesting in your mail ? > > On Sun, Jul 29, 2018 at 11:57:47PM +0200, Oliver Pinter wrote: >> On 7/29/18, Konstantin Belousov <kib@freebsd.org> wrote: >> > +ENTRY(copyin_smap) >> > + PUSH_FRAME_POINTER >> > + movq PCPU(CURPCB),%rax >> > + movq $copyin_fault,PCB_ONFAULT(%rax) >> > + testq %rdx,%rdx /* anything to do? */ >> > + jz done_copyin >> > + >> > + /* >> > + * make sure address is valid >> > + */ >> > + movq %rdi,%rax >> > + addq %rdx,%rax >> > + jc copyin_fault >> > + movq $VM_MAXUSER_ADDRESS,%rcx >> > + cmpq %rcx,%rax >> > + ja copyin_fault >> > + >> > + xchgq %rdi,%rsi >> > + movq %rdx,%rcx >> > + movb %cl,%al >> > + shrq $3,%rcx /* copy longword-wise */ >> >> missing cld from here > In fact not. It is copyin_nosmap that got unneeded cld. > > See r327820, apparently I mis-merged this commit into the SMAP branch. > >> >> > + stac >> > + rep >> > + movsq >> > + movb %al,%cl >> > + andb $7,%cl /* copy remaining bytes */ >> > je done_copyin >> > rep >> > movsb >> > + clac > >> > +ENTRY(copyinstr_smap) >> > + PUSH_FRAME_POINTER >> > + movq %rdx,%r8 /* %r8 = maxlen */ >> > + movq %rcx,%r9 /* %r9 = *len */ >> > + xchgq %rdi,%rsi /* %rdi = from, %rsi = to */ >> > + movq PCPU(CURPCB),%rcx >> > + movq $cpystrflt,PCB_ONFAULT(%rcx) >> > + >> > + movq $VM_MAXUSER_ADDRESS,%rax >> > + >> > + /* make sure 'from' is within bounds */ >> > + subq %rsi,%rax >> > + jbe cpystrflt >> > + >> > + /* restrict maxlen to <= VM_MAXUSER_ADDRESS-from */ >> > + cmpq %rdx,%rax >> > + jae 1f >> > + movq %rax,%rdx >> > + movq %rax,%r8 >> > +1: >> > + incq %rdx >> >> missing cld here > Same. > >> >> > + >> > +2: >> > + decq %rdx >> > + jz copyinstr_succ >> > >> cpystrflt_x: >> /* set *lencopied and return %eax */ >> movq PCPU(CURPCB),%rcx >> movq $0,PCB_ONFAULT(%rcx) >> >> testq %r9,%r9 >> jz 1f >> subq %rdx,%r8 >> movq %r8,(%r9) << Here you access user-space, with cleared >> RFLAGS.AC from the fault handler. > How does this instruction access userspace ? I do not see. As far as I remember from 4 years, the r9 may contained a user-space address in 10-STABLE in the case of starting the init. I've a stac/clac pair in my internal version, but I haven't found yet the relevant commit message. For a quick grep around - http://ix.io/1fje - I haven't found yet this place, so it's looks good in your version. > >> 1: >> POP_FRAME_POINTER >> ret > > So the patch below removes unneeded (mismerged) cld's left in the > support.S. > > diff --git a/sys/amd64/amd64/support.S b/sys/amd64/amd64/support.S > index 9b8b2a40461..0aa307e6895 100644 > --- a/sys/amd64/amd64/support.S > +++ b/sys/amd64/amd64/support.S > @@ -307,7 +307,6 @@ ENTRY(copyout_smap) > movq %rdx,%rcx > > shrq $3,%rcx > - cld > stac > rep > movsq > @@ -358,7 +357,6 @@ ENTRY(copyin_nosmap) > movq %rdx,%rcx > movb %cl,%al > shrq $3,%rcx /* copy longword-wise */ > - cld > rep > movsq > movb %al,%cl > @@ -887,7 +885,6 @@ ENTRY(copyinstr_nosmap) > movq %rax,%r8 > 1: > incq %rdx > - cld > > 2: > decq %rdx Looks fine to me. >
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CAPQ4ffsWKQG9vAna9ORefNW1DYBv4=32s0TDLTe%2Bi6BB96vrdw>