diff mbox series

[2/2] Add scripts/oss-fuzz/build.sh

Message ID 20240621191434.5046-2-tamas@tklengyel.com (mailing list archive)
State Superseded
Headers show
Series [1/2] Add libfuzzer target to fuzz/x86_instruction_emulator | expand

Commit Message

Tamas K Lengyel June 21, 2024, 7:14 p.m. UTC
The build integration script for oss-fuzz targets.

Signed-off-by: Tamas K Lengyel <tamas@tklengyel.com>
---
 scripts/oss-fuzz/build.sh | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)
 create mode 100755 scripts/oss-fuzz/build.sh

Comments

Julien Grall June 24, 2024, 9:58 p.m. UTC | #1
Hi,

On 21/06/2024 20:14, Tamas K Lengyel wrote:
> The build integration script for oss-fuzz targets.

Do you have any details how this is meant and/or will be used?

I also couldn't find a cover letter. For series with more than one 
patch, it is recommended to have one as it help threading and could also 
give some insight on what you are aiming to do.

> 
> Signed-off-by: Tamas K Lengyel <tamas@tklengyel.com>
> ---
>   scripts/oss-fuzz/build.sh | 22 ++++++++++++++++++++++
>   1 file changed, 22 insertions(+)
>   create mode 100755 scripts/oss-fuzz/build.sh
> 
> diff --git a/scripts/oss-fuzz/build.sh b/scripts/oss-fuzz/build.sh
> new file mode 100755
> index 0000000000..48528bbfc2
> --- /dev/null
> +++ b/scripts/oss-fuzz/build.sh

Depending on the answer above, we may want to consider to create the 
directory oss-fuzz under automation or maybe tools/fuzz/.

> @@ -0,0 +1,22 @@
> +#!/bin/bash -eu
> +# Copyright 2024 Google LLC

I am a bit confused with this copyright. Is this script taken from 
somewhere?

> +#
> +# Licensed under the Apache License, Version 2.0 (the "License");
> +# you may not use this file except in compliance with the License.
> +# You may obtain a copy of the License at
> +#
> +#      http://www.apache.org/licenses/LICENSE-2.0
> +#
> +# Unless required by applicable law or agreed to in writing, software
> +# distributed under the License is distributed on an "AS IS" BASIS,
> +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
> +# See the License for the specific language governing permissions and
> +# limitations under the License.
> +#
> +################################################################################
> +
> +cd xen
> +./configure clang=y --disable-stubdom --disable-pvshim --disable-docs --disable-xen

Looking at the help from ./configure, 'clang=y' is not mentioned and it 
doesn't make any difference in the config.log. Can you clarify why this 
was added?

> +make clang=y -C tools/include
> +make clang=y -C tools/fuzz/x86_instruction_emulator libfuzzer-harness
> +cp tools/fuzz/x86_instruction_emulator/libfuzzer-harness $OUT/x86_instruction_emulator

Who will be defining $OUT?

Cheers,
Tamas K Lengyel June 24, 2024, 10:18 p.m. UTC | #2
On Mon, Jun 24, 2024 at 5:58 PM Julien Grall <julien@xen.org> wrote:
>
> Hi,
>
> On 21/06/2024 20:14, Tamas K Lengyel wrote:
> > The build integration script for oss-fuzz targets.
>
> Do you have any details how this is meant and/or will be used?

https://google.github.io/oss-fuzz/getting-started/new-project-guide/#buildsh

>
> I also couldn't find a cover letter. For series with more than one
> patch, it is recommended to have one as it help threading and could also
> give some insight on what you are aiming to do.
>
> >
> > Signed-off-by: Tamas K Lengyel <tamas@tklengyel.com>
> > ---
> >   scripts/oss-fuzz/build.sh | 22 ++++++++++++++++++++++
> >   1 file changed, 22 insertions(+)
> >   create mode 100755 scripts/oss-fuzz/build.sh
> >
> > diff --git a/scripts/oss-fuzz/build.sh b/scripts/oss-fuzz/build.sh
> > new file mode 100755
> > index 0000000000..48528bbfc2
> > --- /dev/null
> > +++ b/scripts/oss-fuzz/build.sh
>
> Depending on the answer above, we may want to consider to create the
> directory oss-fuzz under automation or maybe tools/fuzz/.

I'm fine with moving it wherever.

>
> > @@ -0,0 +1,22 @@
> > +#!/bin/bash -eu
> > +# Copyright 2024 Google LLC
>
> I am a bit confused with this copyright. Is this script taken from
> somewhere?

Yes, I took an existing build.sh from oss-fuzz, it is recommended to
have the more complex part of build.sh as part of the upstream
repository so that additional targets/fixes can be merged there
instead of opening PRs on oss-fuzz directly. With this setup the
build.sh I merge to oss-fuzz will just just this build.sh in the Xen
repository. See
https://github.com/tklengyel/oss-fuzz/commit/552317ae9d24ef1c00d87595516cc364bc33b662.

