Versions Compared

Key

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

...

Code Block
bgColor#FFCCCC
langc
typedef struct object {   //* AGeneric generic struct -- Thestruct: 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;  //* forgotForgot to close fin2 !! */
  }

  //* ... moreMore code ... */

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

This is just a small example; in much larger examples, errors like this would be are even harder to detect.

Compliant Solution

...

Code Block
bgColor#CCCCFF
langc
//* ... assumeAssume the same struct asused abovepreviously ... */

errno_t do_something(void) {
  FILE *fin1, *fin2;
  object_t *obj;
  errno_t ret_val = NOERR; //* Initially assume a successful return value */

  fin1 = fopen("some_file", "r");
  if (fin == NULL) {
    ret_val = errno;
    goto FAIL_FIN1;
  }

  fin2 = fopen("some_other_file", "r");
  if (fin2 == NULL) {
    ret_val = errno;
    goto FAIL_FIN2;
  }

  obj = malloc(sizeof(object_t));
  if (obj == NULL) {
    ret_val = errno;
    goto FAIL_OBJ;
  }

  //* ... moreMore code ... */

SUCCESS:     //* Clean up everything */
  free(obj);

FAIL_OBJ:   //* Otherwise, close only the resources we opened */
  fclose(fin2);

FAIL_FIN2:
  fclose(fin1);

FAIL_FIN1:
  return ret_val;
}

...

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 of the function that have not 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;

  /* ... */

  /* copyCopy 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;

  /* ... cleanupCleanup 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); /* blockingBlocking */
bad_fork_cleanup_files:
  exit_files(p); /* blockingBlocking */
bad_fork_cleanup_semundo:
  exit_sem(p);
bad_fork_cleanup_audit:
  audit_free(p);

  /* ... moreMore cleanup code ... */

fork_out:
  return ERR_PTR(retval);
}

...

Recommendation

Severity

Likelihood

Remediation Cost

Priority

Level

MEM12-C

lowLow

probableProbable

mediumMedium

P4

L3

Bibliography

Linux Kernel Sourcecode (v2.6.xx)2.6.29, kernel/fork.c, the copy_process() Function
[Seacord 2013]Chapter 4, "Dynamic Memory Management"

...