On Fri, 14 Jan 2022 09:25:53 +0100
Moritz Buhl <[email protected]> wrote:
> I thought I'd walk through my understanding of the code
> to make it easier to review.
>
> from sys/dev/ipmi.c:
>
> 1019 int
> 1020 get_sdr(struct ipmi_softc *sc, u_int16_t recid, u_int16_t *nxtrec)
> 1021 {
> ...
> 1046 psdr = malloc(sdrlen, M_DEVBUF, M_NOWAIT);
> 1047 if (psdr == NULL)
> 1048 return (1);
> Now psdr is allocated. In the following loop it is freed correctly on
> failure:
> 1062 free(psdr, M_DEVBUF, sdrlen);
> 1063 return (1);
> If everything works, we attach it to a list:
> 1067 /* Add SDR to sensor list, if not wanted, free buffer */
> 1068 if (add_sdr_sensor(sc, psdr, sdrlen) == 0)
> 1069 free(psdr, M_DEVBUF, sdrlen);
> 1070
> 1071 return (0);
> So if add_sdr_sensor doesn't return 0 it has to keep a reference
> to psdr somewhere.
>
> 1335 int
> 1336 add_sdr_sensor(struct ipmi_softc *sc, u_int8_t *psdr, int sdrlen)
> 1337 {
> There are two switch cases that both call add_child_sensors and set
> rc with it, if no previous failures occour:
> 1349 rc = add_child_sensors(sc, psdr, 1, s1->sensor_num,
> 1350 s1->sensor_type, s1->event_code, 0, s1->entity_id,
> name);
> 1351 break;
> and
> 1358 rc = add_child_sensors(sc, psdr, s2->share1 & 0xF,
> 1359 s2->sensor_num, s2->sensor_type, s2->event_code,
> 1360 s2->share2 & 0x7F, s2->entity_id, name);
> 1361 break;
> These two calls are the only locations that can make add_sdr_sensor
> return something else than 0.
>
> 1370 int
> 1371 add_child_sensors(struct ipmi_softc *sc, u_int8_t *psdr, int count,
> 1372 int sensor_num, int sensor_type, int ext_type, int sensor_base,
> 1373 int entity, const char *name)
> 1374 {
> So as mentioned before, this function needs to return 0 or keep a
> reference to psdr.
> This is not the case in two situations, but to keep it simple I
> will only show one:
> 1388 psensor = malloc(sizeof(*psensor), M_DEVBUF, M_NOWAIT |
> M_ZERO);
> 1389 if (psensor == NULL)
> 1390 break;
> ...
> 1420 return (1);
>
> I hope this helps
> mbuhl
>
Hi, llvm/scan-build agrees.
http://www.netzbasis.de/openbsd/scan-build/kernel/2022-01-12-131800-47421-1/report-c7382e.html#EndPath
Patch seems correct, but I'm not a dev and I don't have the hardware eighter.
Greetings Ben
