On 01.05.2018 10:43, Borislav Petkov wrote: > On Tue, May 01, 2018 at 12:27:51AM +0200, Maciej S. Szmigiero wrote: >> 1) -EINVAL maps to a valid return value of 4294967274 bytes. >> We have a different behavior for invalid data in the container file >> (including too large lengths) than for grave errors like a failed memory >> allocation. > > WTF?
Could you please elaborate this comment? -EINVAL cast to unsigned int is 4294967274 and this value is also a valid count of bytes to skip that this function can return. The "grave errors" behavior comes from the existing code, a comment in code above verify_and_add_patch() says: "a grave error like a memory allocation has failed and the driver cannot continue functioning normally. In such cases, we tear down everything we've used up so far and exit." >> 2) This function single caller (__load_microcode_amd()) normalized any >> error that verify_and_add_patch() returned to UCODE_ERROR anyway, > > So? This means that there is no loss of information here. The function these three points are about (verify_and_add_patch()) is declared as "static", so it cannot be called from any other kernel code. >> 3) The existing code uses a convention that zero return value means >> 'terminate processing' for the parse_container() function in the early >> loader which normally returns a 'bytes consumed' value, as this function >> does. > > parse_container() could very well change its convention to return > negative on error and positive value if the loop is supposed to skip > bytes. > Yes, but then the problem from the point 1) above will be introduced also to parse_container(). Maciej
