Skip to content

s3:utils: smbget fix a memory leak

Jones Syue requested to merge samba-team/devel/samba:jones2019_fix_smbget_memory_leak into master

Using smbget to download files recursively (-R).

If smbget found that a file is already existed in the destination, smbget would said 'File exists', return early, and 'newname' allocated memory is never freed, this is found by valgrind.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=15403

# valgrind --leak-check=full --show-leak-kinds=definite smbget -R -U${ac}%${pw} smb://${ip}/src
==13050== Memcheck, a memory error detector
==13050== Copyright (C) 2002-2015, and GNU GPL'd, by Julian Seward et al.
==13050== Using Valgrind-3.12.0 and LibVEX; rerun with -h for copyright info
==13050== Command: smbget -R -Ujones%${pw} smb://127.0.0.1/src
==13050==
Using workgroup WORKGROUP, user jones
Can't open file: File exists
Failed to download /file: File exists
==13050==
==13050== HEAP SUMMARY:
==13050==     in use at exit: 121,424 bytes in 263 blocks
==13050==   total heap usage: 5,466 allocs, 5,203 frees, 927,883 bytes allocated
==13050==
==13050== 6 bytes in 1 blocks are definitely lost in loss record 6 of 151
==13050==    at 0x4C28B15: malloc (in /root/bin/valgrind-3.12.0/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==13050==    by 0x5B7E9D7: vasprintf (vasprintf.c:76)
==13050==    by 0x5B5D2C6: asprintf (asprintf.c:35)
==13050==    by 0x10BC29: smb_download_dir (smbget.c:211)
==13050==    by 0x10A549: main (smbget.c:1037)
==13050==
... <cut> ...

Checklist

  • Commits have Signed-off-by: with name/author being identical to the commit author
  • (optional) This MR is just one part towards a larger feature.
  • (optional, if backport required) Bugzilla bug filed and BUG: tag added
  • Test suite updated with functionality tests
  • Test suite updated with negative tests
  • Documentation updated
  • CI timeout is 3h or higher (see Settings/CICD/General pipelines/ Timeout)

Reviewer's checklist:

  • There is a test suite reasonably covering new functionality or modifications
  • Function naming, parameters, return values, types, etc., are consistent and according to README.Coding.md
  • This feature/change has adequate documentation added
  • No obvious mistakes in the code

Merge request reports