On Fri, 9 Jul 2021 at 06:17, David Gibson <da...@gibson.dropbear.id.au> wrote: > > From: Alexey Kardashevskiy <a...@ozlabs.ru> > > The PAPR platform describes an OS environment that's presented by > a combination of a hypervisor and firmware. The features it specifies > require collaboration between the firmware and the hypervisor.
Hi; Coverity reports issues in this code: > +static uint32_t vof_package_to_path(const void *fdt, uint32_t phandle, > + uint32_t buf, uint32_t len) > +{ > + uint32_t ret = -1; Here we declare 'ret' as an unsigned type... > + char tmp[VOF_MAX_PATH] = ""; > + > + ret = phandle_to_path(fdt, phandle, tmp, sizeof(tmp)); > + if (ret > 0) { ...so this is doing an unsigned comparison, which means that the negative values returned from phandle_to_path() (whose return type is 'int') will not be detected (viewed as unsigned values they will all be positive and >2GB). > + if (VOF_MEM_WRITE(buf, tmp, ret) != MEMTX_OK) { This then means that we will attempt to write >2GB of data here... > + ret = -1; > + } > + } > + > + trace_vof_package_to_path(phandle, tmp, ret); > + > + return ret; > +} > + > +static uint32_t vof_instance_to_path(void *fdt, Vof *vof, uint32_t ihandle, > + uint32_t buf, uint32_t len) > +{ > + uint32_t ret = -1; > + uint32_t phandle = vof_instance_to_package(vof, ihandle); > + char tmp[VOF_MAX_PATH] = ""; > + > + if (phandle != -1) { > + ret = phandle_to_path(fdt, phandle, tmp, sizeof(tmp)); > + if (ret > 0) { This function has the same problem. > + if (VOF_MEM_WRITE(buf, tmp, ret) != MEMTX_OK) { > + ret = -1; > + } > + } > + } > + trace_vof_instance_to_path(ihandle, phandle, tmp, ret); > + > + return ret; > +} thanks -- PMM