On Tue, Jan 04, 2022 at 08:09:16PM +0800, zhanchengbin wrote: > In setup_tdb(), need free tdb_dir and db->tdb_fn before return, > otherwise it will cause memory leak. This patch is not correct. tdb_dir is returned by profile_get_string(), and it does *not* need to be freed. In fact, if you had tested this using valgrind or AddressSanitizer, it would have failed because tdb_dir is in the *middle* of an allocated block[1], and this would have corrupted the heap data structures leading to all sorts of potential problems. [1] And that allocated block is freed by profile_release() In addition, tdb->tdb_fn will be freed by e2fsck_free_dir_info(), and so freeing it on the error path in setup_tdb() will result in a double-free when e2fsck_free_dir_info() gets called. I'm guessing you didn't actually test this patch with the code paths in question --- that is, by triggering an error while using something like ASan or valgrid? Note that corrupting the heap may lead to an exploitable security bug, so if you have applied this patch in your production version of e2fsprogs, I suggest that you revert it. Cheers, - Ted
