Project

General

Profile

Actions

Bug #12637

closed

ses_facility.c topo methods are not properly terminated

Added by Robert Mustacchi about 1 year ago. Updated 11 months ago.

Status:
Closed
Priority:
Normal
Category:
lib - userland libraries
Start date:
Due date:
% Done:

100%

Estimated time:
Difficulty:
Medium
Tags:
Gerrit CR:

Description

When a series of FMA topo methods are registered, the resulting array must be null terminated. After auditing the source tree, I have found two cases of this right now: ses_indicator_methods and ses_sensor_methods. Tracking down the history here, it appears that this occurred in the initial integration of these changes back in:

commit d91236fe104c7ea63142e053b22a39c8a30d304b
Author: eschrock <none@none>
Date:   Fri Aug 1 18:36:31 2008 -0700

    PSARC 2008/485 SES Sensors and Enumerator
    6720433 SES enumerator should provide controller revision information
    6720435 SES enumerator should prefer description over class-description
    6720452 SES enumerator should support indicators and sensors
    6722807 SES enumerator should work with internal enclosures
    6722809 want a way to identify enclosures as internal
    6722811 SES enumerator should prefer elements with known status
    6723603 x86 xmlgen topo scripts should make use of propmap
    6732875 typo in fan-hc-topology.xmlgen
    6732879 broken logic in pad_process()

I've looked a bit into smatch to try and catch this, but it appears that upstream is working on similar support right now so we'll wait and come back to that later. For the time being, the best we can do is a manual audit to make sure that nothing else makes it through there.

Actions #1

Updated by Electric Monk about 1 year ago

  • Gerrit CR set to 636
Actions #2

Updated by Robert Mustacchi 11 months ago

Unfortunately, folks with the hardware who were in a best position to test weren't able to get that done. However, we know that the code is incorrect because fma iterates a null terminated array and we believe we've historically been getting lucky. We know from topo_method_register(), that it walks looking for a NULL tm_name:

134 int
135 topo_method_register(topo_mod_t *mod, tnode_t *node, const topo_method_t *mp)
136 {
137         topo_imethod_t *imp;
138         const topo_method_t *meth;
139 
140         /*
141          * Initialize module methods
142          */
143         for (meth = &mp[0]; meth->tm_name != NULL; meth++) {
144 
Actions #3

Updated by Electric Monk 11 months ago

  • Status changed from New to Closed
  • % Done changed from 90 to 100

git commit ab26215b1a80ead55969e925a597044ad4185a34

commit  ab26215b1a80ead55969e925a597044ad4185a34
Author: Robert Mustacchi <rm@fingolfin.org>
Date:   2020-08-30T19:10:53.000Z

    12637 ses_facility.c topo methods are not properly terminated
    Reviewed by: John Levon <levon@movementarian.org>
    Approved by: Richard Lowe <richlowe@richlowe.net>

Actions

Also available in: Atom PDF