Attention is currently required from: Edward O'Callaghan, Anastasia Klimchuk.

View Change

1 comment:

• File dummyflasher.c:

  • Patch Set #2, Line 1416:

    data->refs_cnt += ((dummy_buses_supported & BUS_PROG) != BUS_NONE);
    	data->refs_cnt += ((dummy_buses_supported & BUS_NONSPI) != BUS_NONE);
    	data->refs_cnt += ((dummy_buses_supported & BUS_SPI) != BUS_NONE);
    
    #if 0
    	data->refs_cnt += (dummy_buses_supported & BUS_PROG) ? 1 : 0;
    	data->refs_cnt += (dummy_buses_supported & BUS_NONSPI) ? 1 : 0;
    	data->refs_cnt += (dummy_buses_supported & BUS_SPI) ? 1 : 0;
    #endif
    

    I have another idea! :) What if you add `refs_cnt++` into every if below. […]

    I thought about it at first sight. But this approach may potentially cause flashrom to crash.

    If the dummy programmer registers more than one bus, then the next accident can happen => If the first condition the programmer enters fails because `register_*_master()` could not register shutdown (`register_shutdown()`), then it will call shutdown immediately (`mst->shutdown(data)`, opaque.c:55). And since we only have one owner in refs_cnt at that moment, data will be released.

    When the dummy tries to enter the second condition and increase `data->refs_cnt`, flashrom will crash because this memory isn't valid anymore.

    This should be mentioned in the code, but I cannot come up with brief comment for that.

To view, visit change 72408. To unsubscribe, or for help writing mail filters, visit settings.