[PATCH 0/3] Fuzz/x86emual and x86emul/test fixes

[PATCH 0/3] Fuzz/x86emual and x86emul/test fixes

[Wei Liu]

Wei Liu (3):
  fuzz/x86emul: avoid race in link farm rune
  x86emul/test: avoid race in link farm rune
  gitignore: ignore asm soft link in fuzz and x86emul test

 .gitignore                                   | 2 ++
 tools/fuzz/x86_instruction_emulator/Makefile | 8 ++++++--
 tools/tests/x86_emulator/Makefile            | 8 ++++++--
 3 files changed, 14 insertions(+), 4 deletions(-)

-- 
2.11.0


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[PATCH 1/3] fuzz/x86emul: avoid race in link farm rune

[Wei Liu]

Several `ln -sf` can race with each other and cause error like:

14:43:56 00:07:06 O: ln: cannot remove 'asm': No such file or directory

Provide dedicated targets for soft-linking directories.

Reported-by: Andrew Cooper <andrew.cooper3@citrix.com>
Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
 tools/fuzz/x86_instruction_emulator/Makefile | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/tools/fuzz/x86_instruction_emulator/Makefile b/tools/fuzz/x86_instruction_emulator/Makefile
index fede7e9afd..0cd49753cf 100644
--- a/tools/fuzz/x86_instruction_emulator/Makefile
+++ b/tools/fuzz/x86_instruction_emulator/Makefile
@@ -8,12 +8,16 @@ else
 x86-instruction-emulator-fuzzer-all:
 endif
 
-x86_emulate/x86_emulate.c x86_emulate/x86_emulate.h:
+x86_emulate:
 	[ -L x86_emulate ] || ln -sf $(XEN_ROOT)/xen/arch/x86/x86_emulate .
 
-asm/x86-vendors.h asm/x86-defns.h asm/msr-index.h:
+x86_emulate/x86_emulate.c x86_emulate/x86_emulate.h: x86_emulate
+
+asm:
 	[ -L asm ] || ln -sf $(XEN_ROOT)/xen/include/asm-x86 asm
 
+asm/x86-vendors.h asm/x86-defns.h asm/msr-index.h: asm
+
 x86_emulate.c x86_emulate.h: %:
 	[ -L $* ] || ln -sf $(XEN_ROOT)/tools/tests/x86_emulator/$*
 
-- 
2.11.0


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH 1/3] fuzz/x86emul: avoid race in link farm rune

[Andrew Cooper]

On 16/02/17 18:56, Wei Liu wrote:
> Several `ln -sf` can race with each other and cause error like:
>
> 14:43:56 00:07:06 O: ln: cannot remove 'asm': No such file or directory
>
> Provide dedicated targets for soft-linking directories.
>
> Reported-by: Andrew Cooper <andrew.cooper3@citrix.com>
> Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> ---
>  tools/fuzz/x86_instruction_emulator/Makefile | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/tools/fuzz/x86_instruction_emulator/Makefile b/tools/fuzz/x86_instruction_emulator/Makefile
> index fede7e9afd..0cd49753cf 100644
> --- a/tools/fuzz/x86_instruction_emulator/Makefile
> +++ b/tools/fuzz/x86_instruction_emulator/Makefile
> @@ -8,12 +8,16 @@ else
>  x86-instruction-emulator-fuzzer-all:
>  endif
>  
> -x86_emulate/x86_emulate.c x86_emulate/x86_emulate.h:
> +x86_emulate:
>  	[ -L x86_emulate ] || ln -sf $(XEN_ROOT)/xen/arch/x86/x86_emulate .
>  
> -asm/x86-vendors.h asm/x86-defns.h asm/msr-index.h:
> +x86_emulate/x86_emulate.c x86_emulate/x86_emulate.h: x86_emulate

You should be able to do this:

x86_emulate/%: x86_emulate

> +
> +asm:
>  	[ -L asm ] || ln -sf $(XEN_ROOT)/xen/include/asm-x86 asm
>  
> +asm/x86-vendors.h asm/x86-defns.h asm/msr-index.h: asm

And this:

asm/%: asm

Otherwise, Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>

> +
>  x86_emulate.c x86_emulate.h: %:
>  	[ -L $* ] || ln -sf $(XEN_ROOT)/tools/tests/x86_emulator/$*
>  


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH 1/3] fuzz/x86emul: avoid race in link farm rune