>
> > +#
> > +# Licensed under the Apache License, Version 2.0 (the "License");
> > +# you may not use this file except in compliance with the License.
> > +# You may obtain a copy of the License at
> > +#
> > +#      http://www.apache.org/licenses/LICENSE-2.0
> > +#
> > +# Unless required by applicable law or agreed to in writing, software
> > +# distributed under the License is distributed on an "AS IS" BASIS,
> > +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
> > +# See the License for the specific language governing permissions and
> > +# limitations under the License.
> > +#
> > +################################################################################
> > +
> > +cd xen
> > +./configure clang=y --disable-stubdom --disable-pvshim --disable-docs --disable-xen
>
> Looking at the help from ./configure, 'clang=y' is not mentioned and it
> doesn't make any difference in the config.log. Can you clarify why this
> was added?

Just throwing stuff at the wall till I was able to get a clang build.
If it's indeed not needed I can remove it.

>
> > +make clang=y -C tools/include
> > +make clang=y -C tools/fuzz/x86_instruction_emulator libfuzzer-harness
> > +cp tools/fuzz/x86_instruction_emulator/libfuzzer-harness $OUT/x86_instruction_emulator
>
> Who will be defining $OUT?

oss-fuzz

Tamas
Jan Beulich June 25, 2024, 9:16 a.m. UTC | #3
On 21.06.2024 21:14, Tamas K Lengyel wrote:
> --- /dev/null
> +++ b/scripts/oss-fuzz/build.sh
> @@ -0,0 +1,22 @@
> +#!/bin/bash -eu
> +# Copyright 2024 Google LLC
> +#
> +# Licensed under the Apache License, Version 2.0 (the "License");
> +# you may not use this file except in compliance with the License.
> +# You may obtain a copy of the License at
> +#
> +#      http://www.apache.org/licenses/LICENSE-2.0
> +#
> +# Unless required by applicable law or agreed to in writing, software
> +# distributed under the License is distributed on an "AS IS" BASIS,
> +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
> +# See the License for the specific language governing permissions and
> +# limitations under the License.
> +#
> +################################################################################

I'm a little concerned here, but maybe I shouldn't be. According to what
I'm reading, the Apache 2.0 license is at least not entirely compatible
with GPLv2. While apparently the issue is solely with linking in Apache-
licensed code, I wonder whether us not having a respective file under
./LICENSES/ (and no pre-cooked SPDX identifier to use) actually has a
reason possibly excluding the use of such code in the project.

> +cd xen
> +./configure clang=y --disable-stubdom --disable-pvshim --disable-docs --disable-xen
> +make clang=y -C tools/include
> +make clang=y -C tools/fuzz/x86_instruction_emulator libfuzzer-harness
> +cp tools/fuzz/x86_instruction_emulator/libfuzzer-harness $OUT/x86_instruction_emulator

In addition to what Julien said, I further think that filename / directory
name are too generic for a file with this pretty specific contents.

Jan
Tamas K Lengyel June 25, 2024, 11:15 a.m. UTC | #4
On Tue, Jun 25, 2024 at 5:17 AM Jan Beulich <jbeulich@suse.com> wrote:
>
> On 21.06.2024 21:14, Tamas K Lengyel wrote:
> > --- /dev/null
> > +++ b/scripts/oss-fuzz/build.sh
> > @@ -0,0 +1,22 @@
> > +#!/bin/bash -eu
> > +# Copyright 2024 Google LLC
> > +#
> > +# Licensed under the Apache License, Version 2.0 (the "License");
> > +# you may not use this file except in compliance with the License.
> > +# You may obtain a copy of the License at
> > +#
> > +#      http://www.apache.org/licenses/LICENSE-2.0
> > +#
> > +# Unless required by applicable law or agreed to in writing, software
> > +# distributed under the License is distributed on an "AS IS" BASIS,
> > +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
> > +# See the License for the specific language governing permissions and
> > +# limitations under the License.
> > +#
> > +################################################################################
>
> I'm a little concerned here, but maybe I shouldn't be. According to what
> I'm reading, the Apache 2.0 license is at least not entirely compatible
> with GPLv2. While apparently the issue is solely with linking in Apache-
> licensed code, I wonder whether us not having a respective file under
> ./LICENSES/ (and no pre-cooked SPDX identifier to use) actually has a
> reason possibly excluding the use of such code in the project.
>
> > +cd xen
> > +./configure clang=y --disable-stubdom --disable-pvshim --disable-docs --disable-xen
> > +make clang=y -C tools/include
> > +make clang=y -C tools/fuzz/x86_instruction_emulator libfuzzer-harness
> > +cp tools/fuzz/x86_instruction_emulator/libfuzzer-harness $OUT/x86_instruction_emulator
>
> In addition to what Julien said, I further think that filename / directory
> name are too generic for a file with this pretty specific contents.

I don't really get your concern here?

Tamas
Tamas K Lengyel June 25, 2024, 11:18 a.m. UTC | #5
On Tue, Jun 25, 2024 at 5:17 AM Jan Beulich <jbeulich@suse.com> wrote:
>
> On 21.06.2024 21:14, Tamas K Lengyel wrote:
> > --- /dev/null
> > +++ b/scripts/oss-fuzz/build.sh
> > @@ -0,0 +1,22 @@
> > +#!/bin/bash -eu
> > +# Copyright 2024 Google LLC
> > +#
> > +# Licensed under the Apache License, Version 2.0 (the "License");
> > +# you may not use this file except in compliance with the License.
> > +# You may obtain a copy of the License at
> > +#
> > +#      http://www.apache.org/licenses/LICENSE-2.0
> > +#
> > +# Unless required by applicable law or agreed to in writing, software
> > +# distributed under the License is distributed on an "AS IS" BASIS,
> > +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
> > +# See the License for the specific language governing permissions and
> > +# limitations under the License.
> > +#
> > +################################################################################
>
> I'm a little concerned here, but maybe I shouldn't be. According to what
> I'm reading, the Apache 2.0 license is at least not entirely compatible
> with GPLv2. While apparently the issue is solely with linking in Apache-
> licensed code, I wonder whether us not having a respective file under
> ./LICENSES/ (and no pre-cooked SPDX identifier to use) actually has a
> reason possibly excluding the use of such code in the project.

