* [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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).