[Bug 203649] makefs: Coverity CID 1305659: Unclear whether reaction on malloc failure suffices.

[bugzilla-noreply at freebsd.org]

https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=203649

            Bug ID: 203649
           Summary: makefs: Coverity CID 1305659: Unclear whether reaction
                    on malloc failure suffices.
           Product: Base System
           Version: 11.0-CURRENT
          Hardware: Any
                OS: Any
            Status: New
          Severity: Affects Some People
          Priority: ---
         Component: bin
          Assignee: freebsd-bugs at FreeBSD.org
          Reporter: scdbackup at gmx.net

usr.sbin/makefs/cd9660.c

CID 1305659: Dereference before null check (REVERSE_INULL)i
  check_after_deref: Null-checking var suggests that it may be null,
  but it has already been dereferenced on all paths leading to the check.

431        if (var)
432                free(var);

--------------- Source analysis:

Indeed the function should bail out when allocation fails.

327        if ((var = strdup(option)) == NULL)
328                err(1, "allocating memory for copy of option string");

If err() does not finally call exit(), then the program runs
into a SIGSEGV by

331        val = strchr(var, '=');

The function cd9660_parse_opts() gets called by usr.sbin/makefs/makefs.c

                                if (! fstype->parse_options(p, &fsoptions))
                                        usage();

usage() calls exit(1).
So i assume that return NULL would be the way to indicate error
and cause abort. But usage() will indicate a user error where
a resource shortage is the reason.

--------------- Remedy proposal:

Call exit(1) if no memory is available. (Unless you can find the
definition of err() and verify that it calls exit().)

-        if ((var = strdup(option)) == NULL)
+        if ((var = strdup(option)) == NULL) {
                 err(1, "allocating memory for copy of option string");
+                exit(1);
+        }

In any case remove the test which made Coverity suspicious.

-        if (var)
-                free(var);
+        free(var);

-- 
You are receiving this mail because:
You are the assignee for the bug.