Versions Compared

Key

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

...

Code Block
bgColor#FFCCCC
langc
typedef struct object {  /* Generic struct: 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;
}

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

...

/* ... 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 */ fin1 = fopen("some_file", "r"); if (fin1 == 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; } /* ... More 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; }
Code Block
bgColor#CCCCFF
langc

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

...

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); }
Code Block
bgColor#CCCCFF
langc

Risk Assessment

Failure to free allocated memory or close opened files results in a memory leak and possibly unexpected results.

...

Tool

Version

Checker

Description

CERT C Rules implemented in the LDRA tool suite
Include Page
LDRA_V
LDRA_V
50 DPartially Implemented

...

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

 

...

Image Modified