[patch sbin/iked/pfkey.c] replacing select with poll
Hi there,
Similar to the nfsd patch recently, replacing select() with poll() and removing
some unecessary variables.
ok?
Index: pfkey.c
===
RCS file: /cvs/src/sbin/iked/pfkey.c,v
retrieving revision 1.34
diff -u -p -u -r1.34 pfkey.c
--- pfkey.c 6 May 2014 10:24:22 - 1.34
+++ pfkey.c 7 May 2014 00:20:06 -
@@ -35,6 +35,7 @@
#include stdlib.h
#include unistd.h
#include event.h
+#include poll.h
#include iked.h
#include ikev2.h
@@ -43,7 +44,6 @@
#define IOV_CNT 20
#define PFKEYV2_CHUNK sizeof(u_int64_t)
-#define PFKEY_REPLY_TIMEOUT 1000
static u_int32_t sadb_msg_seq = 0;
static u_int sadb_decoupled = 0;
@@ -1086,11 +1086,10 @@ pfkey_reply(int sd, u_int8_t **datap, ss
{
struct pfkey_message*pm;
struct sadb_msg hdr;
- struct timeval tv;
+ struct pollfdpfd;
ssize_t len;
u_int8_t*data;
- fd_set *fds;
- int n;
+ int ret, timeout;
for (;;) {
/*
@@ -1100,24 +1099,17 @@ pfkey_reply(int sd, u_int8_t **datap, ss
* and if it is not readable in that time, we fail
* the read.
*/
- n = howmany(sd + 1, NFDBITS);
- if ((fds = calloc(n, sizeof(fd_mask))) == NULL) {
- log_warn(%s: calloc(%lu, %lu) failed, __func__,
- (unsigned long) n,
- (unsigned long) sizeof(fd_mask));
- return (-1);
- }
- FD_SET(sd, fds);
- tv.tv_sec = 0;
- tv.tv_usec = PFKEY_REPLY_TIMEOUT;
- n = select(sd + 1, fds, 0, 0, tv);
- free(fds);
- if (n == -1) {
- log_warn(%s: select(%d, fds, 0, 0, tv) failed,
- __func__, sd + 1);
+
+ timeout = 1;
+ pfd.fd = sd;
+ pfd.events = POLLIN;
+ ret = poll(pfd, 1, timeout);
+
+ if (ret == -1) {
+ log_warn(%s: poll, __func__);
return (-1);
}
- if (n == 0) {
+ if (ret == 0) {
log_warnx(%s: no reply from PF_KEY, __func__);
return (-1);
}
--
Peter Malone pe...@petermalone.org
[patch usr.bin/pkill/pkill.c malloc memset = calloc
Hi there, I thought I had sent in all of these, but this was in my drafts and I didn't see it after searching the list my sent items. ok? Index: pkill.c === RCS file: /cvs/src/usr.bin/pkill/pkill.c,v retrieving revision 1.34 diff -u -p -u -r1.34 pkill.c --- pkill.c 12 Nov 2013 13:54:51 - 1.34 +++ pkill.c 7 May 2014 01:08:45 - @@ -277,9 +277,8 @@ main(int argc, char **argv) * Allocate memory which will be used to keep track of the * selection. */ - if ((selected = malloc(nproc)) == NULL) + if ((selected = calloc(1, nproc)) == NULL) errx(STATUS_ERROR, memory allocation failure); - memset(selected, 0, nproc); /* * Refine the selection. -- Peter Malone pe...@petermalone.org
Re: [patch src/usr.bin/mg/undo.c] replace malloc memset with calloc
On 04/24/14 04:09, Stuart Henderson wrote: On 2014/04/24 04:26, Miod Vallat wrote: Same as the others, this time with src/usr.bin/mg/undo.c You are now losing a memset() in the `rec doesn't come from malloc' code path. From the number and types of diff being sent, I am guessing these are tool-generated (coccinelle?); while this is a valid technique it is important that they are also carefully checked. Understood - thanks to Stuart, Miod, Ted team. I'll check my inventory but I believe last night's run was the last of them. Next up is a bunch of files which could benefit from using poll(). There was around five I spotted I believe, including the one Ted raised last night.
Re: sudo alloc.c -- cleanup required?
Great explanation - thanks. On 04/24/14 08:28, Todd C. Miller wrote: Sudo runs on more systems thsan just OpenBSD and so has a lot of configure goo and defines as a result. There's really no point in removing that. Any changes made to the sudo in OpenBSD just makes updates harder. The alloc functions implement integer overflow checks that are not present on most systems as well as a malloc(0) check that has caught bugs in the past. Nothing in sudo should be calling malloc with a zero size. - todd
Re: [patch sbin/nfsd/nfsd.c] replace malloc memset with calloc
The more I think about this. if we want to continue the select()
NULL functionality then timeout should be a negative value for poll(),
as NULL is unrecognized and zero is a zero-length timeout.
On 04/24/14 18:08, Peter Malone wrote:
As promised. I cleaned up some of the code as well.
One item of note - select() took a NULL value for timeout. I did not want to do
this with poll(), so I defaulted with 10 instead. We should probably discuss
this. I didn't even try to compile poll() with NULL - I don't think it's a good
idea and I'm sure the compiler wouldn't be happy. Thoughts?
Index: nfsd.c
===
RCS file: /cvs/src/sbin/nfsd/nfsd.c,v
retrieving revision 1.32
diff -u -p -u -r1.32 nfsd.c
--- nfsd.c 11 Mar 2013 17:40:10 - 1.32
+++ nfsd.c 24 Apr 2014 22:00:20 -
@@ -103,13 +103,10 @@ main(int argc, char *argv[])
{
struct nfsd_args nfsdargs;
struct sockaddr_in inetaddr, inetpeer;
- fd_set *ready, *sockbits;
- size_t fd_size;
- int ch, connect_type_cnt, i, maxsock = 0, msgsock;
+ int ch, connect_type_cnt, i;
int nfsdcnt = DEFNFSDCNT, on, reregister = 0, sock;
int udpflag = 0, tcpflag = 0, tcpsock;
const char *errstr = NULL;
- socklen_t len;
/* Start by writing to both console and log. */
openlog(nfsd, LOG_PID | LOG_PERROR, LOG_DAEMON);
@@ -264,7 +261,6 @@ main(int argc, char *argv[])
syslog(LOG_ERR, can't register tcp with portmap);
return (1);
}
- maxsock = tcpsock;
connect_type_cnt++;
}
@@ -274,33 +270,29 @@ main(int argc, char *argv[])
setproctitle(master);
/*
-* Allocate space for the fd_set pointers and fill in sockbits
-*/
- fd_size = howmany(maxsock + 1, NFDBITS) * sizeof(fd_mask);
- sockbits = malloc(fd_size);
- ready = malloc(fd_size);
- if (sockbits == NULL || ready == NULL) {
- syslog(LOG_ERR, cannot allocate memory);
- return (1);
- }
- memset(sockbits, 0, fd_size);
- if (tcpflag)
- FD_SET(tcpsock, sockbits);
-
- /*
* Loop forever accepting connections and passing the sockets
* into the kernel for the mounts.
*/
for (;;) {
- memcpy(ready, sockbits, fd_size);
+ struct pollfd pfd;
+ struct sockaddr_in inetpeer;
+ int ret, msgsock, timeout;
+ socklen_t len;
+
+ pfd.fd = tcpsock;
+ pfd.events = POLLIN;
+ timeout = 10;
+
if (connect_type_cnt 1) {
- if (select(maxsock + 1,
- ready, NULL, NULL, NULL) 1) {
- syslog(LOG_ERR, select failed: %m);
+ ret = poll(pfd, 1, timeout);
+ if (ret 1) {
+ syslog(LOG_ERR, poll failed: %m);
return (1);
}
+
}
- if (tcpflag FD_ISSET(tcpsock, ready)) {
+
+ if (tcpflag) {
len = sizeof(inetpeer);
if ((msgsock = accept(tcpsock,
(struct sockaddr *)inetpeer, len)) 0) {
On Wed, 23 Apr 2014 21:56:56 -0400
Peter Malone pe...@petermalone.org wrote:
Sounds good. I'll work on that tomorrow afternoon/evening.
On 04/23/14 21:55, Ted Unangst wrote:
On Wed, Apr 23, 2014 at 21:38, Peter Malone wrote:
Hi,
Similar to the others. malloc memset replacement with calloc, this time
in sbin/nfsd/nfsd.c
fd_size = howmany(maxsock + 1, NFDBITS) * sizeof(fd_mask);
- sockbits = malloc(fd_size);
+ sockbits = calloc(1, fd_size);
ready = malloc(fd_size);
As with ping6, I think this is better converted to poll.
[patch usr.sbin/rtsold/rtsold.c] replace malloc memset with calloc
Hi,
Here's another malloc memset = calloc replacement.
Index: rtsold.c
===
RCS file: /cvs/src/usr.sbin/rtsold/rtsold.c,v
retrieving revision 1.50
diff -u -p -u -r1.50 rtsold.c
--- rtsold.c21 Oct 2013 08:46:07 - 1.50
+++ rtsold.c24 Apr 2014 23:49:30 -
@@ -324,12 +324,11 @@ ifconfig(char *ifname)
return(-1);
}
- if ((ifinfo = malloc(sizeof(*ifinfo))) == NULL) {
+ if ((ifinfo = calloc(1, sizeof(*ifinfo))) == NULL) {
warnmsg(LOG_ERR, __func__, memory allocation failed);
free(sdl);
return(-1);
}
- memset(ifinfo, 0, sizeof(*ifinfo));
ifinfo-sdl = sdl;
strncpy(ifinfo-ifname, ifname, sizeof(ifinfo-ifname));
--
Peter Malone pe...@petermalone.org
[patch l2tpd.c] malloc memset = calloc
Hi,
Here's another.
Index: l2tpd.c
===
RCS file: /cvs/src/usr.sbin/npppd/l2tp/l2tpd.c,v
retrieving revision 1.14
diff -u -p -u -r1.14 l2tpd.c
--- l2tpd.c 22 Mar 2014 04:32:39 - 1.14
+++ l2tpd.c 25 Apr 2014 00:09:33 -
@@ -162,12 +162,11 @@ l2tpd_add_listener(l2tpd *_this, int idx
__func__, slist_length(_this-listener), idx);
goto fail;
}
- if ((plistener = malloc(sizeof(l2tpd_listener))) == NULL) {
- l2tpd_log(_this, LOG_ERR, malloc() failed in %s: %m,
+ if ((plistener = calloc(1, sizeof(l2tpd_listener))) == NULL) {
+ l2tpd_log(_this, LOG_ERR, calloc() failed in %s: %m,
__func__);
goto fail;
}
- memset(plistener, 0, sizeof(l2tpd_listener));
L2TPD_ASSERT(sizeof(plistener-bind) = addr-sa_len);
memcpy(plistener-bind, addr, addr-sa_len);
--
Peter Malone pe...@petermalone.org
[patch l2tp_ctrl.c] malloc memset = calloc
Hi,
Here's another.
Index: l2tp_ctrl.c
===
RCS file: /cvs/src/usr.sbin/npppd/l2tp/l2tp_ctrl.c,v
retrieving revision 1.16
diff -u -p -u -r1.16 l2tp_ctrl.c
--- l2tp_ctrl.c 13 Sep 2013 03:25:27 - 1.16
+++ l2tp_ctrl.c 25 Apr 2014 00:13:13 -
@@ -108,10 +108,9 @@ l2tp_ctrl_create(void)
{
l2tp_ctrl *_this;
- if ((_this = malloc(sizeof(l2tp_ctrl))) == NULL)
+ if ((_this = calloc(1, sizeof(l2tp_ctrl))) == NULL)
return NULL;
- memset(_this, 0, sizeof(l2tp_ctrl));
return (l2tp_ctrl *)_this;
}
--
Peter Malone pe...@petermalone.org
[patch parse.c] malloc memset = calloc
Hi,
Another malloc memset = calloc.
Index: parse.c
===
RCS file: /cvs/src/lib/libusbhid/parse.c,v
retrieving revision 1.7
diff -u -p -u -r1.7 parse.c
--- parse.c 16 Jul 2012 19:57:17 - 1.7
+++ parse.c 25 Apr 2014 00:29:52 -
@@ -91,10 +91,9 @@ hid_start_parse(report_desc_t d, int kin
{
struct hid_data *s;
- s = malloc(sizeof *s);
+ s = calloc(1, sizeof *s);
if (s == NULL)
return (NULL);
- memset(s, 0, sizeof *s);
s-start = s-p = d-data;
s-end = d-data + d-size;
s-kindset = kindset;
--
Peter Malone pe...@petermalone.org
Re: [patch sbin/nfsd/nfsd.c] replace malloc memset with calloc
Thanks Ted. I sent through another patch with timeout = -1. Passing INFTIM sounds like the best option though. Hopefully the rest checks out. On 04/24/14 20:30, Ted Unangst wrote: On Thu, Apr 24, 2014 at 19:27, Peter Malone wrote: The more I think about this. if we want to continue the select() NULL functionality then timeout should be a negative value for poll(), as NULL is unrecognized and zero is a zero-length timeout. The standard way to effect no timeout for poll is to pass INFTIM (which is defined to be -1). I'll check the rest of the diff and make that change if the rest is good.
[patch bin/cp/utils.c] replace malloc memset with calloc
Hi,
Similar to previous patches replacing malloc memset with calloc. This time in
src/bin/cp/utils.c
Please let me know if this doesn't paste correctly this time. I suspect it's ok
now. If not, I may just cry.
I also attached it (I hope that's not frowned upon).
Cheers!
Index: utils.c
===
RCS file: /cvs/src/bin/cp/utils.c,v
retrieving revision 1.33
diff -u -p -u -r1.33 utils.c
--- utils.c 11 Jul 2012 16:19:24 - 1.33
+++ utils.c 24 Apr 2014 00:51:42 -
@@ -63,10 +63,9 @@ copy_file(FTSENT *entp, int dne)
err(1, malloc);
}
if (!zeroes) {
- zeroes = malloc(MAXBSIZE);
+ zeroes = calloc(1, MAXBSIZE);
if (!zeroes)
- err(1, malloc);
- memset(zeroes, 0, MAXBSIZE);
+ err(1, calloc);
}
if ((from_fd = open(entp-fts_path, O_RDONLY, 0)) == -1) {
--
Peter Malone pe...@petermalone.org
utils.patch
Description: Binary data
[patch bin/systrace/intercept.c] replace malloc memset with calloc
Hi, Similar to previous patches replacing malloc memset with calloc, this time in src/bin/systrace/intercept.c Index: intercept.c === RCS file: /cvs/src/bin/systrace/intercept.c,v retrieving revision 1.60 diff -u -p -u -r1.60 intercept.c --- intercept.c 4 Dec 2012 02:24:47 - 1.60 +++ intercept.c 24 Apr 2014 01:20:47 - @@ -155,12 +155,11 @@ intercept_register_translation(char *emu errx(1, %s: %s-%s: can't find call back, __func__, emulation, name); - tlnew = malloc(sizeof(struct intercept_translate)); + tlnew = calloc(1, sizeof(struct intercept_translate)); if (tlnew == NULL) - err(1, %s: %s-%s: malloc, + err(1, %s: %s-%s: calloc, __func__, emulation, name); - memcpy(tlnew, tl, sizeof(struct intercept_translate)); tlnew-off = offset; TAILQ_INSERT_TAIL(tmp-tls, tlnew, next); -- Peter Malone pe...@petermalone.org intercept.c Description: Binary data
[patch sbin/nfsd/nfsd.c] replace malloc memset with calloc
Hi,
Similar to the others. malloc memset replacement with calloc, this time in
sbin/nfsd/nfsd.c
Index: nfsd.c
===
RCS file: /cvs/src/sbin/nfsd/nfsd.c,v
retrieving revision 1.32
diff -u -p -u -r1.32 nfsd.c
--- nfsd.c 11 Mar 2013 17:40:10 - 1.32
+++ nfsd.c 24 Apr 2014 01:32:04 -
@@ -277,13 +277,12 @@ main(int argc, char *argv[])
* Allocate space for the fd_set pointers and fill in sockbits
*/
fd_size = howmany(maxsock + 1, NFDBITS) * sizeof(fd_mask);
- sockbits = malloc(fd_size);
+ sockbits = calloc(1, fd_size);
ready = malloc(fd_size);
if (sockbits == NULL || ready == NULL) {
syslog(LOG_ERR, cannot allocate memory);
return (1);
}
- memset(sockbits, 0, fd_size);
if (tcpflag)
FD_SET(tcpsock, sockbits);
--
Peter Malone pe...@petermalone.org
Re: [patch bin/systrace/intercept.c] replace malloc memset with calloc
As Ned Flanders infamously said... Take out the crayolas and color me tickled pink. Here's the _actual_ patch. Index: intercept.c === RCS file: /cvs/src/bin/systrace/intercept.c,v retrieving revision 1.60 diff -u -p -u -r1.60 intercept.c --- intercept.c 4 Dec 2012 02:24:47 - 1.60 +++ intercept.c 24 Apr 2014 01:42:57 - @@ -426,10 +426,9 @@ intercept_getpid(pid_t pid) if (tmp) return (tmp); - if ((tmp = malloc(sizeof(struct intercept_pid))) == NULL) - err(1, %s: malloc, __func__); + if ((tmp = calloc(1, sizeof(struct intercept_pid))) == NULL) + err(1, %s: calloc, __func__); - memset(tmp, 0, sizeof(struct intercept_pid)); tmp-pid = pid; SPLAY_INSERT(pidtree, pids, tmp); On Wed, 23 Apr 2014 21:35:59 -0400 Ted Unangst t...@tedunangst.com wrote: On Wed, Apr 23, 2014 at 21:24, Peter Malone wrote: Hi, Similar to previous patches replacing malloc memset with calloc, this time in src/bin/systrace/intercept.c - tlnew = malloc(sizeof(struct intercept_translate)); + tlnew = calloc(1, sizeof(struct intercept_translate)); if (tlnew == NULL) - err(1, %s: %s-%s: malloc, + err(1, %s: %s-%s: calloc, __func__, emulation, name); - memcpy(tlnew, tl, sizeof(struct intercept_translate)); Ha, tricked you. :) That's a memcpy. -- Peter Malone pe...@petermalone.org
Re: [patch sbin/nfsd/nfsd.c] replace malloc memset with calloc
Sounds good. I'll work on that tomorrow afternoon/evening. On 04/23/14 21:55, Ted Unangst wrote: On Wed, Apr 23, 2014 at 21:38, Peter Malone wrote: Hi, Similar to the others. malloc memset replacement with calloc, this time in sbin/nfsd/nfsd.c fd_size = howmany(maxsock + 1, NFDBITS) * sizeof(fd_mask); - sockbits = malloc(fd_size); + sockbits = calloc(1, fd_size); ready = malloc(fd_size); As with ping6, I think this is better converted to poll.
sudo alloc.c -- cleanup required?
Hi,
I see something with sudo which I would like to raise with the team. This
impacts most of the source files in the sudo package.
Before I go down this rat hole, I wanted to run it by you folks to see if you
agree.
/*
* If there is no SIZE_MAX or SIZE_T_MAX we have to assume that size_t
* could be signed (as it is on SunOS 4.x). This just means that
* emalloc2() and erealloc3() cannot allocate huge amounts on such a
* platform but that is OK since sudo doesn't need to do so anyway.
*/
#ifndef SIZE_MAX
# ifdef SIZE_T_MAX
# define SIZE_MAX SIZE_T_MAX
# else
# define SIZE_MAX INT_MAX
# endif /* SIZE_T_MAX */
#endif /* SIZE_MAX */
SunOS 4.x? The latest version is 11.x I believe. Can we do without this?
Lets look at emalloc() and emalloc2()
/*
* emalloc() calls the system malloc(3) and exits with an error if
* malloc(3) fails.
*/
void *
emalloc(size)
size_t size;
{
void *ptr;
if (size == 0)
errorx(1, internal error, tried to emalloc(0));
if ((ptr = malloc(size)) == NULL)
errorx(1, unable to allocate memory);
return(ptr);
}
/*
* emalloc2() allocates nmemb * size bytes and exits with an error
* if overflow would occur or if the system malloc(3) fails.
*/
void *
emalloc2(nmemb, size)
size_t nmemb;
size_t size;
{
void *ptr;
if (nmemb == 0 || size == 0)
errorx(1, internal error, tried to emalloc2(0));
if (nmemb SIZE_MAX / size)
errorx(1, internal error, emalloc2() overflow);
size *= nmemb;
if ((ptr = malloc(size)) == NULL)
errorx(1, unable to allocate memory);
return(ptr);
}
I'm failing to see the need for these two functions. This could be implemented
with less code.
In fact, most of alloc.c contains wrapper functions. Some are useful, like
estrdup(), but could be implemented better (it calls the malloc wrapper).
Any thoughts on this?
--
Peter Malone pe...@petermalone.org
[patch usr.sbin/snmpd/mib.c] replace malloc memset with calloc
Hi,
Same as the others. Replace malloc memset with calloc in usr.sbin/snmpd/mib.c
Index: mib.c
===
RCS file: /cvs/src/usr.sbin/snmpd/mib.c,v
retrieving revision 1.67
diff -u -p -u -r1.67 mib.c
--- mib.c 8 Apr 2014 14:04:11 - 1.67
+++ mib.c 24 Apr 2014 02:29:26 -
@@ -2818,9 +2818,8 @@ mib_carpifget(u_int idx)
return (NULL);
}
- cif = malloc(sizeof(struct carpif));
+ cif = calloc(1, sizeof(struct carpif));
if (cif != NULL) {
- memset(cif, 0, sizeof(struct carpif));
memcpy(cif-carpr, carpr, sizeof(struct carpreq));
memcpy(cif-kif, kif, sizeof(struct kif));
}
--
Peter Malone pe...@petermalone.org
[patch usr.sbin/rwhod/rwhod.c] replace malloc memset with calloc
Hi, Same as the others, replace malloc memset with calloc. This time in usr.sbin/rwhod/rwhod.c. Good night! Index: rwhod.c === RCS file: /cvs/src/usr.sbin/rwhod/rwhod.c,v retrieving revision 1.36 diff -u -p -u -r1.36 rwhod.c --- rwhod.c 9 Jan 2014 05:04:03 - 1.36 +++ rwhod.c 24 Apr 2014 02:33:08 - @@ -530,10 +530,9 @@ configure(void) if (np != NULL) continue; len = sizeof(*np) + dstaddr-sa_len + sdl-sdl_nlen + 1; - np = (struct neighbor *)malloc(len); + np = calloc(1, len); if (np == NULL) - quit(malloc of neighbor structure); - memset(np, 0, len); + quit(calloc of neighbor structure); np-n_flags = flags; np-n_addr = (struct sockaddr *)(np + 1); np-n_addrlen = dstaddr-sa_len; -- Peter Malone pe...@petermalone.org
[patch dso_lib.c] replace malloc memset with calloc
Hi,
Theo has been working on these patches over the past few days. I noticed
an issue with dso_lib.c and fixed it accordingly.
# cvs diff -u -r1.10 dso_lib.c
Index: dso_lib.c
===
RCS file: /cvs/src/lib/libssl/src/crypto/dso/dso_lib.c,v
retrieving revision 1.10
diff -u -p -u -r1.10 dso_lib.c
--- dso_lib.c19 Apr 2014 15:30:17 -1.10
+++ dso_lib.c21 Apr 2014 21:03:33 -
@@ -107,12 +107,11 @@ DSO_new_method(DSO_METHOD *meth)
* to stealing the best available method. Will fallback
* to DSO_METH_null() in the worst case. */
default_DSO_meth = DSO_METHOD_openssl();
-ret = (DSO *)malloc(sizeof(DSO));
+ret = calloc(1, sizeof(DSO));
if (ret == NULL) {
DSOerr(DSO_F_DSO_NEW_METHOD, ERR_R_MALLOC_FAILURE);
return (NULL);
}
-memset(ret, 0, sizeof(DSO));
ret-meth_data = sk_void_new_null();
if (ret-meth_data == NULL) {
/* sk_new doesn't generate any errors so we do */
#
[patch ping.c] replace malloc memset with calloc
Hi,
malloc memset can be replaced with calloc in ping.c. Please see below
for patch details:
Index: ping.c
===
RCS file: /cvs/src/sbin/ping/ping.c,v
retrieving revision 1.100
diff -u -p -u -r1.100 ping.c
--- ping.c24 Mar 2014 11:11:49 -1.100
+++ ping.c22 Apr 2014 04:41:56 -
@@ -508,8 +508,8 @@ main(int argc, char *argv[])
catcher(0);/* start things going */
fdmasks = howmany(s+1, NFDBITS) * sizeof(fd_mask);
-if ((fdmaskp = (fd_set *)malloc(fdmasks)) == NULL)
-err(1, malloc);
+if ((fdmaskp = calloc(1, fdmasks)) == NULL)
+err(1, calloc);
for (;;) {
struct sockaddr_in from;
@@ -521,7 +521,6 @@ main(int argc, char *argv[])
pinger();
timeout.tv_sec = 0;
timeout.tv_usec = 1;
-memset(fdmaskp, 0, fdmasks);
FD_SET(s, fdmaskp);
if (select(s + 1, (fd_set *)fdmaskp, (fd_set *)NULL,
(fd_set *)NULL, timeout) 1)
Re: [patch] courier-imap-4.13 imapd patch replacing malloc, strcat and strcpy with asprintf
That's it in a nutshell, essentially.
I'll take a stab at it, until I get frustrated.
Perhaps my time would be better suited to something else which could
help OpenBSD. For the time being, however, I'll give this a shot.
On 04/20/14 02:18, Maxime Villard wrote:
Le 20/04/2014 02:38, Peter Malone a écrit :
Hi,
I'm using OpenBSD 5.5. courier-imap-4.13 is in the ports tree and it's
quite a mess. I started looking at it today with the hope of just
replacing some of the malloc,strcat strcpy calls with asprintf, but it
became clear before long that there's lots more issues with this code.
Regardless, here's a patch which fixes some of the malloc,strcat
strcpy spaghetti in imapd.c
I figure you guys are fairly busy with OpenSSL right now so if it's OK
with you I'll get working on the rest of this code.
I spoke with the courier-imap team and their response was less than
satisfactory - claiming asprintf code won't compile on non-linux
platforms.
courier-imap is *totally buggy*. Not only because there are big
programming mistakes - at a time I started to fix many issues like
double-free, use-after-free, leaks, etc., before letting down -,
but also because this team doesn't give a fuck at all.
Give a quick glance at the code: it is just a great mess, with bugs
everywhere, no indentation, and probably no review. Clearly, there
are poor - not to say *no* - considerations for security. That's the
idea I made myself of this project, and you seem to have come to the
same conclusion.
--- imapd_orig.c Sat Apr 19 19:38:18 2014
+++ imapd.c Sat Apr 19 20:09:48 2014
@@ -343,9 +343,8 @@
if (q)
{
- r=malloc(strlen(q)+sizeof(/.));
- if (!r) write_error_exit(0);
- strcat(strcpy(r, q), /.);
+ if(asprintf(r, %s%s, q, /.) == -1)
+ write_error_exit(0);
if (access(r, 0) == 0)
{
free(r);
@@ -1373,11 +1372,9 @@
static char *alloc_filename(const char *mbox, const char *name)
{
-char *p=malloc(strlen(mbox)+strlen(name)+sizeof(/cur/));
-
- if (!p) write_error_exit(0);
-
- strcat(strcat(strcpy(p, mbox), /cur/), name);
+ char *p;
+ if(asprintf(p, %s%s%s, mbox, /cur/, name) == -1)
+ write_error_exit(0);
return (p);
}
@@ -1812,14 +1809,12 @@
{
#if EXPLICITDIRSYNC
- char *p=malloc(strlen(folder)+sizeof(/new));
+ char *p;
int fd;
-
- if (!p)
+
+ if(asprintf(p, %s%s, folder, /new) == -1)
write_error_exit(0);
- p=strcat(strcpy(p, folder), /new);
-
fd=open(p, O_RDONLY);
if (fd = 0)
@@ -1828,7 +1823,8 @@
close(fd);
}
- p=strcat(strcpy(p, folder), /cur);
+ if(asprintf(p, %s%s, folder, /cur) == -1)
+ write_error_exit(0);
fd=open(p, O_RDONLY);
@@ -2212,12 +2208,10 @@
{
struct imapscaninfo minfo;
- char *p=malloc(strlen(new_path)+sizeof(/ IMAPDB));
+ char *p;
+ if(asprintf(p, %s%s, new_path, / IMAPDB) == -1)
+write_error_exit(0);
- if (!p)
- write_error_exit(0);
-
- strcat(strcpy(p, new_path), / IMAPDB);
unlink(p);
free(p);
imapscan_init(minfo);
@@ -2448,14 +2442,12 @@
}
return 0;
}
- q=malloc(strlen(p)+sizeof(/new));
- if (!q)
+ if(asprintf(q, %s%s, p, /new) == -1)
{
free(p);
maildir_aclt_list_destroy(aclt_list);
return -1;
}
- strcat(strcpy(q, p), /new);
free(p);
if (maildir_aclt_list_add(aclt_list,
@@ -4216,11 +4208,10 @@
if (p (p=maildir_shareddir(., p+1)) != NULL)
{
int rc;
- char *q=malloc(strlen(p)+sizeof(/shared));
-
- if (!q) write_error_exit(0);
-
- strcat(strcpy(q, p), /shared);
+ char *q;
+
+ if(asprintf(q, %s%s, p, /shared) == -1)
+ write_error_exit(0);
free(p);
rc=append(tag, tok-tokenbuf, q);
free(q);
@@ -6041,10 +6032,9 @@
isshared=0;
if (is_sharedsubdir(cqinfo.destmailbox))
{
- char *p=malloc(strlen(cqinfo.destmailbox)+sizeof(/shared));
-
- if (!p) write_error_exit(0);
- strcat(strcpy(p, cqinfo.destmailbox), /shared);
+ char *p;
+ if(asprintf(p, %s%s, cqinfo.destmailbox, /shared) == -1)
+ write_error_exit(0);
free(mailbox);
mailbox=cqinfo.destmailbox=p;
[patch] courier-imap-4.13 imapd patch replacing malloc, strcat and strcpy with asprintf
Hi,
I'm using OpenBSD 5.5. courier-imap-4.13 is in the ports tree and it's
quite a mess. I started looking at it today with the hope of just
replacing some of the malloc,strcat strcpy calls with asprintf, but it
became clear before long that there's lots more issues with this code.
Regardless, here's a patch which fixes some of the malloc,strcat
strcpy spaghetti in imapd.c
I figure you guys are fairly busy with OpenSSL right now so if it's OK
with you I'll get working on the rest of this code.
I spoke with the courier-imap team and their response was less than
satisfactory - claiming asprintf code won't compile on non-linux
platforms.
--- imapd_orig.c Sat Apr 19 19:38:18 2014
+++ imapd.c Sat Apr 19 20:09:48 2014
@@ -343,9 +343,8 @@
if (q)
{
- r=malloc(strlen(q)+sizeof(/.));
- if (!r) write_error_exit(0);
- strcat(strcpy(r, q), /.);
+ if(asprintf(r, %s%s, q, /.) == -1)
+ write_error_exit(0);
if (access(r, 0) == 0)
{
free(r);
@@ -1373,11 +1372,9 @@
static char *alloc_filename(const char *mbox, const char *name)
{
-char *p=malloc(strlen(mbox)+strlen(name)+sizeof(/cur/));
-
- if (!p) write_error_exit(0);
-
- strcat(strcat(strcpy(p, mbox), /cur/), name);
+ char *p;
+ if(asprintf(p, %s%s%s, mbox, /cur/, name) == -1)
+ write_error_exit(0);
return (p);
}
@@ -1812,14 +1809,12 @@
{
#if EXPLICITDIRSYNC
- char *p=malloc(strlen(folder)+sizeof(/new));
+ char *p;
int fd;
-
- if (!p)
+
+ if(asprintf(p, %s%s, folder, /new) == -1)
write_error_exit(0);
- p=strcat(strcpy(p, folder), /new);
-
fd=open(p, O_RDONLY);
if (fd = 0)
@@ -1828,7 +1823,8 @@
close(fd);
}
- p=strcat(strcpy(p, folder), /cur);
+ if(asprintf(p, %s%s, folder, /cur) == -1)
+ write_error_exit(0);
fd=open(p, O_RDONLY);
@@ -2212,12 +2208,10 @@
{
struct imapscaninfo minfo;
- char *p=malloc(strlen(new_path)+sizeof(/ IMAPDB));
+ char *p;
+ if(asprintf(p, %s%s, new_path, / IMAPDB) == -1)
+write_error_exit(0);
- if (!p)
- write_error_exit(0);
-
- strcat(strcpy(p, new_path), / IMAPDB);
unlink(p);
free(p);
imapscan_init(minfo);
@@ -2448,14 +2442,12 @@
}
return 0;
}
- q=malloc(strlen(p)+sizeof(/new));
- if (!q)
+ if(asprintf(q, %s%s, p, /new) == -1)
{
free(p);
maildir_aclt_list_destroy(aclt_list);
return -1;
}
- strcat(strcpy(q, p), /new);
free(p);
if (maildir_aclt_list_add(aclt_list,
@@ -4216,11 +4208,10 @@
if (p (p=maildir_shareddir(., p+1)) != NULL)
{
int rc;
- char *q=malloc(strlen(p)+sizeof(/shared));
-
- if (!q) write_error_exit(0);
-
- strcat(strcpy(q, p), /shared);
+ char *q;
+
+ if(asprintf(q, %s%s, p, /shared) == -1)
+ write_error_exit(0);
free(p);
rc=append(tag, tok-tokenbuf, q);
free(q);
@@ -6041,10 +6032,9 @@
isshared=0;
if (is_sharedsubdir(cqinfo.destmailbox))
{
- char *p=malloc(strlen(cqinfo.destmailbox)+sizeof(/shared));
-
- if (!p) write_error_exit(0);
- strcat(strcpy(p, cqinfo.destmailbox), /shared);
+ char *p;
+ if(asprintf(p, %s%s, cqinfo.destmailbox, /shared) == -1)
+ write_error_exit(0);
free(mailbox);
mailbox=cqinfo.destmailbox=p;
--- imapd_orig.c Sat Apr 19 19:38:18 2014
+++ imapd.c Sat Apr 19 20:09:48 2014
@@ -343,9 +343,8 @@
if (q)
{
- r=malloc(strlen(q)+sizeof(/.));
- if (!r) write_error_exit(0);
- strcat(strcpy(r, q), /.);
+ if(asprintf(r, %s%s, q, /.) == -1)
+write_error_exit(0);
if (access(r, 0) == 0)
{
free(r);
@@ -1373,11 +1372,9 @@
static char *alloc_filename(const char *mbox, const char *name)
{
-char *p=malloc(strlen(mbox)+strlen(name)+sizeof(/cur/));
-
- if (!p) write_error_exit(0);
-
- strcat(strcat(strcpy(p, mbox), /cur/), name);
+ char *p;
+ if(asprintf(p, %s%s%s, mbox, /cur/, name) == -1)
+ write_error_exit(0);
return (p);
}
@@ -1812,14 +1809,12 @@
{
#if EXPLICITDIRSYNC
- char *p=malloc(strlen(folder)+sizeof(/new));
+ char *p;
int fd;
-
- if (!p)
+
+ if(asprintf(p, %s%s, folder, /new) == -1)
write_error_exit(0);
- p=strcat(strcpy(p, folder), /new);
-
fd=open(p, O_RDONLY);
if (fd = 0)
@@ -1828,7 +1823,8 @@
close(fd);
}
- p=strcat(strcpy(p, folder), /cur);
+ if(asprintf(p, %s%s, folder, /cur) == -1)
+ write_error_exit(0);
fd=open(p, O_RDONLY);
@@ -2212,12 +2208,10 @@
{
struct imapscaninfo minfo;
- char *p=malloc(strlen(new_path)+sizeof(/ IMAPDB));
+ char *p;
+ if(asprintf(p, %s%s, new_path, / IMAPDB) == -1)
+write_error_exit(0);
- if (!p)
- write_error_exit(0);
-
- strcat(strcpy(p, new_path), / IMAPDB);
unlink(p);
free(p);
imapscan_init(minfo);
@@ -2448,14 +2442,12 @@
}
return 0;
}
- q=malloc(strlen(p)+sizeof(/new));
- if (!q)
+ if(asprintf(q, %s%s, p, /new) == -1)
{
free(p);
maildir_aclt_list_destroy(aclt_list);
return -1;
}
- strcat(strcpy(q, p), /new);
free(p);
if (maildir_aclt_list_add(aclt_list,
@@ -4216,11 +4208,10 @@
if (p (p=maildir_shareddir(., p+1)) != NULL)
{
int rc;
-char *q=malloc(strlen(p)+sizeof(/shared));
-
-if (!q) write_error_exit(0);
-
-strcat(strcpy(q, p), /shared);
+char
