Messages in this thread | | | From | Ard Biesheuvel <> | Date | Wed, 2 Nov 2016 13:57:58 +0000 | Subject | Re: [PATCH v2] EFI loader: remove redundant code |
| |
On 2 November 2016 at 13:56, Eugene Korenevsky <ekorenevsky@gmail.com> wrote: >> > *e820ext is always NULL in 'alloc_e820ext()' (see the code of 'exit_boot()'). >> > Without loss of generality we can replace freeing with returning >> > EFI_INVALID_PARAMETER. So if the caller would ever incorrectly pass non-NULL >> > *e820ext, he will obtain a returned error code. >> > >> >> What exactly are you trying to fix here? Adding new artificial failure >> modes is hardly worth it when all you are doing is cleaning up code >> that by itself is correct to begin with, but is simply never called. > > This code (free_pool, assignments) is dead whether it is correct or > not. So it is to be removed. > The check gives some assurance that memory is not leaked if the calling > code is changed. >
No, it doesn't. This code executes in the context of the UEFI firmware, which is riddled with bugs on older x86 platforms (as well as non newer non-x86 platforms). By returning an error at runtime, you may be breaking the boot for someone who wil have *no* idea whatsoever what is going on.
If you want to clean this up in a way that prevents future issues, please use something like BUILD_BUG()
| |