| Message ID | 20201105221905.1350-2-dbuono@linux.vnet.ibm.com |
|---|---|
| State | Accepted |
| Commit | aba378dee666fe2aa07f3d318fdf904f454389af |
| Headers | show |
| Series | Add support for Control-Flow Integrity | expand |
On 201105 1718, Daniele Buono wrote: > LLVM's linker, LLD, supports the keyword "INSERT AFTER", starting with > version 11. > However, when multiple sections are defined in the same "INSERT AFTER", > they are added in a reversed order, compared to BFD's LD. > > This patch makes fork_fuzz.ld generic enough to work with both linkers. > Each section now has its own "INSERT AFTER" keyword, so proper ordering is > defined between the sections added. > Hi Daniele, Good to know that LLVM now has support for "INSERT AFTER" :) I compared the resulting symbols between __FUZZ_COUNTERS_{START,END} (after linking with BFD) before/after this patch, and they look good. I also ran a test-build with OSS-Fuzz container and confirmed that the resulting binary also had proper symbols. Reviewed-by: Alexander Bulekov <alxndr@bu.edu> Tested-by: Alexander Bulekov <alxndr@bu.edu> Thanks > Signed-off-by: Daniele Buono <dbuono@linux.vnet.ibm.com> > --- > tests/qtest/fuzz/fork_fuzz.ld | 12 +++++++++++- > 1 file changed, 11 insertions(+), 1 deletion(-) > > diff --git a/tests/qtest/fuzz/fork_fuzz.ld b/tests/qtest/fuzz/fork_fuzz.ld > index bfb667ed06..cfb88b7fdb 100644 > --- a/tests/qtest/fuzz/fork_fuzz.ld > +++ b/tests/qtest/fuzz/fork_fuzz.ld > @@ -16,6 +16,11 @@ SECTIONS > /* Lowest stack counter */ > *(__sancov_lowest_stack); > } > +} > +INSERT AFTER .data; > + > +SECTIONS > +{ > .data.fuzz_ordered : > { > /* > @@ -34,6 +39,11 @@ SECTIONS > */ > *(.bss._ZN6fuzzer3TPCE); > } > +} > +INSERT AFTER .data.fuzz_start; > + > +SECTIONS > +{ > .data.fuzz_end : ALIGN(4K) > { > __FUZZ_COUNTERS_END = .; > @@ -43,4 +53,4 @@ SECTIONS > * Don't overwrite the SECTIONS in the default linker script. Instead insert the > * above into the default script > */ > -INSERT AFTER .data; > +INSERT AFTER .data.fuzz_ordered; > -- > 2.17.1 >
Thanks Alex, do you think you could also give it a try linking with LLD? just add --extra-ldflags="-fuse-ld=lld" I do see some small differences when moving from BFD ro LLD, but they should not be of importance. The position of the data.fuzz* is kept. size -A on qemu-fuzz-i386, LTO DISABLED: BFD section size addr [...] .got 10704 29849128 .data 1160800 29859840 __sancov_pcs 3362992 31020640 .data.fuzz_start 210187 34385920 .data.fuzz_ordered 211456 34596352 .bss 9659608 34807808 .comment 225 0 [...] BFD section size addr [...] .got 816 27824632 .got.plt 9992 27825448 .data 1160808 27839536 .data.fuzz_start 210187 29003776 .data.fuzz_ordered 211456 29214208 .data.fuzz_end 0 29425664 .tm_clone_table 0 29425664 __sancov_pcs 3362992 29425664 .bss 9659624 32788672 I tried running the fuzzer and didn't seem to have any issues, but I haven't tried a test-build with OSS-Fuzz. Is there a info somewhere on how to do that? Thanks, Daniele On 11/6/2020 9:50 AM, Alexander Bulekov wrote: > On 201105 1718, Daniele Buono wrote: >> LLVM's linker, LLD, supports the keyword "INSERT AFTER", starting with >> version 11. >> However, when multiple sections are defined in the same "INSERT AFTER", >> they are added in a reversed order, compared to BFD's LD. >> >> This patch makes fork_fuzz.ld generic enough to work with both linkers. >> Each section now has its own "INSERT AFTER" keyword, so proper ordering is >> defined between the sections added. >> > > Hi Daniele, > Good to know that LLVM now has support for "INSERT AFTER" :) > > I compared the resulting symbols between __FUZZ_COUNTERS_{START,END} > (after linking with BFD) before/after this patch, and they look good. I > also ran a test-build with OSS-Fuzz container and confirmed that the > resulting binary also had proper symbols. > > Reviewed-by: Alexander Bulekov <alxndr@bu.edu> > Tested-by: Alexander Bulekov <alxndr@bu.edu> > > Thanks > >> Signed-off-by: Daniele Buono <dbuono@linux.vnet.ibm.com> >> --- >> tests/qtest/fuzz/fork_fuzz.ld | 12 +++++++++++- >> 1 file changed, 11 insertions(+), 1 deletion(-) >> >> diff --git a/tests/qtest/fuzz/fork_fuzz.ld b/tests/qtest/fuzz/fork_fuzz.ld >> index bfb667ed06..cfb88b7fdb 100644 >> --- a/tests/qtest/fuzz/fork_fuzz.ld >> +++ b/tests/qtest/fuzz/fork_fuzz.ld >> @@ -16,6 +16,11 @@ SECTIONS >> /* Lowest stack counter */ >> *(__sancov_lowest_stack); >> } >> +} >> +INSERT AFTER .data; >> + >> +SECTIONS >> +{ >> .data.fuzz_ordered : >> { >> /* >> @@ -34,6 +39,11 @@ SECTIONS >> */ >> *(.bss._ZN6fuzzer3TPCE); >> } >> +} >> +INSERT AFTER .data.fuzz_start; >> + >> +SECTIONS >> +{ >> .data.fuzz_end : ALIGN(4K) >> { >> __FUZZ_COUNTERS_END = .; >> @@ -43,4 +53,4 @@ SECTIONS >> * Don't overwrite the SECTIONS in the default linker script. Instead insert the >> * above into the default script >> */ >> -INSERT AFTER .data; >> +INSERT AFTER .data.fuzz_ordered; >> -- >> 2.17.1 >> >
On 201119 1706, Daniele Buono wrote: > Thanks Alex, > do you think you could also give it a try linking with LLD? > > just add --extra-ldflags="-fuse-ld=lld" > > I do see some small differences when moving from BFD ro LLD, but they should > not be of importance. The position of the data.fuzz* is kept. > > size -A on qemu-fuzz-i386, LTO DISABLED: > > BFD > section size addr > [...] > .got 10704 29849128 > .data 1160800 29859840 > __sancov_pcs 3362992 31020640 > .data.fuzz_start 210187 34385920 > .data.fuzz_ordered 211456 34596352 > .bss 9659608 34807808 > .comment 225 0 > [...] > > BFD > section size addr > [...] > .got 816 27824632 > .got.plt 9992 27825448 > .data 1160808 27839536 > .data.fuzz_start 210187 29003776 > .data.fuzz_ordered 211456 29214208 > .data.fuzz_end 0 29425664 > .tm_clone_table 0 29425664 > __sancov_pcs 3362992 29425664 > .bss 9659624 32788672 > > I tried running the fuzzer and didn't seem to have any issues, but I > haven't tried a test-build with OSS-Fuzz. Is there a info somewhere > on how to do that? > > Thanks, > Daniele > Hi Daniele, Sorry for the late response. I tried linking the fuzzer with lld, and everything looks good. OSS-Fuzz just runs scripts/oss-fuzz/build.sh with some environment variables set (CC, CXX, CFLAGS, LIB_FUZZING_ENGINE ...). To get this to work with that script, we would just need to pass the corresponding arguments to ./configure and find a way to locate and specify llvm-ar-{11,12,...}. So far I haven't noticed too much of a performance increase with -flto, but I need to run some more tests. We probably spend too much time in the kernel (fork()-ing for each input, etc) for the gains to show up. -Alex > On 11/6/2020 9:50 AM, Alexander Bulekov wrote: > > On 201105 1718, Daniele Buono wrote: > > > LLVM's linker, LLD, supports the keyword "INSERT AFTER", starting with > > > version 11. > > > However, when multiple sections are defined in the same "INSERT AFTER", > > > they are added in a reversed order, compared to BFD's LD. > > > > > > This patch makes fork_fuzz.ld generic enough to work with both linkers. > > > Each section now has its own "INSERT AFTER" keyword, so proper ordering is > > > defined between the sections added. > > > > > > > Hi Daniele, > > Good to know that LLVM now has support for "INSERT AFTER" :) > > > > I compared the resulting symbols between __FUZZ_COUNTERS_{START,END} > > (after linking with BFD) before/after this patch, and they look good. I > > also ran a test-build with OSS-Fuzz container and confirmed that the > > resulting binary also had proper symbols. > > > > Reviewed-by: Alexander Bulekov <alxndr@bu.edu> > > Tested-by: Alexander Bulekov <alxndr@bu.edu> > > > > Thanks > > > > > Signed-off-by: Daniele Buono <dbuono@linux.vnet.ibm.com> > > > --- > > > tests/qtest/fuzz/fork_fuzz.ld | 12 +++++++++++- > > > 1 file changed, 11 insertions(+), 1 deletion(-) > > > > > > diff --git a/tests/qtest/fuzz/fork_fuzz.ld b/tests/qtest/fuzz/fork_fuzz.ld > > > index bfb667ed06..cfb88b7fdb 100644 > > > --- a/tests/qtest/fuzz/fork_fuzz.ld > > > +++ b/tests/qtest/fuzz/fork_fuzz.ld > > > @@ -16,6 +16,11 @@ SECTIONS > > > /* Lowest stack counter */ > > > *(__sancov_lowest_stack); > > > } > > > +} > > > +INSERT AFTER .data; > > > + > > > +SECTIONS > > > +{ > > > .data.fuzz_ordered : > > > { > > > /* > > > @@ -34,6 +39,11 @@ SECTIONS > > > */ > > > *(.bss._ZN6fuzzer3TPCE); > > > } > > > +} > > > +INSERT AFTER .data.fuzz_start; > > > + > > > +SECTIONS > > > +{ > > > .data.fuzz_end : ALIGN(4K) > > > { > > > __FUZZ_COUNTERS_END = .; > > > @@ -43,4 +53,4 @@ SECTIONS > > > * Don't overwrite the SECTIONS in the default linker script. Instead insert the > > > * above into the default script > > > */ > > > -INSERT AFTER .data; > > > +INSERT AFTER .data.fuzz_ordered; > > > -- > > > 2.17.1 > > > > >
diff --git a/tests/qtest/fuzz/fork_fuzz.ld b/tests/qtest/fuzz/fork_fuzz.ld index bfb667ed06..cfb88b7fdb 100644 --- a/tests/qtest/fuzz/fork_fuzz.ld +++ b/tests/qtest/fuzz/fork_fuzz.ld @@ -16,6 +16,11 @@ SECTIONS /* Lowest stack counter */ *(__sancov_lowest_stack); } +} +INSERT AFTER .data; + +SECTIONS +{ .data.fuzz_ordered : { /* @@ -34,6 +39,11 @@ SECTIONS */ *(.bss._ZN6fuzzer3TPCE); } +} +INSERT AFTER .data.fuzz_start; + +SECTIONS +{ .data.fuzz_end : ALIGN(4K) { __FUZZ_COUNTERS_END = .; @@ -43,4 +53,4 @@ SECTIONS * Don't overwrite the SECTIONS in the default linker script. Instead insert the * above into the default script */ -INSERT AFTER .data; +INSERT AFTER .data.fuzz_ordered;
LLVM's linker, LLD, supports the keyword "INSERT AFTER", starting with version 11. However, when multiple sections are defined in the same "INSERT AFTER", they are added in a reversed order, compared to BFD's LD. This patch makes fork_fuzz.ld generic enough to work with both linkers. Each section now has its own "INSERT AFTER" keyword, so proper ordering is defined between the sections added. Signed-off-by: Daniele Buono <dbuono@linux.vnet.ibm.com> --- tests/qtest/fuzz/fork_fuzz.ld | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-)