Many functions require the allocation of multiple resources. Failing and returning somewhere in the middle of this function without freeing all of the allocated resources could produce a memory leak. It is a common error to forget to free one (or all) of the resources in this manner, so a goto
- chain is the simplest and cleanest way to organize exits when order is preservedwhile preserving the order of freed resources.
Noncompliant Code Example (POSIX)
In this noncompliant example, exit code is written for every instance in which the function can terminate prematurely. Notice how failing to allocate obj2
close fin2
produces a memory leak and fails to close the opened fileresource leak, leaving an open file descriptor.
Please note that these examples assume errno_t
and NOERR
to be defined, as recommended in DCL09-C. Declare functions that return errno with a return type of errno_t. An equivalent compatible example would define errno_t
as an int
and NOERR
as zero.
These examples also assume that errno
is set if fopen()
or malloc()
fail. These are guaranteed by POSIX but not by C11. See ERR30-C. Take care when reading errno for more details.
Code Block | ||||
---|---|---|---|---|
| ||||
typedef struct object { //* A genericGeneric struct -- The: contents don't matter */ int propertyA, propertyB, propertyC; } object_t; errno_t do_something(void){ FILE *fin1, *finfin2; object_t *obj1, *obj2obj; errno_t ret_val; finfin1 = fopen("some_file", "r"); if (finfin1 == NULL) { return errno; } obj1fin2 = malloc(sizeof(object_t))fopen("some_other_file", "r"); if (obj1fin2 == NULL){ ret_val = errno; { fclose(finfin1); return ret_valerrno; } obj2obj = malloc(sizeof(object_t)); if (obj2obj == NULL) { ret_val = errno; fclose(finfin1); return ret_val; //* Forgot to freeclose obj1 fin2!! */ } //* ... moreMore code ... */ fclose(fin1); fclose(fin2); free(obj); return NOERR; } |
This is also just a small example; in much larger examples, errors like this would be are even harder to detect.
Compliant Solution
...
In this revised version, we have used a goto-chain in replacement of each individual return segment. If there is no error, control flow will fall through to the SUCCESS
label, release all of the resources and return NOERR
. In the case of an error, the return value will be set to errno
, control flow will jump to the proper failure label, and the appropriate resources will be released before returning.
Wiki Markup |
---|
Please note that these examples assume {{errno_t}} and {{NOERR}} to be defined, as requested by \[[DCL09-C. Declare functions that return an errno error code with a return type of errno_t|DCL09-C. Declare functions that return an errno error code with a return type of errno_t]\]. An equivalent compatible example would define {{errno_t}} as an {{int}} and {{NOERR}} as zero. |
(POSIX, Nested Ifs)
This compliant solution uses nested if statements to properly close files and free memory in the case that any error occurs. When the number of resources to manage is small (3 in this example), nested if statements will be simpler than a goto chain.
Code Block | ||||
---|---|---|---|---|
| ||||
/* ... Assume the same struct used previously ... */
errno_t do_something(void) {
FILE *fin1, *fin2;
object_t *obj;
errno_t ret_val = NOERR; /* Initially assume a successful return value */
if ((fin1 = fopen("some_file", "r")) != NULL) {
if ((fin2 = fopen("some_other_file", "r")) != NULL) {
if ((obj = malloc(sizeof(object_t))) != NULL) {
/* ... More code ... */
/* Clean-up & handle errors */
free(obj);
} else {
ret_val = errno;
}
fclose(fin2);
} else {
ret_val = errno;
}
fclose(fin1);
} else {
ret_val = errno;
}
return ret_val;
} |
Compliant Solution (POSIX, Goto Chain)
Occasionally, the number of resources to manage in one function will be too large to permit using nested ifs to manage them.
In this revised version, a goto
chain replaces each individual return segment. If no error occurs, control flow falls through to the SUCCESS
label, releases all of the resources, and returns NOERR
. If an error occurs, the return value is set to errno
, control flow jumps to the proper failure label, and the appropriate resources are released before returning.
Code Block | ||||
---|---|---|---|---|
| ||||
/* ... Assume the same struct used previously ... */ | ||||
Code Block | ||||
| ||||
// ... assume the same struct as above ... errno_t do_something(void) { FILE *fin1, *finfin2; object_t *obj1, *obj2obj; errno_t ret_val = NOERR; //* Initially assume a successful return value */ finfin1 = fopen("some_file", "r"); if (finfin1 == NULL) { ret_val = errno; goto FAIL_FINFIN1; } obj1fin2 = malloc(sizeof(object_t))fopen("some_other_file", "r"); if (obj1fin2 == NULL) { ret_val = errno; goto FAIL_OBJ1FIN2; } obj2obj = malloc(sizeof(object_t)); if (obj2obj == NULL) { ret_val = errno; goto FAIL_OBJ2OBJ; } //* ... moreMore code ... */ SUCCESS: //* Clean Freeup everything */ free(obj2obj); FAIL_OBJ2OBJ: //* Otherwise, freeclose only the objectsresources we opened allocated*/ freefclose(obj1fin2); FAIL_OBJ1FIN2: fclose(finfin1); FAIL_FINFIN1: return ret_val; } |
The benefits of this method are that This method is beneficial because the code is cleaner and we prevent the rewriting of , and the programmer does not need to rewrite similar code upon every function error.
Note that this guideline does not advocate more general uses of goto, which is still considered harmful. The use of goto in these cases is specifically to transfer control within a single function body.
Compliant Solution (copy_process()
from Linux kernel)
Some effective examples of goto
chains are quite large. This compliant solution is an excerpt from the Linux kernel. This is the copy_process
function from kernel/fork.c
from version 2.6.29 of the kernel.
The function uses 17 goto
labels (not all displayed here) to perform cleanup code should any internal function yield an error code. If no errors occur, the program returns a pointer to the new process p
. If any error occurs, the program diverts control to a particular goto
label, which performs cleanup for sections of the function that have currently been successfully executed but not for sections that have not yet been executed. Consequently, only resources that were successfully opened are actually closed.
All comments in this excerpt were added to indicate additional code in the kernel not displayed here.
Code Block | ||||
---|---|---|---|---|
| ||||
static struct task_struct *copy_process(unsigned long clone_flags, unsigned long stack_start, struct pt_regs *regs, unsigned long stack_size, int __user *child_tidptr, struct pid *pid, int trace) { int retval; struct task_struct *p; int cgroup_callbacks_done = 0; if ((clone_flags & (CLONE_NEWNS|CLONE_FS)) == (CLONE_NEWNS|CLONE_FS)) return ERR_PTR(-EINVAL); /* ... */ retval = security_task_create(clone_flags); if (retval) goto fork_out; retval = -ENOMEM; p = dup_task_struct(current); if (!p) goto fork_out; /* ... */ /* Copy all the process information */ if ((retval = copy_semundo(clone_flags, p))) goto bad_fork_cleanup_audit; if ((retval = copy_files(clone_flags, p))) goto bad_fork_cleanup_semundo; if ((retval = copy_fs(clone_flags, p))) goto bad_fork_cleanup_files; if ((retval = copy_sighand(clone_flags, p))) goto bad_fork_cleanup_fs; if ((retval = copy_signal(clone_flags, p))) goto bad_fork_cleanup_sighand; if ((retval = copy_mm(clone_flags, p))) goto bad_fork_cleanup_signal; if ((retval = copy_namespaces(clone_flags, p))) goto bad_fork_cleanup_mm; if ((retval = copy_io(clone_flags, p))) goto bad_fork_cleanup_namespaces; retval = copy_thread(0, clone_flags, stack_start, stack_size, p, regs); if (retval) goto bad_fork_cleanup_io; /* ... */ return p; /* ... Cleanup code starts here ... */ bad_fork_cleanup_io: put_io_context(p->io_context); bad_fork_cleanup_namespaces: exit_task_namespaces(p); bad_fork_cleanup_mm: if (p->mm) mmput(p->mm); bad_fork_cleanup_signal: cleanup_signal(p); bad_fork_cleanup_sighand: __cleanup_sighand(p->sighand); bad_fork_cleanup_fs: exit_fs(p); /* Blocking */ bad_fork_cleanup_files: exit_files(p); /* Blocking */ bad_fork_cleanup_semundo: exit_sem(p); bad_fork_cleanup_audit: audit_free(p); /* ... More cleanup code ... */ fork_out: return ERR_PTR(retval); } |
Risk Assessment
Failure to free allocated memory or close opened files results in a memory leak and possibly unexpected results.
Recommendation | Severity | Likelihood | Remediation Cost | Priority | Level |
---|---|---|---|---|---|
MEM12-C |
Low |
Probable |
Medium |
P3
L3
References
P4 | L3 |
Automated Detection
Tool | Version | Checker | Description | ||||||
---|---|---|---|---|---|---|---|---|---|
Klocwork |
| MLK.MIGHT MLK.MUST MLK.RET.MIGHT MLK.RET.MUST RH.LEAK | |||||||
LDRA tool suite |
| 50 D | Partially implemented | ||||||
Parasoft C/C++test |
| CERT_C-MEM12-a | Ensure resources are freed | ||||||
PC-lint Plus |
| 429 | Assistance provided | ||||||
Polyspace Bug Finder |
| Checks for memory leak and resource leak (rec. partially covered) |
Bibliography
Dijkstra, Edgar, "Go To Statement Considered Harmful.", 1968 | |
Linux Kernel Sourcecode (v2.6.xx) | 2.6.29, kernel/fork.c , the copy_process() Function |
[Seacord 2013] | Chapter 4, "Dynamic Memory Management" |
...
\[[ISO/IEC 9899:1999|AA. C References#ISO/IEC 9899-1999]\] Section 7.20.3, "Memory management functions"
\[[ISO/IEC 9899:1999|AA. C References#ISO/IEC 9899-1999]\] Section 7.19.5, "File access functions"
\[[Seacord 05|AA. C References#Seacord 05]\] Chapter 4, "Dynamic Memory Management" Wiki Markup