Re: [PATCH] powerpc: Initialize local variable fdt to NULL in elf64_load()

Re: [PATCH] powerpc: Initialize local variable fdt to NULL in elf64_load()

[Lakshmi Ramasubramanian]

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.
> 
> Signed-off-by: Lakshmi Ramasubramanian <nramas@linux.microsoft.com>
> Reported-by: kernel test robot <lkp@intel.com>
> Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
> ---
>   arch/powerpc/kexec/elf_64.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/powerpc/kexec/elf_64.c b/arch/powerpc/kexec/elf_64.c
> index 5a569bb51349..0051440c1f77 100644
> --- a/arch/powerpc/kexec/elf_64.c
> +++ b/arch/powerpc/kexec/elf_64.c
> @@ -32,7 +32,7 @@ static void *elf64_load(struct kimage *image, char *kernel_buf,
>   	int ret;
>   	unsigned long kernel_load_addr;
>   	unsigned long initrd_load_addr = 0, fdt_load_addr;
> -	void *fdt;
> +	void *fdt = NULL;
>   	const void *slave_code;
>   	struct elfhdr ehdr;
>   	char *modified_cmdline = NULL;
> 

thanks,
  -lakshmi

Re: [PATCH] powerpc: Initialize local variable fdt to NULL in elf64_load()

[Daniel Axtens]

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...

The out label reads in part:

	/* Make kimage_file_post_load_cleanup free the fdt buffer for us. */
	return ret ? ERR_PTR(ret) : fdt;

As far as I can tell, any time we get a non-zero ret, we're going to
return an error pointer rather than the uninitialised value...

(btw, it does look like we might leak fdt if we have an error after we
successfully kmalloc it.)

Am I missing something? Can you link to the report for the kernel test
robot or from Dan? 

FWIW, I think it's worth including this patch _anyway_ because initing
local variables is good practice, but I'm just not sure on the
justification.

Kind regards,
Daniel

>> Signed-off-by: Lakshmi Ramasubramanian <nramas@linux.microsoft.com>
>> Reported-by: kernel test robot <lkp@intel.com>
>> Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
>> ---
>>   arch/powerpc/kexec/elf_64.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>> 
>> diff --git a/arch/powerpc/kexec/elf_64.c b/arch/powerpc/kexec/elf_64.c
>> index 5a569bb51349..0051440c1f77 100644
>> --- a/arch/powerpc/kexec/elf_64.c
>> +++ b/arch/powerpc/kexec/elf_64.c
>> @@ -32,7 +32,7 @@ static void *elf64_load(struct kimage *image, char *kernel_buf,
>>   	int ret;
>>   	unsigned long kernel_load_addr;
>>   	unsigned long initrd_load_addr = 0, fdt_load_addr;
>> -	void *fdt;
>> +	void *fdt = NULL;
>>   	const void *slave_code;
>>   	struct elfhdr ehdr;
>>   	char *modified_cmdline = NULL;
>> 
>
> thanks,
>   -lakshmi

Re: [PATCH] powerpc: Initialize local variable fdt to NULL in elf64_load()

[Christophe Leroy]

Le 16/04/2021 à 08:44, Daniel Axtens a écrit :
> 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...
> 
> The out label reads in part:
> 
> 	/* Make kimage_file_post_load_cleanup free the fdt buffer for us. */
> 	return ret ? ERR_PTR(ret) : fdt;
> 
> As far as I can tell, any time we get a non-zero ret, we're going to
> return an error pointer rather than the uninitialised value...

I don't think GCC is smart enough to detect that.

> 
> (btw, it does look like we might leak fdt if we have an error after we
> successfully kmalloc it.)
> 
> Am I missing something? Can you link to the report for the kernel test
> robot or from Dan?
> 
> FWIW, I think it's worth including this patch _anyway_ because initing
> local variables is good practice, but I'm just not sure on the
> justification.

I don't think local systematically initing local variables is a good practice at all, as it leads to 
bugs where you get a wrong value because of pathes where you forgot to set the correct value.

If you don't init local variable at declaration and forget to set it in some pathes, the compiler 
will detect it and warn you.
If you init the local variable with an arbitrary value at declaration and forget to set it later in 
some pathes, the compiler won't be able to detect it and you will go with the wrong value.

Christophe

> 
> Kind regards,
> Daniel
> 
>>> Signed-off-by: Lakshmi Ramasubramanian <nramas@linux.microsoft.com>
>>> Reported-by: kernel test robot <lkp@intel.com>
>>> Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
>>> ---
>>>    arch/powerpc/kexec/elf_64.c | 2 +-
>>>    1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/arch/powerpc/kexec/elf_64.c b/arch/powerpc/kexec/elf_64.c
>>> index 5a569bb51349..0051440c1f77 100644
>>> --- a/arch/powerpc/kexec/elf_64.c
>>> +++ b/arch/powerpc/kexec/elf_64.c
>>> @@ -32,7 +32,7 @@ static void *elf64_load(struct kimage *image, char *kernel_buf,
>>>    	int ret;
>>>    	unsigned long kernel_load_addr;
>>>    	unsigned long initrd_load_addr = 0, fdt_load_addr;
>>> -	void *fdt;
>>> +	void *fdt = NULL;
>>>    	const void *slave_code;
>>>    	struct elfhdr ehdr;
>>>    	char *modified_cmdline = NULL;
>>>
>>
>> thanks,
>>    -lakshmi

Re: [PATCH] powerpc: Initialize local variable fdt to NULL in elf64_load()

[Dan Carpenter]

On Fri, Apr 16, 2021 at 04:44:30PM +1000, Daniel Axtens wrote:
> 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!

Don't be!  It just disables static checker warnings and hides bugs.
The kbuild emails are archived but the email is mangled and unreadable.
https://www.mail-archive.com/kbuild@lists.01.org/msg06371.html

I think maybe you're not on the most recent code.  In linux-next this
code looks like:

arch/powerpc/kexec/elf_64.c
    27  static void *elf64_load(struct kimage *image, char *kernel_buf,
    28                          unsigned long kernel_len, char *initrd,
    29                          unsigned long initrd_len, char *cmdline,
    30                          unsigned long cmdline_len)
    31  {
    32          int ret;
    33          unsigned long kernel_load_addr;
    34          unsigned long initrd_load_addr = 0, fdt_load_addr;
    35          void *fdt;
    36          const void *slave_code;
    37          struct elfhdr ehdr;
    38          char *modified_cmdline = NULL;
    39          struct kexec_elf_info elf_info;
    40          struct kexec_buf kbuf = { .image = image, .buf_min = 0,
    41                                    .buf_max = ppc64_rma_size };
    42          struct kexec_buf pbuf = { .image = image, .buf_min = 0,
    43                                    .buf_max = ppc64_rma_size, .top_down = true,
    44                                    .mem = KEXEC_BUF_MEM_UNKNOWN };
    45  
    46          ret = kexec_build_elf_info(kernel_buf, kernel_len, &ehdr, &elf_info);
    47          if (ret)
    48                  goto out;
                        ^^^^^^^^
I really despise "goto out;" because freeing things which haven't been
allocated is always dangerous.

[ snip ].


   143  out:
   144          kfree(modified_cmdline);
   145          kexec_free_elf_info(&elf_info);
                                     ^^^^^^^^
There is a possibility that "elf_info" has holds uninitialized stack
data if elf_read_ehdr() fails so that's probably fixing as well.  kexec()
is root only so this can't be exploited.

   146  
   147          /*
   148           * Once FDT buffer has been successfully passed to kexec_add_buffer(),
   149           * the FDT buffer address is saved in image->arch.fdt. In that case,
   150           * the memory cannot be freed here in case of any other error.
   151           */
   152          if (ret && !image->arch.fdt)
   153                  kvfree(fdt);
                               ^^^
Uninitialized.

   154  
   155          return ret ? ERR_PTR(ret) : NULL;
   156  }

regards,
dan carpenter

Re: [PATCH] powerpc: Initialize local variable fdt to NULL in elf64_load()

[Dan Carpenter]

On Fri, Apr 16, 2021 at 09:00:12AM +0200, Christophe Leroy wrote:
> 
> 
> Le 16/04/2021 à 08:44, Daniel Axtens a écrit :
> > 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...
> > 
> > The out label reads in part:
> > 
> > 	/* Make kimage_file_post_load_cleanup free the fdt buffer for us. */
> > 	return ret ? ERR_PTR(ret) : fdt;
> > 
> > As far as I can tell, any time we get a non-zero ret, we're going to
> > return an error pointer rather than the uninitialised value...
> 
> I don't think GCC is smart enough to detect that.
> 

We disabled uninitialized variable checking for GCC.

But actually is something that has been on my mind recently.  Smatch is
supposed to parse this correctly but there is a bug that affects powerpc
and I don't know how to debug it.  The kbuild bot is doing cross
platform compiles but I don't have one set up on myself.  Could someone
with Smatch installed test something for me?

Or if you don't have Smatch installed then you should definitely install
it.  :P
https://www.spinics.net/lists/smatch/msg00568.html

Apply the patch from below and edit the path to point to the correct
directory.  Then run kchecker and email me the output?

~/path/to/smatch_scripts/kchecker arch/powerpc/kernel/hw_breakpoint.c

regads,
dan carpenter

diff --git a/arch/powerpc/kernel/hw_breakpoint.c b/arch/powerpc/kernel/hw_breakpoint.c
index 8fc7a14e4d71..f2dfba54e14d 100644
--- a/arch/powerpc/kernel/hw_breakpoint.c
+++ b/arch/powerpc/kernel/hw_breakpoint.c
@@ -167,13 +167,19 @@ static bool can_co_exist(struct breakpoint *b, struct perf_event *bp)
 	return !(alternate_infra_bp(b, bp) && bp_addr_range_overlap(b->bp, bp));
 }
 
+#include "/home/XXX/path/to/smatch/check_debug.h"
 static int task_bps_add(struct perf_event *bp)
 {
 	struct breakpoint *tmp;
 
 	tmp = alloc_breakpoint(bp);
-	if (IS_ERR(tmp))
+	__smatch_about(tmp);
+	__smatch_debug_on();
+	if (IS_ERR(tmp)) {
+		__smatch_debug_off();
+		__smatch_about(tmp);
 		return PTR_ERR(tmp);
+	}
 
 	list_add(&tmp->list, &task_bps);
 	return 0;

Re: [PATCH] powerpc: Initialize local variable fdt to NULL in elf64_load()

[Michael Ellerman]

Daniel Axtens <dja@axtens.net> writes:
>> 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...
>
> The out label reads in part:
>
> 	/* Make kimage_file_post_load_cleanup free the fdt buffer for us. */
> 	return ret ? ERR_PTR(ret) : fdt;
>
> As far as I can tell, any time we get a non-zero ret, we're going to
> return an error pointer rather than the uninitialised value...
>
> (btw, it does look like we might leak fdt if we have an error after we
> successfully kmalloc it.)
>
> Am I missing something? Can you link to the report for the kernel test
> robot or from Dan? 
>
> FWIW, I think it's worth including this patch _anyway_ because initing
> local variables is good practice, but I'm just not sure on the
> justification.

Why is it good practice?

It defeats -Wuninitialized. So you're guaranteed to be returning
something initialised, but not necessarily initialised to the right
value.

In a case like this NULL seems like a safe choice, but it's still wrong.
The function is meant to return a pointer to the successfully allocated
fdt, or an ERR_PTR() value. NULL is neither of those.

I agree there are security reasons that initialising stack variables is
desirable, but I think that should be handled by the compiler, not at
the source level.

cheers

Re: [PATCH] powerpc: Initialize local variable fdt to NULL in elf64_load()

[Michael Ellerman]

Dan Carpenter <dan.carpenter@oracle.com> writes:
> On Fri, Apr 16, 2021 at 09:00:12AM +0200, Christophe Leroy wrote:
>> Le 16/04/2021 à 08:44, Daniel Axtens a écrit :
>> > > On 4/15/21 12:14 PM, Lakshmi Ramasubramanian wrote:
>> > > 
>> > > > 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...
>> > 
>> > The out label reads in part:
>> > 
>> > 	/* Make kimage_file_post_load_cleanup free the fdt buffer for us. */
>> > 	return ret ? ERR_PTR(ret) : fdt;
>> > 
>> > As far as I can tell, any time we get a non-zero ret, we're going to
>> > return an error pointer rather than the uninitialised value...
>> 
>> I don't think GCC is smart enough to detect that.
>> 
>
> We disabled uninitialized variable checking for GCC.

We disabled -Wmaybe-uninitialized, but that doesn't disable *all*
uninitialized warnings does it?

I wish we hadn't disabled it, it's already led to bugs slipping through.

> But actually is something that has been on my mind recently.  Smatch is
> supposed to parse this correctly but there is a bug that affects powerpc
> and I don't know how to debug it.  The kbuild bot is doing cross
> platform compiles but I don't have one set up on myself.  Could someone
> with Smatch installed test something for me?
>
> Or if you don't have Smatch installed then you should definitely install
> it.  :P
> https://www.spinics.net/lists/smatch/msg00568.html

I have smatch installed, and even run it sometimes ;)


> Apply the patch from below and edit the path to point to the correct
> directory.  Then run kchecker and email me the output?

That gave me:

    CC      arch/powerpc/kernel/hw_breakpoint.o
  /home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c: In function ‘task_bps_add’:
  /home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:176:16: error: passing argument 1 of ‘__smatch_about’ makes integer from pointer without a cast [-Werror=int-conversion]
    176 | __smatch_about(tmp);
        |                ^~~
        |                |
        |                struct breakpoint *
  In file included from /home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:170:
  /home/michael/smatch/check_debug.h:4:40: note: expected ‘long int’ but argument is of type ‘struct breakpoint *’
      4 | static inline void __smatch_about(long var){}
        |                                   ~~~~~^~~
  /home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:180:21: error: passing argument 1 of ‘__smatch_about’ makes integer from pointer without a cast [-Werror=int-conversion]
    180 |      __smatch_about(tmp);
        |                     ^~~
        |                     |
        |                     struct breakpoint *
  In file included from /home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:170:
  /home/michael/smatch/check_debug.h:4:40: note: expected ‘long int’ but argument is of type ‘struct breakpoint *’
      4 | static inline void __smatch_about(long var){}
        |                                   ~~~~~^~~
  cc1: all warnings being treated as errors


Which looks like it didn't work.

Right, needs tmp cast to long.

Output below, hope it helps. Happy to test other things.

cheers


  GEN     Makefile
  CHECK   /home/michael/linux/scripts/mod/empty.c
  CALL    /home/michael/linux/scripts/checksyscalls.sh
  CALL    /home/michael/linux/scripts/atomic/check-atomics.sh
  CHECK   /home/michael/linux/arch/powerpc/kernel/vdso64/vgettimeofday.c
  CC      arch/powerpc/kernel/hw_breakpoint.o
  CHECK   /home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:177 task_bps_add() ---- about ----
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:177 task_bps_add() implied: tmp = 's64min-(-4096),(-12),4096-s64max'
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:177 task_bps_add() buf size: 'tmp' 0 elements, 0 bytes (rl = (-1),32)
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:177 task_bps_add() strlen: 'tmp'  characters
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:177 task_bps_add() real absolute: tmp = 's64min-(-4096),(-12),4096-s64max'
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:177 task_bps_add() mtag = 0 offset = 0 rl = ''
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:177 task_bps_add() [register_smatch_extra] tmp = '4096-ptr_max,(-12)' [merged] (4096-ptr_max,(-12), 4096-ptr_max, (-12), 4096-ptr_max, (-12))
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:177 task_bps_add() [register_modification_hooks] tmp = 'tmp = alloc_breakpoint(bp)'
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:177 task_bps_add() [register_buf_size] tmp = '(-1),32' [merged] ((-1), (-1),32, 32)
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:177 task_bps_add() [register_comparison_links] tmp = '__fake_param_0x7da6ac1c7450_0 vs tmp, return 0x7da6ac1c7360 vs tmp'
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:177 task_bps_add() [register_fresh_alloc] tmp = 'undefined'
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:177 task_bps_add() [check_deref] tmp = 'ok'
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:177 task_bps_add() [check_uninitialized] tmp = 'initialized' [merged]
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:177 task_bps_add() [register_assigned_expr] tmp = 'alloc_breakpoint(bp)'
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:177 task_bps_add() [register_assigned_expr_links] tmp = '__fake_param_0x7da6ac1c7450_0'
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:178 task_bps_add() set_state change [register_definition_db_callbacks] 'db_incomplete' incomplete => incomplete
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:178 task_bps_add() __set_sm change [register_definition_db_callbacks] db_incomplete = 'incomplete' (was incomplete)
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() set_state change [register_statement_count] 'stmts' 10 => 11
179 in __split_whole_condition
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() 179 in split_conditions (IS_ERR(tmp))
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() set_state new [register_buf_size] '__fake_param_0x7da6ac1c7630_0' (-1),32
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() set_state new [register_smatch_extra] '__fake_param_0x7da6ac1c7630_0' 4096-ptr_max,(-12)
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm change [register_smatch_extra] __fake_param_0x7da6ac1c7630_0 = '4096-ptr_max,(-12)' [merged] (4096-ptr_max,(-12), 4096-ptr_max, (-12), 4096-ptr_max, (-12)) (was 4096-ptr_max,(-12))
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm change [register_smatch_extra] tmp = '4096-ptr_max,(-12)' [merged] (4096-ptr_max,(-12), 4096-ptr_max, (-12), 4096-ptr_max, (-12)) (was 4096-ptr_max,(-12))
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm change [register_smatch_extra] __fake_param_0x7da6ac1c7630_0 = '4096-ptr_max,(-12)' [merged] (4096-ptr_max,(-12), 4096-ptr_max, (-12), 4096-ptr_max, (-12)) (was 4096-ptr_max,(-12))
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() set_state new [register_comparison] '__fake_param_0x7da6ac1c7630_0 vs return 0x7da6ac1c7360' ==
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() set_state new [register_comparison_links] '__fake_param_0x7da6ac1c7630_0' __fake_param_0x7da6ac1c7630_0 vs return 0x7da6ac1c7360
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() set_state new [register_comparison] '__fake_param_0x7da6ac1c7630_0 vs tmp' ==
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() set_state change [register_comparison_links] '__fake_param_0x7da6ac1c7630_0' __fake_param_0x7da6ac1c7630_0 vs return 0x7da6ac1c7360 => __fake_param_0x7da6ac1c7630_0 vs return 0x7da6ac1c7360, __fake_param_0x7da6ac1c7630_0 vs tmp
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() set_state change [register_comparison_links] 'tmp' __fake_param_0x7da6ac1c7450_0 vs tmp, return 0x7da6ac1c7360 vs tmp => __fake_param_0x7da6ac1c7450_0 vs tmp, __fake_param_0x7da6ac1c7630_0 vs tmp, return 0x7da6ac1c7360 vs tmp
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() set_state new [register_assigned_expr] '__fake_param_0x7da6ac1c7630_0' tmp
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() set_state change [register_assigned_expr_links] 'tmp' __fake_param_0x7da6ac1c7450_0 => __fake_param_0x7da6ac1c7450_0, __fake_param_0x7da6ac1c7630_0
debug: insert into caller_info values ('/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c', 'task_bps_add', 'IS_ERR', 138154805589552, 1, 0, -1, '%call_marker%', 'bool(*)(void*)');
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() insert into caller_info values ('/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c', 'task_bps_add', 'IS_ERR', 138154805589552, 1, 0, -1, '%call_marker%', 'bool(*)(void*)');
debug: insert into caller_info values ('/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c', 'task_bps_add', 'IS_ERR', 138154805589552, 1, 1001, 0, '$', '4096-ptr_max,(-12)');
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() insert into caller_info values ('/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c', 'task_bps_add', 'IS_ERR', 138154805589552, 1, 1001, 0, '$', '4096-ptr_max,(-12)');
debug: insert into caller_info values ('/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c', 'task_bps_add', 'IS_ERR', 138154805589552, 1, 1014, 0, '$', 'r alloc_breakpoint');
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() insert into caller_info values ('/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c', 'task_bps_add', 'IS_ERR', 138154805589552, 1, 1014, 0, '$', 'r alloc_breakpoint');
debug: insert into caller_info values ('/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c', 'task_bps_add', 'IS_ERR', 138154805589552, 1, 1002, 0, '$', '(-1),32');
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() insert into caller_info values ('/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c', 'task_bps_add', 'IS_ERR', 138154805589552, 1, 1002, 0, '$', '(-1),32');
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() set_state change [register_modification_hooks] '*tmp' __smatch_about(tmp) => IS_ERR(tmp)
inline function:  IS_ERR
/home/michael/linux/include/linux/err.h:34 IS_ERR() set_state new [internal] 'unnull_path' true
debug: select call_id, type, parameter, key, value from caller_info where call_id = 138154805589552;
/home/michael/linux/include/linux/err.h:34 IS_ERR() select call_id, type, parameter, key, value from caller_info where call_id = 138154805589552;
138154805589552, 0, -1, %call_marker%, bool(*)(void*)
138154805589552, 1001, 0, $, 4096-ptr_max,(-12)
138154805589552, 1014, 0, $, r alloc_breakpoint
138154805589552, 1002, 0, $, (-1),32
/home/michael/linux/include/linux/err.h:34 IS_ERR() set_state new [register_kernel_user_data2] 'this_function' called
/home/michael/linux/include/linux/err.h:34 IS_ERR() set_state new [register_smatch_extra] 'ptr' 4096-ptr_max,(-12)
/home/michael/linux/include/linux/err.h:34 IS_ERR() set_state new [register_buf_size] 'ptr' (-1),32
/home/michael/linux/include/linux/err.h:34 IS_ERR() __set_sm new [register_smatch_extra] ptr = '4096-ptr_max,(-12)'
/home/michael/linux/include/linux/err.h:34 IS_ERR() __set_sm new [register_buf_size] ptr = '(-1),32'
/home/michael/linux/include/linux/err.h:34 IS_ERR() __set_sm new [register_kernel_user_data2] this_function = 'called'
mem-db: insert into function_type values ('/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c', 'IS_ERR', 1, 0, 'void*');
/home/michael/linux/include/linux/err.h:34 IS_ERR() set_state new [check_dereferences_param] 'ptr' param
/home/michael/linux/include/linux/err.h:34 IS_ERR() set_state new [register_comparison] 'ptr vs ptr orig' ==
/home/michael/linux/include/linux/err.h:34 IS_ERR() set_state new [register_comparison_links] 'ptr' ptr vs ptr orig
/home/michael/linux/include/linux/err.h:34 IS_ERR() set_state new [register_statement_count] 'stmts' 1
/home/michael/linux/include/linux/err.h:36 IS_ERR() set_state change [register_statement_count] 'stmts' 1 => 2
36 in __handle_condition_assigns
/home/michael/linux/include/linux/err.h:36 IS_ERR() 36 in split_conditions (!!((ptr) >= -4095))
/home/michael/linux/include/linux/err.h:36 IS_ERR() 36 in split_conditions (!((ptr) >= -4095))
/home/michael/linux/include/linux/err.h:36 IS_ERR() 36 in split_conditions (((ptr) >= -4095))
/home/michael/linux/include/linux/err.h:36 IS_ERR() set_true_false_states [register_smatch_extra] 'ptr'.  Was 4096-ptr_max,(-12).  Now T:(-12) F:4096-ptr_max
/home/michael/linux/include/linux/err.h:36 IS_ERR() set_true_false_states [register_parsed_conditions] 'condition 0x7da6aed21be0'.  Was (null).  Now T:true_path F:false_path
/home/michael/linux/include/linux/err.h:36 IS_ERR() set_state new [register_smatch_extra] '__sm_fake_0x7da6aed219b0' 1
/home/michael/linux/include/linux/err.h:36 IS_ERR() set_state new [register_smatch_extra] '__sm_fake_0x7da6aed219b0' 0
/home/michael/linux/include/linux/err.h:36 IS_ERR() merge [register_smatch_extra] 'ptr' 4096-ptr_max(L 36) + (-12)(L 36) => 4096-ptr_max,(-12) (4096-ptr_max,(-12), (-12), 4096-ptr_max)
/home/michael/linux/include/linux/err.h:36 IS_ERR() merge [register_parsed_conditions] 'condition 0x7da6aed21be0' false_path(L 36) + true_path(L 36) => merged (merged, false_path, true_path)
/home/michael/linux/include/linux/err.h:36 IS_ERR() merge [register_smatch_extra] '__sm_fake_0x7da6aed219b0' 1(L 36) + 0(L 36) => 0-1 (0-1, 0, 1)
/home/michael/linux/include/linux/err.h:36 IS_ERR() merge [register_smatch_extra] 'ptr' (-12)(L 36) + 4096-ptr_max(L 36) => 4096-ptr_max,(-12) (4096-ptr_max,(-12), 4096-ptr_max, (-12))
/home/michael/linux/include/linux/err.h:36 IS_ERR() merge [register_parsed_conditions] 'condition 0x7da6aed21be0' true_path(L 36) + false_path(L 36) => merged (merged, false_path, true_path)
/home/michael/linux/include/linux/err.h:36 IS_ERR() __set_sm new [register_smatch_extra] __sm_fake_0x7da6aed219b0 = '0-1' [merged] (0-1, 0, 1, 0, 1)
/home/michael/linux/include/linux/err.h:36 IS_ERR() __set_sm change [register_smatch_extra] ptr = '4096-ptr_max,(-12)' [merged] (4096-ptr_max,(-12), 4096-ptr_max, (-12), 4096-ptr_max, (-12)) (was 4096-ptr_max,(-12))
/home/michael/linux/include/linux/err.h:36 IS_ERR() __set_sm change [register_parsed_conditions] condition 0x7da6aed21be0 = 'merged' [merged] (merged, false_path, true_path) (was merged)
36 done __handle_condition_assigns
/home/michael/linux/include/linux/err.h:36 IS_ERR() set_state new [register_assigned_expr] '__sm_fake_0x7da6aed219b0' !!((ptr) >= -4095)
36 in __split_whole_condition
/home/michael/linux/include/linux/err.h:36 IS_ERR() 36 in split_conditions (!!((ptr) >= -4095))
/home/michael/linux/include/linux/err.h:36 IS_ERR() 36 in split_conditions (!((ptr) >= -4095))
/home/michael/linux/include/linux/err.h:36 IS_ERR() 36 in split_conditions (((ptr) >= -4095))
/home/michael/linux/include/linux/err.h:36 IS_ERR() __set_true_false_sm [register_smatch_extra] '__sm_fake_0x7da6aed219b0'.  Was 0-1.  Now T:1 F:(null)
/home/michael/linux/include/linux/err.h:36 IS_ERR() __set_true_false_sm [register_smatch_extra] 'ptr'.  Was 4096-ptr_max,(-12).  Now T:(-12) F:(null)
/home/michael/linux/include/linux/err.h:36 IS_ERR() __set_true_false_sm [register_parsed_conditions] 'condition 0x7da6aed21be0'.  Was merged.  Now T:true_path F:(null)
/home/michael/linux/include/linux/err.h:36 IS_ERR() __set_true_false_sm [register_smatch_extra] '__sm_fake_0x7da6aed219b0'.  Was 0-1.  Now T:(null) F:0
/home/michael/linux/include/linux/err.h:36 IS_ERR() __set_true_false_sm [register_smatch_extra] 'ptr'.  Was 4096-ptr_max,(-12).  Now T:(null) F:4096-ptr_max
/home/michael/linux/include/linux/err.h:36 IS_ERR() __set_true_false_sm [register_parsed_conditions] 'condition 0x7da6aed21be0'.  Was merged.  Now T:(null) F:false_path
/home/michael/linux/include/linux/err.h:36 IS_ERR() set_true_false_states [register_smatch_extra] 'ptr'.  Was 4096-ptr_max,(-12).  Now T:(-12) F:4096-ptr_max
/home/michael/linux/include/linux/err.h:36 IS_ERR() set_true_false_states [register_parsed_conditions] 'condition 0x7da6aed21be0'.  Was merged.  Now T:true_path F:false_path
36 done __split_whole_condition
/home/michael/linux/include/linux/err.h:36 IS_ERR() set_state new [register_returns_early] 'return_ranges' 1
mem-db: insert into return_states values ('/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c', 'IS_ERR', 138154805589552, 93, '1', 1, 0, -1, '', 'bool(*)(void*)');
mem-db: insert into return_states values ('/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c', 'IS_ERR', 138154805589552, 93, '1', 1, 103, 0, '$', '(-12)');
mem-db: insert into return_states values ('/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c', 'IS_ERR', 138154805589552, 93, '1', 1, 1037, -1, '', '2');
/home/michael/linux/include/linux/err.h:36 IS_ERR() set_state new [register_returns_early] 'return_ranges' 0
mem-db: insert into return_states values ('/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c', 'IS_ERR', 138154805589552, 94, '0', 1, 0, -1, '', 'bool(*)(void*)');
/home/michael/linux/include/linux/err.h:36 IS_ERR() merge [register_smatch_extra] '__sm_fake_0x7da6aed219b0' 1(L 36) + 0(L 36) => 0-1 (0-1, 0, 1)
/home/michael/linux/include/linux/err.h:36 IS_ERR() merge [register_smatch_extra] 'ptr' (-12)(L 36) + 4096-ptr_max(L 36) => 4096-ptr_max,(-12) (4096-ptr_max,(-12), 4096-ptr_max, (-12))
/home/michael/linux/include/linux/err.h:36 IS_ERR() merge [register_returns_early] 'return_ranges' 1(L 36) + 0(L 36) => merged (0, 1, merged)
/home/michael/linux/include/linux/err.h:36 IS_ERR() merge [register_parsed_conditions] 'condition 0x7da6aed21be0' true_path(L 36) + false_path(L 36) => merged (merged, false_path, true_path)
mem-db: insert into return_states values ('/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c', 'IS_ERR', 138154805589552, 94, '0', 1, 103, 0, '$', '4096-ptr_max');
mem-db: insert into return_states values ('/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c', 'IS_ERR', 138154805589552, 94, '0', 1, 1037, -1, '', '2');
/home/michael/linux/include/linux/err.h:36 IS_ERR() merge [register_smatch_extra] '__sm_fake_0x7da6aed219b0' 0(L 36) + 1(L 36) => 0-1 (0-1, 1, 0)
/home/michael/linux/include/linux/err.h:36 IS_ERR() merge [register_smatch_extra] 'ptr' 4096-ptr_max(L 36) + (-12)(L 36) => 4096-ptr_max,(-12) (4096-ptr_max,(-12), (-12), 4096-ptr_max)
/home/michael/linux/include/linux/err.h:36 IS_ERR() merge [register_returns_early] 'return_ranges' 0(L 36) + 1(L 36) => merged (0, 1, merged)
/home/michael/linux/include/linux/err.h:36 IS_ERR() merge [register_parsed_conditions] 'condition 0x7da6aed21be0' false_path(L 36) + true_path(L 36) => merged (merged, false_path, true_path)
debug: select function, type, parameter, key, value from return_implies where call_id = '138154805589552';
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() select function, type, parameter, key, value from return_implies where call_id = '138154805589552';
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm new [register_smatch_extra] __fake_param_0x7da6ac1c7630_0 = '4096-ptr_max,(-12)' [merged] (4096-ptr_max,(-12), 4096-ptr_max, (-12), 4096-ptr_max, (-12))
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_true_false_sm [register_smatch_extra] '__fake_param_0x7da6ac1c7630_0'.  Was 4096-ptr_max,(-12).  Now T:4096-ptr_max,(-12) F:4096-ptr_max,(-12)
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm change [register_smatch_extra] tmp = '4096-ptr_max,(-12)' [merged] (4096-ptr_max,(-12), 4096-ptr_max, (-12), 4096-ptr_max, (-12)) (was 4096-ptr_max,(-12))
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_true_false_sm [register_smatch_extra] 'tmp'.  Was 4096-ptr_max,(-12).  Now T:4096-ptr_max,(-12) F:4096-ptr_max,(-12)
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm change [register_modification_hooks] *tmp = 'IS_ERR(tmp)' (was __smatch_about(tmp))
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_true_false_sm [register_modification_hooks] '*tmp'.  Was IS_ERR(tmp).  Now T:IS_ERR(tmp) F:IS_ERR(tmp)
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm new [register_buf_size] __fake_param_0x7da6ac1c7630_0 = '(-1),32'
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_true_false_sm [register_buf_size] '__fake_param_0x7da6ac1c7630_0'.  Was (-1),32.  Now T:(-1),32 F:(-1),32
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm new [register_comparison] __fake_param_0x7da6ac1c7630_0 vs return 0x7da6ac1c7360 = '=='
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_true_false_sm [register_comparison] '__fake_param_0x7da6ac1c7630_0 vs return 0x7da6ac1c7360'.  Was ==.  Now T:== F:==
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm new [register_comparison] __fake_param_0x7da6ac1c7630_0 vs tmp = '=='
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_true_false_sm [register_comparison] '__fake_param_0x7da6ac1c7630_0 vs tmp'.  Was ==.  Now T:== F:==
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm new [register_comparison_links] __fake_param_0x7da6ac1c7630_0 = '__fake_param_0x7da6ac1c7630_0 vs return 0x7da6ac1c7360, __fake_param_0x7da6ac1c7630_0 vs tmp'
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_true_false_sm [register_comparison_links] '__fake_param_0x7da6ac1c7630_0'.  Was __fake_param_0x7da6ac1c7630_0 vs return 0x7da6ac1c7360, __fake_param_0x7da6ac1c7630_0 vs tmp.  Now T:__fake_param_0x7da6ac1c7630_0 vs return 0x7da6ac1c7360, __fake_param_0x7da6ac1c7630_0 vs tmp F:__fake_param_0x7da6ac1c7630_0 vs return 0x7da6ac1c7360, __fake_param_0x7da6ac1c7630_0 vs tmp
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm change [register_comparison_links] tmp = '__fake_param_0x7da6ac1c7450_0 vs tmp, __fake_param_0x7da6ac1c7630_0 vs tmp, return 0x7da6ac1c7360 vs tmp' (was __fake_param_0x7da6ac1c7450_0 vs tmp, return 0x7da6ac1c7360 vs tmp)
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_true_false_sm [register_comparison_links] 'tmp'.  Was __fake_param_0x7da6ac1c7450_0 vs tmp, __fake_param_0x7da6ac1c7630_0 vs tmp, return 0x7da6ac1c7360 vs tmp.  Now T:__fake_param_0x7da6ac1c7450_0 vs tmp, __fake_param_0x7da6ac1c7630_0 vs tmp, return 0x7da6ac1c7360 vs tmp F:__fake_param_0x7da6ac1c7450_0 vs tmp, __fake_param_0x7da6ac1c7630_0 vs tmp, return 0x7da6ac1c7360 vs tmp
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm new [register_assigned_expr] __fake_param_0x7da6ac1c7630_0 = 'tmp'
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_true_false_sm [register_assigned_expr] '__fake_param_0x7da6ac1c7630_0'.  Was tmp.  Now T:tmp F:tmp
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm change [register_assigned_expr_links] tmp = '__fake_param_0x7da6ac1c7450_0, __fake_param_0x7da6ac1c7630_0' (was __fake_param_0x7da6ac1c7450_0)
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_true_false_sm [register_assigned_expr_links] 'tmp'.  Was __fake_param_0x7da6ac1c7450_0, __fake_param_0x7da6ac1c7630_0.  Now T:__fake_param_0x7da6ac1c7450_0, __fake_param_0x7da6ac1c7630_0 F:__fake_param_0x7da6ac1c7450_0, __fake_param_0x7da6ac1c7630_0
debug: select distinct return from return_states where call_id = '138154805589552';
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() select distinct return from return_states where call_id = '138154805589552';
1
0
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() set_state new [check_err_ptr_deref] 'tmp' err_ptr
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() set_state new [check_err_ptr_deref] 'tmp' checked
debug: select return_id, return, type, parameter, key, value from return_states where call_id = '138154805589552' order by return_id, type;
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() select return_id, return, type, parameter, key, value from return_states where call_id = '138154805589552' order by return_id, type;
93, 1, 0, -1, , bool(*)(void*)
93, 1, 103, 0, $, (-12)
93, 1, 1037, -1, , 2
94, 0, 0, -1, , bool(*)(void*)
94, 0, 103, 0, $, 4096-ptr_max
94, 0, 1037, -1, , 2
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() set_state change [internal] 'unnull_path' true => true
'tmp = (-12)' from 176 is true. [leaf][stree 3]
'tmp = 4096-ptr_max' from 176 is false. [leaf][stree 4]
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm_fake_stree change [register_smatch_extra] __fake_param_0x7da6ac1c7630_0 = '(-12)' [merged] (was 4096-ptr_max,(-12))
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm_fake_stree change [register_smatch_extra] tmp->bp = '' [merged] (was 0-u64max)
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm_fake_stree change [register_smatch_extra] tmp->list.next = '' [merged] (was 0)
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm_fake_stree change [register_smatch_extra] tmp->list.prev = '' [merged] (was 0)
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm_fake_stree change [register_untracked_param] bp = 'undefined' [merged] (was merged)
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() set_state new [register_smatch_extra_links] 'tmp' tmp
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() set_state change [register_smatch_extra] '__fake_param_0x7da6ac1c7630_0' 4096-ptr_max,(-12) => (-12)
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() set_state change [register_smatch_extra] 'tmp' 4096-ptr_max,(-12) => (-12)
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() set_state change [register_smatch_extra] '__fake_param_0x7da6ac1c7630_0' (-12) => (-12)
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() set_state change [register_smatch_extra_links] 'tmp' tmp => __fake_param_0x7da6ac1c7630_0, tmp
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() set_state change [register_smatch_extra] '__fake_param_0x7da6ac1c7630_0' (-12) => (-12)
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm change [register_smatch_extra] tmp = '(-12)' [merged] (was (-12))
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm change [register_buf_size] tmp = '(-1)' [merged] (was (-1),32)
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm change [check_uninitialized] tmp = 'initialized' [merged] (was initialized)
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() set_state change [register_statement_count] 'stmts' 11 => 13
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() set_state new [internal] 'unnull_path' true
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm_cur_stree new [register_smatch_extra] __fake_param_0x7da6ac1c7630_0 = '4096-ptr_max,(-12)' [merged] (4096-ptr_max,(-12), 4096-ptr_max, (-12), 4096-ptr_max, (-12))
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm_cur_stree new [register_smatch_extra] tmp = '4096-ptr_max,(-12)' [merged] (4096-ptr_max,(-12), 4096-ptr_max, (-12), 4096-ptr_max, (-12))
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm_cur_stree new [register_smatch_extra] tmp->bp = '0-u64max' [merged] (0-u64max, 0-u64max, , 0-u64max, )
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm_cur_stree new [register_smatch_extra] tmp->list.next = '0' [merged] (0, 0, , 0, )
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm_cur_stree new [register_smatch_extra] tmp->list.prev = '0' [merged] (0, 0, , 0, )
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm_cur_stree new [register_modification_hooks] *bp = 'alloc_breakpoint(bp)'
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm_cur_stree new [register_modification_hooks] *tmp = 'IS_ERR(tmp)'
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm_cur_stree new [register_modification_hooks] tmp = 'tmp = alloc_breakpoint(bp)'
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm_cur_stree new [register_definition_db_callbacks] db_incomplete = 'incomplete'
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm_cur_stree new [register_untracked_param] bp = 'merged' [merged] (untracked, merged, undefined)
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm_cur_stree new [register_buf_size] __fake_param_0x7da6ac1c7630_0 = '(-1),32'
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm_cur_stree new [register_buf_size] tmp = '(-1),32' [merged] ((-1), (-1),32, 32)
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm_cur_stree new [register_comparison] __fake_param_0x7da6ac1c7630_0 vs return 0x7da6ac1c7360 = '=='
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm_cur_stree new [register_comparison] __fake_param_0x7da6ac1c7630_0 vs tmp = '=='
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm_cur_stree new [register_comparison] bp vs bp orig = '=='
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm_cur_stree new [register_comparison] return 0x7da6ac1c7360 vs tmp = '=='
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm_cur_stree new [register_comparison_links] __fake_param_0x7da6ac1c7630_0 = '__fake_param_0x7da6ac1c7630_0 vs return 0x7da6ac1c7360, __fake_param_0x7da6ac1c7630_0 vs tmp'
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm_cur_stree new [register_comparison_links] bp = 'bp vs bp orig'
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm_cur_stree new [register_comparison_links] return 0x7da6ac1c7360 = 'return 0x7da6ac1c7360 vs tmp'
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm_cur_stree new [register_comparison_links] tmp = '__fake_param_0x7da6ac1c7450_0 vs tmp, __fake_param_0x7da6ac1c7630_0 vs tmp, return 0x7da6ac1c7360 vs tmp'
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm_cur_stree new [register_statement_count] stmts = '11'
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm_cur_stree new [register_fresh_alloc] tmp = 'undefined'
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm_cur_stree new [check_deref] tmp = 'ok'
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm_cur_stree new [check_dereferences_param] bp = 'param'
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm_cur_stree new [check_uninitialized] tmp = 'initialized' [merged]
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm_cur_stree new [register_assigned_expr] __fake_param_0x7da6ac1c7630_0 = 'tmp'
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm_cur_stree new [register_assigned_expr] tmp = 'alloc_breakpoint(bp)'
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm_cur_stree new [register_assigned_expr_links] tmp = '__fake_param_0x7da6ac1c7450_0, __fake_param_0x7da6ac1c7630_0'
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm_cur_stree change [internal] unnull_path = 'true' [merged] (was true)
debug: select return_id, return, type, parameter, key, value from return_states where call_id = '138154805589552' order by return_id, type;
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() select return_id, return, type, parameter, key, value from return_states where call_id = '138154805589552' order by return_id, type;
93, 1, 0, -1, , bool(*)(void*)
93, 1, 103, 0, $, (-12)
93, 1, 1037, -1, , 2
94, 0, 0, -1, , bool(*)(void*)
94, 0, 103, 0, $, 4096-ptr_max
94, 0, 1037, -1, , 2
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() set_state change [internal] 'unnull_path' true => true
'tmp = (-12)' from 176 is false. [leaf][stree 3]
'tmp = 4096-ptr_max' from 176 is true. [leaf][stree 4]
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm_fake_stree change [register_smatch_extra] __fake_param_0x7da6ac1c7630_0 = '4096-ptr_max' [merged] (was 4096-ptr_max,(-12))
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm_fake_stree change [register_smatch_extra] tmp->bp = '0-u64max' [merged] (was 0-u64max)
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm_fake_stree change [register_smatch_extra] tmp->list.next = '0' [merged] (was 0)
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm_fake_stree change [register_smatch_extra] tmp->list.prev = '0' [merged] (was 0)
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm_fake_stree change [register_untracked_param] bp = 'untracked' [merged] (was merged)
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() set_state new [register_smatch_extra_links] 'tmp' tmp
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() set_state change [register_smatch_extra] '__fake_param_0x7da6ac1c7630_0' 4096-ptr_max,(-12) => 4096-ptr_max
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() set_state change [register_smatch_extra] 'tmp' 4096-ptr_max,(-12) => 4096-ptr_max
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() set_state change [register_smatch_extra] '__fake_param_0x7da6ac1c7630_0' 4096-ptr_max => 4096-ptr_max
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() set_state change [register_smatch_extra_links] 'tmp' tmp => __fake_param_0x7da6ac1c7630_0, tmp
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() set_state change [register_smatch_extra] '__fake_param_0x7da6ac1c7630_0' 4096-ptr_max => 4096-ptr_max
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm change [register_smatch_extra] tmp = '4096-ptr_max' [merged] (was 4096-ptr_max)
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm change [register_buf_size] tmp = '32' [merged] (was (-1),32)
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm change [check_uninitialized] tmp = 'initialized' [merged] (was initialized)
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() set_state change [register_statement_count] 'stmts' 11 => 13
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() set_state new [internal] 'unnull_path' true
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm_cur_stree new [register_smatch_extra] __fake_param_0x7da6ac1c7630_0 = '4096-ptr_max,(-12)' [merged] (4096-ptr_max,(-12), 4096-ptr_max, (-12), 4096-ptr_max, (-12))
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm_cur_stree new [register_smatch_extra] tmp = '4096-ptr_max,(-12)' [merged] (4096-ptr_max,(-12), 4096-ptr_max, (-12), 4096-ptr_max, (-12))
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm_cur_stree new [register_smatch_extra] tmp->bp = '0-u64max' [merged] (0-u64max, 0-u64max, , 0-u64max, )
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm_cur_stree new [register_smatch_extra] tmp->list.next = '0' [merged] (0, 0, , 0, )
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm_cur_stree new [register_smatch_extra] tmp->list.prev = '0' [merged] (0, 0, , 0, )
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm_cur_stree new [register_modification_hooks] *bp = 'alloc_breakpoint(bp)'
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm_cur_stree new [register_modification_hooks] *tmp = 'IS_ERR(tmp)'
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm_cur_stree new [register_modification_hooks] tmp = 'tmp = alloc_breakpoint(bp)'
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm_cur_stree new [register_definition_db_callbacks] db_incomplete = 'incomplete'
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm_cur_stree new [register_untracked_param] bp = 'merged' [merged] (untracked, merged, undefined)
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm_cur_stree new [register_buf_size] __fake_param_0x7da6ac1c7630_0 = '(-1),32'
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm_cur_stree new [register_buf_size] tmp = '(-1),32' [merged] ((-1), (-1),32, 32)
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm_cur_stree new [register_comparison] __fake_param_0x7da6ac1c7630_0 vs return 0x7da6ac1c7360 = '=='
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm_cur_stree new [register_comparison] __fake_param_0x7da6ac1c7630_0 vs tmp = '=='
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm_cur_stree new [register_comparison] bp vs bp orig = '=='
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm_cur_stree new [register_comparison] return 0x7da6ac1c7360 vs tmp = '=='
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm_cur_stree new [register_comparison_links] __fake_param_0x7da6ac1c7630_0 = '__fake_param_0x7da6ac1c7630_0 vs return 0x7da6ac1c7360, __fake_param_0x7da6ac1c7630_0 vs tmp'
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm_cur_stree new [register_comparison_links] bp = 'bp vs bp orig'
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm_cur_stree new [register_comparison_links] return 0x7da6ac1c7360 = 'return 0x7da6ac1c7360 vs tmp'
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm_cur_stree new [register_comparison_links] tmp = '__fake_param_0x7da6ac1c7450_0 vs tmp, __fake_param_0x7da6ac1c7630_0 vs tmp, return 0x7da6ac1c7360 vs tmp'
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm_cur_stree new [register_statement_count] stmts = '11'
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm_cur_stree new [register_fresh_alloc] tmp = 'undefined'
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm_cur_stree new [check_deref] tmp = 'ok'
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm_cur_stree new [check_dereferences_param] bp = 'param'
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm_cur_stree new [check_uninitialized] tmp = 'initialized' [merged]
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm_cur_stree new [register_assigned_expr] __fake_param_0x7da6ac1c7630_0 = 'tmp'
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm_cur_stree new [register_assigned_expr] tmp = 'alloc_breakpoint(bp)'
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm_cur_stree new [register_assigned_expr_links] tmp = '__fake_param_0x7da6ac1c7450_0, __fake_param_0x7da6ac1c7630_0'
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm_cur_stree change [internal] unnull_path = 'true' [merged] (was true)
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_true_false_sm [register_smatch_extra] '__fake_param_0x7da6ac1c7630_0'.  Was 4096-ptr_max,(-12).  Now T:(-12) F:(null)
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_true_false_sm [register_smatch_extra] 'tmp'.  Was 4096-ptr_max,(-12).  Now T:(-12) F:(null)
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_true_false_sm [register_smatch_extra] 'tmp->bp'.  Was 0-u64max.  Now T: F:(null)
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_true_false_sm [register_smatch_extra] 'tmp->list.next'.  Was 0.  Now T: F:(null)
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_true_false_sm [register_smatch_extra] 'tmp->list.prev'.  Was 0.  Now T: F:(null)
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_true_false_sm [register_smatch_extra_links] 'tmp'.  Was (null).  Now T:__fake_param_0x7da6ac1c7630_0, tmp F:(null)
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_true_false_sm [register_untracked_param] 'bp'.  Was merged.  Now T:undefined F:(null)
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_true_false_sm [register_buf_size] 'tmp'.  Was (-1),32.  Now T:(-1) F:(null)
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_true_false_sm [register_statement_count] 'stmts'.  Was 11.  Now T:13 F:(null)
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_true_false_sm [check_uninitialized] 'tmp'.  Was initialized.  Now T:initialized F:(null)
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_true_false_sm [internal] 'unnull_path'.  Was true.  Now T:true F:(null)
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_true_false_sm [register_smatch_extra] '__fake_param_0x7da6ac1c7630_0'.  Was 4096-ptr_max,(-12).  Now T:(null) F:4096-ptr_max
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_true_false_sm [register_smatch_extra] 'tmp'.  Was 4096-ptr_max,(-12).  Now T:(null) F:4096-ptr_max
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_true_false_sm [register_smatch_extra] 'tmp->bp'.  Was 0-u64max.  Now T:(null) F:0-u64max
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_true_false_sm [register_smatch_extra] 'tmp->list.next'.  Was 0.  Now T:(null) F:0
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_true_false_sm [register_smatch_extra] 'tmp->list.prev'.  Was 0.  Now T:(null) F:0
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_true_false_sm [register_smatch_extra_links] 'tmp'.  Was (null).  Now T:(null) F:__fake_param_0x7da6ac1c7630_0, tmp
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_true_false_sm [register_untracked_param] 'bp'.  Was merged.  Now T:(null) F:untracked
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_true_false_sm [register_buf_size] 'tmp'.  Was (-1),32.  Now T:(null) F:32
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_true_false_sm [register_statement_count] 'stmts'.  Was 11.  Now T:(null) F:13
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_true_false_sm [check_uninitialized] 'tmp'.  Was initialized.  Now T:(null) F:initialized
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_true_false_sm [internal] 'unnull_path'.  Was true.  Now T:(null) F:true
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_true_false_sm [check_err_ptr_deref] 'tmp'.  Was (null).  Now T:err_ptr F:(null)
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_true_false_sm [check_err_ptr_deref] 'tmp'.  Was (null).  Now T:(null) F:checked
debug: select distinct return from return_states where call_id = '138154805589552';
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() select distinct return from return_states where call_id = '138154805589552';
1
0
debug: select distinct return from return_states where call_id = '138154805589552';
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() select distinct return from return_states where call_id = '138154805589552';
1
0
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() set_true_false_states [register_parsed_conditions] 'condition 0x7da6ac1c7630'.  Was (null).  Now T:true_path F:false_path
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm change [internal] unnull_path = 'true' (was true)
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_true_false_sm [internal] 'unnull_path'.  Was true.  Now T:true F:true
179 done __split_whole_condition
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() set_state change [register_statement_count] 'stmts' 13 => 14
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:180 task_bps_add() set_state change [register_statement_count] 'stmts' 14 => 15
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:181 task_bps_add() ---- about ----
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:181 task_bps_add() implied: tmp = '(-12)'
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:181 task_bps_add() buf size: 'tmp' 0 elements, 0 bytes
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:181 task_bps_add() strlen: 'tmp'  characters
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:181 task_bps_add() real absolute: tmp = '(-12)'
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:181 task_bps_add() mtag = 0 offset = 0 rl = ''
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:181 task_bps_add() [register_smatch_extra] tmp = '(-12)' [merged]
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:181 task_bps_add() [register_smatch_extra_links] tmp = '__fake_param_0x7da6ac1c7630_0, tmp'
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:181 task_bps_add() [register_modification_hooks] tmp = 'tmp = alloc_breakpoint(bp)'
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:181 task_bps_add() [register_buf_size] tmp = '(-1)' [merged]
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:181 task_bps_add() [register_comparison_links] tmp = '__fake_param_0x7da6ac1c7450_0 vs tmp, __fake_param_0x7da6ac1c7630_0 vs tmp, __fake_param_0x7da6ac1c77c0_0 vs tmp, return 0x7da6ac1c7360 vs tmp'
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:181 task_bps_add() [register_fresh_alloc] tmp = 'undefined'
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:181 task_bps_add() [check_deref] tmp = 'ok'
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:181 task_bps_add() [check_uninitialized] tmp = 'initialized' [merged]
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:181 task_bps_add() [check_err_ptr_deref] tmp = 'err_ptr'
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:181 task_bps_add() [register_assigned_expr] tmp = 'alloc_breakpoint(bp)'
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:181 task_bps_add() [register_assigned_expr_links] tmp = '__fake_param_0x7da6ac1c7450_0, __fake_param_0x7da6ac1c7630_0, __fake_param_0x7da6ac1c77c0_0'
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:178 task_bps_add() set_state change [register_definition_db_callbacks] 'db_incomplete' incomplete => incomplete
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:178 task_bps_add() __set_sm change [register_definition_db_callbacks] db_incomplete = 'incomplete' (was incomplete)
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() set_state change [register_statement_count] 'stmts' 5 => 6
179 in __split_whole_condition
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() 179 in split_conditions (IS_ERR(tmp))
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() set_state new [register_smatch_extra] '__fake_param_0x7da6ac1c7630_0' 0-u64max
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm change [register_smatch_extra] __fake_param_0x7da6ac1c7630_0 = '0-u64max' (was 0-u64max)
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm change [register_smatch_extra] tmp = '0-u64max' (was 0-u64max)
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm change [register_smatch_extra] __fake_param_0x7da6ac1c7630_0 = '0-u64max' (was 0-u64max)
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() set_state new [register_assigned_expr] '__fake_param_0x7da6ac1c7630_0' tmp
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() set_state change [register_assigned_expr_links] 'tmp' __fake_param_0x7da6ac1c7450_0 => __fake_param_0x7da6ac1c7450_0, __fake_param_0x7da6ac1c7630_0
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() set_state change [register_modification_hooks] '*tmp' __smatch_about(tmp) => IS_ERR(tmp)
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm new [register_smatch_extra] __fake_param_0x7da6ac1c7630_0 = '0-u64max'
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_true_false_sm [register_smatch_extra] '__fake_param_0x7da6ac1c7630_0'.  Was 0-u64max.  Now T:0-u64max F:0-u64max
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm change [register_smatch_extra] tmp = '0-u64max' (was 0-u64max)
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_true_false_sm [register_smatch_extra] 'tmp'.  Was 0-u64max.  Now T:0-u64max F:0-u64max
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm change [register_modification_hooks] *tmp = 'IS_ERR(tmp)' (was __smatch_about(tmp))
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_true_false_sm [register_modification_hooks] '*tmp'.  Was IS_ERR(tmp).  Now T:IS_ERR(tmp) F:IS_ERR(tmp)
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm new [register_assigned_expr] __fake_param_0x7da6ac1c7630_0 = 'tmp'
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_true_false_sm [register_assigned_expr] '__fake_param_0x7da6ac1c7630_0'.  Was tmp.  Now T:tmp F:tmp
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm change [register_assigned_expr_links] tmp = '__fake_param_0x7da6ac1c7450_0, __fake_param_0x7da6ac1c7630_0' (was __fake_param_0x7da6ac1c7450_0)
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_true_false_sm [register_assigned_expr_links] 'tmp'.  Was __fake_param_0x7da6ac1c7450_0, __fake_param_0x7da6ac1c7630_0.  Now T:__fake_param_0x7da6ac1c7450_0, __fake_param_0x7da6ac1c7630_0 F:__fake_param_0x7da6ac1c7450_0, __fake_param_0x7da6ac1c7630_0
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() set_state new [check_err_ptr_deref] 'tmp' err_ptr
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() set_state new [check_err_ptr_deref] 'tmp' checked
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() set_state change [register_definition_db_callbacks] 'db_incomplete' incomplete => incomplete
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() set_state new [internal] 'unnull_path' true
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm_cur_stree new [register_smatch_extra] __fake_param_0x7da6ac1c7630_0 = '0-u64max'
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm_cur_stree new [register_smatch_extra] bp = '4096-ptr_max'
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm_cur_stree new [register_smatch_extra] bp->overflow_handler = '4096-ptr_max'
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm_cur_stree new [register_smatch_extra] tmp = '0-u64max'
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm_cur_stree new [register_modification_hooks] *bp = 'alloc_breakpoint(bp)'
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm_cur_stree new [register_modification_hooks] *tmp = 'IS_ERR(tmp)'
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm_cur_stree new [register_modification_hooks] tmp = 'tmp = alloc_breakpoint(bp)'
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm_cur_stree new [register_definition_db_callbacks] db_incomplete = 'incomplete'
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm_cur_stree new [register_untracked_param] bp = 'untracked'
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm_cur_stree new [register_comparison] bp vs bp orig = '=='
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm_cur_stree new [register_comparison_links] bp = 'bp vs bp orig'
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm_cur_stree new [register_statement_count] stmts = '6'
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm_cur_stree new [register_kernel_user_data2] this_function = 'called'
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm_cur_stree new [check_deref] tmp = 'ok'
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm_cur_stree new [check_dereferences_param] bp = 'param'
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm_cur_stree new [check_uninitialized] tmp = 'initialized'
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm_cur_stree new [register_assigned_expr] __fake_param_0x7da6ac1c7630_0 = 'tmp'
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm_cur_stree new [register_assigned_expr] tmp = 'alloc_breakpoint(bp)'
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm_cur_stree new [register_assigned_expr_links] tmp = '__fake_param_0x7da6ac1c7450_0, __fake_param_0x7da6ac1c7630_0'
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm_cur_stree change [internal] unnull_path = 'true' (was true)
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() set_state change [register_definition_db_callbacks] 'db_incomplete' incomplete => incomplete
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() set_state new [internal] 'unnull_path' true
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm_cur_stree new [register_smatch_extra] __fake_param_0x7da6ac1c7630_0 = '0-u64max'
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm_cur_stree new [register_smatch_extra] bp = '4096-ptr_max'
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm_cur_stree new [register_smatch_extra] bp->overflow_handler = '4096-ptr_max'
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm_cur_stree new [register_smatch_extra] tmp = '0-u64max'
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm_cur_stree new [register_modification_hooks] *bp = 'alloc_breakpoint(bp)'
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm_cur_stree new [register_modification_hooks] *tmp = 'IS_ERR(tmp)'
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm_cur_stree new [register_modification_hooks] tmp = 'tmp = alloc_breakpoint(bp)'
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm_cur_stree new [register_definition_db_callbacks] db_incomplete = 'incomplete'
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm_cur_stree new [register_untracked_param] bp = 'untracked'
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm_cur_stree new [register_comparison] bp vs bp orig = '=='
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm_cur_stree new [register_comparison_links] bp = 'bp vs bp orig'
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm_cur_stree new [register_statement_count] stmts = '6'
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm_cur_stree new [register_kernel_user_data2] this_function = 'called'
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm_cur_stree new [check_deref] tmp = 'ok'
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm_cur_stree new [check_dereferences_param] bp = 'param'
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm_cur_stree new [check_uninitialized] tmp = 'initialized'
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm_cur_stree new [register_assigned_expr] __fake_param_0x7da6ac1c7630_0 = 'tmp'
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm_cur_stree new [register_assigned_expr] tmp = 'alloc_breakpoint(bp)'
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm_cur_stree new [register_assigned_expr_links] tmp = '__fake_param_0x7da6ac1c7450_0, __fake_param_0x7da6ac1c7630_0'
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm_cur_stree change [internal] unnull_path = 'true' (was true)
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_true_false_sm [register_definition_db_callbacks] 'db_incomplete'.  Was incomplete.  Now T:incomplete F:(null)
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_true_false_sm [register_definition_db_callbacks] 'db_incomplete'.  Was incomplete.  Now T:(null) F:incomplete
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() set_state new [register_untracked_param] 'tmp' untracked
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_true_false_sm [check_err_ptr_deref] 'tmp'.  Was (null).  Now T:err_ptr F:(null)
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_true_false_sm [check_err_ptr_deref] 'tmp'.  Was (null).  Now T:(null) F:checked
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() set_true_false_states [register_parsed_conditions] 'condition 0x7da6ac1c7630'.  Was (null).  Now T:true_path F:false_path
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm new [register_untracked_param] tmp = 'untracked'
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_true_false_sm [register_untracked_param] 'tmp'.  Was untracked.  Now T:untracked F:untracked
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm change [internal] unnull_path = 'true' (was true)
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_true_false_sm [internal] 'unnull_path'.  Was true.  Now T:true F:true
179 done __split_whole_condition
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() set_state change [register_statement_count] 'stmts' 6 => 7
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:180 task_bps_add() set_state change [register_statement_count] 'stmts' 7 => 8
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:178 task_bps_add() set_state change [register_definition_db_callbacks] 'db_incomplete' incomplete => incomplete
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:178 task_bps_add() __set_sm change [register_definition_db_callbacks] db_incomplete = 'incomplete' (was incomplete)
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() set_state change [register_statement_count] 'stmts' 5 => 6
179 in __split_whole_condition
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() 179 in split_conditions (IS_ERR(tmp))
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() set_state new [register_smatch_extra] '__fake_param_0x7da6ac1c7630_0' 0-u64max
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm change [register_smatch_extra] __fake_param_0x7da6ac1c7630_0 = '0-u64max' (was 0-u64max)
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm change [register_smatch_extra] tmp = '0-u64max' (was 0-u64max)
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm change [register_smatch_extra] __fake_param_0x7da6ac1c7630_0 = '0-u64max' (was 0-u64max)
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() set_state new [register_assigned_expr] '__fake_param_0x7da6ac1c7630_0' tmp
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() set_state change [register_assigned_expr_links] 'tmp' __fake_param_0x7da6ac1c7450_0 => __fake_param_0x7da6ac1c7450_0, __fake_param_0x7da6ac1c7630_0
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() set_state change [register_modification_hooks] '*tmp' __smatch_about(tmp) => IS_ERR(tmp)
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm new [register_smatch_extra] __fake_param_0x7da6ac1c7630_0 = '0-u64max'
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_true_false_sm [register_smatch_extra] '__fake_param_0x7da6ac1c7630_0'.  Was 0-u64max.  Now T:0-u64max F:0-u64max
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm change [register_smatch_extra] tmp = '0-u64max' (was 0-u64max)
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_true_false_sm [register_smatch_extra] 'tmp'.  Was 0-u64max.  Now T:0-u64max F:0-u64max
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm change [register_modification_hooks] *tmp = 'IS_ERR(tmp)' (was __smatch_about(tmp))
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_true_false_sm [register_modification_hooks] '*tmp'.  Was IS_ERR(tmp).  Now T:IS_ERR(tmp) F:IS_ERR(tmp)
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm new [register_assigned_expr] __fake_param_0x7da6ac1c7630_0 = 'tmp'
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_true_false_sm [register_assigned_expr] '__fake_param_0x7da6ac1c7630_0'.  Was tmp.  Now T:tmp F:tmp
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm change [register_assigned_expr_links] tmp = '__fake_param_0x7da6ac1c7450_0, __fake_param_0x7da6ac1c7630_0' (was __fake_param_0x7da6ac1c7450_0)
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_true_false_sm [register_assigned_expr_links] 'tmp'.  Was __fake_param_0x7da6ac1c7450_0, __fake_param_0x7da6ac1c7630_0.  Now T:__fake_param_0x7da6ac1c7450_0, __fake_param_0x7da6ac1c7630_0 F:__fake_param_0x7da6ac1c7450_0, __fake_param_0x7da6ac1c7630_0
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() set_state new [check_err_ptr_deref] 'tmp' err_ptr
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() set_state new [check_err_ptr_deref] 'tmp' checked
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() set_state change [register_definition_db_callbacks] 'db_incomplete' incomplete => incomplete
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() set_state new [internal] 'unnull_path' true
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm_cur_stree new [register_smatch_extra] __fake_param_0x7da6ac1c7630_0 = '0-u64max'
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm_cur_stree new [register_smatch_extra] bp = '4096-ptr_max'
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm_cur_stree new [register_smatch_extra] bp->attr.bp_addr = '0-13835058055282163711'
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm_cur_stree new [register_smatch_extra] bp->cpu = '(-1)'
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm_cur_stree new [register_smatch_extra] bp->hw.target = '1-u64max'
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm_cur_stree new [register_smatch_extra] bp->overflow_handler = '0-4644892087836254207,4644892087836254209-u64max'
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm_cur_stree new [register_smatch_extra] tmp = '0-u64max'
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm_cur_stree new [register_modification_hooks] *bp = 'alloc_breakpoint(bp)'
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm_cur_stree new [register_modification_hooks] *tmp = 'IS_ERR(tmp)'
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm_cur_stree new [register_modification_hooks] tmp = 'tmp = alloc_breakpoint(bp)'
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm_cur_stree new [register_definition_db_callbacks] db_incomplete = 'incomplete'
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm_cur_stree new [register_untracked_param] bp = 'untracked'
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm_cur_stree new [register_capped] bp->hw.target = 'capped'
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm_cur_stree new [register_comparison] bp vs bp orig = '=='
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm_cur_stree new [register_comparison_links] bp = 'bp vs bp orig'
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm_cur_stree new [register_statement_count] stmts = '6'
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm_cur_stree new [register_kernel_user_data2] this_function = 'called'
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm_cur_stree new [check_deref] tmp = 'ok'
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm_cur_stree new [check_dereferences_param] bp = 'param'
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm_cur_stree new [check_uninitialized] tmp = 'initialized'
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm_cur_stree new [register_assigned_expr] __fake_param_0x7da6ac1c7630_0 = 'tmp'
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm_cur_stree new [register_assigned_expr] tmp = 'alloc_breakpoint(bp)'
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm_cur_stree new [register_assigned_expr_links] tmp = '__fake_param_0x7da6ac1c7450_0, __fake_param_0x7da6ac1c7630_0'
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm_cur_stree change [internal] unnull_path = 'true' (was true)
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() set_state change [register_definition_db_callbacks] 'db_incomplete' incomplete => incomplete
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() set_state new [internal] 'unnull_path' true
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm_cur_stree new [register_smatch_extra] __fake_param_0x7da6ac1c7630_0 = '0-u64max'
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm_cur_stree new [register_smatch_extra] bp = '4096-ptr_max'
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm_cur_stree new [register_smatch_extra] bp->attr.bp_addr = '0-13835058055282163711'
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm_cur_stree new [register_smatch_extra] bp->cpu = '(-1)'
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm_cur_stree new [register_smatch_extra] bp->hw.target = '1-u64max'
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm_cur_stree new [register_smatch_extra] bp->overflow_handler = '0-4644892087836254207,4644892087836254209-u64max'
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm_cur_stree new [register_smatch_extra] tmp = '0-u64max'
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm_cur_stree new [register_modification_hooks] *bp = 'alloc_breakpoint(bp)'
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm_cur_stree new [register_modification_hooks] *tmp = 'IS_ERR(tmp)'
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm_cur_stree new [register_modification_hooks] tmp = 'tmp = alloc_breakpoint(bp)'
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm_cur_stree new [register_definition_db_callbacks] db_incomplete = 'incomplete'
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm_cur_stree new [register_untracked_param] bp = 'untracked'
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm_cur_stree new [register_capped] bp->hw.target = 'capped'
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm_cur_stree new [register_comparison] bp vs bp orig = '=='
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm_cur_stree new [register_comparison_links] bp = 'bp vs bp orig'
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm_cur_stree new [register_statement_count] stmts = '6'
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm_cur_stree new [register_kernel_user_data2] this_function = 'called'
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm_cur_stree new [check_deref] tmp = 'ok'
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm_cur_stree new [check_dereferences_param] bp = 'param'
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm_cur_stree new [check_uninitialized] tmp = 'initialized'
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm_cur_stree new [register_assigned_expr] __fake_param_0x7da6ac1c7630_0 = 'tmp'
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm_cur_stree new [register_assigned_expr] tmp = 'alloc_breakpoint(bp)'
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm_cur_stree new [register_assigned_expr_links] tmp = '__fake_param_0x7da6ac1c7450_0, __fake_param_0x7da6ac1c7630_0'
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm_cur_stree change [internal] unnull_path = 'true' (was true)
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_true_false_sm [register_definition_db_callbacks] 'db_incomplete'.  Was incomplete.  Now T:incomplete F:(null)
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_true_false_sm [register_definition_db_callbacks] 'db_incomplete'.  Was incomplete.  Now T:(null) F:incomplete
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() set_state new [register_untracked_param] 'tmp' untracked
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_true_false_sm [check_err_ptr_deref] 'tmp'.  Was (null).  Now T:err_ptr F:(null)
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_true_false_sm [check_err_ptr_deref] 'tmp'.  Was (null).  Now T:(null) F:checked
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() set_true_false_states [register_parsed_conditions] 'condition 0x7da6ac1c7630'.  Was (null).  Now T:true_path F:false_path
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm new [register_untracked_param] tmp = 'untracked'
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_true_false_sm [register_untracked_param] 'tmp'.  Was untracked.  Now T:untracked F:untracked
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm change [internal] unnull_path = 'true' (was true)
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_true_false_sm [internal] 'unnull_path'.  Was true.  Now T:true F:true
179 done __split_whole_condition
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() set_state change [register_statement_count] 'stmts' 6 => 7
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:180 task_bps_add() set_state change [register_statement_count] 'stmts' 7 => 8
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:178 task_bps_add() set_state change [register_definition_db_callbacks] 'db_incomplete' incomplete => incomplete
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:178 task_bps_add() __set_sm change [register_definition_db_callbacks] db_incomplete = 'incomplete' (was incomplete)
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() set_state change [register_statement_count] 'stmts' 5 => 6
179 in __split_whole_condition
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() 179 in split_conditions (IS_ERR(tmp))
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() set_state new [register_smatch_extra] '__fake_param_0x7da6ac1c7630_0' 0-u64max
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm change [register_smatch_extra] __fake_param_0x7da6ac1c7630_0 = '0-u64max' (was 0-u64max)
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm change [register_smatch_extra] tmp = '0-u64max' (was 0-u64max)
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm change [register_smatch_extra] __fake_param_0x7da6ac1c7630_0 = '0-u64max' (was 0-u64max)
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() set_state new [register_comparison] '__fake_param_0x7da6ac1c7630_0 vs return 0x7da6ac1c7360' ==
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() set_state new [register_comparison_links] '__fake_param_0x7da6ac1c7630_0' __fake_param_0x7da6ac1c7630_0 vs return 0x7da6ac1c7360
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() set_state new [register_comparison] '__fake_param_0x7da6ac1c7630_0 vs tmp' ==
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() set_state change [register_comparison_links] '__fake_param_0x7da6ac1c7630_0' __fake_param_0x7da6ac1c7630_0 vs return 0x7da6ac1c7360 => __fake_param_0x7da6ac1c7630_0 vs return 0x7da6ac1c7360, __fake_param_0x7da6ac1c7630_0 vs tmp
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() set_state change [register_comparison_links] 'tmp' __fake_param_0x7da6ac1c7450_0 vs tmp, return 0x7da6ac1c7360 vs tmp => __fake_param_0x7da6ac1c7450_0 vs tmp, __fake_param_0x7da6ac1c7630_0 vs tmp, return 0x7da6ac1c7360 vs tmp
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() set_state new [register_assigned_expr] '__fake_param_0x7da6ac1c7630_0' tmp
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() set_state change [register_assigned_expr_links] 'tmp' __fake_param_0x7da6ac1c7450_0 => __fake_param_0x7da6ac1c7450_0, __fake_param_0x7da6ac1c7630_0
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() set_state change [register_modification_hooks] '*tmp' __smatch_about(tmp) => IS_ERR(tmp)
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm new [register_smatch_extra] __fake_param_0x7da6ac1c7630_0 = '0-u64max'
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_true_false_sm [register_smatch_extra] '__fake_param_0x7da6ac1c7630_0'.  Was 0-u64max.  Now T:0-u64max F:0-u64max
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm change [register_smatch_extra] tmp = '0-u64max' (was 0-u64max)
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_true_false_sm [register_smatch_extra] 'tmp'.  Was 0-u64max.  Now T:0-u64max F:0-u64max
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm change [register_modification_hooks] *tmp = 'IS_ERR(tmp)' (was __smatch_about(tmp))
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_true_false_sm [register_modification_hooks] '*tmp'.  Was IS_ERR(tmp).  Now T:IS_ERR(tmp) F:IS_ERR(tmp)
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm new [register_comparison] __fake_param_0x7da6ac1c7630_0 vs return 0x7da6ac1c7360 = '=='
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_true_false_sm [register_comparison] '__fake_param_0x7da6ac1c7630_0 vs return 0x7da6ac1c7360'.  Was ==.  Now T:== F:==
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm new [register_comparison] __fake_param_0x7da6ac1c7630_0 vs tmp = '=='
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_true_false_sm [register_comparison] '__fake_param_0x7da6ac1c7630_0 vs tmp'.  Was ==.  Now T:== F:==
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm new [register_comparison_links] __fake_param_0x7da6ac1c7630_0 = '__fake_param_0x7da6ac1c7630_0 vs return 0x7da6ac1c7360, __fake_param_0x7da6ac1c7630_0 vs tmp'
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_true_false_sm [register_comparison_links] '__fake_param_0x7da6ac1c7630_0'.  Was __fake_param_0x7da6ac1c7630_0 vs return 0x7da6ac1c7360, __fake_param_0x7da6ac1c7630_0 vs tmp.  Now T:__fake_param_0x7da6ac1c7630_0 vs return 0x7da6ac1c7360, __fake_param_0x7da6ac1c7630_0 vs tmp F:__fake_param_0x7da6ac1c7630_0 vs return 0x7da6ac1c7360, __fake_param_0x7da6ac1c7630_0 vs tmp
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm change [register_comparison_links] tmp = '__fake_param_0x7da6ac1c7450_0 vs tmp, __fake_param_0x7da6ac1c7630_0 vs tmp, return 0x7da6ac1c7360 vs tmp' (was __fake_param_0x7da6ac1c7450_0 vs tmp, return 0x7da6ac1c7360 vs tmp)
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_true_false_sm [register_comparison_links] 'tmp'.  Was __fake_param_0x7da6ac1c7450_0 vs tmp, __fake_param_0x7da6ac1c7630_0 vs tmp, return 0x7da6ac1c7360 vs tmp.  Now T:__fake_param_0x7da6ac1c7450_0 vs tmp, __fake_param_0x7da6ac1c7630_0 vs tmp, return 0x7da6ac1c7360 vs tmp F:__fake_param_0x7da6ac1c7450_0 vs tmp, __fake_param_0x7da6ac1c7630_0 vs tmp, return 0x7da6ac1c7360 vs tmp
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm new [register_assigned_expr] __fake_param_0x7da6ac1c7630_0 = 'tmp'
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_true_false_sm [register_assigned_expr] '__fake_param_0x7da6ac1c7630_0'.  Was tmp.  Now T:tmp F:tmp
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm change [register_assigned_expr_links] tmp = '__fake_param_0x7da6ac1c7450_0, __fake_param_0x7da6ac1c7630_0' (was __fake_param_0x7da6ac1c7450_0)
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_true_false_sm [register_assigned_expr_links] 'tmp'.  Was __fake_param_0x7da6ac1c7450_0, __fake_param_0x7da6ac1c7630_0.  Now T:__fake_param_0x7da6ac1c7450_0, __fake_param_0x7da6ac1c7630_0 F:__fake_param_0x7da6ac1c7450_0, __fake_param_0x7da6ac1c7630_0
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() set_state new [check_err_ptr_deref] 'tmp' err_ptr
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() set_state new [check_err_ptr_deref] 'tmp' checked
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() set_state change [register_definition_db_callbacks] 'db_incomplete' incomplete => incomplete
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() set_state new [internal] 'unnull_path' true
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm_cur_stree new [register_smatch_extra] __fake_param_0x7da6ac1c7630_0 = '0-u64max'
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm_cur_stree new [register_smatch_extra] bp = '4096-ptr_max'
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm_cur_stree new [register_smatch_extra] bp->attr.bp_addr = '0-13835058055282163711'
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm_cur_stree new [register_smatch_extra] bp->overflow_handler = '0-4644892087836254207,4644892087836254209-u64max'
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm_cur_stree new [register_smatch_extra] tmp = '0-u64max'
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm_cur_stree new [register_modification_hooks] *bp = 'alloc_breakpoint(bp)'
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm_cur_stree new [register_modification_hooks] *tmp = 'IS_ERR(tmp)'
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm_cur_stree new [register_modification_hooks] tmp = 'tmp = alloc_breakpoint(bp)'
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm_cur_stree new [register_definition_db_callbacks] db_incomplete = 'incomplete'
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm_cur_stree new [register_untracked_param] bp = 'untracked'
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm_cur_stree new [register_comparison] __fake_param_0x7da6ac1c7630_0 vs return 0x7da6ac1c7360 = '=='
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm_cur_stree new [register_comparison] __fake_param_0x7da6ac1c7630_0 vs tmp = '=='
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm_cur_stree new [register_comparison] bp vs bp orig = '=='
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm_cur_stree new [register_comparison] return 0x7da6ac1c7360 vs tmp = '=='
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm_cur_stree new [register_comparison_links] __fake_param_0x7da6ac1c7630_0 = '__fake_param_0x7da6ac1c7630_0 vs return 0x7da6ac1c7360, __fake_param_0x7da6ac1c7630_0 vs tmp'
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm_cur_stree new [register_comparison_links] bp = 'bp vs bp orig'
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm_cur_stree new [register_comparison_links] return 0x7da6ac1c7360 = 'return 0x7da6ac1c7360 vs tmp'
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm_cur_stree new [register_comparison_links] tmp = '__fake_param_0x7da6ac1c7450_0 vs tmp, __fake_param_0x7da6ac1c7630_0 vs tmp, return 0x7da6ac1c7360 vs tmp'
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm_cur_stree new [register_statement_count] stmts = '6'
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm_cur_stree new [register_kernel_user_data2] this_function = 'called'
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm_cur_stree new [check_deref] tmp = 'ok'
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm_cur_stree new [check_dereferences_param] bp = 'param'
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm_cur_stree new [check_uninitialized] tmp = 'initialized'
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm_cur_stree new [register_assigned_expr] __fake_param_0x7da6ac1c7630_0 = 'tmp'
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm_cur_stree new [register_assigned_expr] tmp = 'alloc_breakpoint(bp)'
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm_cur_stree new [register_assigned_expr_links] tmp = '__fake_param_0x7da6ac1c7450_0, __fake_param_0x7da6ac1c7630_0'
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm_cur_stree change [internal] unnull_path = 'true' (was true)
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() set_state change [register_definition_db_callbacks] 'db_incomplete' incomplete => incomplete
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() set_state new [internal] 'unnull_path' true
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm_cur_stree new [register_smatch_extra] __fake_param_0x7da6ac1c7630_0 = '0-u64max'
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm_cur_stree new [register_smatch_extra] bp = '4096-ptr_max'
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm_cur_stree new [register_smatch_extra] bp->attr.bp_addr = '0-13835058055282163711'
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm_cur_stree new [register_smatch_extra] bp->overflow_handler = '0-4644892087836254207,4644892087836254209-u64max'
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm_cur_stree new [register_smatch_extra] tmp = '0-u64max'
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm_cur_stree new [register_modification_hooks] *bp = 'alloc_breakpoint(bp)'
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm_cur_stree new [register_modification_hooks] *tmp = 'IS_ERR(tmp)'
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm_cur_stree new [register_modification_hooks] tmp = 'tmp = alloc_breakpoint(bp)'
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm_cur_stree new [register_definition_db_callbacks] db_incomplete = 'incomplete'
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm_cur_stree new [register_untracked_param] bp = 'untracked'
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm_cur_stree new [register_comparison] __fake_param_0x7da6ac1c7630_0 vs return 0x7da6ac1c7360 = '=='
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm_cur_stree new [register_comparison] __fake_param_0x7da6ac1c7630_0 vs tmp = '=='
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm_cur_stree new [register_comparison] bp vs bp orig = '=='
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm_cur_stree new [register_comparison] return 0x7da6ac1c7360 vs tmp = '=='
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm_cur_stree new [register_comparison_links] __fake_param_0x7da6ac1c7630_0 = '__fake_param_0x7da6ac1c7630_0 vs return 0x7da6ac1c7360, __fake_param_0x7da6ac1c7630_0 vs tmp'
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm_cur_stree new [register_comparison_links] bp = 'bp vs bp orig'
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm_cur_stree new [register_comparison_links] return 0x7da6ac1c7360 = 'return 0x7da6ac1c7360 vs tmp'
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm_cur_stree new [register_comparison_links] tmp = '__fake_param_0x7da6ac1c7450_0 vs tmp, __fake_param_0x7da6ac1c7630_0 vs tmp, return 0x7da6ac1c7360 vs tmp'
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm_cur_stree new [register_statement_count] stmts = '6'
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm_cur_stree new [register_kernel_user_data2] this_function = 'called'
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm_cur_stree new [check_deref] tmp = 'ok'
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm_cur_stree new [check_dereferences_param] bp = 'param'
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm_cur_stree new [check_uninitialized] tmp = 'initialized'
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm_cur_stree new [register_assigned_expr] __fake_param_0x7da6ac1c7630_0 = 'tmp'
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm_cur_stree new [register_assigned_expr] tmp = 'alloc_breakpoint(bp)'
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm_cur_stree new [register_assigned_expr_links] tmp = '__fake_param_0x7da6ac1c7450_0, __fake_param_0x7da6ac1c7630_0'
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm_cur_stree change [internal] unnull_path = 'true' (was true)
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_true_false_sm [register_definition_db_callbacks] 'db_incomplete'.  Was incomplete.  Now T:incomplete F:(null)
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_true_false_sm [register_definition_db_callbacks] 'db_incomplete'.  Was incomplete.  Now T:(null) F:incomplete
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() set_state new [register_untracked_param] 'tmp' untracked
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_true_false_sm [check_err_ptr_deref] 'tmp'.  Was (null).  Now T:err_ptr F:(null)
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_true_false_sm [check_err_ptr_deref] 'tmp'.  Was (null).  Now T:(null) F:checked
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() set_true_false_states [register_parsed_conditions] 'condition 0x7da6ac1c7630'.  Was (null).  Now T:true_path F:false_path
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm new [register_untracked_param] tmp = 'untracked'
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_true_false_sm [register_untracked_param] 'tmp'.  Was untracked.  Now T:untracked F:untracked
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_sm change [internal] unnull_path = 'true' (was true)
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() __set_true_false_sm [internal] 'unnull_path'.  Was true.  Now T:true F:true
179 done __split_whole_condition
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:179 task_bps_add() set_state change [register_statement_count] 'stmts' 6 => 7
/home/michael/linux/arch/powerpc/kernel/hw_breakpoint.c:180 task_bps_add() set_state change [register_statement_count] 'stmts' 7 => 8

Re: [PATCH] powerpc: Initialize local variable fdt to NULL in elf64_load()

[Lakshmi Ramasubramanian]

On 4/16/21 2:05 AM, Michael Ellerman wrote:

> Daniel Axtens <dja@axtens.net> writes:
>>> 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...
>>
>> The out label reads in part:
>>
>> 	/* Make kimage_file_post_load_cleanup free the fdt buffer for us. */
>> 	return ret ? ERR_PTR(ret) : fdt;
>>
>> As far as I can tell, any time we get a non-zero ret, we're going to
>> return an error pointer rather than the uninitialised value...

As Dan pointed out, the new code is in linux-next.

I have copied the new one below - the function doesn't return fdt, but 
instead sets it in the arch specific field (please see the link to the 
updated elf_64.c below).

https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git/tree/arch/powerpc/kexec/elf_64.c?h=for-next

>>
>> (btw, it does look like we might leak fdt if we have an error after we
>> successfully kmalloc it.)
>>
>> Am I missing something? Can you link to the report for the kernel test
>> robot or from Dan?

/*
          * Once FDT buffer has been successfully passed to 
kexec_add_buffer(),
          * the FDT buffer address is saved in image->arch.fdt. In that 
case,
          * the memory cannot be freed here in case of any other error.
          */
         if (ret && !image->arch.fdt)
                 kvfree(fdt);

         return ret ? ERR_PTR(ret) : NULL;

In case of an error, the memory allocated for fdt is freed unless it has 
already been passed to kexec_add_buffer().

thanks,
  -lakshmi

>>
>> FWIW, I think it's worth including this patch _anyway_ because initing
>> local variables is good practice, but I'm just not sure on the
>> justification.
> 
> Why is it good practice?
> 
> It defeats -Wuninitialized. So you're guaranteed to be returning
> something initialised, but not necessarily initialised to the right
> value.
> 
> In a case like this NULL seems like a safe choice, but it's still wrong.
> The function is meant to return a pointer to the successfully allocated
> fdt, or an ERR_PTR() value. NULL is neither of those.
> 
> I agree there are security reasons that initialising stack variables is
> desirable, but I think that should be handled by the compiler, not at
> the source level.
> 
> cheers
> 

Re: [PATCH] powerpc: Initialize local variable fdt to NULL in elf64_load()

[Michael Ellerman]

Lakshmi Ramasubramanian <nramas@linux.microsoft.com> writes:
> On 4/16/21 2:05 AM, Michael Ellerman wrote:
>
>> Daniel Axtens <dja@axtens.net> writes:
>>>> 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...
>>>
>>> The out label reads in part:
>>>
>>> 	/* Make kimage_file_post_load_cleanup free the fdt buffer for us. */
>>> 	return ret ? ERR_PTR(ret) : fdt;
>>>
>>> As far as I can tell, any time we get a non-zero ret, we're going to
>>> return an error pointer rather than the uninitialised value...
>
> As Dan pointed out, the new code is in linux-next.
>
> I have copied the new one below - the function doesn't return fdt, but 
> instead sets it in the arch specific field (please see the link to the 
> updated elf_64.c below).
>
> https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git/tree/arch/powerpc/kexec/elf_64.c?h=for-next
>
>>>
>>> (btw, it does look like we might leak fdt if we have an error after we
>>> successfully kmalloc it.)
>>>
>>> Am I missing something? Can you link to the report for the kernel test
>>> robot or from Dan?
>
> /*
>           * Once FDT buffer has been successfully passed to 
> kexec_add_buffer(),
>           * the FDT buffer address is saved in image->arch.fdt. In that 
> case,
>           * the memory cannot be freed here in case of any other error.
>           */
>          if (ret && !image->arch.fdt)
>                  kvfree(fdt);
>
>          return ret ? ERR_PTR(ret) : NULL;
>
> In case of an error, the memory allocated for fdt is freed unless it has 
> already been passed to kexec_add_buffer().

It feels like the root of the problem is that the kvfree of fdt is in
the wrong place. It's only allocated later in the function, so the error
path should reflect that. Something like the patch below.

cheers


diff --git a/arch/powerpc/kexec/elf_64.c b/arch/powerpc/kexec/elf_64.c
index 5a569bb51349..02662e72c53d 100644
--- a/arch/powerpc/kexec/elf_64.c
+++ b/arch/powerpc/kexec/elf_64.c
@@ -114,7 +114,7 @@ static void *elf64_load(struct kimage *image, char *kernel_buf,
 	ret = setup_new_fdt_ppc64(image, fdt, initrd_load_addr,
 				  initrd_len, cmdline);
 	if (ret)
-		goto out;
+		goto out_free_fdt;
 
 	fdt_pack(fdt);
 
@@ -125,7 +125,7 @@ static void *elf64_load(struct kimage *image, char *kernel_buf,
 	kbuf.mem = KEXEC_BUF_MEM_UNKNOWN;
 	ret = kexec_add_buffer(&kbuf);
 	if (ret)
-		goto out;
+		goto out_free_fdt;
 
 	/* FDT will be freed in arch_kimage_file_post_load_cleanup */
 	image->arch.fdt = fdt;
@@ -140,18 +140,14 @@ static void *elf64_load(struct kimage *image, char *kernel_buf,
 	if (ret)
 		pr_err("Error setting up the purgatory.\n");
 
+	goto out;
+
+out_free_fdt:
+	kvfree(fdt);
 out:
 	kfree(modified_cmdline);
 	kexec_free_elf_info(&elf_info);
 
-	/*
-	 * Once FDT buffer has been successfully passed to kexec_add_buffer(),
-	 * the FDT buffer address is saved in image->arch.fdt. In that case,
-	 * the memory cannot be freed here in case of any other error.
-	 */
-	if (ret && !image->arch.fdt)
-		kvfree(fdt);
-
 	return ret ? ERR_PTR(ret) : NULL;
 }
 

Re: [PATCH] powerpc: Initialize local variable fdt to NULL in elf64_load()

[Lakshmi Ramasubramanian]

On 4/19/21 4:30 PM, Michael Ellerman wrote:
> Lakshmi Ramasubramanian <nramas@linux.microsoft.com> writes:
>> On 4/16/21 2:05 AM, Michael Ellerman wrote:
>>
>>> Daniel Axtens <dja@axtens.net> writes:
>>>>> 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...
>>>>
>>>> The out label reads in part:
>>>>
>>>> 	/* Make kimage_file_post_load_cleanup free the fdt buffer for us. */
>>>> 	return ret ? ERR_PTR(ret) : fdt;
>>>>
>>>> As far as I can tell, any time we get a non-zero ret, we're going to
>>>> return an error pointer rather than the uninitialised value...
>>
>> As Dan pointed out, the new code is in linux-next.
>>
>> I have copied the new one below - the function doesn't return fdt, but
>> instead sets it in the arch specific field (please see the link to the
>> updated elf_64.c below).
>>
>> https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git/tree/arch/powerpc/kexec/elf_64.c?h=for-next
>>
>>>>
>>>> (btw, it does look like we might leak fdt if we have an error after we
>>>> successfully kmalloc it.)
>>>>
>>>> Am I missing something? Can you link to the report for the kernel test
>>>> robot or from Dan?
>>
>> /*
>>            * Once FDT buffer has been successfully passed to
>> kexec_add_buffer(),
>>            * the FDT buffer address is saved in image->arch.fdt. In that
>> case,
>>            * the memory cannot be freed here in case of any other error.
>>            */
>>           if (ret && !image->arch.fdt)
>>                   kvfree(fdt);
>>
>>           return ret ? ERR_PTR(ret) : NULL;
>>
>> In case of an error, the memory allocated for fdt is freed unless it has
>> already been passed to kexec_add_buffer().
> 
> It feels like the root of the problem is that the kvfree of fdt is in
> the wrong place. It's only allocated later in the function, so the error
> path should reflect that. Something like the patch below.
> 
> cheers
> 
> 
> diff --git a/arch/powerpc/kexec/elf_64.c b/arch/powerpc/kexec/elf_64.c
> index 5a569bb51349..02662e72c53d 100644
> --- a/arch/powerpc/kexec/elf_64.c
> +++ b/arch/powerpc/kexec/elf_64.c
> @@ -114,7 +114,7 @@ static void *elf64_load(struct kimage *image, char *kernel_buf,
>   	ret = setup_new_fdt_ppc64(image, fdt, initrd_load_addr,
>   				  initrd_len, cmdline);
>   	if (ret)
> -		goto out;
> +		goto out_free_fdt;
>   
>   	fdt_pack(fdt);
>   
> @@ -125,7 +125,7 @@ static void *elf64_load(struct kimage *image, char *kernel_buf,
>   	kbuf.mem = KEXEC_BUF_MEM_UNKNOWN;
>   	ret = kexec_add_buffer(&kbuf);
>   	if (ret)
> -		goto out;
> +		goto out_free_fdt;
>   
>   	/* FDT will be freed in arch_kimage_file_post_load_cleanup */
>   	image->arch.fdt = fdt;
> @@ -140,18 +140,14 @@ static void *elf64_load(struct kimage *image, char *kernel_buf,
>   	if (ret)
>   		pr_err("Error setting up the purgatory.\n");
>   
> +	goto out;
> +
> +out_free_fdt:
> +	kvfree(fdt);
>   out:
>   	kfree(modified_cmdline);
>   	kexec_free_elf_info(&elf_info);
>   
> -	/*
> -	 * Once FDT buffer has been successfully passed to kexec_add_buffer(),
> -	 * the FDT buffer address is saved in image->arch.fdt. In that case,
> -	 * the memory cannot be freed here in case of any other error.
> -	 */
> -	if (ret && !image->arch.fdt)
> -		kvfree(fdt);
> -
>   	return ret ? ERR_PTR(ret) : NULL;
>   }
>   
> 

This looks good to me. Thanks Michael.

I'll post the updated patch shortly.

  -lakshmi

Re: [PATCH] powerpc: Initialize local variable fdt to NULL in elf64_load()

[Dan Carpenter]

On Tue, Apr 20, 2021 at 09:30:16AM +1000, Michael Ellerman wrote:
> Lakshmi Ramasubramanian <nramas@linux.microsoft.com> writes:
> > On 4/16/21 2:05 AM, Michael Ellerman wrote:
> >
> >> Daniel Axtens <dja@axtens.net> writes:
> >>>> 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...
> >>>
> >>> The out label reads in part:
> >>>
> >>> 	/* Make kimage_file_post_load_cleanup free the fdt buffer for us. */
> >>> 	return ret ? ERR_PTR(ret) : fdt;
> >>>
> >>> As far as I can tell, any time we get a non-zero ret, we're going to
> >>> return an error pointer rather than the uninitialised value...
> >
> > As Dan pointed out, the new code is in linux-next.
> >
> > I have copied the new one below - the function doesn't return fdt, but 
> > instead sets it in the arch specific field (please see the link to the 
> > updated elf_64.c below).
> >
> > https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git/tree/arch/powerpc/kexec/elf_64.c?h=for-next 
> >
> >>>
> >>> (btw, it does look like we might leak fdt if we have an error after we
> >>> successfully kmalloc it.)
> >>>
> >>> Am I missing something? Can you link to the report for the kernel test
> >>> robot or from Dan?
> >
> > /*
> >           * Once FDT buffer has been successfully passed to 
> > kexec_add_buffer(),
> >           * the FDT buffer address is saved in image->arch.fdt. In that 
> > case,
> >           * the memory cannot be freed here in case of any other error.
> >           */
> >          if (ret && !image->arch.fdt)
> >                  kvfree(fdt);
> >
> >          return ret ? ERR_PTR(ret) : NULL;
> >
> > In case of an error, the memory allocated for fdt is freed unless it has 
> > already been passed to kexec_add_buffer().
> 
> It feels like the root of the problem is that the kvfree of fdt is in
> the wrong place. It's only allocated later in the function, so the error
> path should reflect that. Something like the patch below.
> 
> cheers
> 
> 
> diff --git a/arch/powerpc/kexec/elf_64.c b/arch/powerpc/kexec/elf_64.c
> index 5a569bb51349..02662e72c53d 100644
> --- a/arch/powerpc/kexec/elf_64.c
> +++ b/arch/powerpc/kexec/elf_64.c
> @@ -114,7 +114,7 @@ static void *elf64_load(struct kimage *image, char *kernel_buf,
>  	ret = setup_new_fdt_ppc64(image, fdt, initrd_load_addr,
>  				  initrd_len, cmdline);
>  	if (ret)
> -		goto out;
> +		goto out_free_fdt;
>  
>  	fdt_pack(fdt);
>  
> @@ -125,7 +125,7 @@ static void *elf64_load(struct kimage *image, char *kernel_buf,
>  	kbuf.mem = KEXEC_BUF_MEM_UNKNOWN;
>  	ret = kexec_add_buffer(&kbuf);
>  	if (ret)
> -		goto out;
> +		goto out_free_fdt;
>  
>  	/* FDT will be freed in arch_kimage_file_post_load_cleanup */
>  	image->arch.fdt = fdt;
> @@ -140,18 +140,14 @@ static void *elf64_load(struct kimage *image, char *kernel_buf,
>  	if (ret)
>  		pr_err("Error setting up the purgatory.\n");
>  
> +	goto out;

This will leak.  It would need to be something like:

	if (ret) {
		pr_err("Error setting up the purgatory.\n");
		goto out_free_fdt;
	}

	goto out;

But we should also fix the uninitialized variable of "elf_info" if
kexec_build_elf_info() fails.

> +
> +out_free_fdt:
> +	kvfree(fdt);
>  out:
>  	kfree(modified_cmdline);
>  	kexec_free_elf_info(&elf_info);
>  
> -	/*
> -	 * Once FDT buffer has been successfully passed to kexec_add_buffer(),
> -	 * the FDT buffer address is saved in image->arch.fdt. In that case,
> -	 * the memory cannot be freed here in case of any other error.
> -	 */
> -	if (ret && !image->arch.fdt)
> -		kvfree(fdt);
> -
>  	return ret ? ERR_PTR(ret) : NULL;
>  }

regards,
dan carpenter

Re: [PATCH] powerpc: Initialize local variable fdt to NULL in elf64_load()

[Lakshmi Ramasubramanian]

On 4/19/21 10:00 PM, Dan Carpenter wrote:
> On Tue, Apr 20, 2021 at 09:30:16AM +1000, Michael Ellerman wrote:
>> Lakshmi Ramasubramanian <nramas@linux.microsoft.com> writes:
>>> On 4/16/21 2:05 AM, Michael Ellerman wrote:
>>>
>>>> Daniel Axtens <dja@axtens.net> writes:
>>>>>> 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...
>>>>>
>>>>> The out label reads in part:
>>>>>
>>>>> 	/* Make kimage_file_post_load_cleanup free the fdt buffer for us. */
>>>>> 	return ret ? ERR_PTR(ret) : fdt;
>>>>>
>>>>> As far as I can tell, any time we get a non-zero ret, we're going to
>>>>> return an error pointer rather than the uninitialised value...
>>>
>>> As Dan pointed out, the new code is in linux-next.
>>>
>>> I have copied the new one below - the function doesn't return fdt, but
>>> instead sets it in the arch specific field (please see the link to the
>>> updated elf_64.c below).
>>>
>>> https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git/tree/arch/powerpc/kexec/elf_64.c?h=for-next
>>>
>>>>>
>>>>> (btw, it does look like we might leak fdt if we have an error after we
>>>>> successfully kmalloc it.)
>>>>>
>>>>> Am I missing something? Can you link to the report for the kernel test
>>>>> robot or from Dan?
>>>
>>> /*
>>>            * Once FDT buffer has been successfully passed to
>>> kexec_add_buffer(),
>>>            * the FDT buffer address is saved in image->arch.fdt. In that
>>> case,
>>>            * the memory cannot be freed here in case of any other error.
>>>            */
>>>           if (ret && !image->arch.fdt)
>>>                   kvfree(fdt);
>>>
>>>           return ret ? ERR_PTR(ret) : NULL;
>>>
>>> In case of an error, the memory allocated for fdt is freed unless it has
>>> already been passed to kexec_add_buffer().
>>
>> It feels like the root of the problem is that the kvfree of fdt is in
>> the wrong place. It's only allocated later in the function, so the error
>> path should reflect that. Something like the patch below.
>>
>> cheers
>>
>>
>> diff --git a/arch/powerpc/kexec/elf_64.c b/arch/powerpc/kexec/elf_64.c
>> index 5a569bb51349..02662e72c53d 100644
>> --- a/arch/powerpc/kexec/elf_64.c
>> +++ b/arch/powerpc/kexec/elf_64.c
>> @@ -114,7 +114,7 @@ static void *elf64_load(struct kimage *image, char *kernel_buf,
>>   	ret = setup_new_fdt_ppc64(image, fdt, initrd_load_addr,
>>   				  initrd_len, cmdline);
>>   	if (ret)
>> -		goto out;
>> +		goto out_free_fdt;
>>   
>>   	fdt_pack(fdt);
>>   
>> @@ -125,7 +125,7 @@ static void *elf64_load(struct kimage *image, char *kernel_buf,
>>   	kbuf.mem = KEXEC_BUF_MEM_UNKNOWN;
>>   	ret = kexec_add_buffer(&kbuf);
>>   	if (ret)
>> -		goto out;
>> +		goto out_free_fdt;
>>   
>>   	/* FDT will be freed in arch_kimage_file_post_load_cleanup */
>>   	image->arch.fdt = fdt;
>> @@ -140,18 +140,14 @@ static void *elf64_load(struct kimage *image, char *kernel_buf,
>>   	if (ret)
>>   		pr_err("Error setting up the purgatory.\n");
>>   
>> +	goto out;
> 
> This will leak.  It would need to be something like:
> 
> 	if (ret) {
> 		pr_err("Error setting up the purgatory.\n");
> 		goto out_free_fdt;
> 	}
Once "fdt" buffer is successfully passed to kexec_add_buffer() it cannot 
be freed here - it will be freed when the kexec cleanup function is called.

> 
> 	goto out;
> 
> But we should also fix the uninitialized variable of "elf_info" if
> kexec_build_elf_info() fails.

kexec_build_elf_info() frees elf_info and zeroes it in error paths, 
except when elf_read_ehdr() fails. So, I think it is better to 
initialize the local variable "elf_info" before calling 
kexec_build_elf_info().

	memset(&elf_info, 0, sizeof(elf_info));

thanks,
  -lakshmi

> 
>> +
>> +out_free_fdt:
>> +	kvfree(fdt);
>>   out:
>>   	kfree(modified_cmdline);
>>   	kexec_free_elf_info(&elf_info);
>>   
>> -	/*
>> -	 * Once FDT buffer has been successfully passed to kexec_add_buffer(),
>> -	 * the FDT buffer address is saved in image->arch.fdt. In that case,
>> -	 * the memory cannot be freed here in case of any other error.
>> -	 */
>> -	if (ret && !image->arch.fdt)
>> -		kvfree(fdt);
>> -
>>   	return ret ? ERR_PTR(ret) : NULL;
>>   }
> 
> regards,
> dan carpenter
> 

Re: [PATCH] powerpc: Initialize local variable fdt to NULL in elf64_load()

[Rob Herring]

On Tue, Apr 20, 2021 at 12:20 AM Lakshmi Ramasubramanian
<nramas@linux.microsoft.com> wrote:
>
> On 4/19/21 10:00 PM, Dan Carpenter wrote:
> > On Tue, Apr 20, 2021 at 09:30:16AM +1000, Michael Ellerman wrote:
> >> Lakshmi Ramasubramanian <nramas@linux.microsoft.com> writes:
> >>> On 4/16/21 2:05 AM, Michael Ellerman wrote:
> >>>
> >>>> Daniel Axtens <dja@axtens.net> writes:
> >>>>>> 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...
> >>>>>
> >>>>> The out label reads in part:
> >>>>>
> >>>>>   /* Make kimage_file_post_load_cleanup free the fdt buffer for us. */
> >>>>>   return ret ? ERR_PTR(ret) : fdt;
> >>>>>
> >>>>> As far as I can tell, any time we get a non-zero ret, we're going to
> >>>>> return an error pointer rather than the uninitialised value...
> >>>
> >>> As Dan pointed out, the new code is in linux-next.
> >>>
> >>> I have copied the new one below - the function doesn't return fdt, but
> >>> instead sets it in the arch specific field (please see the link to the
> >>> updated elf_64.c below).
> >>>
> >>> https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git/tree/arch/powerpc/kexec/elf_64.c?h=for-next
> >>>
> >>>>>
> >>>>> (btw, it does look like we might leak fdt if we have an error after we
> >>>>> successfully kmalloc it.)
> >>>>>
> >>>>> Am I missing something? Can you link to the report for the kernel test
> >>>>> robot or from Dan?
> >>>
> >>> /*
> >>>            * Once FDT buffer has been successfully passed to
> >>> kexec_add_buffer(),
> >>>            * the FDT buffer address is saved in image->arch.fdt. In that
> >>> case,
> >>>            * the memory cannot be freed here in case of any other error.
> >>>            */
> >>>           if (ret && !image->arch.fdt)
> >>>                   kvfree(fdt);
> >>>
> >>>           return ret ? ERR_PTR(ret) : NULL;
> >>>
> >>> In case of an error, the memory allocated for fdt is freed unless it has
> >>> already been passed to kexec_add_buffer().
> >>
> >> It feels like the root of the problem is that the kvfree of fdt is in
> >> the wrong place. It's only allocated later in the function, so the error
> >> path should reflect that. Something like the patch below.
> >>
> >> cheers
> >>
> >>
> >> diff --git a/arch/powerpc/kexec/elf_64.c b/arch/powerpc/kexec/elf_64.c
> >> index 5a569bb51349..02662e72c53d 100644
> >> --- a/arch/powerpc/kexec/elf_64.c
> >> +++ b/arch/powerpc/kexec/elf_64.c
> >> @@ -114,7 +114,7 @@ static void *elf64_load(struct kimage *image, char *kernel_buf,
> >>      ret = setup_new_fdt_ppc64(image, fdt, initrd_load_addr,
> >>                                initrd_len, cmdline);
> >>      if (ret)
> >> -            goto out;
> >> +            goto out_free_fdt;
> >>
> >>      fdt_pack(fdt);
> >>
> >> @@ -125,7 +125,7 @@ static void *elf64_load(struct kimage *image, char *kernel_buf,
> >>      kbuf.mem = KEXEC_BUF_MEM_UNKNOWN;
> >>      ret = kexec_add_buffer(&kbuf);
> >>      if (ret)
> >> -            goto out;
> >> +            goto out_free_fdt;
> >>
> >>      /* FDT will be freed in arch_kimage_file_post_load_cleanup */
> >>      image->arch.fdt = fdt;
> >> @@ -140,18 +140,14 @@ static void *elf64_load(struct kimage *image, char *kernel_buf,
> >>      if (ret)
> >>              pr_err("Error setting up the purgatory.\n");
> >>
> >> +    goto out;
> >
> > This will leak.  It would need to be something like:
> >
> >       if (ret) {
> >               pr_err("Error setting up the purgatory.\n");
> >               goto out_free_fdt;
> >       }
> Once "fdt" buffer is successfully passed to kexec_add_buffer() it cannot
> be freed here - it will be freed when the kexec cleanup function is called.

That may be the case currently, but really if a function returns an
error it should have undone anything it did like memory allocations. I
don't think you should do that to fix this issue, but it would be a
good clean-up.

Rob

Re: [PATCH] powerpc: Initialize local variable fdt to NULL in elf64_load()

[Lakshmi Ramasubramanian]

On 4/20/21 6:06 AM, Rob Herring wrote:
> On Tue, Apr 20, 2021 at 12:20 AM Lakshmi Ramasubramanian
> <nramas@linux.microsoft.com> wrote:
>>
>> On 4/19/21 10:00 PM, Dan Carpenter wrote:
>>> On Tue, Apr 20, 2021 at 09:30:16AM +1000, Michael Ellerman wrote:
>>>> Lakshmi Ramasubramanian <nramas@linux.microsoft.com> writes:
>>>>> On 4/16/21 2:05 AM, Michael Ellerman wrote:
>>>>>
>>>>>> Daniel Axtens <dja@axtens.net> writes:
>>>>>>>> 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...
>>>>>>>
>>>>>>> The out label reads in part:
>>>>>>>
>>>>>>>    /* Make kimage_file_post_load_cleanup free the fdt buffer for us. */
>>>>>>>    return ret ? ERR_PTR(ret) : fdt;
>>>>>>>
>>>>>>> As far as I can tell, any time we get a non-zero ret, we're going to
>>>>>>> return an error pointer rather than the uninitialised value...
>>>>>
>>>>> As Dan pointed out, the new code is in linux-next.
>>>>>
>>>>> I have copied the new one below - the function doesn't return fdt, but
>>>>> instead sets it in the arch specific field (please see the link to the
>>>>> updated elf_64.c below).
>>>>>
>>>>> https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git/tree/arch/powerpc/kexec/elf_64.c?h=for-next
>>>>>
>>>>>>>
>>>>>>> (btw, it does look like we might leak fdt if we have an error after we
>>>>>>> successfully kmalloc it.)
>>>>>>>
>>>>>>> Am I missing something? Can you link to the report for the kernel test
>>>>>>> robot or from Dan?
>>>>>
>>>>> /*
>>>>>             * Once FDT buffer has been successfully passed to
>>>>> kexec_add_buffer(),
>>>>>             * the FDT buffer address is saved in image->arch.fdt. In that
>>>>> case,
>>>>>             * the memory cannot be freed here in case of any other error.
>>>>>             */
>>>>>            if (ret && !image->arch.fdt)
>>>>>                    kvfree(fdt);
>>>>>
>>>>>            return ret ? ERR_PTR(ret) : NULL;
>>>>>
>>>>> In case of an error, the memory allocated for fdt is freed unless it has
>>>>> already been passed to kexec_add_buffer().
>>>>
>>>> It feels like the root of the problem is that the kvfree of fdt is in
>>>> the wrong place. It's only allocated later in the function, so the error
>>>> path should reflect that. Something like the patch below.
>>>>
>>>> cheers
>>>>
>>>>
>>>> diff --git a/arch/powerpc/kexec/elf_64.c b/arch/powerpc/kexec/elf_64.c
>>>> index 5a569bb51349..02662e72c53d 100644
>>>> --- a/arch/powerpc/kexec/elf_64.c
>>>> +++ b/arch/powerpc/kexec/elf_64.c
>>>> @@ -114,7 +114,7 @@ static void *elf64_load(struct kimage *image, char *kernel_buf,
>>>>       ret = setup_new_fdt_ppc64(image, fdt, initrd_load_addr,
>>>>                                 initrd_len, cmdline);
>>>>       if (ret)
>>>> -            goto out;
>>>> +            goto out_free_fdt;
>>>>
>>>>       fdt_pack(fdt);
>>>>
>>>> @@ -125,7 +125,7 @@ static void *elf64_load(struct kimage *image, char *kernel_buf,
>>>>       kbuf.mem = KEXEC_BUF_MEM_UNKNOWN;
>>>>       ret = kexec_add_buffer(&kbuf);
>>>>       if (ret)
>>>> -            goto out;
>>>> +            goto out_free_fdt;
>>>>
>>>>       /* FDT will be freed in arch_kimage_file_post_load_cleanup */
>>>>       image->arch.fdt = fdt;
>>>> @@ -140,18 +140,14 @@ static void *elf64_load(struct kimage *image, char *kernel_buf,
>>>>       if (ret)
>>>>               pr_err("Error setting up the purgatory.\n");
>>>>
>>>> +    goto out;
>>>
>>> This will leak.  It would need to be something like:
>>>
>>>        if (ret) {
>>>                pr_err("Error setting up the purgatory.\n");
>>>                goto out_free_fdt;
>>>        }
>> Once "fdt" buffer is successfully passed to kexec_add_buffer() it cannot
>> be freed here - it will be freed when the kexec cleanup function is called.
> 
> That may be the case currently, but really if a function returns an
> error it should have undone anything it did like memory allocations. I
> don't think you should do that to fix this issue, but it would be a
> good clean-up.
> 

I agree - in case of an error the function should do a proper clean-up.
Just to be clear - for now, I will leave this as is. Correct?

In my patch, I will do the following changes:

  => Free "fdt" when possible (as Michael had suggested in his patch)
  => Zero out "elf_info" struct at the start of the function.

thanks,
  -lakshmi

Re: [PATCH] powerpc: Initialize local variable fdt to NULL in elf64_load()

[Lakshmi Ramasubramanian]

On 4/20/21 7:42 AM, Lakshmi Ramasubramanian wrote:
> On 4/20/21 6:06 AM, Rob Herring wrote:
>> On Tue, Apr 20, 2021 at 12:20 AM Lakshmi Ramasubramanian
>> <nramas@linux.microsoft.com> wrote:
>>>
>>> On 4/19/21 10:00 PM, Dan Carpenter wrote:
>>>> On Tue, Apr 20, 2021 at 09:30:16AM +1000, Michael Ellerman wrote:
>>>>> Lakshmi Ramasubramanian <nramas@linux.microsoft.com> writes:
>>>>>> On 4/16/21 2:05 AM, Michael Ellerman wrote:
>>>>>>
>>>>>>> Daniel Axtens <dja@axtens.net> writes:
>>>>>>>>> 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...
>>>>>>>>
>>>>>>>> The out label reads in part:
>>>>>>>>
>>>>>>>>    /* Make kimage_file_post_load_cleanup free the fdt buffer for 
>>>>>>>> us. */
>>>>>>>>    return ret ? ERR_PTR(ret) : fdt;
>>>>>>>>
>>>>>>>> As far as I can tell, any time we get a non-zero ret, we're 
>>>>>>>> going to
>>>>>>>> return an error pointer rather than the uninitialised value...
>>>>>>
>>>>>> As Dan pointed out, the new code is in linux-next.
>>>>>>
>>>>>> I have copied the new one below - the function doesn't return fdt, 
>>>>>> but
>>>>>> instead sets it in the arch specific field (please see the link to 
>>>>>> the
>>>>>> updated elf_64.c below).
>>>>>>
>>>>>> https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git/tree/arch/powerpc/kexec/elf_64.c?h=for-next 
>>>>>>
>>>>>>
>>>>>>>>
>>>>>>>> (btw, it does look like we might leak fdt if we have an error 
>>>>>>>> after we
>>>>>>>> successfully kmalloc it.)
>>>>>>>>
>>>>>>>> Am I missing something? Can you link to the report for the 
>>>>>>>> kernel test
>>>>>>>> robot or from Dan?
>>>>>>
>>>>>> /*
>>>>>>             * Once FDT buffer has been successfully passed to
>>>>>> kexec_add_buffer(),
>>>>>>             * the FDT buffer address is saved in image->arch.fdt. 
>>>>>> In that
>>>>>> case,
>>>>>>             * the memory cannot be freed here in case of any other 
>>>>>> error.
>>>>>>             */
>>>>>>            if (ret && !image->arch.fdt)
>>>>>>                    kvfree(fdt);
>>>>>>
>>>>>>            return ret ? ERR_PTR(ret) : NULL;
>>>>>>
>>>>>> In case of an error, the memory allocated for fdt is freed unless 
>>>>>> it has
>>>>>> already been passed to kexec_add_buffer().
>>>>>
>>>>> It feels like the root of the problem is that the kvfree of fdt is in
>>>>> the wrong place. It's only allocated later in the function, so the 
>>>>> error
>>>>> path should reflect that. Something like the patch below.
>>>>>
>>>>> cheers
>>>>>
>>>>>
>>>>> diff --git a/arch/powerpc/kexec/elf_64.c b/arch/powerpc/kexec/elf_64.c
>>>>> index 5a569bb51349..02662e72c53d 100644
>>>>> --- a/arch/powerpc/kexec/elf_64.c
>>>>> +++ b/arch/powerpc/kexec/elf_64.c
>>>>> @@ -114,7 +114,7 @@ static void *elf64_load(struct kimage *image, 
>>>>> char *kernel_buf,
>>>>>       ret = setup_new_fdt_ppc64(image, fdt, initrd_load_addr,
>>>>>                                 initrd_len, cmdline);
>>>>>       if (ret)
>>>>> -            goto out;
>>>>> +            goto out_free_fdt;
>>>>>
>>>>>       fdt_pack(fdt);
>>>>>
>>>>> @@ -125,7 +125,7 @@ static void *elf64_load(struct kimage *image, 
>>>>> char *kernel_buf,
>>>>>       kbuf.mem = KEXEC_BUF_MEM_UNKNOWN;
>>>>>       ret = kexec_add_buffer(&kbuf);
>>>>>       if (ret)
>>>>> -            goto out;
>>>>> +            goto out_free_fdt;
>>>>>
>>>>>       /* FDT will be freed in arch_kimage_file_post_load_cleanup */
>>>>>       image->arch.fdt = fdt;
>>>>> @@ -140,18 +140,14 @@ static void *elf64_load(struct kimage *image, 
>>>>> char *kernel_buf,
>>>>>       if (ret)
>>>>>               pr_err("Error setting up the purgatory.\n");
>>>>>
>>>>> +    goto out;
>>>>
>>>> This will leak.  It would need to be something like:
>>>>
>>>>        if (ret) {
>>>>                pr_err("Error setting up the purgatory.\n");
>>>>                goto out_free_fdt;
>>>>        }
>>> Once "fdt" buffer is successfully passed to kexec_add_buffer() it cannot
>>> be freed here - it will be freed when the kexec cleanup function is 
>>> called.
>>
>> That may be the case currently, but really if a function returns an
>> error it should have undone anything it did like memory allocations. I
>> don't think you should do that to fix this issue, but it would be a
>> good clean-up.
>>
> 
> I agree - in case of an error the function should do a proper clean-up.
> Just to be clear - for now, I will leave this as is. Correct?
> 
> In my patch, I will do the following changes:
> 
>   => Free "fdt" when possible (as Michael had suggested in his patch)
>   => Zero out "elf_info" struct at the start of the function.
> 

Instead of zeroing out "elf_info", I think it would be better to return 
an error immediately, instead of the "goto out;", if 
kexec_build_elf_info() fails.

    ret = kexec_build_elf_info(kernel_buf, kernel_len, &ehdr, &elf_info);
    if (ret)
      return ERR_PTR(ret);

  -lakshmi

Re: [PATCH] powerpc: Initialize local variable fdt to NULL in elf64_load()

[Rob Herring]

On Tue, Apr 20, 2021 at 10:04 AM Lakshmi Ramasubramanian
<nramas@linux.microsoft.com> wrote:
>
> On 4/20/21 7:42 AM, Lakshmi Ramasubramanian wrote:
> > On 4/20/21 6:06 AM, Rob Herring wrote:
> >> On Tue, Apr 20, 2021 at 12:20 AM Lakshmi Ramasubramanian
> >> <nramas@linux.microsoft.com> wrote:
> >>>
> >>> On 4/19/21 10:00 PM, Dan Carpenter wrote:
> >>>> On Tue, Apr 20, 2021 at 09:30:16AM +1000, Michael Ellerman wrote:
> >>>>> Lakshmi Ramasubramanian <nramas@linux.microsoft.com> writes:
> >>>>>> On 4/16/21 2:05 AM, Michael Ellerman wrote:
> >>>>>>
> >>>>>>> Daniel Axtens <dja@axtens.net> writes:
> >>>>>>>>> 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...
> >>>>>>>>
> >>>>>>>> The out label reads in part:
> >>>>>>>>
> >>>>>>>>    /* Make kimage_file_post_load_cleanup free the fdt buffer for
> >>>>>>>> us. */
> >>>>>>>>    return ret ? ERR_PTR(ret) : fdt;
> >>>>>>>>
> >>>>>>>> As far as I can tell, any time we get a non-zero ret, we're
> >>>>>>>> going to
> >>>>>>>> return an error pointer rather than the uninitialised value...
> >>>>>>
> >>>>>> As Dan pointed out, the new code is in linux-next.
> >>>>>>
> >>>>>> I have copied the new one below - the function doesn't return fdt,
> >>>>>> but
> >>>>>> instead sets it in the arch specific field (please see the link to
> >>>>>> the
> >>>>>> updated elf_64.c below).
> >>>>>>
> >>>>>> https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git/tree/arch/powerpc/kexec/elf_64.c?h=for-next
> >>>>>>
> >>>>>>
> >>>>>>>>
> >>>>>>>> (btw, it does look like we might leak fdt if we have an error
> >>>>>>>> after we
> >>>>>>>> successfully kmalloc it.)
> >>>>>>>>
> >>>>>>>> Am I missing something? Can you link to the report for the
> >>>>>>>> kernel test
> >>>>>>>> robot or from Dan?
> >>>>>>
> >>>>>> /*
> >>>>>>             * Once FDT buffer has been successfully passed to
> >>>>>> kexec_add_buffer(),
> >>>>>>             * the FDT buffer address is saved in image->arch.fdt.
> >>>>>> In that
> >>>>>> case,
> >>>>>>             * the memory cannot be freed here in case of any other
> >>>>>> error.
> >>>>>>             */
> >>>>>>            if (ret && !image->arch.fdt)
> >>>>>>                    kvfree(fdt);
> >>>>>>
> >>>>>>            return ret ? ERR_PTR(ret) : NULL;
> >>>>>>
> >>>>>> In case of an error, the memory allocated for fdt is freed unless
> >>>>>> it has
> >>>>>> already been passed to kexec_add_buffer().
> >>>>>
> >>>>> It feels like the root of the problem is that the kvfree of fdt is in
> >>>>> the wrong place. It's only allocated later in the function, so the
> >>>>> error
> >>>>> path should reflect that. Something like the patch below.
> >>>>>
> >>>>> cheers
> >>>>>
> >>>>>
> >>>>> diff --git a/arch/powerpc/kexec/elf_64.c b/arch/powerpc/kexec/elf_64.c
> >>>>> index 5a569bb51349..02662e72c53d 100644
> >>>>> --- a/arch/powerpc/kexec/elf_64.c
> >>>>> +++ b/arch/powerpc/kexec/elf_64.c
> >>>>> @@ -114,7 +114,7 @@ static void *elf64_load(struct kimage *image,
> >>>>> char *kernel_buf,
> >>>>>       ret = setup_new_fdt_ppc64(image, fdt, initrd_load_addr,
> >>>>>                                 initrd_len, cmdline);
> >>>>>       if (ret)
> >>>>> -            goto out;
> >>>>> +            goto out_free_fdt;
> >>>>>
> >>>>>       fdt_pack(fdt);
> >>>>>
> >>>>> @@ -125,7 +125,7 @@ static void *elf64_load(struct kimage *image,
> >>>>> char *kernel_buf,
> >>>>>       kbuf.mem = KEXEC_BUF_MEM_UNKNOWN;
> >>>>>       ret = kexec_add_buffer(&kbuf);
> >>>>>       if (ret)
> >>>>> -            goto out;
> >>>>> +            goto out_free_fdt;
> >>>>>
> >>>>>       /* FDT will be freed in arch_kimage_file_post_load_cleanup */
> >>>>>       image->arch.fdt = fdt;
> >>>>> @@ -140,18 +140,14 @@ static void *elf64_load(struct kimage *image,
> >>>>> char *kernel_buf,
> >>>>>       if (ret)
> >>>>>               pr_err("Error setting up the purgatory.\n");
> >>>>>
> >>>>> +    goto out;
> >>>>
> >>>> This will leak.  It would need to be something like:
> >>>>
> >>>>        if (ret) {
> >>>>                pr_err("Error setting up the purgatory.\n");
> >>>>                goto out_free_fdt;
> >>>>        }
> >>> Once "fdt" buffer is successfully passed to kexec_add_buffer() it cannot
> >>> be freed here - it will be freed when the kexec cleanup function is
> >>> called.
> >>
> >> That may be the case currently, but really if a function returns an
> >> error it should have undone anything it did like memory allocations. I
> >> don't think you should do that to fix this issue, but it would be a
> >> good clean-up.
> >>
> >
> > I agree - in case of an error the function should do a proper clean-up.
> > Just to be clear - for now, I will leave this as is. Correct?

Yes.

> > In my patch, I will do the following changes:
> >
> >   => Free "fdt" when possible (as Michael had suggested in his patch)
> >   => Zero out "elf_info" struct at the start of the function.
> >
>
> Instead of zeroing out "elf_info", I think it would be better to return
> an error immediately, instead of the "goto out;", if
> kexec_build_elf_info() fails.
>
>     ret = kexec_build_elf_info(kernel_buf, kernel_len, &ehdr, &elf_info);
>     if (ret)
>       return ERR_PTR(ret);

I thought kexec_build_elf_info() can return an error and allocated
memory, so that would leak memory.

Rob

Re: [PATCH] powerpc: Initialize local variable fdt to NULL in elf64_load()

[Lakshmi Ramasubramanian]

On 4/20/21 8:47 AM, Rob Herring wrote:
> On Tue, Apr 20, 2021 at 10:04 AM Lakshmi Ramasubramanian
> <nramas@linux.microsoft.com> wrote:
>>
>> On 4/20/21 7:42 AM, Lakshmi Ramasubramanian wrote:
>>> On 4/20/21 6:06 AM, Rob Herring wrote:
>>>> On Tue, Apr 20, 2021 at 12:20 AM Lakshmi Ramasubramanian
>>>> <nramas@linux.microsoft.com> wrote:
>>>>>
>>>>> On 4/19/21 10:00 PM, Dan Carpenter wrote:
>>>>>> On Tue, Apr 20, 2021 at 09:30:16AM +1000, Michael Ellerman wrote:
>>>>>>> Lakshmi Ramasubramanian <nramas@linux.microsoft.com> writes:
>>>>>>>> On 4/16/21 2:05 AM, Michael Ellerman wrote:
>>>>>>>>
>>>>>>>>> Daniel Axtens <dja@axtens.net> writes:
>>>>>>>>>>> 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...
>>>>>>>>>>
>>>>>>>>>> The out label reads in part:
>>>>>>>>>>
>>>>>>>>>>     /* Make kimage_file_post_load_cleanup free the fdt buffer for
>>>>>>>>>> us. */
>>>>>>>>>>     return ret ? ERR_PTR(ret) : fdt;
>>>>>>>>>>
>>>>>>>>>> As far as I can tell, any time we get a non-zero ret, we're
>>>>>>>>>> going to
>>>>>>>>>> return an error pointer rather than the uninitialised value...
>>>>>>>>
>>>>>>>> As Dan pointed out, the new code is in linux-next.
>>>>>>>>
>>>>>>>> I have copied the new one below - the function doesn't return fdt,
>>>>>>>> but
>>>>>>>> instead sets it in the arch specific field (please see the link to
>>>>>>>> the
>>>>>>>> updated elf_64.c below).
>>>>>>>>
>>>>>>>> https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git/tree/arch/powerpc/kexec/elf_64.c?h=for-next
>>>>>>>>
>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> (btw, it does look like we might leak fdt if we have an error
>>>>>>>>>> after we
>>>>>>>>>> successfully kmalloc it.)
>>>>>>>>>>
>>>>>>>>>> Am I missing something? Can you link to the report for the
>>>>>>>>>> kernel test
>>>>>>>>>> robot or from Dan?
>>>>>>>>
>>>>>>>> /*
>>>>>>>>              * Once FDT buffer has been successfully passed to
>>>>>>>> kexec_add_buffer(),
>>>>>>>>              * the FDT buffer address is saved in image->arch.fdt.
>>>>>>>> In that
>>>>>>>> case,
>>>>>>>>              * the memory cannot be freed here in case of any other
>>>>>>>> error.
>>>>>>>>              */
>>>>>>>>             if (ret && !image->arch.fdt)
>>>>>>>>                     kvfree(fdt);
>>>>>>>>
>>>>>>>>             return ret ? ERR_PTR(ret) : NULL;
>>>>>>>>
>>>>>>>> In case of an error, the memory allocated for fdt is freed unless
>>>>>>>> it has
>>>>>>>> already been passed to kexec_add_buffer().
>>>>>>>
>>>>>>> It feels like the root of the problem is that the kvfree of fdt is in
>>>>>>> the wrong place. It's only allocated later in the function, so the
>>>>>>> error
>>>>>>> path should reflect that. Something like the patch below.
>>>>>>>
>>>>>>> cheers
>>>>>>>
>>>>>>>
>>>>>>> diff --git a/arch/powerpc/kexec/elf_64.c b/arch/powerpc/kexec/elf_64.c
>>>>>>> index 5a569bb51349..02662e72c53d 100644
>>>>>>> --- a/arch/powerpc/kexec/elf_64.c
>>>>>>> +++ b/arch/powerpc/kexec/elf_64.c
>>>>>>> @@ -114,7 +114,7 @@ static void *elf64_load(struct kimage *image,
>>>>>>> char *kernel_buf,
>>>>>>>        ret = setup_new_fdt_ppc64(image, fdt, initrd_load_addr,
>>>>>>>                                  initrd_len, cmdline);
>>>>>>>        if (ret)
>>>>>>> -            goto out;
>>>>>>> +            goto out_free_fdt;
>>>>>>>
>>>>>>>        fdt_pack(fdt);
>>>>>>>
>>>>>>> @@ -125,7 +125,7 @@ static void *elf64_load(struct kimage *image,
>>>>>>> char *kernel_buf,
>>>>>>>        kbuf.mem = KEXEC_BUF_MEM_UNKNOWN;
>>>>>>>        ret = kexec_add_buffer(&kbuf);
>>>>>>>        if (ret)
>>>>>>> -            goto out;
>>>>>>> +            goto out_free_fdt;
>>>>>>>
>>>>>>>        /* FDT will be freed in arch_kimage_file_post_load_cleanup */
>>>>>>>        image->arch.fdt = fdt;
>>>>>>> @@ -140,18 +140,14 @@ static void *elf64_load(struct kimage *image,
>>>>>>> char *kernel_buf,
>>>>>>>        if (ret)
>>>>>>>                pr_err("Error setting up the purgatory.\n");
>>>>>>>
>>>>>>> +    goto out;
>>>>>>
>>>>>> This will leak.  It would need to be something like:
>>>>>>
>>>>>>         if (ret) {
>>>>>>                 pr_err("Error setting up the purgatory.\n");
>>>>>>                 goto out_free_fdt;
>>>>>>         }
>>>>> Once "fdt" buffer is successfully passed to kexec_add_buffer() it cannot
>>>>> be freed here - it will be freed when the kexec cleanup function is
>>>>> called.
>>>>
>>>> That may be the case currently, but really if a function returns an
>>>> error it should have undone anything it did like memory allocations. I
>>>> don't think you should do that to fix this issue, but it would be a
>>>> good clean-up.
>>>>
>>>
>>> I agree - in case of an error the function should do a proper clean-up.
>>> Just to be clear - for now, I will leave this as is. Correct?
> 
> Yes.
okay.

> 
>>> In my patch, I will do the following changes:
>>>
>>>    => Free "fdt" when possible (as Michael had suggested in his patch)
>>>    => Zero out "elf_info" struct at the start of the function.
>>>
>>
>> Instead of zeroing out "elf_info", I think it would be better to return
>> an error immediately, instead of the "goto out;", if
>> kexec_build_elf_info() fails.
>>
>>      ret = kexec_build_elf_info(kernel_buf, kernel_len, &ehdr, &elf_info);
>>      if (ret)
>>        return ERR_PTR(ret);
> 
> I thought kexec_build_elf_info() can return an error and allocated
> memory, so that would leak memory.
> 

I looked at kexec_build_elf_info() more - it does free elf_info, if it 
allocated it but encountered an error after the allocation. So it does a 
proper clean-up in case of an error.

  -lakshmi

Re: [PATCH] powerpc: Initialize local variable fdt to NULL in elf64_load()

[Daniel Axtens]

Daniel Axtens <dja@axtens.net> writes:

> 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?)

I was reading the version in powerpc/next, clearly I should have looked
at linux-next. Having said that, I think I will leave the rest of the
bikeshedding to the rest of you, you all seem to have it in hand :)

Kind regards,
Daniel

>
> The out label reads in part:
>
> 	/* Make kimage_file_post_load_cleanup free the fdt buffer for us. */
> 	return ret ? ERR_PTR(ret) : fdt;
>
> As far as I can tell, any time we get a non-zero ret, we're going to
> return an error pointer rather than the uninitialised value...
>
> (btw, it does look like we might leak fdt if we have an error after we
> successfully kmalloc it.)
>
> Am I missing something? Can you link to the report for the kernel test
> robot or from Dan? 
>
> FWIW, I think it's worth including this patch _anyway_ because initing
> local variables is good practice, but I'm just not sure on the
> justification.
>
> Kind regards,
> Daniel
>
>>> Signed-off-by: Lakshmi Ramasubramanian <nramas@linux.microsoft.com>
>>> Reported-by: kernel test robot <lkp@intel.com>
>>> Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
>>> ---
>>>   arch/powerpc/kexec/elf_64.c | 2 +-
>>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>> 
>>> diff --git a/arch/powerpc/kexec/elf_64.c b/arch/powerpc/kexec/elf_64.c
>>> index 5a569bb51349..0051440c1f77 100644
>>> --- a/arch/powerpc/kexec/elf_64.c
>>> +++ b/arch/powerpc/kexec/elf_64.c
>>> @@ -32,7 +32,7 @@ static void *elf64_load(struct kimage *image, char *kernel_buf,
>>>   	int ret;
>>>   	unsigned long kernel_load_addr;
>>>   	unsigned long initrd_load_addr = 0, fdt_load_addr;
>>> -	void *fdt;
>>> +	void *fdt = NULL;
>>>   	const void *slave_code;
>>>   	struct elfhdr ehdr;
>>>   	char *modified_cmdline = NULL;
>>> 
>>
>> thanks,
>>   -lakshmi

RE: [PATCH] powerpc: Initialize local variable fdt to NULL in elf64_load()

[David Laight]

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!

I also think there is a new option to gcc (or clang?) to initialise
on-stack structures and arrays to ensure garbage isn't passed.
That seems to be a horrid performance hit!
Especially in userspace where large stack allocations are almost free.

Any auto-initialise ought to be with a semi-random value
(especially not zero) so that it is never right and doesn't
lead to lazy coding.

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)

Re: [PATCH] powerpc: Initialize local variable fdt to NULL in elf64_load()

[Dan Carpenter]

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

Re: [PATCH] powerpc: Initialize local variable fdt to NULL in elf64_load()

[Segher Boessenkool]

On Thu, Apr 22, 2021 at 08:05:27AM +0000, David Laight wrote:
> > (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!

Yeah, that's the worst of all possible worlds.

> I also think there is a new option to gcc (or clang?) to initialise
> on-stack structures and arrays to ensure garbage isn't passed.
> That seems to be a horrid performance hit!
> Especially in userspace where large stack allocations are almost free.
> 
> Any auto-initialise ought to be with a semi-random value
> (especially not zero) so that it is never right and doesn't
> lead to lazy coding.

Many compilers did something like this, decades ago -- for debug builds.


Segher

Re: [PATCH] powerpc: Initialize local variable fdt to NULL in elf64_load()

[Michael Ellerman]

Daniel Axtens <dja@axtens.net> writes:
> Daniel Axtens <dja@axtens.net> writes:
>
>> 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?)

This is where I say, "yes, Android" and you say "ugh no I meant a real
distro", and I say "well ...".

But yeah doesn't help us much.

cheers

RE: [PATCH] powerpc: Initialize local variable fdt to NULL in elf64_load()

[David Laight]

From: Michael Ellerman
> Sent: 23 April 2021 14:51
...
> > (Does anyone - and by anyone I mean any large distro - compile with
> > local variables inited by the compiler?)
> 
> This is where I say, "yes, Android" and you say "ugh no I meant a real
> distro", and I say "well ...".
> 
> But yeah doesn't help us much.

And it doesn't seem to stop my phone crashing either :-)

Of course, I've absolutely no way of finding out where it is crashing.
Nor where the massive memory leaks are that means it need rebooting
every few days.

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)

Re: [PATCH] powerpc: Initialize local variable fdt to NULL in elf64_load()

[Rob Herring]

On Fri, Apr 23, 2021 at 9:42 AM David Laight <David.Laight@aculab.com> wrote:
>
> From: Michael Ellerman
> > Sent: 23 April 2021 14:51
> ...
> > > (Does anyone - and by anyone I mean any large distro - compile with
> > > local variables inited by the compiler?)
> >
> > This is where I say, "yes, Android" and you say "ugh no I meant a real
> > distro", and I say "well ...".
> >
> > But yeah doesn't help us much.
>
> And it doesn't seem to stop my phone crashing either :-)
>
> Of course, I've absolutely no way of finding out where it is crashing.
> Nor where the massive memory leaks are that means it need rebooting
> every few days.

You need a new phone. :) My Pixel3 uptime is sitting at 194 hours
currently. It would be more, but those annoying security updates
require reboots. I have had phones that I setup to reboot every night
though.

Rob