[Wei Liu]

On Thu, Feb 16, 2017 at 07:10:41PM +0000, Andrew Cooper wrote:
> On 16/02/17 18:56, Wei Liu wrote:
> > Several `ln -sf` can race with each other and cause error like:
> >
> > 14:43:56 00:07:06 O: ln: cannot remove 'asm': No such file or directory
> >
> > Provide dedicated targets for soft-linking directories.
> >
> > Reported-by: Andrew Cooper <andrew.cooper3@citrix.com>
> > Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> > ---
> >  tools/fuzz/x86_instruction_emulator/Makefile | 8 ++++++--
> >  1 file changed, 6 insertions(+), 2 deletions(-)
> >
> > diff --git a/tools/fuzz/x86_instruction_emulator/Makefile b/tools/fuzz/x86_instruction_emulator/Makefile
> > index fede7e9afd..0cd49753cf 100644
> > --- a/tools/fuzz/x86_instruction_emulator/Makefile
> > +++ b/tools/fuzz/x86_instruction_emulator/Makefile
> > @@ -8,12 +8,16 @@ else
> >  x86-instruction-emulator-fuzzer-all:
> >  endif
> >  
> > -x86_emulate/x86_emulate.c x86_emulate/x86_emulate.h:
> > +x86_emulate:
> >  	[ -L x86_emulate ] || ln -sf $(XEN_ROOT)/xen/arch/x86/x86_emulate .
> >  
> > -asm/x86-vendors.h asm/x86-defns.h asm/msr-index.h:
> > +x86_emulate/x86_emulate.c x86_emulate/x86_emulate.h: x86_emulate
> 
> You should be able to do this:
> 
> x86_emulate/%: x86_emulate
> 

Using % won't work. Jan made a similar comment during the first
iteration of the fuzzer series. It turned out that we needed to list
explicitly every file because of the other rules explicitly listed
those files.


> > +
> > +asm:
> >  	[ -L asm ] || ln -sf $(XEN_ROOT)/xen/include/asm-x86 asm
> >  
> > +asm/x86-vendors.h asm/x86-defns.h asm/msr-index.h: asm
> 
> And this:
> 
> asm/%: asm
> 

This can be made work by eliminating the explicit listing of individual
files because we're linking asm-x86 wholesale.

Wei.

> Otherwise, Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
> 
> > +
> >  x86_emulate.c x86_emulate.h: %:
> >  	[ -L $* ] || ln -sf $(XEN_ROOT)/tools/tests/x86_emulator/$*
> >  
> 

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH 1/3] fuzz/x86emul: avoid race in link farm rune

[Jan Beulich]

>>> On 17.02.17 at 11:12, <wei.liu2@citrix.com> wrote:
> On Thu, Feb 16, 2017 at 07:10:41PM +0000, Andrew Cooper wrote:
>> On 16/02/17 18:56, Wei Liu wrote:
>> > --- a/tools/fuzz/x86_instruction_emulator/Makefile
>> > +++ b/tools/fuzz/x86_instruction_emulator/Makefile
>> > @@ -8,12 +8,16 @@ else
>> >  x86-instruction-emulator-fuzzer-all:
>> >  endif
>> >  
>> > -x86_emulate/x86_emulate.c x86_emulate/x86_emulate.h:
>> > +x86_emulate:
>> >  	[ -L x86_emulate ] || ln -sf $(XEN_ROOT)/xen/arch/x86/x86_emulate .
>> >  
>> > -asm/x86-vendors.h asm/x86-defns.h asm/msr-index.h:
>> > +x86_emulate/x86_emulate.c x86_emulate/x86_emulate.h: x86_emulate
>> 
>> You should be able to do this:
>> 
>> x86_emulate/%: x86_emulate
>> 
> 
> Using % won't work. Jan made a similar comment during the first
> iteration of the fuzzer series. It turned out that we needed to list
> explicitly every file because of the other rules explicitly listed
> those files.

There was something in that area, but explicitly mentioning files
elsewhere should not prevent this dependency to work as long as
the files don't exist without the rule's commands having run.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH 1/3] fuzz/x86emul: avoid race in link farm rune

[Wei Liu]

