Skip to content

Commit

Permalink
Check return values of calls to malloc() everywhere. Try to let progr…
Browse files Browse the repository at this point in the history
…ams continue running.
  • Loading branch information
ytrezq committed Jun 26, 2016
1 parent 2707823 commit 417647a
Show file tree
Hide file tree
Showing 30 changed files with 136 additions and 59 deletions.
9 changes: 9 additions & 0 deletions doc/doc-txt/ChangeLog
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,15 @@ LC/01 Prefer the use of size_t for variables representing sizes. Even if most
LC/02 Some values representing maximum path size were hard coded.
They are now replaced with the PATH_MAX macro.

LC/03 As everybody knows, malloc() can fails by returning 0. The return values
weren’t checked everywhere.
The values are checked manually in order handle the situation in way that
let the program continue running. Otherwise, replace direct calls to
malloc() with store_malloc() from the project standard memory management
facilities in order to stop the program.
Except if it isn’t possible to call store_malloc() or that some ressources
cleanup need to done.


Exim version 4.87
-----------------
Expand Down
8 changes: 4 additions & 4 deletions src/OS/Makefile-Base
Original file line number Diff line number Diff line change
Expand Up @@ -408,9 +408,9 @@ exim_tidydb: $(OBJ_TIDYDB)

# The utility for building dbm files

exim_dbmbuild: exim_dbmbuild.o
exim_dbmbuild: util-store.o exim_dbmbuild.o
@echo "$(LNCC) -o exim_dbmbuild"
$(FE)$(LNCC) $(CFLAGS) $(INCLUDE) -o exim_dbmbuild $(LFLAGS) exim_dbmbuild.o \
$(FE)$(LNCC) $(CFLAGS) $(INCLUDE) -o exim_dbmbuild $(LFLAGS) exim_dbmbuild.o util-store.o \
$(LIBS) $(EXTRALIBS) $(DBMLIB)
@if [ x"$(STRIP_COMMAND)" != x"" ]; then \
echo $(STRIP_COMMAND) exim_dbmbuild; \
Expand All @@ -421,11 +421,11 @@ exim_dbmbuild: exim_dbmbuild.o

# The utility for locking a mailbox while messing around with it

exim_lock: exim_lock.c os.h
exim_lock: util-store.o exim_lock.c os.h
@echo "$(CC) exim_lock.c"
$(FE)$(CC) -c $(CFLAGS) $(INCLUDE) exim_lock.c
@echo "$(LNCC) -o exim_lock"
$(FE)$(LNCC) -o exim_lock $(LFLAGS) exim_lock.o \
$(FE)$(LNCC) -o exim_lock $(LFLAGS) exim_lock.o util-store.o \
$(LIBS) $(EXTRALIBS)
@if [ x"$(STRIP_COMMAND)" != x"" ]; then \
echo $(STRIP_COMMAND) exim_lock; \
Expand Down
3 changes: 2 additions & 1 deletion src/exim_monitor/em_version.c
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

#include "mytypes.h"
#include "macros.h"
#include "store.h"
#include <string.h>
#include <stdlib.h>

Expand All @@ -25,7 +26,7 @@ Ustrcpy(today, __DATE__);
if (today[4] == ' ') i = 1;
today[3] = today[6] = '-';

version_date = (uschar *)malloc(32);
version_date = (uschar *)store_malloc(32);
version_date[0] = 0;
Ustrncat(version_date, today+4+i, 3-i);
Ustrncat(version_date, today, 4);
Expand Down
4 changes: 2 additions & 2 deletions src/exim_monitor/em_xs.c
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ void xs_SetValues(Widget w, Cardinal num_args, ...)
{
int i;
va_list ap;
Arg *aa = (num_args > 15)? (Arg *)malloc(num_args*sizeof(Arg)) : xs_temparg;
Arg *aa = (num_args > 15)? (Arg *)store_malloc(num_args*sizeof(Arg)) : xs_temparg;
va_start(ap, num_args);
for (i = 0; i < num_args; i++)
{
Expand All @@ -39,7 +39,7 @@ for (i = 0; i < num_args; i++)
}
va_end(ap);
XtSetValues(w, aa, num_args);
if (num_args > 15) free(aa);
if (num_args > 15) store_free(aa);
}

/* End of em_xs.c */
11 changes: 6 additions & 5 deletions src/src/auths/auth-spa.c
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,7 @@ extern int DEBUGLEVEL;

