search

epixoip / od6config

Configuration tool for AMD Overdrive6 devices.
19 stars 5 forks source link

ADLODPerformanceLevel array subscript is above array bounds #7

Closed philsmd closed 8 years ago

philsmd commented 8 years ago

While compiling oclHashcat on a system with (seemingly) a new(er) gcc version I've seen (for the first time) this compiler warning about ADLODPerformanceLevels.aLevels[x]:

array subscript is above array bounds [-Warray-bounds]

After discussing this a little bit w/ @epixoip, I've opened a new issue on oclHashcat's github repo.

This issue does for sure also affect od6config, therefore I decided that it's worth to open a ticket also here (to document it here too).

The problem seems to be that the ADLODPerformanceLevels struct which is defined within the file include/adl_structures.h (you can also see the struct definition here) of the ADL SDK only has 1 single ADLODPerformanceLevel array item:

typedef struct ADLODPerformanceLevels
{
    ...
    ADLODPerformanceLevel aLevels [1];
} ADLODPerformanceLevels;

but for instance these lines of code use a subscription greater than 0 (.aLevels[1]):

It is still not perfectly clear why this aLevels struct entry has an array of only one single item. As discussed within the new issue opened at oclHashcat's github repository, there probably should be at least 2 ADLODPerformanceLevel within the ADLODPerformanceLevels struct (or it ideally should be allocated dynamically and the size should depend on iNumberOfPerformanceLevels).

A valid fix/hack could be to increase the number of items of type ADLODPerformanceLevel to 2 (at least) such that there is no more risk that memory is getting read and/or written to that shouldn't be touched at all.

After all, it seems that this is definitely a problem of the ADL SDK struct definition vs ADL SDK sample code and ideally AMD should fix this code/confusion. But, as we've experienced a lot of times, AMD probably won't respond/fix anything within a reasonable amount of time (or not at all).

I suggest that we could fix both issues (oclHashcat's and this new od6config issue) with the same fix/solution, therefore it would be great if we could coordinate on this issue and apply the same "fix".

Thank you very much

philsmd commented 8 years ago

Note: oclHashcat now (temporarily?) solves the issue by applying this patch https://github.com/hashcat/oclHashcat/pull/255 A similar fix could be applied on od6config by changing the number of array items of the aLevels arrays in https://github.com/epixoip/od6config/blob/master/ADL_SDK/include/adl_structures.h

epixoip commented 8 years ago

Yeah I saw you make that change, I'll probably make the same change

Edit: made the chantge