On Fri, Feb 17, 2017 at 03:33:26AM -0700, Jan Beulich wrote:
> >>> On 17.02.17 at 11:12, <wei.liu2@citrix.com> wrote:
> > On Thu, Feb 16, 2017 at 07:10:41PM +0000, Andrew Cooper wrote:
> >> On 16/02/17 18:56, Wei Liu wrote:
> >> > --- a/tools/fuzz/x86_instruction_emulator/Makefile
> >> > +++ b/tools/fuzz/x86_instruction_emulator/Makefile
> >> > @@ -8,12 +8,16 @@ else
> >> >  x86-instruction-emulator-fuzzer-all:
> >> >  endif
> >> >  
> >> > -x86_emulate/x86_emulate.c x86_emulate/x86_emulate.h:
> >> > +x86_emulate:
> >> >  	[ -L x86_emulate ] || ln -sf $(XEN_ROOT)/xen/arch/x86/x86_emulate .
> >> >  
> >> > -asm/x86-vendors.h asm/x86-defns.h asm/msr-index.h:
> >> > +x86_emulate/x86_emulate.c x86_emulate/x86_emulate.h: x86_emulate
> >> 
> >> You should be able to do this:
> >> 
> >> x86_emulate/%: x86_emulate
> >> 
> > 
> > Using % won't work. Jan made a similar comment during the first
> > iteration of the fuzzer series. It turned out that we needed to list
> > explicitly every file because of the other rules explicitly listed
> > those files.
> 
> There was something in that area, but explicitly mentioning files
> elsewhere should not prevent this dependency to work as long as
> the files don't exist without the rule's commands having run.
> 

I didn't dig out the old email so the description of the cause might be
inaccurate.

Do you want me to do anything other than explicitly listing every file
as we did before? I'm not sure I get your request.

Wei.

> Jan
> 

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH 1/3] fuzz/x86emul: avoid race in link farm rune

[Jan Beulich]

>>> On 17.02.17 at 12:36, <wei.liu2@citrix.com> wrote:
> On Fri, Feb 17, 2017 at 03:33:26AM -0700, Jan Beulich wrote:
>> >>> On 17.02.17 at 11:12, <wei.liu2@citrix.com> wrote:
>> > On Thu, Feb 16, 2017 at 07:10:41PM +0000, Andrew Cooper wrote:
>> >> On 16/02/17 18:56, Wei Liu wrote:
>> >> > --- a/tools/fuzz/x86_instruction_emulator/Makefile
>> >> > +++ b/tools/fuzz/x86_instruction_emulator/Makefile
>> >> > @@ -8,12 +8,16 @@ else
>> >> >  x86-instruction-emulator-fuzzer-all:
>> >> >  endif
>> >> >  
>> >> > -x86_emulate/x86_emulate.c x86_emulate/x86_emulate.h:
>> >> > +x86_emulate:
>> >> >  	[ -L x86_emulate ] || ln -sf $(XEN_ROOT)/xen/arch/x86/x86_emulate .
>> >> >  
>> >> > -asm/x86-vendors.h asm/x86-defns.h asm/msr-index.h:
>> >> > +x86_emulate/x86_emulate.c x86_emulate/x86_emulate.h: x86_emulate
>> >> 
>> >> You should be able to do this:
>> >> 
>> >> x86_emulate/%: x86_emulate
>> >> 
>> > 
>> > Using % won't work. Jan made a similar comment during the first
>> > iteration of the fuzzer series. It turned out that we needed to list
>> > explicitly every file because of the other rules explicitly listed
>> > those files.
>> 
>> There was something in that area, but explicitly mentioning files
>> elsewhere should not prevent this dependency to work as long as
>> the files don't exist without the rule's commands having run.
> 
> I didn't dig out the old email so the description of the cause might be
> inaccurate.
> 
> Do you want me to do anything other than explicitly listing every file
> as we did before? I'm not sure I get your request.

Well, I'd prefer the shorter variant Andrew did suggest, so if that's
not possible to be used here, I think it would help if the reason was
recorded (in the commit message) now, to avoid later having the
discussion yet another time.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH 1/3] fuzz/x86emul: avoid race in link farm rune

[Wei Liu]

