On Thu, Apr 22, 2021 at 08:05:27AM +0000, David Laight wrote: > From: Daniel Axtens > > Sent: 22 April 2021 03:21 > > > > > Hi Lakshmi, > > > > > >> On 4/15/21 12:14 PM, Lakshmi Ramasubramanian wrote: > > >> > > >> Sorry - missed copying device-tree and powerpc mailing lists. > > >> > > >>> There are a few "goto out;" statements before the local variable "fdt" > > >>> is initialized through the call to of_kexec_alloc_and_setup_fdt() in > > >>> elf64_load(). This will result in an uninitialized "fdt" being passed > > >>> to kvfree() in this function if there is an error before the call to > > >>> of_kexec_alloc_and_setup_fdt(). > > >>> > > >>> Initialize the local variable "fdt" to NULL. > > >>> > > > I'm a huge fan of initialising local variables! But I'm struggling to > > > find the code path that will lead to an uninit fdt being returned... > > > > OK, so perhaps this was putting it too strongly. I have been bitten > > by uninitialised things enough in C that I may have taken a slightly > > overly-agressive view of fixing them in the source rather than the > > compiler. I do think compiler-level mitigations are better, and I take > > the point that we don't want to defeat compiler checking. > > > > (Does anyone - and by anyone I mean any large distro - compile with > > local variables inited by the compiler?) > > There are compilers that initialise locals to zero for 'debug' builds > and leave the 'random' for optimised 'release' builds. > Lets not test what we are releasing! We're eventually going to move to a world where initializing to zero it the default for the kernel. I think people will still want to initialize to a poison value for debug builds. Initializing to zero is better for debugging because it's more predictable. An it avoid information leaks. And dereferencing random uninitialized pointers is a privilege escalation but dereferencing a NULL is just an Oops. The speed impact is not very significant because (conceptually) it only needs to be done where there is a compiler warning about uninitialized variables. It's slightly more complicated in real life. In this case, the compiler doesn't know what happens inside the kexec_build_elf_info() function so it silences the warning. And GCC silences warnings if the variable is initialized inside a loop even when it doesn't know that we enter the loop. regards, dan carpenter