#include <sys/types.h> /* For size_t */
#include "auth-spa.h"
#include "../store.h"
#include <assert.h>
#include <ctype.h>
#include <stdio.h>
Expand Down Expand Up @@ -1401,7 +1402,7 @@ spa_build_auth_request (SPAAuthRequest * request, char *user, char *domain)
SIVAL (&request->flags, 0, 0x0000b207); /* have to figure out what these mean */
spa_string_add (request, user, u);
spa_string_add (request, domain, domain);
free (u);
store_free (u);
}


Expand Down Expand Up @@ -1483,8 +1484,8 @@ spa_build_auth_response (SPAAuthChallenge * challenge,

response->flags = challenge->flags;

free (d);
free (u);
store_free (d);
store_free (u);
}
#endif

Expand Down Expand Up @@ -1537,6 +1538,6 @@ spa_build_auth_response (SPAAuthChallenge * challenge,
spa_string_add (response, sessionKey, NULL);
response->flags = challenge->flags;

if (d != NULL) free (d);
free (u);
if (d != NULL) store_free (d);
store_free (u);
}
2 changes: 1 addition & 1 deletion src/src/auths/call_pam.c
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ for (i = 0; i < num_msg; i++)
break;

default: /* Must be an error of some sort... */
free (reply);
store_free (reply);
pam_conv_had_error = TRUE;
return PAM_CONV_ERR;
}
Expand Down
2 changes: 1 addition & 1 deletion src/src/auths/gsasl_exim.c
Original file line number Diff line number Diff line change
Expand Up @@ -358,7 +358,7 @@ auth_gsasl_server(auth_instance *ablock, uschar *initial_data)
auth_get_no64_data((uschar **)&received, (uschar *)to_send);

if (to_send) {
free(to_send);
store_free(to_send);
to_send = NULL;
}

Expand Down
4 changes: 2 additions & 2 deletions src/src/auths/heimdal_gssapi.c
Original file line number Diff line number Diff line change
Expand Up @@ -160,8 +160,8 @@ auth_heimdal_gssapi_init(auth_instance *ablock)
principal ? principal : "??",
entry.vno,
enctype_s ? enctype_s : "??");
free(principal);
free(enctype_s);
store_free(principal);
store_free(enctype_s);
krb5_kt_free_entry(context, &entry);
}
krc = krb5_kt_end_seq_get(context, keytab, &cursor);
Expand Down
4 changes: 4 additions & 0 deletions src/src/buildconfig.c
Original file line number Diff line number Diff line change
Expand Up @@ -688,6 +688,10 @@ else if (isgroup)
while (*p != 0) if (*p++ == ':') count++;

vector = malloc((count+1) * sizeof(uid_t));
if (!vector) {
printf("memory allocation falied");
return 1;
}
vector[0] = (uid_t)count;

for (i = 1, j = 0; i <= count; list++, i++)
Expand Down
5 changes: 5 additions & 0 deletions src/src/dbfn.c
Original file line number Diff line number Diff line change
Expand Up @@ -465,6 +465,11 @@ spool_directory = argv[1];
debug_selector = D_all - D_memory;
debug_file = stderr;
big_buffer = malloc(big_buffer_size);
if (!big_buffer)
{
printf("Memory allocation failed!\n");
return 1;
}

for (i = 0; i < max_db; i++) dbblock[i].dbptr = NULL;

Expand Down
17 changes: 9 additions & 8 deletions src/src/dbstuff.h
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ utilities as well as the main Exim binary. */
/* ************************* tdb interface ************************ */

#include <tdb.h>
#include "store.h"

/* Basic DB type */
#define EXIM_DB TDB_CONTEXT
Expand Down Expand Up @@ -64,17 +65,17 @@ tdb_traverse to be called) */

/* EXIM_DBCREATE_CURSOR - initialize for scanning operation */
#define EXIM_DBCREATE_CURSOR(db, cursor) { \
*(cursor) = malloc(sizeof(TDB_DATA)); (*(cursor))->dptr = NULL; }
*(cursor) = store_malloc(sizeof(TDB_DATA)); (*(cursor))->dptr = NULL; }

/* EXIM_DBSCAN - This is complicated because we have to free the last datum
free() must not die when passed NULL */
#define EXIM_DBSCAN(db, key, data, first, cursor) \
(key = (first ? tdb_firstkey(db) : tdb_nextkey(db, *(cursor))), \
free((cursor)->dptr), *(cursor) = key, \
store_free((cursor)->dptr), *(cursor) = key, \
key.dptr != NULL)

