qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Greg Kurz <groug@kaod.org>
To: Peter Maydell <peter.maydell@linaro.org>
Cc: qemu-ppc <qemu-ppc@nongnu.org>,
	QEMU Developers <qemu-devel@nongnu.org>,
	David Gibson <david@gibson.dropbear.id.au>
Subject: Re: [PULL 06/30] spapr: Do PHB hoplug sanity check at pre-plug
Date: Tue, 15 Dec 2020 18:31:36 +0100	[thread overview]
Message-ID: <20201215183136.0f58cb35@bahia.lan> (raw)
In-Reply-To: <CAFEAcA8sOMxFJN=t69y=oSruh4a+LMUNWHX60wOEZZ8Cr=64OQ@mail.gmail.com>

On Tue, 15 Dec 2020 16:56:36 +0000
Peter Maydell <peter.maydell@linaro.org> wrote:

> On Mon, 14 Dec 2020 at 04:58, David Gibson <david@gibson.dropbear.id.au> wrote:
> >
> > From: Greg Kurz <groug@kaod.org>
> >
> > We currently detect that a PHB index is already in use at plug time.
> > But this can be decteted at pre-plug in order to error out earlier.
> >
> > This allows to pass &error_abort to spapr_drc_attach() and to end
> > up with a plug handler that doesn't need to report errors anymore.
> >
> > Signed-off-by: Greg Kurz <groug@kaod.org>
> > Message-Id: <20201120234208.683521-8-groug@kaod.org>
> > Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> 
> Hi; this change seems to have nudged one of Coverity's
> heuristics into deciding that spapr_drc_by_id() can return
> NULL (because its return value is checked here, I suspect),
> so it reports CID 1437757, 1437758, where spapr_add_lmbs()
> and spapr_memory_unplug_request() both take the return value
> of spapr_drc_by_id() and pass it directly to spapr_drc_index(),
> which will crash if it is passed a NULL pointer.
> 
> Is it impossible for spapr_drc_by_id() to return NULL in
> those functions (ie Coverity false positive) or is there
> a missing error check ?
> 

No, all DRC objects are created before any of these two
functions are called. Each function happens to loop over
the full list of memory DRCs a few line above the offending
call sites and already assert spapr_drc_by_id() doesn't
return NULL. But I guess Coverity isn't smart enough to
detect that.

I'll post a patch to add some more assertions.

> thanks
> -- PMM



  reply	other threads:[~2020-12-15 17:35 UTC|newest]

Thread overview: 35+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-12-14  4:57 [PULL 00/30] ppc-for-6.0 queue 20201214 David Gibson
2020-12-14  4:57 ` [PULL 01/30] spapr/xive: Turn some sanity checks into assertions David Gibson
2020-12-14  4:57 ` [PULL 02/30] spapr/xics: Drop unused argument to xics_kvm_has_broken_disconnect() David Gibson
2020-12-14  4:57 ` [PULL 03/30] spapr: Do PCI device hotplug sanity checks at pre-plug only David Gibson
2020-12-14  4:57 ` [PULL 04/30] spapr: Do NVDIMM/PC-DIMM " David Gibson
2020-12-14  4:57 ` [PULL 05/30] spapr: Make PHB placement functions and spapr_pre_plug_phb() return status David Gibson
2020-12-14  4:57 ` [PULL 06/30] spapr: Do PHB hoplug sanity check at pre-plug David Gibson
2020-12-15 16:56   ` Peter Maydell
2020-12-15 17:31     ` Greg Kurz [this message]
2020-12-14  4:57 ` [PULL 07/30] spapr: Do TPM proxy hotplug sanity checks " David Gibson
2020-12-14  4:57 ` [PULL 08/30] target/ppc: replaced the TODO with LOG_UNIMP and add break for silence warnings David Gibson
2020-12-14  4:57 ` [PULL 09/30] ppc: Add a missing break for PPC6xx_INPUT_TBEN David Gibson
2020-12-14  4:57 ` [PULL 10/30] ppc/translate: Fix unordered f64/f128 comparisons David Gibson
2020-12-14  4:57 ` [PULL 11/30] ppc/translate: Turn the helper macros into functions David Gibson
2020-12-14  4:57 ` [PULL 12/30] ppc/translate: Delay NaN checking after comparison David Gibson
2020-12-14  4:57 ` [PULL 13/30] ppc/translate: Raise exceptions after setting the cc David Gibson
2020-12-14  4:57 ` [PULL 14/30] ppc/translate: Rewrite gen_lxvdsx to use gvec primitives David Gibson
2020-12-14  4:57 ` [PULL 15/30] hw/ppc/spapr_tpm_proxy: Fix hexadecimal format string specifier David Gibson
2020-12-14  4:57 ` [PULL 16/30] xive: Add trace events David Gibson
2020-12-14  4:57 ` [PULL 17/30] spapr: Fix pre-2.10 dummy ICP hack David Gibson
2020-12-14  4:57 ` [PULL 18/30] spapr: Abort if ppc_set_compat() fails for hot-plugged CPUs David Gibson
2020-12-14  4:57 ` [PULL 19/30] spapr: Simplify error path of spapr_core_plug() David Gibson
2020-12-14  4:57 ` [PULL 20/30] spapr: spapr_drc_attach() cannot fail David Gibson
2020-12-14  4:57 ` [PULL 21/30] target/ppc: Remove "compat" property of server class POWER CPUs David Gibson
2020-12-14  4:57 ` [PULL 22/30] hw/ppc: Do not re-read the clock on pre_save if doing savevm David Gibson
2020-12-14  4:58 ` [PULL 23/30] MAINTAINERS: Add Greg Kurz as co-maintainer for ppc David Gibson
2020-12-14  4:58 ` [PULL 24/30] ppc/e500: Free irqs array to avoid memleak David Gibson
2020-12-14  4:58 ` [PULL 25/30] ppc/translate: Use POWERPC_MMU_64 to detect 64-bit MMU models David Gibson
2020-12-14  4:58 ` [PULL 26/30] target/ppc: Introduce an mmu_is_64bit() helper David Gibson
2020-12-14  4:58 ` [PULL 27/30] spapr: Pass sPAPR machine state down to spapr_pci_switch_vga() David Gibson
2020-12-14  4:58 ` [PULL 28/30] spapr: Don't use qdev_get_machine() in spapr_msi_write() David Gibson
2020-12-14  4:58 ` [PULL 29/30] spapr: Pass sPAPR machine state to some RTAS events handling functions David Gibson
2020-12-14  4:58 ` [PULL 30/30] spapr.c: set a 'kvm-type' default value instead of relying on NULL David Gibson
2020-12-14 18:53 ` [PULL 00/30] ppc-for-6.0 queue 20201214 Peter Maydell
  -- strict thread matches above, loose matches on Subject: below --
2020-12-11  4:14 [PULL 00/30] ppc-for-6.0 queue 20201211 David Gibson
2020-12-11  4:14 ` [PULL 06/30] spapr: Do PHB hoplug sanity check at pre-plug David Gibson

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20201215183136.0f58cb35@bahia.lan \
    --to=groug@kaod.org \
    --cc=david@gibson.dropbear.id.au \
    --cc=peter.maydell@linaro.org \
    --cc=qemu-devel@nongnu.org \
    --cc=qemu-ppc@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).