Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <20220922083610.235936-1-xu.panda@zte.com.cn>
In-Reply-To: <20220922083610.235936-1-xu.panda@zte.com.cn>
From:   David Gow <davidgow@google.com>
Date:   Thu, 22 Sep 2022 18:09:28 +0800
Message-ID: <CABVgOSkjmuWDVwOW5PQRAtDYJGjHjcc_8Gg4JjhiT41Kez1rcw@mail.gmail.com>
Subject: Re: [PATCH linux-next] kunit: tool: use absolute path for wget
To:     cgel.zte@gmail.com
Cc:     Brendan Higgins <brendan.higgins@linux.dev>,
        paul.walmsley@sifive.com, palmer@dabbelt.com,
        aou@eecs.berkeley.edu, Shuah Khan <skhan@linuxfoundation.org>,
        Daniel Latypov <dlatypov@google.com>,
        "open list:KERNEL SELFTEST FRAMEWORK" 
        <linux-kselftest@vger.kernel.org>,
        KUnit Development <kunit-dev@googlegroups.com>,
        linux-riscv <linux-riscv@lists.infradead.org>,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
        Xu Panda <xu.panda@zte.com.cn>, Zeal Robot <zealci@zte.com.cn>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Thu, Sep 22, 2022 at 4:36 PM <cgel.zte@gmail.com> wrote:
>
> From: Xu Panda <xu.panda@zte.com.cn>
>
> Not using absolute path when invoking wget can lead to serious
> security issues.
>
> Reported-by: Zeal Robot <zealci@zte.com.cn>
> Signed-off-by: Xu Panda <xu.panda@zte.com.cn>
> ---

This seems mostly okay to me -- we'd be abandoning people who have
wget in an unusual location, but I don't think there are many people
who want to run KUnit under RISC-V, have wget in a non-standard
location, and can't acquire the bios file themselves.

So this is:
Reviewed-by: David Gow <davidgow@google.com>

However, would a patch like this make _more_ sense? It looks like (at
least on Debian and Arch), the OpenSBI bios is installed as part of
the appropriate qemu package anyway, into a standard location.
---
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])
---

That way, we could avoid using wget at all. (I did confirm that this
is the only use of it anywhere in kunit_tool.)

The other options would be to use some python library to download it?

Thoughts?
-- David

>  tools/testing/kunit/qemu_configs/riscv.py | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/tools/testing/kunit/qemu_configs/riscv.py b/tools/testing/kunit/qemu_configs/riscv.py
> index 6207be146d26..c3dcd654ca15 100644
> --- a/tools/testing/kunit/qemu_configs/riscv.py
> +++ b/tools/testing/kunit/qemu_configs/riscv.py
> @@ -11,7 +11,7 @@ if not os.path.isfile(OPENSBI_FILE):
>               '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)
> +               os.system('/usr/bin/wget ' + GITHUB_OPENSBI_URL)
>         else:
>                 sys.exit()
>
> --
> 2.15.2
>
> --
> You received this message because you are subscribed to the Google Groups "KUnit Development" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to kunit-dev+unsubscribe@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/kunit-dev/20220922083610.235936-1-xu.panda%40zte.com.cn.