[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PULL 06/30] spapr: Do PHB hoplug sanity check at pre-plug
From: |
Greg Kurz |
Subject: |
Re: [PULL 06/30] spapr: Do PHB hoplug sanity check at pre-plug |
Date: |
Tue, 15 Dec 2020 18:31:36 +0100 |
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
- [PULL 00/30] ppc-for-6.0 queue 20201214, David Gibson, 2020/12/13
- [PULL 05/30] spapr: Make PHB placement functions and spapr_pre_plug_phb() return status, David Gibson, 2020/12/13
- [PULL 01/30] spapr/xive: Turn some sanity checks into assertions, David Gibson, 2020/12/13
- [PULL 02/30] spapr/xics: Drop unused argument to xics_kvm_has_broken_disconnect(), David Gibson, 2020/12/13
- [PULL 08/30] target/ppc: replaced the TODO with LOG_UNIMP and add break for silence warnings, David Gibson, 2020/12/13
- [PULL 06/30] spapr: Do PHB hoplug sanity check at pre-plug, David Gibson, 2020/12/13
- [PULL 03/30] spapr: Do PCI device hotplug sanity checks at pre-plug only, David Gibson, 2020/12/13
- [PULL 07/30] spapr: Do TPM proxy hotplug sanity checks at pre-plug, David Gibson, 2020/12/13
- [PULL 10/30] ppc/translate: Fix unordered f64/f128 comparisons, David Gibson, 2020/12/13
- [PULL 12/30] ppc/translate: Delay NaN checking after comparison, David Gibson, 2020/12/13
- [PULL 04/30] spapr: Do NVDIMM/PC-DIMM device hotplug sanity checks at pre-plug only, David Gibson, 2020/12/13
- [PULL 09/30] ppc: Add a missing break for PPC6xx_INPUT_TBEN, David Gibson, 2020/12/13
- [PULL 13/30] ppc/translate: Raise exceptions after setting the cc, David Gibson, 2020/12/13
- [PULL 18/30] spapr: Abort if ppc_set_compat() fails for hot-plugged CPUs, David Gibson, 2020/12/13
- [PULL 19/30] spapr: Simplify error path of spapr_core_plug(), David Gibson, 2020/12/13
- [PULL 14/30] ppc/translate: Rewrite gen_lxvdsx to use gvec primitives, David Gibson, 2020/12/13