Skip site navigation (1)Skip section navigation (2)
Date:      Thu, 15 Jun 2023 18:41:33 +0000
From:      bugzilla-noreply@freebsd.org
To:        bugs@FreeBSD.org
Subject:   [Bug 272018] pci_read_vpd() does not always load VPD even when valid VPD exists because of state machine bug
Message-ID:  <bug-272018-227-vsedqOtZLc@https.bugs.freebsd.org/bugzilla/>
In-Reply-To: <bug-272018-227@https.bugs.freebsd.org/bugzilla/>
References:  <bug-272018-227@https.bugs.freebsd.org/bugzilla/>

next in thread | previous in thread | raw e-mail | index | archive | help
https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=3D272018

--- Comment #1 from Dave Baukus <daveb@spectralogic.com> ---
The state machine to read a device's VPD in pci_vpd_read() has this stateme=
nt
at the end of the top level switch() statement:

if (cfg->vpd.vpd_ident =3D=3D NULL || cfg->vpd.vpd_ident[0] =3D=3D '\0') {
       pci_printf(cfg, "no valid vpd ident found\n");
       state =3D -2;
}

The issue is the cfg->vpd.vpd_ident[0] =3D=3D '\0' check.

On the first iteration with state =3D=3D 0, case 0: if a "name" of 2 is dis=
covered
(name =3D (byte >> 3) & 0xf) then the code falls into a sub-switch() statem=
ent
where if everything else is valid then vpd_ident is allocated:

switch(name)
case 0x2:
    ...
    ...
    ...
    cfg->vpd.vpd_ident =3D malloc(remain + 1, M_DEVBUF, M_WAITOK);
    i =3D 0;
    state =3D 1;
    break;

Now when the code falls out of the top level switch() the above check trigg=
ers.
The code has not assigned anything to cfg->vpd.vpd_ident[0]; it is therefore
random luck weather or not a device with valid VPD gets its VPD cached.

My quick hack: assign a characther not '\0' to cfg->vpd.vpd_ident[0] after =
it
is allocated. It will be overwritten in case 1 of the top level switch() on=
 the
next iteration; I'm sure there's a more elegant solution.

--=20
You are receiving this mail because:
You are the assignee for the bug.=



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?bug-272018-227-vsedqOtZLc>