[PATCH] pci: Handle failed calloc in decode_regions()

[Christian Gmeiner]

Hi

>
> Hi,
>
> On Tue, Mar 21, 2023 at 08:57:18AM +0100, Christian Gmeiner wrote:
> > Am So., 4. Dez. 2022 um 22:22 Uhr schrieb Pierre-Clément Tosi
> > <ptosi at google.com>:
> > >
> > > Hi,
> > >
> > > On Fri, Dec 02, 2022 at 08:38:37PM +0100, sean at geanix.com wrote:
> > > >
> > > > Quoting Pierre-Clément Tosi <ptosi at google.com>:
> > > >
> > > > > Add a check for calloc() failing to allocate the requested memory.
> > > > >
> > > > > Make decode_regions() return an error code.
> > > > >
> > > > > Cc: Bin Meng <bmeng.cn at gmail.com>
> > > > > Cc: Simon Glass <sjg at chromium.org>
> > > > > Cc: Stefan Roese <sr at denx.de>
> > > > > Signed-off-by: Pierre-Clément Tosi <ptosi at google.com>
> > > > > ---
> > > >
> > > > Hi,
> > > >
> > > > We upgraded from v2022.04 -> v2022.10, and this PATCH breaks PCI (scsi/sata)
> > > > support for x86_64.
> > > > I have seen this in qemu, i havn't had the time to test this on real hardware.
> > > >
> > > > Steps to reproduce:
> > > > $ make efi-x86_payload64_defconfig && make -j32 && scripts/build-efi.sh -sPrp
> > >
> >
> > Breaks my use case too and is basically a revert of
> > https://source.denx.de/u-boot/u-boot/-/commit/f2825f6ec0bb50e7bd9376828a32212f1961f979
> > In my use case we are using coreboot for different Intel SoCs with a
> > generic U-Boot payload.
> >
> > > Decompiling the resulting u-boot.dtb shows
> > >
> > >     pci {
> > >             compatible = "pci-x86";
> > >             u-boot,dm-pre-reloc;
> > >     };
> > >
> >
> > Yes.. that is what can be found here:
> > https://source.denx.de/u-boot/u-boot/-/blob/master/arch/x86/dts/coreboot.dts#L34
> >
> >
> > > which isn't supported by the patch as we return -EINVAL when missing "ranges".
> > > The rationale here was that the property seemed mandatory (see [1] and [2]); was
> > > this a misunderstanding of potential corner cases?
> > >
> >
> > At the moment I would like to revert this change.
> >
>
> Thanks for sharing such a corner case.
>
> IMO, normal operation shouldn't rely on errors being silently skipped because
> return values are being blindly ignored. Instead, we could limit EINVAL to
> libfdt errors that aren't FDT_ERR_NOTFOUND, which would address your use-case,
> where "ranges" is missing from the DT node.
>
> Is your issue fixed by this patch:
>
> https://patchwork.ozlabs.org/project/uboot/patch/20230220194927.476708-8-sjg@chromium.org/
>

Yes .. the regression is fixed with this patch \o/

-- 
greets
--
Christian Gmeiner, MSc

https://christian-gmeiner.info/privacypolicy