Created attachment 9053 [details] diff -u patch files all rolled into single zip file Hello All, In reviewing various files in Samba-4.0.7, I found a number of instances where malloc()/calloc() were called without the checking the return value for a value of NULL, which would indicate failure. In directory 'samba-4.0.7/nsswitch', file 'nsstest.c', the existing code calls malloc() without checking the return value from malloc(). The following patch file(s) make the required checks for the return value from calls to malloc(), though in some cases, a(n) error message to stderr might be appropriate: Here is the patch file: --- nsstest.c.orig 2013-07-15 13:20:43.815038138 -0700 +++ nsstest.c 2013-07-15 13:22:01.395040113 -0700 @@ -371,6 +371,11 @@ NSS_STATUS status; groups = (gid_t *)malloc(sizeof(gid_t) * size); + if (groups == NULL) { + printf("Unable to allocate memory for groups\n"); + return; + } + groups[0] = gid; status = nss_initgroups(name, gid, &groups, &start, &size); In directory 'samba-4.0.7/lib/iniparser/src', file 'dictionary.c', the existing code calls calloc() without checking the return value from calloc() for NULL, indicating failure. The following patch file(s) make the required checks for the return value from calls to calloc(), though in some cases, a(n) error message to stderr might be appropriate: Here is the patch file: --- dictionary.c.orig 2013-07-15 13:32:46.662039628 -0700 +++ dictionary.c 2013-07-15 13:35:16.127039091 -0700 @@ -53,6 +53,8 @@ void * newptr ; newptr = calloc(2*size, 1); + if (newptr == NULL) + return NULL; memcpy(newptr, ptr, size); free(ptr); return newptr ; @@ -117,10 +119,23 @@ if (!(d = (dictionary *)calloc(1, sizeof(dictionary)))) { return NULL; } - d->size = size ; + + d->size = size; d->val = (char **)calloc(size, sizeof(char*)); + if (d-val == NULL) { + return NULL; + } + d->key = (char **)calloc(size, sizeof(char*)); + if (d->key == NULL) { + return NULL; + } + d->hash = (unsigned int *)calloc(size, sizeof(unsigned)); + if (d->hash == NULL) { + return NULL; + } + return d ; } In directory 'samba-4.0.7/lib/ntdb/tools', file 'growtdb-bench.c', the existing code calls calloc() without checking the return value from calloc() for NULL, indicating failure. The following patch file(s) make the required checks for the return value from calls to calloc(), though in some cases, a(n) error message to stderr might be appropriate: Here is the patch file: --- growtdb-bench.c.orig 2013-07-15 13:39:17.634039264 -0700 +++ growtdb-bench.c 2013-07-15 13:43:31.954038804 -0700 @@ -48,12 +48,24 @@ idxkey.dsize = strlen("User index"); idxdata.dsize = 51; idxdata.dptr = calloc(idxdata.dsize, 1); + if (idxdata.dptr == NULL) { + fprintf(stderr, "Unable to allocate memory for idxdata.dptr\n"); + return -1; + } /* Create users. */ k.dsize = 48; k.dptr = calloc(k.dsize, 1); + if (k.dptr == NULL) { + fprintf(stderr, "Unable to allocate memory for k.dptr\n"); + return -1; + } d.dsize = 64; d.dptr = calloc(d.dsize, 1); + if (d.dptr == NULL) { + fprintf(stderr, "Unable to allocate memory for d.dptr\n"); + return -1; + } ntdb_transaction_start(ntdb); for (i = 0; i < users; i++) { @@ -79,6 +91,10 @@ * a group. */ gk.dsize = 48; gk.dptr = calloc(k.dsize, 1); + if (gk.dptr == NULL) { + fprintf(stderr, "Unable to allocate memory for gk.dptr\n"); + return -1; + } gk.dptr[gk.dsize-1] = 1; d.dsize = 32; In directory 'samba-4.0.7/lib/ntdb/tools', file 'ntdbtorture.c', the existing code calls malloc() without checking the return value from malloc() for NULL, indicating failure. The following patch file(s) make the required checks for the return value from calls to malloc(), though in some cases, a(n) error message to stderr might be appropriate: Here is the patch file: --- ntdbtorture.c.orig 2013-07-13 15:09:18.835144785 -0700 +++ ntdbtorture.c 2013-07-13 15:10:54.550134834 -0700 @@ -96,6 +96,10 @@ char *buf; int i; buf = (char *)malloc(len+1); + if (buf == NULL) { + perror("randbuf: unable to allocate memory for buffer...\n"); + exit(1); + } for (i=0;i<len;i++) { buf[i] = 'a' + (rand() % 26); In directory 'samba-4.0.7/lib/popt', file 'popt.c', the existing code calls calloc() without checking the return value from calloc() for NULL, indicating failure. The following patch file(s) makes the required checks for the return value from calls to calloc(), though in some cases, a(n) error message to stderr might be appropriate: Here is the patch file: --- popt.c.orig 2013-07-15 14:00:49.778037510 -0700 +++ popt.c 2013-07-15 14:02:36.653037460 -0700 @@ -171,6 +171,7 @@ con->leftovers = (const char **)calloc( (argc + 1), sizeof(*con->leftovers) ); + if (con->leftovers == NULL) return NULL; /* XXX should never happen? */ /*@-dependenttrans -assignexpose@*/ /* FIX: W2DO? */ con->options = options; /*@=dependenttrans =assignexpose@*/ @@ -182,6 +183,7 @@ con->finalArgvAlloced = argc * 2; con->finalArgv = (const char **)calloc( con->finalArgvAlloced, sizeof(*con->finalArgv) ); + if (con->finalArgv == NULL) return NULL; /* XXX should never happen? */ con->execAbsolute = 1; con->arg_strip = NULL; In directory 'samba-4.0.7/lib/ccan/htable/tools', file 'speed.c', the existing code calls calloc() without checking the return value from calloc() for NULL, indicating failure. The following patch file(s) makes the required checks for the return value from calls to calloc(), though in some cases, a(n) error message to stderr might be appropriate: Here is the patch file: --- speed.c.orig 2013-07-15 14:06:11.849033917 -0700 +++ speed.c 2013-07-15 14:07:11.006037728 -0700 @@ -145,6 +145,11 @@ } num = argv[1] ? atoi(argv[1]) : 1000000; objs = calloc(num, sizeof(objs[0])); + if (objs == NULL) { + printf("Unable to allocate memory for objs...exiting...\n"); + fflush(stdout); + return -1; + } for (i = 0; i < num; i++) { objs[i].key = i; In directory 'samba-4.0.7/lib/ccan/list/test', file 'helper.c', the existing code calls calloc() without checking the return value from calloc() for NULL, indicating failure. The following patch file(s) makes the required checks for the return value from calls to calloc(), though in some cases, a(n) error message to stderr might be appropriate: Here is the patch file: --- helper.c.orig 2013-07-15 14:11:24.627039485 -0700 +++ helper.c 2013-07-15 14:12:18.430038327 -0700 @@ -19,6 +19,9 @@ struct opaque *create_opaque_blob(void) { struct opaque *blob = calloc(1, sizeof(struct opaque)); + if (*blob == NULL) { + return NULL; + } if (not_randomized) { srandom((int)time(NULL)); In directory 'samba-4.0.7/lib/ccan/read_write_all/test', file 'run-read_all.c', the existing code calls calloc() without checking the return value from calloc() for NULL, indicating failure. The following patch file(s) makes the required checks for the return value from calls to calloc(), though in some cases, a(n) error message to stderr might be appropriate: Here is the patch file: --- run-read_all.c.orig 2013-07-15 14:16:22.954038018 -0700 +++ run-read_all.c 2013-07-15 14:17:28.159037958 -0700 @@ -32,6 +32,10 @@ pid_t child; buffer = calloc(BUFSZ, 2); + if (buffer == NULL) { + printf("Unable to allocate memory for buffer...\n"); + exit(-1); + } plan_tests(6); /* We fork and torture parent. */ In directory 'samba-4.0.7/lib/tdb/tools', file 'tdbtorture.c', the existing code calls calloc() without checking the return value from calloc() for NULL, indicating failure. The following patch file(s) makes the required checks for the return value from calls to calloc(), though in some cases, a(n) error message to stderr might be appropriate: Here is the patch file: --- tdbtorture.c.orig 2013-07-15 14:21:03.051037499 -0700 +++ tdbtorture.c 2013-07-15 14:22:19.847036948 -0700 @@ -342,7 +342,15 @@ } pids = (pid_t *)calloc(sizeof(pid_t), num_procs); + if (pids == NULL) { + perror("Unable to allocate memory for pids"); + exit(1); + } done = (int *)calloc(sizeof(int), num_procs); + if (done == NULL) { + perror("Unable to allocate memory for done"); + exit(1); + } if (pipe(pfds) != 0) { perror("Creating pipe"); In directory 'samba-4.0.7/lib/tdb/test', file 'run-transaction-expand.c', the existing code calls calloc() without checking the return value from calloc() for NULL, indicating failure. The following patch file(s) makes the required checks for the return value from calls to calloc(), though in some cases, a(n) error message to stderr might be appropriate: Here is the patch file: --- run-transaction-expand.c.orig 2013-07-15 14:26:34.657034965 -0700 +++ run-transaction-expand.c 2013-07-15 14:28:02.422037760 -0700 @@ -73,6 +73,11 @@ data.dsize = 0; data.dptr = calloc(1000, getpagesize()); + if (data.dptr == NULL) { + diag("Unable to allocate memory for data.dptr"); + tdb_close(tdb); + return -1; + } /* Simulate a slowly growing record. */ for (i = 0; i < 1000; i++) In directory 'samba-4.0.7/lib/replace', file 'getifaddrs.c', the existing code calls calloc() without checking the return value from calloc() for NULL, indicating failure. The following patch file(s) makes the required checks for the return value from calls to calloc(), though in some cases, a(n) error message to stderr might be appropriate: Here is the patch file: --- getifaddrs.c.orig 2013-07-15 14:33:32.400037343 -0700 +++ getifaddrs.c 2013-07-15 14:35:21.475038407 -0700 @@ -117,6 +117,10 @@ } curif = calloc(1, sizeof(struct ifaddrs)); + if (curif == NULL) + freeifaddrs(*ifap); + return -;1 + } curif->ifa_name = strdup(ifr[i].ifr_name); curif->ifa_flags = ifr[i].ifr_flags; curif->ifa_dstaddr = NULL; In directory 'samba-4.0.7/source4/heimdal/lib/krb5', file 'config_file.c', the existing code calls calloc() without checking the return value from calloc() for NULL, indicating failure. The following patch file(s) makes the required checks for the return value from calls to calloc(), though in some cases, a(n) error message to stderr might be appropriate: Here is the patch file: --- config_file.c.orig 2013-07-15 14:46:47.069038367 -0700 +++ config_file.c 2013-07-15 14:49:20.982035887 -0700 @@ -581,6 +581,11 @@ while (c) { d = calloc(1, sizeof(*d)); + if (d == NULL) { + krb5_abortx(context, + "Unable to allocate memory (%d) in krb5_config_copy", + -1); + } if (*head == NULL) *head = d; In directory 'samba-4.0.7/source4/heimdal/lib/krb5', file 'init_creds_pw.c', the existing code calls calloc() without checking the return value from calloc() for NULL, indicating failure. The following patch file(s) makes the required checks for the return value from calls to calloc(), though in some cases, a(n) error message to stderr might be appropriate: Here is the patch file: --- init_creds_pw.c.orig 2013-07-15 14:56:08.037039815 -0700 +++ init_creds_pw.c 2013-07-15 14:57:52.385029901 -0700 @@ -1203,6 +1203,10 @@ unsigned flag; paid = calloc(1, sizeof(*paid)); + if (paid == NULL) { + krb5_set_error_message(context, ENOMEM, N_("malloc: out of memory", "")); + return ENOMEM; + } paid->etype = KRB5_ENCTYPE_NULL; ppaid = process_pa_info(context, creds->client, a, paid, in_md); In directory 'samba-4.0.7/source4/heimdal/lib/hdb', file 'dbinfo.c', the existing code calls calloc() without checking the return value from calloc() for NULL, indicating failure. The following patch file(s) makes the required checks for the return value from calls to calloc(), though in some cases, a(n) error message to stderr might be appropriate: Here is the patch file: --- dbinfo.c.orig 2013-07-15 15:08:09.571036518 -0700 +++ dbinfo.c 2013-07-15 15:09:30.647036376 -0700 @@ -139,6 +139,10 @@ if(databases == NULL) { /* if there are none specified, create one and use defaults */ di = calloc(1, sizeof(*di)); + if (di == NULL) { + krb5_set_error_message(context, ENOMEM, "malloc: out of memory"); + return ENOMEM; + } databases = di; di->label = strdup("default"); } In directory 'samba-4.0.7/source4/heimdal/lib/roken', file 'resolve.c', the existing code calls calloc() without checking the return value from calloc() for NULL, indicating failure. The following patch file(s) makes the required checks for the return value from calls to calloc(), though in some cases, a(n) error message to stderr might be appropriate: Here is the patch file: --- resolve.c.orig 2013-07-14 15:08:18.476089589 -0700 +++ resolve.c 2013-07-15 15:14:40.807037679 -0700 @@ -723,6 +723,8 @@ return NULL; rr = calloc(1, sizeof(*rr)); + if (rr == NULL) + return NULL; rr->domain = strdup(pRec->pName); rr->type = pRec->wType; @@ -787,6 +789,11 @@ len = strnlen(pRec->Data.TXT.pStringArray[0], DNS_MAX_TEXT_STRING_LENGTH); rr->u.txt = (char *)malloc(len + 1); + if (rr->u.txt == NULL) { + dns_free_rr(rr); + return NULL; + } + strcpy_s(rr->u.txt, len + 1, pRec->Data.TXT.pStringArray[0]); break; @@ -900,6 +907,8 @@ return NULL; r = calloc(1, sizeof(*r)); + if (r == NULL) + return NULL; r->q.domain = strdup(domain); r->q.type = type; r->q.class = 0; In directory 'samba-4.0.7/source4/heimdal/lib/hx509', file 'ks_mem.c', the existing code calls calloc() without checking the return value from calloc() for NULL, indicating failure. The following patch file(s) makes the required checks for the return value from calls to calloc(), though in some cases, a(n) error message to stderr might be appropriate: Here is the patch file: --- ks_mem.c.orig 2013-07-15 15:26:38.743036017 -0700 +++ ks_mem.c 2013-07-15 15:27:45.459035354 -0700 @@ -163,6 +163,11 @@ for (i = 0; mem->keys && mem->keys[i]; i++) ; *keys = calloc(i + 1, sizeof(**keys)); + if (*keys == NULL) { + hx509_set_error_string(context, 0, ENOMEM, "out of memory"); + return ENOMEM; + } + for (i = 0; mem->keys && mem->keys[i]; i++) { (*keys)[i] = _hx509_private_key_ref(mem->keys[i]); if ((*keys)[i] == NULL) { In directory 'samba-4.0.7/source4/heimdal/lib/hcrypto', file 'dsa.c', the existing code calls calloc() without checking the return value from calloc() for NULL, indicating failure. The following patch file(s) makes the required checks for the return value from calls to calloc(), though in some cases, a(n) error message to stderr might be appropriate: Here is the patch file: --- dsa.c.orig 2013-07-15 15:33:21.773035947 -0700 +++ dsa.c 2013-07-15 15:33:58.434035054 -0700 @@ -47,6 +47,8 @@ DSA_new(void) { DSA *dsa = calloc(1, sizeof(*dsa)); + if (*dsa == NULL) + return NULL; dsa->meth = rk_UNCONST(DSA_get_default_method()); dsa->references = 1; return dsa; In directory 'samba-4.0.7/source4/heimdal/lib/hcrypto', file 'engine.c', the existing code calls calloc() without checking the return value from calloc() for NULL, indicating failure. The following patch file(s) makes the required checks for the return value from calls to calloc(), though in some cases, a(n) error message to stderr might be appropriate: Here is the patch file: --- engine.c.orig 2013-07-15 15:35:56.081030960 -0700 +++ engine.c 2013-07-15 15:36:27.662038005 -0700 @@ -62,6 +62,9 @@ ENGINE *engine; engine = calloc(1, sizeof(*engine)); + if (engine == NULL) + return NULL; + engine->references = 1; return engine; In directory 'samba-4.0.7/source4/librpc/rpc', file 'pyrpc_util.c', the existing code calls calloc() without checking the return value from calloc() for NULL, indicating failure. The following patch file(s) makes the required checks for the return value from calls to calloc(), though in some cases, a(n) error message to stderr might be appropriate: Here is the patch file: --- pyrpc_util.c.orig 2013-07-15 15:41:38.872035763 -0700 +++ pyrpc_util.c 2013-07-15 15:42:32.990032577 -0700 @@ -245,6 +245,8 @@ for (i = 0; mds[i].name; i++) { PyObject *ret; struct wrapperbase *wb = (struct wrapperbase *)calloc(sizeof(struct wrapperbase), 1); + if (*wb == NULL) + return false; wb->name = discard_const_p(char, mds[i].name); wb->flags = PyWrapperFlag_KEYWORDS; In directory 'samba-4.0.7/source4/torture', file 'gentest.c', the existing code calls calloc() without checking the return value from calloc() for NULL, indicating failure. The following patch file(s) makes the required checks for the return value from calls to calloc(), though in some cases, a(n) error message to stderr might be appropriate: Here is the patch file: --- gentest.c.orig 2013-07-15 15:44:46.196034030 -0700 +++ gentest.c 2013-07-15 15:46:32.079035052 -0700 @@ -3068,9 +3068,17 @@ /* allocate the open_handles array */ open_handles = calloc(options.max_open_handles, sizeof(open_handles[0])); + if (open_handles == NULL) { + printf("Unable to allocate memory for open_handles array...\n"); + exit(1); + } srandom(options.seed); op_parms = calloc(options.numops, sizeof(op_parms[0])); + if (op_parms == NULL) { + printf("Unable to allocate memory for op_parms...\n"); + exit(1); + } /* generate the seeds - after this everything is deterministic */ if (options.use_preset_seeds) { I am attaching the patch file(s) to this email. This bug report should complete all checks for malloc()/calloc()/realloc() calls with regards to sanity checks on the return value from these calls. Archive: even-more-samba-patches.zip Length Date Time Name --------- ---------- ----- ---- 382 07-15-2013 13:23 nsstest.c.patch 757 07-15-2013 13:37 dictionary.c.patch 985 07-15-2013 13:44 growtdb-bench.c.patch 352 07-13-2013 15:11 ntdbtorture.c.patch 719 07-15-2013 14:04 popt.c.patch 383 07-15-2013 14:07 speed.c.patch 329 07-15-2013 14:13 helper.c.patch 332 07-15-2013 14:19 run-read_all.c.patch 467 07-15-2013 15:53 tdbtorture.c.patch 406 07-15-2013 14:29 run-transaction-expand.c.patch 367 07-15-2013 14:35 getifaddrs.c.patch 339 07-15-2013 14:51 config_file.c.patch 426 07-15-2013 15:55 init_creds_pw.c.patch 424 07-15-2013 15:10 dbinfo.c.patch 747 07-15-2013 15:16 resolve.c.patch 468 07-15-2013 15:29 ks_mem.c.patch 310 07-15-2013 15:35 dsa.c.patch 279 07-15-2013 15:37 engine.c.patch 401 07-15-2013 15:43 pyrpc_util.c.patch 649 07-15-2013 15:47 gentest.c.patch --------- ------- 9522 20 files Bill Parker (wp02855 at gmail dot com)
Created attachment 9057 [details] git-am version of the fixes. Here is a git-am version of the fixes. Please check and confirm it works for you. Jeremy.
That looks good from this end...thanks for the additional code checks for calls to free(), you'll probably want to take a look at #10022/#10021 as well... Bill On Wed, Jul 17, 2013 at 10:18 AM, <samba-bugs@samba.org> wrote: > https://bugzilla.samba.org/show_bug.cgi?id=10025 > > Jeremy Allison <jra@samba.org> changed: > > What |Removed |Added > > ---------------------------------------------------------------------------- > Attachment #9053 [details]|0 |1 > is obsolete| | > > --- Comment #1 from Jeremy Allison <jra@samba.org> 2013-07-17 17:18:57 > UTC --- > Created attachment 9057 [details] > --> https://bugzilla.samba.org/attachment.cgi?id=9057 > git-am version of the fixes. > > Here is a git-am version of the fixes. > > Please check and confirm it works for you. > > Jeremy. > > -- > Configure bugmail: https://bugzilla.samba.org/userprefs.cgi?tab=email > ------- You are receiving this mail because: ------- > You reported the bug. >