The script is standalone in a clearly separate folder, not linking
with anything else in the project so there is no license mixing.
Adding an SPDX tag to the file would be fine.

Tamas
Jan Beulich June 25, 2024, 11:40 a.m. UTC | #6
On 25.06.2024 13:15, Tamas K Lengyel wrote:
> On Tue, Jun 25, 2024 at 5:17 AM Jan Beulich <jbeulich@suse.com> wrote:
>>
>> On 21.06.2024 21:14, Tamas K Lengyel wrote:
>>> --- /dev/null
>>> +++ b/scripts/oss-fuzz/build.sh
>>> @@ -0,0 +1,22 @@
>>> +#!/bin/bash -eu
>>> +# Copyright 2024 Google LLC
>>> +#
>>> +# Licensed under the Apache License, Version 2.0 (the "License");
>>> +# you may not use this file except in compliance with the License.
>>> +# You may obtain a copy of the License at
>>> +#
>>> +#      http://www.apache.org/licenses/LICENSE-2.0
>>> +#
>>> +# Unless required by applicable law or agreed to in writing, software
>>> +# distributed under the License is distributed on an "AS IS" BASIS,
>>> +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
>>> +# See the License for the specific language governing permissions and
>>> +# limitations under the License.
>>> +#
>>> +################################################################################
>>
>> I'm a little concerned here, but maybe I shouldn't be. According to what
>> I'm reading, the Apache 2.0 license is at least not entirely compatible
>> with GPLv2. While apparently the issue is solely with linking in Apache-
>> licensed code, I wonder whether us not having a respective file under
>> ./LICENSES/ (and no pre-cooked SPDX identifier to use) actually has a
>> reason possibly excluding the use of such code in the project.
>>
>>> +cd xen
>>> +./configure clang=y --disable-stubdom --disable-pvshim --disable-docs --disable-xen
>>> +make clang=y -C tools/include
>>> +make clang=y -C tools/fuzz/x86_instruction_emulator libfuzzer-harness
>>> +cp tools/fuzz/x86_instruction_emulator/libfuzzer-harness $OUT/x86_instruction_emulator
>>
>> In addition to what Julien said, I further think that filename / directory
>> name are too generic for a file with this pretty specific contents.
> 
> I don't really get your concern here?

The thing that is built is specifically a x86 emulator piece of fuzzing
binary. Neither the directory name nor the file name contain either x86
or (at least) emul.

Jan
Tamas K Lengyel June 25, 2024, 12:39 p.m. UTC | #7
On Tue, Jun 25, 2024 at 7:40 AM Jan Beulich <jbeulich@suse.com> wrote:
>
> On 25.06.2024 13:15, Tamas K Lengyel wrote:
> > On Tue, Jun 25, 2024 at 5:17 AM Jan Beulich <jbeulich@suse.com> wrote:
> >>
> >> On 21.06.2024 21:14, Tamas K Lengyel wrote:
> >>> --- /dev/null
> >>> +++ b/scripts/oss-fuzz/build.sh
> >>> @@ -0,0 +1,22 @@
> >>> +#!/bin/bash -eu
> >>> +# Copyright 2024 Google LLC
> >>> +#
> >>> +# Licensed under the Apache License, Version 2.0 (the "License");
> >>> +# you may not use this file except in compliance with the License.
> >>> +# You may obtain a copy of the License at
> >>> +#
> >>> +#      http://www.apache.org/licenses/LICENSE-2.0
> >>> +#
> >>> +# Unless required by applicable law or agreed to in writing, software
> >>> +# distributed under the License is distributed on an "AS IS" BASIS,
> >>> +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
> >>> +# See the License for the specific language governing permissions and
> >>> +# limitations under the License.
> >>> +#
> >>> +################################################################################
> >>
> >> I'm a little concerned here, but maybe I shouldn't be. According to what
> >> I'm reading, the Apache 2.0 license is at least not entirely compatible
> >> with GPLv2. While apparently the issue is solely with linking in Apache-
> >> licensed code, I wonder whether us not having a respective file under
> >> ./LICENSES/ (and no pre-cooked SPDX identifier to use) actually has a
> >> reason possibly excluding the use of such code in the project.
> >>
> >>> +cd xen
> >>> +./configure clang=y --disable-stubdom --disable-pvshim --disable-docs --disable-xen
> >>> +make clang=y -C tools/include
> >>> +make clang=y -C tools/fuzz/x86_instruction_emulator libfuzzer-harness
> >>> +cp tools/fuzz/x86_instruction_emulator/libfuzzer-harness $OUT/x86_instruction_emulator
> >>
> >> In addition to what Julien said, I further think that filename / directory
> >> name are too generic for a file with this pretty specific contents.
> >
> > I don't really get your concern here?
>
> The thing that is built is specifically a x86 emulator piece of fuzzing
> binary. Neither the directory name nor the file name contain either x86
> or (at least) emul.