On Fri, Feb 17, 2017 at 04:44:45AM -0700, Jan Beulich wrote:
> >>> On 17.02.17 at 12:36, <wei.liu2@citrix.com> wrote:
> > On Fri, Feb 17, 2017 at 03:33:26AM -0700, Jan Beulich wrote:
> >> >>> On 17.02.17 at 11:12, <wei.liu2@citrix.com> wrote:
> >> > On Thu, Feb 16, 2017 at 07:10:41PM +0000, Andrew Cooper wrote:
> >> >> On 16/02/17 18:56, Wei Liu wrote:
> >> >> > --- a/tools/fuzz/x86_instruction_emulator/Makefile
> >> >> > +++ b/tools/fuzz/x86_instruction_emulator/Makefile
> >> >> > @@ -8,12 +8,16 @@ else
> >> >> >  x86-instruction-emulator-fuzzer-all:
> >> >> >  endif
> >> >> >  
> >> >> > -x86_emulate/x86_emulate.c x86_emulate/x86_emulate.h:
> >> >> > +x86_emulate:
> >> >> >  	[ -L x86_emulate ] || ln -sf $(XEN_ROOT)/xen/arch/x86/x86_emulate .
> >> >> >  
> >> >> > -asm/x86-vendors.h asm/x86-defns.h asm/msr-index.h:
> >> >> > +x86_emulate/x86_emulate.c x86_emulate/x86_emulate.h: x86_emulate
> >> >> 
> >> >> You should be able to do this:
> >> >> 
> >> >> x86_emulate/%: x86_emulate
> >> >> 
> >> > 
> >> > Using % won't work. Jan made a similar comment during the first
> >> > iteration of the fuzzer series. It turned out that we needed to list
> >> > explicitly every file because of the other rules explicitly listed
> >> > those files.
> >> 
> >> There was something in that area, but explicitly mentioning files
> >> elsewhere should not prevent this dependency to work as long as
> >> the files don't exist without the rule's commands having run.
> > 
> > I didn't dig out the old email so the description of the cause might be
> > inaccurate.
> > 
> > Do you want me to do anything other than explicitly listing every file
> > as we did before? I'm not sure I get your request.
> 
> Well, I'd prefer the shorter variant Andrew did suggest, so if that's
> not possible to be used here, I think it would help if the reason was
> recorded (in the commit message) now, to avoid later having the
> discussion yet another time.

I think I figure out how to make it work. New version of patches coming
soon.

Wei.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH 1/3] fuzz/x86emul: avoid race in link farm rune

[Jan Beulich]

>>> On 16.02.17 at 19:56, <wei.liu2@citrix.com> wrote:
> --- a/tools/fuzz/x86_instruction_emulator/Makefile
> +++ b/tools/fuzz/x86_instruction_emulator/Makefile
> @@ -8,12 +8,16 @@ else
>  x86-instruction-emulator-fuzzer-all:
>  endif
>  
> -x86_emulate/x86_emulate.c x86_emulate/x86_emulate.h:
> +x86_emulate:
>  	[ -L x86_emulate ] || ln -sf $(XEN_ROOT)/xen/arch/x86/x86_emulate .

In addition to what Andrew has asked for, I would then also appreciate
if you used $@ here and ...

> -asm/x86-vendors.h asm/x86-defns.h asm/msr-index.h:
> +x86_emulate/x86_emulate.c x86_emulate/x86_emulate.h: x86_emulate
> +
> +asm:
>  	[ -L asm ] || ln -sf $(XEN_ROOT)/xen/include/asm-x86 asm

... here along the lines of ...

> +asm/x86-vendors.h asm/x86-defns.h asm/msr-index.h: asm
> +
>  x86_emulate.c x86_emulate.h: %:
>  	[ -L $* ] || ln -sf $(XEN_ROOT)/tools/tests/x86_emulator/$*

the use of $* here. Looking at this last one makes me notice, btw,
that the second argument to ln is missing? Is it portable to assume
that it'll use . in that case?

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH 1/3] fuzz/x86emul: avoid race in link farm rune

[Wei Liu]

On Fri, Feb 17, 2017 at 02:24:15AM -0700, Jan Beulich wrote:
> >>> On 16.02.17 at 19:56, <wei.liu2@citrix.com> wrote:
> > --- a/tools/fuzz/x86_instruction_emulator/Makefile
> > +++ b/tools/fuzz/x86_instruction_emulator/Makefile
> > @@ -8,12 +8,16 @@ else
> >  x86-instruction-emulator-fuzzer-all:
> >  endif
> >  
> > -x86_emulate/x86_emulate.c x86_emulate/x86_emulate.h:
> > +x86_emulate:
> >  	[ -L x86_emulate ] || ln -sf $(XEN_ROOT)/xen/arch/x86/x86_emulate .
> 
> In addition to what Andrew has asked for, I would then also appreciate
> if you used $@ here and ...
> 