/* EXIM_DBDELETE_CURSOR - terminate scanning operation. */
#define EXIM_DBDELETE_CURSOR(cursor) free(cursor)
#define EXIM_DBDELETE_CURSOR(cursor) store_free(cursor)

/* EXIM_DBCLOSE */
#define EXIM_DBCLOSE(db) tdb_close(db)
Expand Down Expand Up @@ -395,7 +396,7 @@ typedef struct {
(*(dbpp))->lkey.dptr = NULL;\
(*(dbpp))->gdbm = gdbm_open(CS name, 0, (((flags) & O_CREAT))?GDBM_WRCREAT:(((flags) & (O_RDWR|O_WRONLY))?GDBM_WRITER:GDBM_READER), mode, 0);\
if ((*(dbpp))->gdbm == NULL) {\
free(*(dbpp));\
store_free(*(dbpp));\
*(dbpp) = NULL;\
}\
}\
Expand Down Expand Up @@ -427,7 +428,7 @@ typedef struct {
/* EXIM_DBSCAN */
#define EXIM_DBSCAN(db, key, data, first, cursor) \
( key = ((first)? gdbm_firstkey(db->gdbm) : gdbm_nextkey(db->gdbm, db->lkey)), \
(((db)->lkey.dptr != NULL)? (free((db)->lkey.dptr),1) : 1),\
(((db)->lkey.dptr != NULL)? (store_free((db)->lkey.dptr),1) : 1),\
db->lkey = key, key.dptr != NULL)

/* EXIM_DBDELETE_CURSOR - terminate scanning operation (null). Make it
Expand All @@ -437,8 +438,8 @@ refer to cursor, to keep picky compilers happy. */
/* EXIM_DBCLOSE */
#define EXIM_DBCLOSE(db) \
{ gdbm_close((db)->gdbm);\
if ((db)->lkey.dptr != NULL) free((db)->lkey.dptr);\
free(db); }
if ((db)->lkey.dptr != NULL) store_free((db)->lkey.dptr);\
store_free(db); }

/* Datum access types - these are intended to be assignable */

Expand All @@ -449,7 +450,7 @@ refer to cursor, to keep picky compilers happy. */
after reading data. */

#define EXIM_DATUM_INIT(datum)
#define EXIM_DATUM_FREE(datum) free(datum.dptr)
#define EXIM_DATUM_FREE(datum) store_free(datum.dptr)

#else /* USE_GDBM */