Because this build script is not necessarily restricted to build only
this one harness in the future. Right now that's the only one that has
a suitable libfuzzer harness, but the reason this build script is here
is to be easily able to add additional fuzzing binaries without the
need to open PRs on the oss-fuzz repo, which as I understand no one
was willing to do in the xen community due to the CLA. Now that the
integration is going to be in oss-fuzz, the only thing you have to do
in the future is add more stuff to this script to get fuzzed. Anything
that's compiled with libfuzzer and copied to $OUT will be picked up by
oss-fuzz automatically. Makes sense?

Tamas
Jan Beulich June 25, 2024, 1:17 p.m. UTC | #8
On 25.06.2024 14:39, Tamas K Lengyel wrote:
> On Tue, Jun 25, 2024 at 7:40 AM Jan Beulich <jbeulich@suse.com> wrote:
>>
>> On 25.06.2024 13:15, Tamas K Lengyel wrote:
>>> On Tue, Jun 25, 2024 at 5:17 AM Jan Beulich <jbeulich@suse.com> wrote:
>>>>
>>>> On 21.06.2024 21:14, Tamas K Lengyel wrote:
>>>>> --- /dev/null
>>>>> +++ b/scripts/oss-fuzz/build.sh
>>>>> @@ -0,0 +1,22 @@
>>>>> +#!/bin/bash -eu
>>>>> +# Copyright 2024 Google LLC
>>>>> +#
>>>>> +# Licensed under the Apache License, Version 2.0 (the "License");
>>>>> +# you may not use this file except in compliance with the License.
>>>>> +# You may obtain a copy of the License at
>>>>> +#
>>>>> +#      http://www.apache.org/licenses/LICENSE-2.0
>>>>> +#
>>>>> +# Unless required by applicable law or agreed to in writing, software
>>>>> +# distributed under the License is distributed on an "AS IS" BASIS,
>>>>> +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
>>>>> +# See the License for the specific language governing permissions and
>>>>> +# limitations under the License.
>>>>> +#
>>>>> +################################################################################
>>>>
>>>> I'm a little concerned here, but maybe I shouldn't be. According to what
>>>> I'm reading, the Apache 2.0 license is at least not entirely compatible
>>>> with GPLv2. While apparently the issue is solely with linking in Apache-
>>>> licensed code, I wonder whether us not having a respective file under
>>>> ./LICENSES/ (and no pre-cooked SPDX identifier to use) actually has a
>>>> reason possibly excluding the use of such code in the project.
>>>>
>>>>> +cd xen
>>>>> +./configure clang=y --disable-stubdom --disable-pvshim --disable-docs --disable-xen
>>>>> +make clang=y -C tools/include
>>>>> +make clang=y -C tools/fuzz/x86_instruction_emulator libfuzzer-harness
>>>>> +cp tools/fuzz/x86_instruction_emulator/libfuzzer-harness $OUT/x86_instruction_emulator
>>>>
>>>> In addition to what Julien said, I further think that filename / directory
>>>> name are too generic for a file with this pretty specific contents.
>>>
>>> I don't really get your concern here?
>>
>> The thing that is built is specifically a x86 emulator piece of fuzzing
>> binary. Neither the directory name nor the file name contain either x86
>> or (at least) emul.
> 
> Because this build script is not necessarily restricted to build only
> this one harness in the future. Right now that's the only one that has
> a suitable libfuzzer harness, but the reason this build script is here
> is to be easily able to add additional fuzzing binaries without the
> need to open PRs on the oss-fuzz repo, which as I understand no one
> was willing to do in the xen community due to the CLA. Now that the
> integration is going to be in oss-fuzz, the only thing you have to do
> in the future is add more stuff to this script to get fuzzed. Anything
> that's compiled with libfuzzer and copied to $OUT will be picked up by
> oss-fuzz automatically. Makes sense?

It does, yes. Yet nothing like that was said in the description. How
should anyone have known there are future possibilities with this script?

Jan
Tamas K Lengyel June 25, 2024, 1:42 p.m. UTC | #9
On Tue, Jun 25, 2024 at 9:18 AM Jan Beulich <jbeulich@suse.com> wrote:
>
> On 25.06.2024 14:39, Tamas K Lengyel wrote:
> > On Tue, Jun 25, 2024 at 7:40 AM Jan Beulich <jbeulich@suse.com> wrote:
> >>
> >> On 25.06.2024 13:15, Tamas K Lengyel wrote:
> >>> On Tue, Jun 25, 2024 at 5:17 AM Jan Beulich <jbeulich@suse.com> wrote:
> >>>>
> >>>> On 21.06.2024 21:14, Tamas K Lengyel wrote:
> >>>>> --- /dev/null
> >>>>> +++ b/scripts/oss-fuzz/build.sh
> >>>>> @@ -0,0 +1,22 @@
> >>>>> +#!/bin/bash -eu
> >>>>> +# Copyright 2024 Google LLC
> >>>>> +#
> >>>>> +# Licensed under the Apache License, Version 2.0 (the "License");
> >>>>> +# you may not use this file except in compliance with the License.
> >>>>> +# You may obtain a copy of the License at
> >>>>> +#
> >>>>> +#      http://www.apache.org/licenses/LICENSE-2.0
> >>>>> +#
> >>>>> +# Unless required by applicable law or agreed to in writing, software
> >>>>> +# distributed under the License is distributed on an "AS IS" BASIS,
> >>>>> +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
> >>>>> +# See the License for the specific language governing permissions and
> >>>>> +# limitations under the License.
> >>>>> +#
> >>>>> +################################################################################
> >>>>
> >>>> I'm a little concerned here, but maybe I shouldn't be. According to what
> >>>> I'm reading, the Apache 2.0 license is at least not entirely compatible
> >>>> with GPLv2. While apparently the issue is solely with linking in Apache-
> >>>> licensed code, I wonder whether us not having a respective file under
> >>>> ./LICENSES/ (and no pre-cooked SPDX identifier to use) actually has a
> >>>> reason possibly excluding the use of such code in the project.
> >>>>
> >>>>> +cd xen
> >>>>> +./configure clang=y --disable-stubdom --disable-pvshim --disable-docs --disable-xen
> >>>>> +make clang=y -C tools/include
> >>>>> +make clang=y -C tools/fuzz/x86_instruction_emulator libfuzzer-harness
> >>>>> +cp tools/fuzz/x86_instruction_emulator/libfuzzer-harness $OUT/x86_instruction_emulator
> >>>>
> >>>> In addition to what Julien said, I further think that filename / directory
> >>>> name are too generic for a file with this pretty specific contents.
> >>>
> >>> I don't really get your concern here?
> >>
> >> The thing that is built is specifically a x86 emulator piece of fuzzing
> >> binary. Neither the directory name nor the file name contain either x86
> >> or (at least) emul.
> >
> > Because this build script is not necessarily restricted to build only
> > this one harness in the future. Right now that's the only one that has
> > a suitable libfuzzer harness, but the reason this build script is here
> > is to be easily able to add additional fuzzing binaries without the
> > need to open PRs on the oss-fuzz repo, which as I understand no one
> > was willing to do in the xen community due to the CLA. Now that the
> > integration is going to be in oss-fuzz, the only thing you have to do
> > in the future is add more stuff to this script to get fuzzed. Anything
> > that's compiled with libfuzzer and copied to $OUT will be picked up by
> > oss-fuzz automatically. Makes sense?
>
> It does, yes. Yet nothing like that was said in the description. How
> should anyone have known there are future possibilities with this script?

Apologies, to me "The build integration script for oss-fuzz targets."
was sufficiently descriptive but it may require some familiarity with
oss-fuzz to get. I can certainly add the above text to the commit
message if that helps.

Tamas
Jan Beulich June 25, 2024, 2:57 p.m. UTC | #10
On 25.06.2024 15:42, Tamas K Lengyel wrote:
> On Tue, Jun 25, 2024 at 9:18 AM Jan Beulich <jbeulich@suse.com> wrote:
>>
>> On 25.06.2024 14:39, Tamas K Lengyel wrote:
>>> On Tue, Jun 25, 2024 at 7:40 AM Jan Beulich <jbeulich@suse.com> wrote:
>>>>
>>>> On 25.06.2024 13:15, Tamas K Lengyel wrote:
>>>>> On Tue, Jun 25, 2024 at 5:17 AM Jan Beulich <jbeulich@suse.com> wrote:
>>>>>>
>>>>>> On 21.06.2024 21:14, Tamas K Lengyel wrote:
>>>>>>> --- /dev/null
>>>>>>> +++ b/scripts/oss-fuzz/build.sh
>>>>>>> @@ -0,0 +1,22 @@
>>>>>>> +#!/bin/bash -eu
>>>>>>> +# Copyright 2024 Google LLC
>>>>>>> +#
>>>>>>> +# Licensed under the Apache License, Version 2.0 (the "License");
>>>>>>> +# you may not use this file except in compliance with the License.
>>>>>>> +# You may obtain a copy of the License at
>>>>>>> +#
>>>>>>> +#      http://www.apache.org/licenses/LICENSE-2.0
>>>>>>> +#
>>>>>>> +# Unless required by applicable law or agreed to in writing, software
>>>>>>> +# distributed under the License is distributed on an "AS IS" BASIS,
>>>>>>> +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
>>>>>>> +# See the License for the specific language governing permissions and
>>>>>>> +# limitations under the License.
>>>>>>> +#
>>>>>>> +################################################################################
>>>>>>
>>>>>> I'm a little concerned here, but maybe I shouldn't be. According to what
>>>>>> I'm reading, the Apache 2.0 license is at least not entirely compatible
>>>>>> with GPLv2. While apparently the issue is solely with linking in Apache-
>>>>>> licensed code, I wonder whether us not having a respective file under
>>>>>> ./LICENSES/ (and no pre-cooked SPDX identifier to use) actually has a
>>>>>> reason possibly excluding the use of such code in the project.
>>>>>>
>>>>>>> +cd xen
>>>>>>> +./configure clang=y --disable-stubdom --disable-pvshim --disable-docs --disable-xen
>>>>>>> +make clang=y -C tools/include
>>>>>>> +make clang=y -C tools/fuzz/x86_instruction_emulator libfuzzer-harness
>>>>>>> +cp tools/fuzz/x86_instruction_emulator/libfuzzer-harness $OUT/x86_instruction_emulator
>>>>>>
>>>>>> In addition to what Julien said, I further think that filename / directory
>>>>>> name are too generic for a file with this pretty specific contents.
>>>>>
>>>>> I don't really get your concern here?
>>>>
>>>> The thing that is built is specifically a x86 emulator piece of fuzzing
>>>> binary. Neither the directory name nor the file name contain either x86
>>>> or (at least) emul.
>>>
>>> Because this build script is not necessarily restricted to build only
>>> this one harness in the future. Right now that's the only one that has
>>> a suitable libfuzzer harness, but the reason this build script is here
>>> is to be easily able to add additional fuzzing binaries without the
>>> need to open PRs on the oss-fuzz repo, which as I understand no one
>>> was willing to do in the xen community due to the CLA. Now that the
>>> integration is going to be in oss-fuzz, the only thing you have to do
>>> in the future is add more stuff to this script to get fuzzed. Anything
>>> that's compiled with libfuzzer and copied to $OUT will be picked up by
>>> oss-fuzz automatically. Makes sense?
>>
>> It does, yes. Yet nothing like that was said in the description. How
>> should anyone have known there are future possibilities with this script?
> 
> Apologies, to me "The build integration script for oss-fuzz targets."
> was sufficiently descriptive but it may require some familiarity with
> oss-fuzz to get. I can certainly add the above text to the commit
> message if that helps.

Yes please, or and abridged variant thereof.

Jan
Julien Grall June 26, 2024, 12:40 p.m. UTC | #11
Hi Tamas,

On 24/06/2024 23:18, Tamas K Lengyel wrote:
> On Mon, Jun 24, 2024 at 5:58 PM Julien Grall <julien@xen.org> wrote:
>>
>> Hi,
>>
>> On 21/06/2024 20:14, Tamas K Lengyel wrote:
>>> The build integration script for oss-fuzz targets.
>>
>> Do you have any details how this is meant and/or will be used?
> 
> https://google.github.io/oss-fuzz/getting-started/new-project-guide/#buildsh
> 
>>
>> I also couldn't find a cover letter. For series with more than one
>> patch, it is recommended to have one as it help threading and could also
>> give some insight on what you are aiming to do.
>>
>>>
>>> Signed-off-by: Tamas K Lengyel <tamas@tklengyel.com>
>>> ---
>>>    scripts/oss-fuzz/build.sh | 22 ++++++++++++++++++++++
>>>    1 file changed, 22 insertions(+)
>>>    create mode 100755 scripts/oss-fuzz/build.sh
>>>
>>> diff --git a/scripts/oss-fuzz/build.sh b/scripts/oss-fuzz/build.sh
>>> new file mode 100755
>>> index 0000000000..48528bbfc2
>>> --- /dev/null
>>> +++ b/scripts/oss-fuzz/build.sh
>>
>> Depending on the answer above, we may want to consider to create the
>> directory oss-fuzz under automation or maybe tools/fuzz/.
> 
> I'm fine with moving it wherever.

What about tools/fuzz then? This is where are all the tooling for the 
fuzzing.

> 
>>
>>> @@ -0,0 +1,22 @@
>>> +#!/bin/bash -eu
>>> +# Copyright 2024 Google LLC
>>
>> I am a bit confused with this copyright. Is this script taken from
>> somewhere?
> 
> Yes, I took an existing build.sh from oss-fuzz,

It is unclear to me what is left from that "existing" build.sh. At least 
everything below seems to be Xen specific.

Anyway, if you want to give the copyright to Google then fair enough, 
but I think you want to use an Origin tag (or similar) to indicate the 
original copy.

>  it is recommended to
> have the more complex part of build.sh as part of the upstream
> repository so that additional targets/fixes can be merged there
> instead of opening PRs on oss-fuzz directly. With this setup the
> build.sh I merge to oss-fuzz will just just this build.sh in the Xen
> repository. See
> https://github.com/tklengyel/oss-fuzz/commit/552317ae9d24ef1c00d87595516cc364bc33b662.
> 
>>
>>> +#
>>> +# Licensed under the Apache License, Version 2.0 (the "License");
>>> +# you may not use this file except in compliance with the License.
>>> +# You may obtain a copy of the License at
>>> +#
>>> +#      http://www.apache.org/licenses/LICENSE-2.0
>>> +#
>>> +# Unless required by applicable law or agreed to in writing, software
>>> +# distributed under the License is distributed on an "AS IS" BASIS,
>>> +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
>>> +# See the License for the specific language governing permissions and
>>> +# limitations under the License.
>>> +#
>>> +################################################################################
>>> +
>>> +cd xen
>>> +./configure clang=y --disable-stubdom --disable-pvshim --disable-docs --disable-xen
>>
>> Looking at the help from ./configure, 'clang=y' is not mentioned and it
>> doesn't make any difference in the config.log. Can you clarify why this
>> was added?
> 
> Just throwing stuff at the wall till I was able to get a clang build.
> If it's indeed not needed I can remove it.
> 
>>
>>> +make clang=y -C tools/include
>>> +make clang=y -C tools/fuzz/x86_instruction_emulator libfuzzer-harness
>>> +cp tools/fuzz/x86_instruction_emulator/libfuzzer-harness $OUT/x86_instruction_emulator
>>
>> Who will be defining $OUT?
> 
> oss-fuzz

Ok. Can you add a link to the documentation in build.sh? This would be 
helpful for the future reader to understand what's $OUT really mean.

Cheers,
Tamas K Lengyel June 26, 2024, 1:20 p.m. UTC | #12
On Wed, Jun 26, 2024 at 8:41 AM Julien Grall <julien@xen.org> wrote:
>
> Hi Tamas,
>
> On 24/06/2024 23:18, Tamas K Lengyel wrote:
> > On Mon, Jun 24, 2024 at 5:58 PM Julien Grall <julien@xen.org> wrote:
> >>
> >> Hi,
> >>
> >> On 21/06/2024 20:14, Tamas K Lengyel wrote:
> >>> The build integration script for oss-fuzz targets.
> >>
> >> Do you have any details how this is meant and/or will be used?
> >
> > https://google.github.io/oss-fuzz/getting-started/new-project-guide/#buildsh
> >
> >>
> >> I also couldn't find a cover letter. For series with more than one
> >> patch, it is recommended to have one as it help threading and could also
> >> give some insight on what you are aiming to do.
> >>
> >>>
> >>> Signed-off-by: Tamas K Lengyel <tamas@tklengyel.com>
> >>> ---
> >>>    scripts/oss-fuzz/build.sh | 22 ++++++++++++++++++++++
> >>>    1 file changed, 22 insertions(+)
> >>>    create mode 100755 scripts/oss-fuzz/build.sh
> >>>
> >>> diff --git a/scripts/oss-fuzz/build.sh b/scripts/oss-fuzz/build.sh
> >>> new file mode 100755
> >>> index 0000000000..48528bbfc2
> >>> --- /dev/null
> >>> +++ b/scripts/oss-fuzz/build.sh
> >>
> >> Depending on the answer above, we may want to consider to create the
> >> directory oss-fuzz under automation or maybe tools/fuzz/.
> >
> > I'm fine with moving it wherever.
>
> What about tools/fuzz then? This is where are all the tooling for the
> fuzzing.
>
> >
> >>
> >>> @@ -0,0 +1,22 @@
> >>> +#!/bin/bash -eu
> >>> +# Copyright 2024 Google LLC
> >>
> >> I am a bit confused with this copyright. Is this script taken from
> >> somewhere?
> >
> > Yes, I took an existing build.sh from oss-fuzz,
>
> It is unclear to me what is left from that "existing" build.sh. At least
> everything below seems to be Xen specific.
>
> Anyway, if you want to give the copyright to Google then fair enough,
> but I think you want to use an Origin tag (or similar) to indicate the
> original copy.
>
> >  it is recommended to
> > have the more complex part of build.sh as part of the upstream
> > repository so that additional targets/fixes can be merged there
> > instead of opening PRs on oss-fuzz directly. With this setup the
> > build.sh I merge to oss-fuzz will just just this build.sh in the Xen
> > repository. See
> > https://github.com/tklengyel/oss-fuzz/commit/552317ae9d24ef1c00d87595516cc364bc33b662.
> >
> >>
> >>> +#
> >>> +# Licensed under the Apache License, Version 2.0 (the "License");
> >>> +# you may not use this file except in compliance with the License.
> >>> +# You may obtain a copy of the License at
> >>> +#
> >>> +#      http://www.apache.org/licenses/LICENSE-2.0
> >>> +#
> >>> +# Unless required by applicable law or agreed to in writing, software
> >>> +# distributed under the License is distributed on an "AS IS" BASIS,
> >>> +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
> >>> +# See the License for the specific language governing permissions and
> >>> +# limitations under the License.
> >>> +#
> >>> +################################################################################
> >>> +
> >>> +cd xen
> >>> +./configure clang=y --disable-stubdom --disable-pvshim --disable-docs --disable-xen
> >>
> >> Looking at the help from ./configure, 'clang=y' is not mentioned and it
> >> doesn't make any difference in the config.log. Can you clarify why this
> >> was added?
> >
> > Just throwing stuff at the wall till I was able to get a clang build.
> > If it's indeed not needed I can remove it.
> >
> >>
> >>> +make clang=y -C tools/include
> >>> +make clang=y -C tools/fuzz/x86_instruction_emulator libfuzzer-harness
> >>> +cp tools/fuzz/x86_instruction_emulator/libfuzzer-harness $OUT/x86_instruction_emulator
> >>
> >> Who will be defining $OUT?
> >
> > oss-fuzz
>
> Ok. Can you add a link to the documentation in build.sh? This would be
> helpful for the future reader to understand what's $OUT really mean.

Sure, it turns out there is already a README.oss-fuzz in tools/fuzz
that points to the oss-fuzz so I don't think there is anything else
needed here, we can just move the build script there.

Tamas
Julien Grall June 26, 2024, 3:11 p.m. UTC | #13
Hi Tamas,

On 26/06/2024 14:20, Tamas K Lengyel wrote:
> On Wed, Jun 26, 2024 at 8:41 AM Julien Grall <julien@xen.org> wrote:
>>
>> Hi Tamas,
>>
>> On 24/06/2024 23:18, Tamas K Lengyel wrote:
>>> On Mon, Jun 24, 2024 at 5:58 PM Julien Grall <julien@xen.org> wrote:
>>>>
>>>> Hi,
>>>>
>>>> On 21/06/2024 20:14, Tamas K Lengyel wrote:
>>>>> The build integration script for oss-fuzz targets.
>>>>
>>>> Do you have any details how this is meant and/or will be used?
>>>
>>> https://google.github.io/oss-fuzz/getting-started/new-project-guide/#buildsh
>>>
>>>>
>>>> I also couldn't find a cover letter. For series with more than one
>>>> patch, it is recommended to have one as it help threading and could also
>>>> give some insight on what you are aiming to do.
>>>>
>>>>>
>>>>> Signed-off-by: Tamas K Lengyel <tamas@tklengyel.com>
>>>>> ---
>>>>>     scripts/oss-fuzz/build.sh | 22 ++++++++++++++++++++++
>>>>>     1 file changed, 22 insertions(+)
>>>>>     create mode 100755 scripts/oss-fuzz/build.sh
>>>>>
>>>>> diff --git a/scripts/oss-fuzz/build.sh b/scripts/oss-fuzz/build.sh
>>>>> new file mode 100755
>>>>> index 0000000000..48528bbfc2
>>>>> --- /dev/null
>>>>> +++ b/scripts/oss-fuzz/build.sh
>>>>
>>>> Depending on the answer above, we may want to consider to create the
>>>> directory oss-fuzz under automation or maybe tools/fuzz/.
>>>
>>> I'm fine with moving it wherever.
>>
>> What about tools/fuzz then? This is where are all the tooling for the
>> fuzzing.
>>
>>>
>>>>
>>>>> @@ -0,0 +1,22 @@
>>>>> +#!/bin/bash -eu
>>>>> +# Copyright 2024 Google LLC
>>>>
>>>> I am a bit confused with this copyright. Is this script taken from
>>>> somewhere?
>>>
>>> Yes, I took an existing build.sh from oss-fuzz,
>>
>> It is unclear to me what is left from that "existing" build.sh. At least
>> everything below seems to be Xen specific.
>>
>> Anyway, if you want to give the copyright to Google then fair enough,
>> but I think you want to use an Origin tag (or similar) to indicate the
>> original copy.
>>
>>>   it is recommended to
>>> have the more complex part of build.sh as part of the upstream
>>> repository so that additional targets/fixes can be merged there
>>> instead of opening PRs on oss-fuzz directly. With this setup the
>>> build.sh I merge to oss-fuzz will just just this build.sh in the Xen
>>> repository. See
>>> https://github.com/tklengyel/oss-fuzz/commit/552317ae9d24ef1c00d87595516cc364bc33b662.
>>>
>>>>
>>>>> +#
>>>>> +# Licensed under the Apache License, Version 2.0 (the "License");
>>>>> +# you may not use this file except in compliance with the License.
>>>>> +# You may obtain a copy of the License at
>>>>> +#
>>>>> +#      http://www.apache.org/licenses/LICENSE-2.0
>>>>> +#
>>>>> +# Unless required by applicable law or agreed to in writing, software
>>>>> +# distributed under the License is distributed on an "AS IS" BASIS,
>>>>> +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
>>>>> +# See the License for the specific language governing permissions and
>>>>> +# limitations under the License.
>>>>> +#
>>>>> +################################################################################
>>>>> +
>>>>> +cd xen
>>>>> +./configure clang=y --disable-stubdom --disable-pvshim --disable-docs --disable-xen
>>>>
>>>> Looking at the help from ./configure, 'clang=y' is not mentioned and it
>>>> doesn't make any difference in the config.log. Can you clarify why this
>>>> was added?
>>>
>>> Just throwing stuff at the wall till I was able to get a clang build.
>>> If it's indeed not needed I can remove it.
>>>
>>>>
>>>>> +make clang=y -C tools/include
>>>>> +make clang=y -C tools/fuzz/x86_instruction_emulator libfuzzer-harness
>>>>> +cp tools/fuzz/x86_instruction_emulator/libfuzzer-harness $OUT/x86_instruction_emulator
>>>>
>>>> Who will be defining $OUT?
>>>
>>> oss-fuzz
>>
>> Ok. Can you add a link to the documentation in build.sh? This would be
>> helpful for the future reader to understand what's $OUT really mean.
> 
> Sure, it turns out there is already a README.oss-fuzz in tools/fuzz
> that points to the oss-fuzz so I don't think there is anything else
> needed here, 

Perfect. I am fine with that.

Cheers,
diff mbox series

Patch

diff --git a/scripts/oss-fuzz/build.sh b/scripts/oss-fuzz/build.sh
new file mode 100755
index 0000000000..48528bbfc2
--- /dev/null
+++ b/scripts/oss-fuzz/build.sh
@@ -0,0 +1,22 @@ 
+#!/bin/bash -eu
+# Copyright 2024 Google LLC
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#      http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+################################################################################
+
+cd xen
+./configure clang=y --disable-stubdom --disable-pvshim --disable-docs --disable-xen
+make clang=y -C tools/include
+make clang=y -C tools/fuzz/x86_instruction_emulator libfuzzer-harness
+cp tools/fuzz/x86_instruction_emulator/libfuzzer-harness $OUT/x86_instruction_emulator