[Samba] [BUG] Potential Memory Leak in ndr_print_struct_string() Usage

[Jennifer Sutton]

On 3/11/25 4:41 pm, 狐狸半面添 via samba wrote:
> Hello,
>   
> I believe I've identified a potential bug related to memory management in the Samba source code.
>   
> Upon reviewing the implementation, it appears that the memory pointed to by the return value of ndr_print_struct_string() has its parent context set to the first parameter, mem_ctx (approximately line 546 in ndr.c).
>   
> This leads to a potential issue: if ndr_print_struct_string() is called with mem_ctx set to NULL (or 0), and the returned string is used directly as a function argument (e.g., within a DEBUG() macro) without being assigned to a variable in the caller's scope, then there is no way to later free the allocated memory. This results in a memory leak.
>   
> This pattern appears in several places in the codebase. One example can be found in dsdb_access.c around lines 52–55:
>   DEBUG(level,("Security context: %s\n",              ndr_print_struct_string(0, (ndr_print_fn_t)ndr_print_security_token, "", token))); DEBUG(level,("Security descriptor: %s\n",              ndr_print_struct_string(0, (ndr_print_fn_t)ndr_print_security_descriptor, "", sd)));
> In these cases, the string is allocated under a NULL memory context (which typically means it's a top-level allocation), but since the return value is used directly in the DEBUG macro and not stored, it cannot be freed later.
>   
> I would appreciate your confirmation on whether this is indeed a concern and how it should be addressed.
>   
> Best regards,
> A Samba beginner

You’re right, that indeed looks like a memory leak. Fortunately the 
ndr_print_struct_string() calls should be happening only at debug level 10.

Ideally the callers would pass in a suitable talloc memory context 
instead of passing zero.

Thank you for letting us know of this bug!

Cheers,
Jennifer (she/her)