Expand Down
4 changes: 2 additions & 2 deletions src/src/dmarc.c
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ static dmarc_exim_p dmarc_policy_description[] = {
static error_block *
add_to_eblock(error_block *eblock, uschar *t1, uschar *t2)
{
error_block *eb = malloc(sizeof(error_block));
error_block *eb = store_malloc(sizeof(error_block));
if (eblock == NULL)
eblock = eb;
else
Expand Down Expand Up @@ -347,7 +347,7 @@ if (!dmarc_abort && !sender_host_authenticated)
libdm_status = opendmarc_policy_fetch_utilized_domain(dmarc_pctx,
dmarc_domain, DMARC_MAXHOSTNAMELEN-1);
dmarc_used_domain = string_copy(dmarc_domain);
free(dmarc_domain);
store_free(dmarc_domain);

if (libdm_status != DMARC_PARSE_OKAY)
log_write(0, LOG_MAIN|LOG_PANIC,
Expand Down
8 changes: 4 additions & 4 deletions src/src/exim.c
Original file line number Diff line number Diff line change
Expand Up @@ -1282,7 +1282,7 @@ for (i = 0;; i++)
yield = string_catn(yield, &size, &ptr, p, ss - p);

#ifdef USE_READLINE
if (fn_readline != NULL) free(readline_line);
if (fn_readline != NULL) store_free(readline_line);
#endif

/* yield can only be NULL if ss==p */
Expand Down Expand Up @@ -3973,7 +3973,7 @@ EXIM_TMPDIR by the build scripts.
if (Ustrncmp(*p, "TMPDIR=", 7) == 0 &&
Ustrcmp(*p+7, EXIM_TMPDIR) != 0)
{
uschar *newp = malloc(Ustrlen(EXIM_TMPDIR) + 8);
uschar *newp = store_malloc(Ustrlen(EXIM_TMPDIR) + 8);
sprintf(CS newp, "TMPDIR=%s", EXIM_TMPDIR);
*p = newp;
DEBUG(D_any) debug_printf("reset TMPDIR=%s in environment\n", EXIM_TMPDIR);
Expand Down Expand Up @@ -4010,15 +4010,15 @@ else
int count = 0;
if (environ) while (*p++ != NULL) count++;
if (envtz == NULL) count++;
newp = new = malloc(sizeof(uschar *) * (count + 1));
newp = new = store_malloc(sizeof(uschar *) * (count + 1));
if (environ) for (p = USS environ; *p != NULL; p++)
{
if (Ustrncmp(*p, "TZ=", 3) == 0) continue;
*newp++ = *p;
}
if (timezone_string != NULL)
{
*newp = malloc(Ustrlen(timezone_string) + 4);
*newp = store_malloc(Ustrlen(timezone_string) + 4);
sprintf(CS *newp++, "TZ=%s", timezone_string);
}
*newp = NULL;
Expand Down
4 changes: 2 additions & 2 deletions src/src/exim_dbmbuild.c
Original file line number Diff line number Diff line change
Expand Up @@ -151,8 +151,8 @@ uschar *bptr;
uschar keybuffer[256];
uschar temp_dbmname[512];
uschar real_dbmname[512];
uschar *buffer = malloc(max_outsize);
uschar *line = malloc(max_insize);
uschar *buffer = store_malloc(max_outsize);
uschar *line = store_malloc(max_insize);

while (argc > 1)
{
Expand Down
5 changes: 3 additions & 2 deletions src/src/exim_lock.c
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ Copyright (c) The Exim Maintainers 2016
*/

#include "os.h"
#include "store.h"

#include <stdio.h>
#include <stdlib.h>
Expand Down Expand Up @@ -299,9 +300,9 @@ if (use_lockfile)
primary_hostname = s.nodename;

len = (int)strlen(filename);
lockname = malloc(len + 8);
lockname = store_malloc(len + 8);
sprintf(lockname, "%s.lock", filename);
hitchname = malloc(len + 32 + (int)strlen(primary_hostname));
hitchname = store_malloc(len + 32 + (int)strlen(primary_hostname));

/* Presumably, this must match appendfile.c */
sprintf(hitchname, "%s.%s.%08x.%08x", lockname, primary_hostname,
Expand Down
6 changes: 6 additions & 0 deletions src/src/expand.c
Original file line number Diff line number Diff line change
Expand Up @@ -7746,6 +7746,12 @@ debug_selector = D_v;
debug_file = stderr;
debug_fd = fileno(debug_file);
big_buffer = malloc(big_buffer_size);
if (!big_buffer)
{
printf("** error Memory allocation failed!\n");
exit(EXIT_FAILURE);
}


for (i = 1; i < argc; i++)
{
Expand Down
6 changes: 6 additions & 0 deletions src/src/hash.c
Original file line number Diff line number Diff line change
Expand Up @@ -787,10 +787,16 @@ for (i = 0; i < sizeof(tests)/sizeof(uschar *); i ++)
/* 1 000 000 repetitions of "a" */

ctest = malloc(1000000);
if(!ctest)
{
printf("Memory allocation failed!\n*** No match ***\n");
exit(EXIT_FAILURE);
}
memset(ctest, 'a', 1000000);

printf("1 000 000 repetitions of 'a'\n");
printf("Should be: %s\n", atest);
free(ctest);
native_sha1_start(&base);
native_sha1_end(&base, ctest, 1000000, digest);
for (j = 0; j < 20; j++) sprintf(s+2*j, "%02X", digest[j]);
Expand Down
4 changes: 2 additions & 2 deletions src/src/lookups/ldap.c
Original file line number Diff line number Diff line change
Expand Up @@ -746,7 +746,7 @@ while ((rc = ldap_result(lcp->ld, msgid, 0, timeoutptr, &result)) ==
#if defined LDAP_LIB_NETSCAPE || defined LDAP_LIB_OPENLDAP2
ldap_memfree(dn);
#else /* OPENLDAP 1, UMich, Solaris */
free(dn);
store_free(dn);
#endif
}
/* Save for later */
Expand Down Expand Up @@ -904,7 +904,7 @@ if (dn != NULL)
#if defined LDAP_LIB_NETSCAPE || defined LDAP_LIB_OPENLDAP2
ldap_memfree(dn);
#else /* OPENLDAP 1, UMich, Solaris */
free(dn);
store_free(dn);
#endif
}

Expand Down
Loading

0 comments on commit 417647a

Please sign in to comment.