Thank you for reporting this!
I have set up a static-analysis branch on git and pushed fixes for these
problems.
The leaks in iso9660_fs.c were straight forward to fix. The iso-info.c
report was a false positive, as getopt_long makes sure that optarg will not
be NULL for option "r".
udf_fs.c needed some addional care. The actual cause of both reported
problems was in extremely intransparent and complex semantics of
udf_ff_traverse. The function actually had three different possible
outcomes:
- if no entry was not found in the current dir, p_udf_dirent was be
freed and NULL returned
- if an entry was found in the current dir, p_udf_dirent was updated and
returned
- if a recursion was needed, p_udf_dirent was discarded and a new dirent
struct returned
The call to udf_ff_traverse in udf_fopen handled only the second and third
outcome correctly. In case of NULL being returned, it would try to free the
already freed dirent struct a second time.
The recursion call in udf_ff_traverse, in contrast, handled only the first
and second outcome correctly. If more than one recursion step was
performed, intermediately created dirent structs were leaked.
My commit changes the semantics of udf_ff_traverse so that it always takes
care of p_udf_dirent. Callers can and must ignore the passed struct
afterwards from now on and process only the value returned by
udf_ff_traverse. This greatly simplifies usage of that function.
Fortunately it was not exposed in the API and only used internally in
udf_fs.c.
I also set up projects for libcdio and libcdio-paranoia on Coverity Scan.
I want to process the results and push fixes, before merging the
static-analysis branch. I'll send invitations for both projects to Rocky
soon. If anyone else would like to have a look at the results, contact me
to get an invitation too.
Robert
Am 17.04.2014 17:55, schrieb Frantisek Kluknavsky:
Hi,
Maybe you want to see results of static analysis. First two defects seem
noteworthy.
libcdio-0.92/lib/udf/udf_fs.c:266:double_free – Calling
"udf_dirent_free(udf_dirent_t *)" frees pointer "p_udf_dirent" which has
already been freed.
libcdio-0.92/src/iso-info.c:159:var_deref_model – Passing null pointer
"optarg" to function "atoll(char const *)", which dereferences it.
libcdio-0.92/lib/iso9660/iso9660_fs.c:1568:leaked_storage – Variable
"p_stat" going out of scope leaks the storage it points to.
libcdio-0.92/lib/iso9660/iso9660_fs.c:757:leaked_storage – Variable
"p_stat" going out of scope leaks the storage it points to.
libcdio-0.92/lib/udf/udf_fs.c:234:dead_error_line – Execution cannot
reach this statement "free(p_udf_dirent->psz_name);".
Have a nice day.
Fero