There is a memory leak in `sys/cam/ctl/ctl.c`. `ctl_copyin_alloc` performs and returns an allocation with `malloc`: static void * ctl_copyin_alloc(void *user_addr, int len, char *error_str, size_t error_str_len) { void *kptr; kptr = malloc(len, M_CTL, M_WAITOK | M_ZERO); if (copyin(user_addr, kptr, len) != 0) { snprintf(error_str, error_str_len, "Error copying %d bytes " "from user address %p to kernel address %p", len, user_addr, kptr); free(kptr, M_CTL); return (NULL); } return (kptr); } `ctl_copyin_args` calls this function, but doesn't free the returned allocation on the condition that the string is not terminated, before going to `bailout`: static struct ctl_be_arg * ctl_copyin_args(int num_args, struct ctl_be_arg *uargs, char *error_str, size_t error_str_len) { ... uint8_t *tmpptr; ... if (args[i].flags & CTL_BEARG_RD) { tmpptr = ctl_copyin_alloc(args[i].value, args[i].vallen, error_str, error_str_len); if (tmpptr == NULL) goto bailout; if ((args[i].flags & CTL_BEARG_ASCII) && (tmpptr[args[i].vallen - 1] != '\0')) { snprintf(error_str, error_str_len, "Argument " "%d value is not NUL-terminated", i); goto bailout; } args[i].kvalue = tmpptr; } else { args[i].kvalue = malloc(args[i].vallen, M_CTL, M_WAITOK | M_ZERO); } } return (args); bailout: ctl_free_args(num_args, args); return (NULL); } Should be: snprintf(error_str, error_str_len, "Argument " "%d value is not NUL-terminated", i); free(tmpptr); goto bailout;
Patch: https://github.com/HardenedBSD/hardenedBSD-playground/commit/88bf3296d98a0728c54a3ac783ac11ea23669132.patch
Created attachment 167620 [details] Patch to fix the memory leak
Please attribute the fix to HardenedBSD.
Shawn, in your patch, setting `tmpptr` to `NULL` after it has been freed is unnecessary since it is out of scope of the `bailout` label, so nothing can use the freed pointer to cause use after free, after the `goto`; however it may be preferable to do so out of good practice.
A commit references this bug: Author: sbruno Date: Tue Apr 19 16:48:15 UTC 2016 New revision: 298279 URL: https://svnweb.freebsd.org/changeset/base/298279 Log: Plug memory leak in ctl(4) when ctl_copyin_args() is called with a non- null terminated ASCII string. PR: 207626 Submitted by: cturt@hardenedbsd.org MFC after: 2 days Changes: head/sys/cam/ctl/ctl.c
A commit references this bug: Author: sbruno Date: Fri May 6 19:11:48 UTC 2016 New revision: 299191 URL: https://svnweb.freebsd.org/changeset/base/299191 Log: MFC r298279 Plug memory leak in ctl(4) when ctl_copyin_args() is called with a non- null terminated ASCII string. PR: 207626 Submitted by: cturt@hardenedbsd.org Changes: _U stable/10/ stable/10/sys/cam/ctl/ctl.c