search

facebook / hhvm

A virtual machine for executing programs written in Hack.
https://hhvm.com
Other
18.21k stars 3k forks source link

Memory leak? #9514

Closed QiAnXinCodeSafe closed 1 month ago

QiAnXinCodeSafe commented 2 months ago

Hi all, This is Qianxin CodeSafe Team, we found a suspicious issue, at https://github.com/facebook/hhvm/blob/6999172b83a52057636859b59031fcbcab74b020/hphp/neo/neo_hdf.c#L480 Assume that the condition value is false,at https://github.com/facebook/hhvm/blob/6999172b83a52057636859b59031fcbcab74b020/hphp/neo/neo_hdf.c#L97 Let's say that name is equal to the value of the NULL，at https://github.com/facebook/hhvm/blob/6999172b83a52057636859b59031fcbcab74b020/hphp/neo/neo_hdf.c#L105 Assume that 'err' and 'STATUS_OK' have unequal values https://github.com/facebook/hhvm/blob/6999172b83a52057636859b59031fcbcab74b020/hphp/neo/neo_hdf.c#L479

anhadjaisingh commented 1 month ago

Hello @QiAnXinCodeSafe, thank you for reporting this issue -

I've gone through the reported code path mentioned here and I suspect that this is a false alarm.

In https://github.com/facebook/hhvm/blob/6999172b83a52057636859b59031fcbcab74b020/hphp/neo/neo_hdf.c#L470-L480 - if we call _alloc_hdf with *name = NULL, *hdf = a valid ptr, dup !=0, then we have:

  1. The calloc call on https://github.com/facebook/hhvm/blob/6999172b83a52057636859b59031fcbcab74b020/hphp/neo/neo_hdf.c#L96 succeeds, so we have some memory allocated
  2. because of name ==NULL and dupl !=0 we end up in https://github.com/facebook/hhvm/blob/6999172b83a52057636859b59031fcbcab74b020/hphp/neo/neo_hdf.c#L121-L133 , so we can potentially return from _alloc_hdf with an error (status != STATUS_OK), and forget to free *hdf that was calloc'd earlier.

However, there are no return calls from _alloc_hdf() where we return with nerr_raise(NERR_*) and don't free *hdf before doing so. For your specified code-path example from the issue - we have an explicit free() - https://github.com/facebook/hhvm/blob/6999172b83a52057636859b59031fcbcab74b020/hphp/neo/neo_hdf.c#L127-L132

However, If you think I'm wrong - please try to provide a reproducible test case that we can use to ideally confirm the memory leak.

QiAnXinCodeSafe commented 1 month ago

Hello, @anhadjaisingh ,thank you very much for your reply.After our further manual audit,this is a false alarm,thanks again for your detailed reply,we will repair and further improve ours ai robot!Wishing you every success.

lexidor commented 1 month ago

Please do such manual verification before filing issues instead of afterwards.