Re: [PATCH] elf/tst-big-note: Improve accuracy of test [BZ #20419]

[Carlos O'Donell <carlos at redhat dot com>]

On 3/1/19 11:41 AM, Florian Weimer wrote:
> elf/tst-big-note: Improve accuracy of test [BZ #20419]
> 
> It is possible that the link editor injects an allocated ABI tag note
> before the artificial, allocated large note in the test.  Note parsing
> in open_verify stops when the first ABI tag note is encountered, so if
> the ABI tag note comes first, the problematic code is not actually
> exercised.

OK. Good catch, and that's done on purpose. The second note is detected
in dl-prop.h, but it immediately causes the function to return without
setting ibt/sht bits. So we don't process it.

> Also tweak the artificial note so that it is a syntactically valid
> 4-byte aligned note, in case the link editor tries to parse notes and
> process them.

OK. Good catch also. Thanks for cleaning up the test.

> Improves the testing part of commit 0065aaaaae51cd60210ec3a7e13.
> 
> 2019-03-01  Florian Weimer  <fweimer@redhat.com>
> 
> 	[BZ #20419]
> 	* elf/tst-big-note-lib.S: Create a syntactically valid note.
> 	* elf/Makefile (tst-big-note-lib.so): Do not link with startup
> 	code, to avoid creating an ABI tag note.
> 	(modules-names-nobuild): Add tst-big-note-lib.
> 
> diff --git a/elf/Makefile b/elf/Makefile
> index 55204073a3..310a37cc13 100644
> --- a/elf/Makefile
> +++ b/elf/Makefile
> @@ -316,8 +316,8 @@ endif
>  modules-execstack-yes = tst-execstack-mod
>  extra-test-objs += $(addsuffix .os,$(strip $(modules-names)))
>  
> -# filtmod1.so has a special rule
> -modules-names-nobuild := filtmod1
> +# filtmod1.so, tst-big-note-lib.so have special rules.
> +modules-names-nobuild := filtmod1 tst-big-note-lib

OK. Right so tst-big-note-lib is in $(modules-names) and so the build
is handled by extra-modules-build, unless you exclude it via
$(modules-names-nobuild), which you do here.

>  
>  tests += $(tests-static)
>  
> @@ -1515,6 +1515,11 @@ tst-libc_dlvsym-static-ENV = \
>  $(objpfx)tst-libc_dlvsym-static.out: $(objpfx)tst-libc_dlvsym-dso.so
>  
>  $(objpfx)tst-big-note: $(objpfx)tst-big-note-lib.so
> +# Avoid creating an ABI tag note, which may come before the
> +# artificial, large note in tst-big-note-lib.o and invalidate the
> +# test.
> +$(objpfx)tst-big-note-lib.so: $(objpfx)tst-big-note-lib.o
> +	$(LINK.o) -shared -o $@ $(LDFLAGS.so) $<

OK. tst-big-note already depends on the DSO.

>  
>  $(objpfx)tst-unwind-ctor: $(objpfx)tst-unwind-ctor-lib.so
>  
> diff --git a/elf/tst-big-note-lib.S b/elf/tst-big-note-lib.S
> index e2008cf4ae..721686fa0e 100644
> --- a/elf/tst-big-note-lib.S
> +++ b/elf/tst-big-note-lib.S
> @@ -20,7 +20,13 @@
>     On a typical Linux system with 8MiB "ulimit -s", that was enough
>     to trigger stack overflow in open_verify.  */
>  
> +#define NOTE_SIZE 8*1024*1024
> +

OK.

>  .pushsection .note.big,"a"
> -.balign 4
> -.fill 8*1024*1024, 1, 0
> +	.balign 4
> +	.long 5 		/* n_namesz.  Length of "GLIBC".  */
> +	.long NOTE_SIZE		/* n_descsz.  */
> +	.long 0			/* n_type.  */
> +	.ascii "GLIBC\0\0\0"	/* Name and alignment to four bytes.  */

OK. Good, alignment filled up here.

> +	.fill NOTE_SIZE, 1, 0
>  .popsection

OK for master.

Reviewed-by: Carlos O'Donell <carlos@redhat.com>