* [PATCH v2 0/2] elf2dmp: Fix minor Coverity nits
@ 2021-09-10 17:06 Philippe Mathieu-Daudé
2021-09-10 17:06 ` [PATCH v2 1/2] elf2dmp: Check curl_easy_setopt() return value Philippe Mathieu-Daudé
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-09-10 17:06 UTC (permalink / raw)
To: qemu-devel; +Cc: Philippe Mathieu-Daudé, Viktor Prutyanov
This is a respin of Peter Maydell series, with slightly different
logic on the first patch. Quoting v1 cover [*]:
Coverity complains about a couple of minor issues in elf2dmp:
* we weren't checking the return value from curl_easy_setopt()
* we might divide by zero if presented with a corrupt PDB file
This patchseries fixes those.
Viktor Prutyanov tested the patch but didn't provided a formal
Tested-by tag, so I haven't included it in the patches.
[*] https://lore.kernel.org/qemu-devel/20210901143910.17112-1-peter.maydell@linaro.org/
Peter Maydell (2):
elf2dmp: Check curl_easy_setopt() return value
elf2dmp: Fail cleanly if PDB file specifies zero block_size
contrib/elf2dmp/download.c | 22 ++++++++++------------
contrib/elf2dmp/pdb.c | 4 ++++
2 files changed, 14 insertions(+), 12 deletions(-)
--
2.31.1
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2 1/2] elf2dmp: Check curl_easy_setopt() return value
2021-09-10 17:06 [PATCH v2 0/2] elf2dmp: Fix minor Coverity nits Philippe Mathieu-Daudé
@ 2021-09-10 17:06 ` Philippe Mathieu-Daudé
2021-09-10 17:17 ` Viktor Prutyanov
2021-09-10 17:06 ` [PATCH v2 2/2] elf2dmp: Fail cleanly if PDB file specifies zero block_size Philippe Mathieu-Daudé
2021-09-16 9:58 ` [PATCH v2 0/2] elf2dmp: Fix minor Coverity nits Peter Maydell
2 siblings, 1 reply; 6+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-09-10 17:06 UTC (permalink / raw)
To: qemu-devel; +Cc: Peter Maydell, Philippe Mathieu-Daudé, Viktor Prutyanov
From: Peter Maydell <peter.maydell@linaro.org>
Coverity points out that we aren't checking the return value
from curl_easy_setopt().
Fixes: Coverity CID 1458895
Inspired-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
Informal T-b tag on
https://lore.kernel.org/qemu-devel/20210909004313.1dadb24e@192.168.1.7/
Tested-by: Viktor Prutyanov <viktor.prutyanov@phystech.edu>
v1 from Peter:
https://lore.kernel.org/qemu-devel/20210901143910.17112-2-peter.maydell@linaro.org/
---
contrib/elf2dmp/download.c | 22 ++++++++++------------
1 file changed, 10 insertions(+), 12 deletions(-)
diff --git a/contrib/elf2dmp/download.c b/contrib/elf2dmp/download.c
index d09e607431f..bd7650a7a27 100644
--- a/contrib/elf2dmp/download.c
+++ b/contrib/elf2dmp/download.c
@@ -25,21 +25,19 @@ int download_url(const char *name, const char *url)
goto out_curl;
}
- curl_easy_setopt(curl, CURLOPT_URL, url);
- curl_easy_setopt(curl, CURLOPT_WRITEFUNCTION, NULL);
- curl_easy_setopt(curl, CURLOPT_WRITEDATA, file);
- curl_easy_setopt(curl, CURLOPT_FOLLOWLOCATION, 1);
- curl_easy_setopt(curl, CURLOPT_NOPROGRESS, 0);
-
- if (curl_easy_perform(curl) != CURLE_OK) {
- err = 1;
- fclose(file);
+ if (curl_easy_setopt(curl, CURLOPT_URL, url) != CURLE_OK
+ || curl_easy_setopt(curl, CURLOPT_WRITEFUNCTION, NULL) != CURLE_OK
+ || curl_easy_setopt(curl, CURLOPT_WRITEDATA, file) != CURLE_OK
+ || curl_easy_setopt(curl, CURLOPT_FOLLOWLOCATION, 1) != CURLE_OK
+ || curl_easy_setopt(curl, CURLOPT_NOPROGRESS, 0) != CURLE_OK
+ || curl_easy_perform(curl) != CURLE_OK) {
unlink(name);
- goto out_curl;
+ fclose(file);
+ err = 1;
+ } else {
+ err = fclose(file);
}
- err = fclose(file);
-
out_curl:
curl_easy_cleanup(curl);
--
2.31.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH v2 2/2] elf2dmp: Fail cleanly if PDB file specifies zero block_size
2021-09-10 17:06 [PATCH v2 0/2] elf2dmp: Fix minor Coverity nits Philippe Mathieu-Daudé
2021-09-10 17:06 ` [PATCH v2 1/2] elf2dmp: Check curl_easy_setopt() return value Philippe Mathieu-Daudé
@ 2021-09-10 17:06 ` Philippe Mathieu-Daudé
2021-09-10 17:13 ` Viktor Prutyanov
2021-09-16 9:58 ` [PATCH v2 0/2] elf2dmp: Fix minor Coverity nits Peter Maydell
2 siblings, 1 reply; 6+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-09-10 17:06 UTC (permalink / raw)
To: qemu-devel; +Cc: Peter Maydell, Philippe Mathieu-Daudé, Viktor Prutyanov
From: Peter Maydell <peter.maydell@linaro.org>
Coverity points out that if the PDB file we're trying to read
has a header specifying a block_size of zero then we will
end up trying to divide by zero in pdb_ds_read_file().
Check for this and fail cleanly instead.
Fixes: Coverity CID 1458869
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Viktor Prutyanov <viktor.prutyanov@phystech.edu>
Message-Id: <20210901143910.17112-3-peter.maydell@linaro.org>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
Informal T-b tag on
https://lore.kernel.org/qemu-devel/20210909004313.1dadb24e@192.168.1.7/
Tested-by: Viktor Prutyanov <viktor.prutyanov@phystech.edu>
---
contrib/elf2dmp/pdb.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/contrib/elf2dmp/pdb.c b/contrib/elf2dmp/pdb.c
index b3a65470680..adcfa7e154c 100644
--- a/contrib/elf2dmp/pdb.c
+++ b/contrib/elf2dmp/pdb.c
@@ -215,6 +215,10 @@ out_symbols:
static int pdb_reader_ds_init(struct pdb_reader *r, PDB_DS_HEADER *hdr)
{
+ if (hdr->block_size == 0) {
+ return 1;
+ }
+
memset(r->file_used, 0, sizeof(r->file_used));
r->ds.header = hdr;
r->ds.toc = pdb_ds_read(hdr, (uint32_t *)((uint8_t *)hdr +
--
2.31.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v2 2/2] elf2dmp: Fail cleanly if PDB file specifies zero block_size
2021-09-10 17:06 ` [PATCH v2 2/2] elf2dmp: Fail cleanly if PDB file specifies zero block_size Philippe Mathieu-Daudé
@ 2021-09-10 17:13 ` Viktor Prutyanov
0 siblings, 0 replies; 6+ messages in thread
From: Viktor Prutyanov @ 2021-09-10 17:13 UTC (permalink / raw)
To: Philippe Mathieu-Daudé; +Cc: Peter Maydell, qemu-devel
Hi,
On Fri, 10 Sep 2021 19:06:56 +0200
Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
> From: Peter Maydell <peter.maydell@linaro.org>
>
> Coverity points out that if the PDB file we're trying to read
> has a header specifying a block_size of zero then we will
> end up trying to divide by zero in pdb_ds_read_file().
> Check for this and fail cleanly instead.
>
> Fixes: Coverity CID 1458869
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> Reviewed-by: Viktor Prutyanov <viktor.prutyanov@phystech.edu>
> Message-Id: <20210901143910.17112-3-peter.maydell@linaro.org>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
> Informal T-b tag on
> https://lore.kernel.org/qemu-devel/20210909004313.1dadb24e@192.168.1.7/
> Tested-by: Viktor Prutyanov <viktor.prutyanov@phystech.edu>
> ---
> contrib/elf2dmp/pdb.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/contrib/elf2dmp/pdb.c b/contrib/elf2dmp/pdb.c
> index b3a65470680..adcfa7e154c 100644
> --- a/contrib/elf2dmp/pdb.c
> +++ b/contrib/elf2dmp/pdb.c
> @@ -215,6 +215,10 @@ out_symbols:
>
> static int pdb_reader_ds_init(struct pdb_reader *r, PDB_DS_HEADER
> *hdr) {
> + if (hdr->block_size == 0) {
> + return 1;
> + }
> +
> memset(r->file_used, 0, sizeof(r->file_used));
> r->ds.header = hdr;
> r->ds.toc = pdb_ds_read(hdr, (uint32_t *)((uint8_t *)hdr +
Tested-by: Viktor Prutyanov <viktor.prutyanov@phystech.edu>
--
Viktor Prutyanov
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2 1/2] elf2dmp: Check curl_easy_setopt() return value
2021-09-10 17:06 ` [PATCH v2 1/2] elf2dmp: Check curl_easy_setopt() return value Philippe Mathieu-Daudé
@ 2021-09-10 17:17 ` Viktor Prutyanov
0 siblings, 0 replies; 6+ messages in thread
From: Viktor Prutyanov @ 2021-09-10 17:17 UTC (permalink / raw)
To: Philippe Mathieu-Daudé; +Cc: Peter Maydell, qemu-devel
Hi,
On Fri, 10 Sep 2021 19:06:55 +0200
Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
> From: Peter Maydell <peter.maydell@linaro.org>
>
> Coverity points out that we aren't checking the return value
> from curl_easy_setopt().
>
> Fixes: Coverity CID 1458895
> Inspired-by: Peter Maydell <peter.maydell@linaro.org>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
> Informal T-b tag on
> https://lore.kernel.org/qemu-devel/20210909004313.1dadb24e@192.168.1.7/
> Tested-by: Viktor Prutyanov <viktor.prutyanov@phystech.edu>
>
> v1 from Peter:
> https://lore.kernel.org/qemu-devel/20210901143910.17112-2-peter.maydell@linaro.org/
> ---
> contrib/elf2dmp/download.c | 22 ++++++++++------------
> 1 file changed, 10 insertions(+), 12 deletions(-)
>
> diff --git a/contrib/elf2dmp/download.c b/contrib/elf2dmp/download.c
> index d09e607431f..bd7650a7a27 100644
> --- a/contrib/elf2dmp/download.c
> +++ b/contrib/elf2dmp/download.c
> @@ -25,21 +25,19 @@ int download_url(const char *name, const char
> *url) goto out_curl;
> }
>
> - curl_easy_setopt(curl, CURLOPT_URL, url);
> - curl_easy_setopt(curl, CURLOPT_WRITEFUNCTION, NULL);
> - curl_easy_setopt(curl, CURLOPT_WRITEDATA, file);
> - curl_easy_setopt(curl, CURLOPT_FOLLOWLOCATION, 1);
> - curl_easy_setopt(curl, CURLOPT_NOPROGRESS, 0);
> -
> - if (curl_easy_perform(curl) != CURLE_OK) {
> - err = 1;
> - fclose(file);
> + if (curl_easy_setopt(curl, CURLOPT_URL, url) != CURLE_OK
> + || curl_easy_setopt(curl, CURLOPT_WRITEFUNCTION, NULL)
> != CURLE_OK
> + || curl_easy_setopt(curl, CURLOPT_WRITEDATA, file) !=
> CURLE_OK
> + || curl_easy_setopt(curl, CURLOPT_FOLLOWLOCATION, 1) !=
> CURLE_OK
> + || curl_easy_setopt(curl, CURLOPT_NOPROGRESS, 0) !=
> CURLE_OK
> + || curl_easy_perform(curl) != CURLE_OK) {
> unlink(name);
> - goto out_curl;
> + fclose(file);
> + err = 1;
> + } else {
> + err = fclose(file);
> }
>
> - err = fclose(file);
> -
> out_curl:
> curl_easy_cleanup(curl);
>
Reviewed-by: Viktor Prutyanov <viktor.prutyanov@phystech.edu>
Tested-by: Viktor Prutyanov <viktor.prutyanov@phystech.edu>
--
Viktor Prutyanov
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2 0/2] elf2dmp: Fix minor Coverity nits
2021-09-10 17:06 [PATCH v2 0/2] elf2dmp: Fix minor Coverity nits Philippe Mathieu-Daudé
2021-09-10 17:06 ` [PATCH v2 1/2] elf2dmp: Check curl_easy_setopt() return value Philippe Mathieu-Daudé
2021-09-10 17:06 ` [PATCH v2 2/2] elf2dmp: Fail cleanly if PDB file specifies zero block_size Philippe Mathieu-Daudé
@ 2021-09-16 9:58 ` Peter Maydell
2 siblings, 0 replies; 6+ messages in thread
From: Peter Maydell @ 2021-09-16 9:58 UTC (permalink / raw)
To: Philippe Mathieu-Daudé; +Cc: QEMU Developers, Viktor Prutyanov
On Fri, 10 Sept 2021 at 18:08, Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
>
> This is a respin of Peter Maydell series, with slightly different
> logic on the first patch. Quoting v1 cover [*]:
>
> Coverity complains about a couple of minor issues in elf2dmp:
> * we weren't checking the return value from curl_easy_setopt()
> * we might divide by zero if presented with a corrupt PDB file
>
> This patchseries fixes those.
>
> Viktor Prutyanov tested the patch but didn't provided a formal
> Tested-by tag, so I haven't included it in the patches.
>
> [*] https://lore.kernel.org/qemu-devel/20210901143910.17112-1-peter.maydell@linaro.org/
>
> Peter Maydell (2):
> elf2dmp: Check curl_easy_setopt() return value
> elf2dmp: Fail cleanly if PDB file specifies zero block_size
Thanks for doing the respin; I'll take this via target-arm.next.
-- PMM
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2021-09-16 10:02 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-10 17:06 [PATCH v2 0/2] elf2dmp: Fix minor Coverity nits Philippe Mathieu-Daudé
2021-09-10 17:06 ` [PATCH v2 1/2] elf2dmp: Check curl_easy_setopt() return value Philippe Mathieu-Daudé
2021-09-10 17:17 ` Viktor Prutyanov
2021-09-10 17:06 ` [PATCH v2 2/2] elf2dmp: Fail cleanly if PDB file specifies zero block_size Philippe Mathieu-Daudé
2021-09-10 17:13 ` Viktor Prutyanov
2021-09-16 9:58 ` [PATCH v2 0/2] elf2dmp: Fix minor Coverity nits Peter Maydell
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.