Messages in this thread | | | Subject | Re: [PATCH v5 4/6] x86/microcode/AMD: Check microcode container data in the late loader | From | "Maciej S. Szmigiero" <> | Date | Tue, 1 May 2018 18:19:56 +0200 |
| |
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
| |