* [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
* 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
* [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 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).