Versions Compared

Key

  • This line was added.
  • This line was removed.
  • Formatting was changed.

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 while preserving the order of freed resources.

...

Please note that these examples assume errno_t and NOERR to be defined, as in recommendation 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.

Code Block
bgColor#FFCCCC
langc

typedef struct object {   // A generic struct -- The contents don't matter
  int propertyA, propertyB, propertyC;
} object_t;


errno_t do_something(void){
  FILE *fin1, *fin2;
  object_t *obj;
  errno_t ret_val;
  
  fin1 = fopen("some_file", "r");
  if (fin1 == NULL) {
    return errno;
  }

  fin2 = fopen("some_other_file", "r");
  if (fin2 == NULL) {
    fclose(fin1);
    return errno;
  }

  obj = malloc(sizeof(object_t));
  if (obj == NULL) {
    ret_val = errno;
    fclose(fin1);
    return ret_val;  // forgot to close fin2 !!
  }

  // ... more code ...

  fclose(fin1);
  fclose(fin2);
  free(obj);
  return NOERR;
}

...

Compliant Solution

In this revised version, we have used a goto chain in replacement of chain replaces each individual return segment. If there is no error occurs, control flow will fall falls through to the SUCCESS label, release releases all of the resources, and return returns NOERR. In the case of If an error occurs, the return value will be value is set to errno, control flow will jump jumps to the proper failure label, and the appropriate resources will be resources are released before returning.

...

This method is beneficial because the code is cleaner, and we do not and the programmer does not need to rewrite similar code upon every function error.

...

Some effective examples of goto chains are quite large. The following compliant code is an excerpt from the Linux kernel. This is the copy_process function from kernel/fork.c from Version version 2.6.29 of the kernel.

The function uses 17 goto labels (not all displayed here) in order 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 the sections of the function that have currently been successfully executed , while not performing cleanup on sections but not for sections of the function that have not been executed yetnot yet been executed. Consequently, only resources that were successfully opened are actually closed.

...

Code Block
bgColor#CCCCFF
langc

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);
}

...

Recommendation

Severity

Likelihood

Remediation Cost

Priority

Level

MEM12-C

low

probable

medium

P4

L3

Related Guidelines

ISO/IEC 9899:19992011 Section 7.2022.3, "Memory management functions"ISO/IEC 9899:1999 ," and Section 7.1921.5, "File access functions"

...