Using $@ is OK.

> > -asm/x86-vendors.h asm/x86-defns.h asm/msr-index.h:
> > +x86_emulate/x86_emulate.c x86_emulate/x86_emulate.h: x86_emulate
> > +
> > +asm:
> >  	[ -L asm ] || ln -sf $(XEN_ROOT)/xen/include/asm-x86 asm
> 
> ... here along the lines of ...
> 
> > +asm/x86-vendors.h asm/x86-defns.h asm/msr-index.h: asm
> > +
> >  x86_emulate.c x86_emulate.h: %:
> >  	[ -L $* ] || ln -sf $(XEN_ROOT)/tools/tests/x86_emulator/$*
> 
> the use of $* here. Looking at this last one makes me notice, btw,
> that the second argument to ln is missing? Is it portable to assume
> that it'll use . in that case?
> 

Yes. From manpage:

ln [OPTION]... TARGET                  (2nd form)
... in the 2nd form, create a link to TARGET in the current directory.

Wei.

> Jan
> 

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[PATCH 2/3] x86emul/test: avoid race in link farm rune

[Wei Liu]

Several `ln -sf` can race with each other.  Provide dedicated targets
for soft-linking directories.

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
 tools/tests/x86_emulator/Makefile | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/tools/tests/x86_emulator/Makefile b/tools/tests/x86_emulator/Makefile
index 9bf36947c0..860e3867d6 100644
--- a/tools/tests/x86_emulator/Makefile
+++ b/tools/tests/x86_emulator/Makefile
@@ -40,12 +40,16 @@ distclean: clean
 .PHONY: install
 install:
 
-x86_emulate/x86_emulate.c x86_emulate/x86_emulate.h:
+x86_emulate:
 	[ -L x86_emulate ] || ln -sf $(XEN_ROOT)/xen/arch/x86/x86_emulate .
 
-asm/x86-vendors.h asm/x86-defns.h asm/msr-index.h:
+x86_emulate/x86_emulate.c x86_emulate/x86_emulate.h: x86_emulate
+
+asm:
 	[ -L asm ] || ln -sf $(XEN_ROOT)/xen/include/asm-x86 asm
 
+asm/x86-vendors.h asm/x86-defns.h asm/msr-index.h: asm
+
 HOSTCFLAGS += $(CFLAGS_xeninclude) -I.
 
 x86.h := asm/x86-vendors.h asm/x86-defns.h asm/msr-index.h
-- 
2.11.0


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[PATCH 3/3] gitignore: ignore asm soft link in fuzz and x86emul test

[Wei Liu]

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
 .gitignore | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/.gitignore b/.gitignore
index 8810c6975a..c8d56d1bdb 100644
--- a/.gitignore
+++ b/.gitignore
@@ -147,6 +147,7 @@ tools/flask/utils/flask-setenforce
 tools/flask/utils/flask-set-bool
 tools/flask/utils/flask-label-pci
 tools/fuzz/libelf/afl-libelf-fuzzer
+tools/fuzz/x86_instruction_emulator/asm
 tools/fuzz/x86_instruction_emulator/x86_emulate*
 tools/fuzz/x86_instruction_emulator/afl-x86-insn-emulator-fuzzer
 tools/helpers/_paths.h
@@ -209,6 +210,7 @@ tools/python/build/*
 tools/security/secpol_tool
 tools/security/xen/*
 tools/security/xensec_tool
+tools/tests/x86_emulator/asm
 tools/tests/x86_emulator/blowfish.bin
 tools/tests/x86_emulator/blowfish.h
 tools/tests/x86_emulator/test_x86_emulator
-- 
2.11.0


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[PATCH 3/3] gitignore: ignore asm soft link in fuzz and x86emul test

[Jan Beulich]

>>> On 16.02.17 at 19:56, <wei.liu2@citrix.com> wrote:
> Signed-off-by: Wei Liu <wei.liu2@citrix.com>

Acked-by: Jan Beulich <jbeulich@suse.com>



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel