When running a RISC-V test kernel under QEMU, we need an OpenSBI BIOS
file. In the original QEMU support patchset, kunit_tool would optionally
download this file from GitHub if it didn't exist, using wget.
These days, it can usually be found in the distro's qemu-system-riscv
package, and is located in /usr/share/qemu on all the distros I tried
(Debian, Arch, OpenSUSE). Use this file, and thereby don't do any
downloading in kunit_tool.
In addition, we used to shell out to whatever 'wget' was in the path,
which could have potentially been used to trick the developer into
running another binary. By not using wget at all, we nicely sidestep
this issue.
Cc: Xu Panda <[email protected]>
Fixes: 87c9c1631788 ("kunit: tool: add support for QEMU")
Reported-by: Zeal Robot <[email protected]>
Signed-off-by: David Gow <[email protected]>
---
This is a replacement for "kunit: tool: use absolute path for wget":
https://lore.kernel.org/linux-kselftest/[email protected]/
Instead of just changing the path to wget, it removes the download
option completely and grabs the opensbi-riscv64-generic-fw_dynamic.bin
from the /usr/share/qemu directory, where the distro package manager
should have put it.
I _think_ this should be okay to treat as a fix: we were always grabbing
this from the QEMU GitHub repository, so it should be widely available.
And if you want to treat the wget use as a security issue, getting rid
of it everywhere would be nice.
Thoughts?
-- David
---
tools/testing/kunit/qemu_configs/riscv.py | 18 +++++++-----------
1 file changed, 7 insertions(+), 11 deletions(-)
diff --git a/tools/testing/kunit/qemu_configs/riscv.py b/tools/testing/kunit/qemu_configs/riscv.py
index 6207be146d26..12a1d525978a 100644
--- a/tools/testing/kunit/qemu_configs/riscv.py
+++ b/tools/testing/kunit/qemu_configs/riscv.py
@@ -3,17 +3,13 @@ import os
import os.path
import sys
-GITHUB_OPENSBI_URL = 'https://github.com/qemu/qemu/raw/master/pc-bios/opensbi-riscv64-generic-fw_dynamic.bin'
-OPENSBI_FILE = os.path.basename(GITHUB_OPENSBI_URL)
+OPENSBI_FILE = 'opensbi-riscv64-generic-fw_dynamic.bin'
+OPENSBI_PATH = '/usr/share/qemu/' + OPENSBI_FILE
-if not os.path.isfile(OPENSBI_FILE):
- print('\n\nOpenSBI file is not in the current working directory.\n'
- 'Would you like me to download it for you from:\n' + GITHUB_OPENSBI_URL + ' ?\n')
- response = input('yes/[no]: ')
- if response.strip() == 'yes':
- os.system('wget ' + GITHUB_OPENSBI_URL)
- else:
- sys.exit()
+if not os.path.isfile(OPENSBI_PATH):
+ print('\n\nOpenSBI bios was not found in "' + OPENSBI_PATH + '".\n'
+ 'Please ensure that qemu-system-riscv is installed, or edit the path in "qemu_configs/riscv.py"\n')
+ sys.exit()
QEMU_ARCH = QemuArchParams(linux_arch='riscv',
kconfig='''
@@ -29,4 +25,4 @@ CONFIG_SERIAL_EARLYCON_RISCV_SBI=y''',
extra_qemu_params=[
'-machine', 'virt',
'-cpu', 'rv64',
- '-bios', 'opensbi-riscv64-generic-fw_dynamic.bin'])
+ '-bios', OPENSBI_PATH])
--
2.37.3.998.g577e59143f-goog
On Thu, Sep 22, 2022 at 10:01 PM David Gow <[email protected]> wrote:
>
> When running a RISC-V test kernel under QEMU, we need an OpenSBI BIOS
> file. In the original QEMU support patchset, kunit_tool would optionally
> download this file from GitHub if it didn't exist, using wget.
>
> These days, it can usually be found in the distro's qemu-system-riscv
> package, and is located in /usr/share/qemu on all the distros I tried
Note: I actually had the BIOS file, but didn't have the
qemu-system-riscv64 binary.
> (Debian, Arch, OpenSUSE). Use this file, and thereby don't do any
> downloading in kunit_tool.
>
> In addition, we used to shell out to whatever 'wget' was in the path,
> which could have potentially been used to trick the developer into
> running another binary. By not using wget at all, we nicely sidestep
> this issue.
>
> Cc: Xu Panda <[email protected]>
> Fixes: 87c9c1631788 ("kunit: tool: add support for QEMU")
> Reported-by: Zeal Robot <[email protected]>
> Signed-off-by: David Gow <[email protected]>
Tested-by: Daniel Latypov <[email protected]>
> ---
>
> This is a replacement for "kunit: tool: use absolute path for wget":
> https://lore.kernel.org/linux-kselftest/[email protected]/
>
> Instead of just changing the path to wget, it removes the download
> option completely and grabs the opensbi-riscv64-generic-fw_dynamic.bin
> from the /usr/share/qemu directory, where the distro package manager
> should have put it.
>
> I _think_ this should be okay to treat as a fix: we were always grabbing
> this from the QEMU GitHub repository, so it should be widely available.
> And if you want to treat the wget use as a security issue, getting rid
> of it everywhere would be nice.
>
> Thoughts?
>
> -- David
>
> ---
> tools/testing/kunit/qemu_configs/riscv.py | 18 +++++++-----------
> 1 file changed, 7 insertions(+), 11 deletions(-)
>
> diff --git a/tools/testing/kunit/qemu_configs/riscv.py b/tools/testing/kunit/qemu_configs/riscv.py
> index 6207be146d26..12a1d525978a 100644
> --- a/tools/testing/kunit/qemu_configs/riscv.py
> +++ b/tools/testing/kunit/qemu_configs/riscv.py
> @@ -3,17 +3,13 @@ import os
> import os.path
> import sys
>
> -GITHUB_OPENSBI_URL = 'https://github.com/qemu/qemu/raw/master/pc-bios/opensbi-riscv64-generic-fw_dynamic.bin'
> -OPENSBI_FILE = os.path.basename(GITHUB_OPENSBI_URL)
> +OPENSBI_FILE = 'opensbi-riscv64-generic-fw_dynamic.bin'
> +OPENSBI_PATH = '/usr/share/qemu/' + OPENSBI_FILE
>
> -if not os.path.isfile(OPENSBI_FILE):
> - print('\n\nOpenSBI file is not in the current working directory.\n'
> - 'Would you like me to download it for you from:\n' + GITHUB_OPENSBI_URL + ' ?\n')
> - response = input('yes/[no]: ')
> - if response.strip() == 'yes':
> - os.system('wget ' + GITHUB_OPENSBI_URL)
> - else:
> - sys.exit()
> +if not os.path.isfile(OPENSBI_PATH):
> + print('\n\nOpenSBI bios was not found in "' + OPENSBI_PATH + '".\n'
> + 'Please ensure that qemu-system-riscv is installed, or edit the path in "qemu_configs/riscv.py"\n')
> + sys.exit()
>
> QEMU_ARCH = QemuArchParams(linux_arch='riscv',
> kconfig='''
> @@ -29,4 +25,4 @@ CONFIG_SERIAL_EARLYCON_RISCV_SBI=y''',
> extra_qemu_params=[
> '-machine', 'virt',
> '-cpu', 'rv64',
> - '-bios', 'opensbi-riscv64-generic-fw_dynamic.bin'])
> + '-bios', OPENSBI_PATH])
Note: OPENSBI_FILE also works for me (i.e. the old behavior).
But I don't care enough to ask that we change what this patch does.
My understanding is that qemu will automatically look in all of these
directories
$ qemu-system-riscv64 -L help
/usr/share/qemu
/usr/share/seabios
/usr/lib/ipxe/qemu
The first one (the one we're using here) is the only one that has the
file for me, so going with this LGTM.
The alternative is to drop the check entirely and just let QEMU figure
out if the file is missing or not, but I think this does give a
substantially nicer error message, so I think this is reasonable to
keep.
Daniel
On Fri, Sep 23, 2022 at 1:01 AM 'David Gow' via KUnit Development
<[email protected]> wrote:
>
> When running a RISC-V test kernel under QEMU, we need an OpenSBI BIOS
> file. In the original QEMU support patchset, kunit_tool would optionally
> download this file from GitHub if it didn't exist, using wget.
>
> These days, it can usually be found in the distro's qemu-system-riscv
> package, and is located in /usr/share/qemu on all the distros I tried
> (Debian, Arch, OpenSUSE). Use this file, and thereby don't do any
> downloading in kunit_tool.
>
> In addition, we used to shell out to whatever 'wget' was in the path,
> which could have potentially been used to trick the developer into
> running another binary. By not using wget at all, we nicely sidestep
> this issue.
>
> Cc: Xu Panda <[email protected]>
> Fixes: 87c9c1631788 ("kunit: tool: add support for QEMU")
> Reported-by: Zeal Robot <[email protected]>
> Signed-off-by: David Gow <[email protected]>
Reviewed-by: Brendan Higgins <[email protected]>