xmalloc(foo*bar) -> xcalloc(foo, bar) for Portable

Darren Tucker dtucker at zip.com.au
Thu May 4 12:35:39 EST 2006


On Thu, May 04, 2006 at 12:18:57PM +1000, Darren Tucker wrote:
> While wandering in auth-pam.c I noticed that there's a few Portable-specific
> escapees from the xmalloc(foo * bar) cleanup.

My grep for xmalloc missed a couple of bare mallocs, new patch below.

> There's also a "probably can't happen" integer overflow in
> ssh-rand-helper.c with the memset:
> 
>         num_cmds = 64;
> -       entcmd = xmalloc(num_cmds * sizeof(entropy_cmd_t));
> +       entcmd = xcalloc(num_cmds, sizeof(entropy_cmd_t));
>         memset(entcmd, '\0', num_cmds * sizeof(entropy_cmd_t));
> 
> Unless sizeof(entropy_cmd_t) is *really* big, it ought to safe :-)

Looking at xcalloc I see it's always safe since it will abort on overflow.
The memset is redundant, though, since calloc zeros and there's a few
similar.  Zap 'em?

Index: auth-pam.c
===================================================================
RCS file: /usr/local/src/security/openssh/cvs/openssh_cvs/auth-pam.c,v
retrieving revision 1.132
diff -u -p -r1.132 auth-pam.c
--- auth-pam.c	26 Mar 2006 03:22:48 -0000	1.132
+++ auth-pam.c	4 May 2006 02:26:39 -0000
@@ -288,7 +288,7 @@ import_environments(Buffer *b)
 
 	/* Import environment from subprocess */
 	num_env = buffer_get_int(b);
-	sshpam_env = xmalloc((num_env + 1) * sizeof(*sshpam_env));
+	sshpam_env = xcalloc(num_env + 1, sizeof(*sshpam_env));
 	debug3("PAM: num env strings %d", num_env);
 	for(i = 0; i < num_env; i++)
 		sshpam_env[i] = buffer_get_string(b, NULL);
