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; the latest version of Coverity has pointed out another bug
in this commit (CID 1487241):
> +static uint32_t vof_setprop(MachineState *ms, void *fdt, Vof *vof,
> + uint32_t nodeph, uint32_t pname,
> + uint32_t valaddr, uint32_t vallen)
> +{
> + char propname[OF_PROPNAME_LEN_MAX + 1];
We don't initialize this array...
> + uint32_t ret = -1;
> + int offset;
> + char trval[64] = "";
> + char nodepath[VOF_MAX_PATH] = "";
> + Object *vmo = object_dynamic_cast(OBJECT(ms), TYPE_VOF_MACHINE_IF);
> + g_autofree char *val = NULL;
> +
> + if (vallen > VOF_MAX_SETPROPLEN) {
> + goto trace_exit;
...and this error-exit check happens before the readstr() that
fills in the propname[] array...
> + }
> + if (readstr(pname, propname, sizeof(propname))) {
> + goto trace_exit;
...and if the readstr() fails then propname[] isn't set either...
> + }
> +trace_exit:
> + trace_vof_setprop(nodeph, propname, trval, vallen, ret);
...but on the 'trace_exit' error path we still try to trace
propname, which will use the %s format string on it, printing
a potentially arbitrary amount of garbage.
We should either initialize the propname array or else
not trace it for these error paths.
thanks
-- PMM
