samba - Nov 2025 - [BUG] Potential Memory Leak in ndr_print_struct_string() Usage

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





sangxin_fox
sangxin_fox at qq.com



 

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)