@@ -335,7 +335,7 @@ sshpam_thread_conv(int n, sshpam_const s
 	if (n <= 0 || n > PAM_MAX_NUM_MSG)
 		return (PAM_CONV_ERR);
 
-	if ((reply = malloc(n * sizeof(*reply))) == NULL)
+	if ((reply = calloc(n, sizeof(*reply))) == NULL)
 		return (PAM_CONV_ERR);
 	memset(reply, 0, n * sizeof(*reply));
 
@@ -533,7 +533,7 @@ sshpam_store_conv(int n, sshpam_const st
 	if (n <= 0 || n > PAM_MAX_NUM_MSG)
 		return (PAM_CONV_ERR);
 
-	if ((reply = malloc(n * sizeof(*reply))) == NULL)
+	if ((reply = calloc(n, sizeof(*reply))) == NULL)
 		return (PAM_CONV_ERR);
 	memset(reply, 0, n * sizeof(*reply));
 
@@ -935,7 +935,7 @@ sshpam_tty_conv(int n, sshpam_const stru
 	if (n <= 0 || n > PAM_MAX_NUM_MSG || !isatty(STDIN_FILENO))
 		return (PAM_CONV_ERR);
 
-	if ((reply = malloc(n * sizeof(*reply))) == NULL)
+	if ((reply = calloc(n, sizeof(*reply))) == NULL)
 		return (PAM_CONV_ERR);
 	memset(reply, 0, n * sizeof(*reply));
 
Index: groupaccess.c
===================================================================
RCS file: /usr/local/src/security/openssh/cvs/openssh_cvs/groupaccess.c,v
retrieving revision 1.10
diff -u -p -r1.10 groupaccess.c
--- groupaccess.c	26 Mar 2006 03:24:49 -0000	1.10
+++ groupaccess.c	4 May 2006 01:56:11 -0000
@@ -52,8 +52,8 @@ ga_init(const char *user, gid_t base)
 	ngroups = MAX(NGROUPS_MAX, sysconf(_SC_NGROUPS_MAX));
 #endif
 
-	groups_bygid = xmalloc(ngroups * sizeof(*groups_bygid));
-	groups_byname = xmalloc(ngroups * sizeof(*groups_byname));
+	groups_bygid = xcalloc(ngroups, sizeof(*groups_bygid));
+	groups_byname = xcalloc(ngroups, sizeof(*groups_byname));
 
 	if (getgrouplist(user, base, groups_bygid, &ngroups) == -1)
 		logit("getgrouplist: groups list too small");
Index: monitor.c
===================================================================
RCS file: /usr/local/src/security/openssh/cvs/openssh_cvs/monitor.c,v
retrieving revision 1.102
diff -u -p -r1.102 monitor.c
--- monitor.c	31 Mar 2006 12:14:24 -0000	1.102
+++ monitor.c	4 May 2006 01:56:40 -0000
@@ -924,7 +924,7 @@ mm_answer_pam_respond(int sock, Buffer *
 	sshpam_authok = NULL;
 	num = buffer_get_int(m);
 	if (num > 0) {
-		resp = xmalloc(num * sizeof(char *));
+		resp = xcalloc(num, sizeof(char *));
 		for (i = 0; i < num; ++i)
 			resp[i] = buffer_get_string(m, NULL);
 		ret = (sshpam_device.respond)(sshpam_ctxt, num, resp);
Index: monitor_wrap.c
===================================================================
RCS file: /usr/local/src/security/openssh/cvs/openssh_cvs/monitor_wrap.c,v
retrieving revision 1.59
diff -u -p -r1.59 monitor_wrap.c
--- monitor_wrap.c	31 Mar 2006 12:13:02 -0000	1.59
+++ monitor_wrap.c	4 May 2006 01:57:46 -0000
@@ -776,8 +776,8 @@ mm_sshpam_query(void *ctx, char **name, 
 	*name = buffer_get_string(&m, NULL);
 	*info = buffer_get_string(&m, NULL);
 	*num = buffer_get_int(&m);
-	*prompts = xmalloc((*num + 1) * sizeof(char *));
-	*echo_on = xmalloc((*num + 1) * sizeof(u_int));
+	*prompts = xcalloc((*num + 1), sizeof(char *));
+	*echo_on = xcalloc((*num + 1), sizeof(u_int));
 	for (i = 0; i < *num; ++i) {
 		(*prompts)[i] = buffer_get_string(&m, NULL);
 		(*echo_on)[i] = buffer_get_int(&m);
Index: scard-opensc.c
===================================================================
RCS file: /usr/local/src/security/openssh/cvs/openssh_cvs/scard-opensc.c,v
retrieving revision 1.15
diff -u -p -r1.15 scard-opensc.c
--- scard-opensc.c	13 May 2004 07:29:35 -0000	1.15
+++ scard-opensc.c	4 May 2006 01:58:41 -0000
@@ -455,7 +455,7 @@ sc_get_keys(const char *id, const char *
 		}
 		key_count = r;
 	}
-	keys = xmalloc(sizeof(Key *) * (key_count*2+1));
+	keys = xcalloc(key_count * 2 + 1, sizeof(Key *));
 	for (i = 0; i < key_count; i++) {
 		sc_pkcs15_object_t *tmp_obj = NULL;
 		cert_id = ((sc_pkcs15_cert_info_t *)(certs[i]->data))->id;
Index: session.c
===================================================================
RCS file: /usr/local/src/security/openssh/cvs/openssh_cvs/session.c,v
retrieving revision 1.327
diff -u -p -r1.327 session.c
--- session.c	23 Apr 2006 02:10:49 -0000	1.327
+++ session.c	4 May 2006 01:59:13 -0000
@@ -984,7 +984,7 @@ do_setup_env(Session *s, const char *she
 
 	/* Initialize the environment. */
 	envsize = 100;
-	env = xmalloc(envsize * sizeof(char *));
+	env = xcalloc(envsize, sizeof(char *));
 	env[0] = NULL;
 
 #ifdef HAVE_CYGWIN
Index: ssh-rand-helper.c
===================================================================
RCS file: /usr/local/src/security/openssh/cvs/openssh_cvs/ssh-rand-helper.c,v
retrieving revision 1.29
diff -u -p -r1.29 ssh-rand-helper.c
--- ssh-rand-helper.c	26 Mar 2006 03:22:48 -0000	1.29
+++ ssh-rand-helper.c	4 May 2006 02:01:13 -0000
@@ -674,7 +674,7 @@ prng_read_commands(char *cmdfilename)
 	}
 
 	num_cmds = 64;
-	entcmd = xmalloc(num_cmds * sizeof(entropy_cmd_t));
+	entcmd = xcalloc(num_cmds, sizeof(entropy_cmd_t));
 	memset(entcmd, '\0', num_cmds * sizeof(entropy_cmd_t));
 
 	/* Read in file */
Index: sshd.c
===================================================================
RCS file: /usr/local/src/security/openssh/cvs/openssh_cvs/sshd.c,v
retrieving revision 1.333
diff -u -p -r1.333 sshd.c
--- sshd.c	26 Mar 2006 03:24:50 -0000	1.333
+++ sshd.c	4 May 2006 02:01:50 -0000
@@ -921,7 +921,7 @@ main(int ac, char **av)
 	/* Save argv. Duplicate so setproctitle emulation doesn't clobber it */
 	saved_argc = ac;
 	rexec_argc = ac;
-	saved_argv = xmalloc(sizeof(*saved_argv) * (ac + 1));
+	saved_argv = xcalloc(ac + 1, sizeof(*saved_argv));
 	for (i = 0; i < ac; i++)
 		saved_argv[i] = xstrdup(av[i]);
 	saved_argv[i] = NULL;
Index: openbsd-compat/bsd-cygwin_util.c
===================================================================
RCS file: /usr/local/src/security/openssh/cvs/openssh_cvs/openbsd-compat/bsd-cygwin_util.c,v
retrieving revision 1.16
diff -u -p -r1.16 bsd-cygwin_util.c
--- openbsd-compat/bsd-cygwin_util.c	25 Mar 2006 13:03:24 -0000	1.16
+++ openbsd-compat/bsd-cygwin_util.c	4 May 2006 02:02:57 -0000
@@ -268,7 +268,7 @@ fetch_windows_environment(void)
 	char **e, **p;
 	unsigned int i, idx = 0;
 
-	p = xmalloc((WENV_SIZ + 1) * sizeof(char *));
+	p = xcalloc(WENV_SIZ + 1, sizeof(char *));
 	for (e = environ; *e != NULL; ++e) {
 		for (i = 0; i < WENV_SIZ; ++i) {
 			if (!strncmp(*e, wenv_arr[i].name, wenv_arr[i].namelen))
Index: openbsd-compat/fake-rfc2553.c
===================================================================
RCS file: /usr/local/src/security/openssh/cvs/openssh_cvs/openbsd-compat/fake-rfc2553.c,v
retrieving revision 1.6
diff -u -p -r1.6 fake-rfc2553.c
--- openbsd-compat/fake-rfc2553.c	25 Mar 2006 13:03:24 -0000	1.6
+++ openbsd-compat/fake-rfc2553.c	4 May 2006 02:28:30 -0000
@@ -115,7 +115,7 @@ addrinfo *malloc_ai(int port, u_long add
 {
 	struct addrinfo *ai;
 
-	ai = malloc(sizeof(*ai) + sizeof(struct sockaddr_in));
+	ai = calloc(sizeof(*ai), sizeof(struct sockaddr_in));
 	if (ai == NULL)
 		return (NULL);
 	
Index: openbsd-compat/setproctitle.c
===================================================================
RCS file: /usr/local/src/security/openssh/cvs/openssh_cvs/openbsd-compat/setproctitle.c,v
retrieving revision 1.9
diff -u -p -r1.9 setproctitle.c
--- openbsd-compat/setproctitle.c	20 Feb 2004 09:38:17 -0000	1.9
+++ openbsd-compat/setproctitle.c	4 May 2006 02:29:03 -0000
@@ -80,7 +80,7 @@ compat_init_setproctitle(int argc, char 
 	/* Fail if we can't allocate room for the new environment */
 	for (i = 0; envp[i] != NULL; i++)
 		;
-	if ((environ = malloc(sizeof(*environ) * (i + 1))) == NULL) {
+	if ((environ = calloc(i + 1, sizeof(*environ))) == NULL) {
 		environ = envp;	/* put it back */
 		return;
 	}

-- 
Darren Tucker (dtucker at zip.com.au)
GPG key 8FF4FA69 / D9A3 86E9 7EEE AF4B B2D4  37C9 C982 80C7 8FF4 FA69
    Good judgement comes with experience. Unfortunately, the experience
usually comes from bad judgement.




More information about the openssh-unix-dev mailing list