On the matter of strlcpy/strlcat acceptance by industry
>From time to time, there are people who say that strlcpy and strlcat are stupid. This is a little frustrating because we just want developers to have an easier time writing/auditing string code to avoid overflows and truncations, especially considering so many standard C APIs require fixed length strings or have other limits, and will in the forceable future. You probably all know about the mainstream users of these functions, like the Linux kernel, or MacOS, or the other BSD's, and Solaris. But there are many, many more, and it is time to show the global strlcpy'ing deniers the reality. I've collected some statistics to see how much upstream software use these functions. I asked Stuart Henderson to collect a "recursive nm .o" for every piece of software built in our ports tree. It's roughly 2GB of text output. For those who don't know, that ports tree is basically a repository of all the application software we supply as an add-on on top of the base operating system. Each of those becomes a package, so that is what we are looking at. They are pretty much the bulk of the commonly-used Unix applications found on all systems. These packages do not generally include things like openssh, perl, or X11, sqlite, or a number of other small things directly integrated into the OpenBSD base. But that's OK, because those I just mentioned do use strlcpy and strlcat in their upstream repositories. So 3535 packages contain .o files, and now we can grep to see what they define or use. In essence, a piece of software will likely fall into one of these catagories: (0) Not use the functions at all. (1) Will assume that the system has the functions in libc. (2) Will have a configure-style "feature-test" which tests if libc contains the functions, and thus turn on a cpp symbol such as HAS_STRLCPY, then use the libc version. Otherwise it will avoid using them... (3) More commonly, if the feature-test fails, it will substitute copies from its own tree. Essentially to cope with glibc. (4) Some software contain their own version, typically copied from us, but renamed. There are many of these. Let's look at these cases backwards, for reasons that become obvious as we move ahead. (4) Who is defining their own versions of the functions, with slightly different names? The obvious names we find are: SDL_strlcpy SDL_utf8strlcpy _iodbcdm_strlcpy _strlcpyascii_safe_strlcpy av_strlcpy cli_strlcpy dt_utf8_strlcpy fc_strlcpy fl_strlcpy flac__strlcpy fz_strlcpy g_strlcpy hd_strlcpy isc_string_strlcpy lg_strlcpy llvm_strlcpyloud_strlcpy mcs_strlcpy mg_strlcpy monoeg_g_strlcpy mowgli_strlcpy my_strlcpy mystrlcpy os_strlcpy pa_strlcpy rb_strlcpy sg_strlcpy sl_strlcpy sm_strlcpy test_evutil_strlcpy test_strlcpytr_strlcpy ut_strlcpy utf8_strlcpyuv_strlcpy vi_strlcpy xstrlcpyzbx_strlcpy SDL_strlcat SDL_strlcpy _iodbcdm_strlcat av_strlcat fc_strlcat fl_strlcat flac__strlcat fz_strlcat g_strlcat hd_strlcat isc_string_strlcat ixp_strlcat mcs_strlcat mowgli_strlcat mystrlcat rb_strlcat sg_strlcat sl_strlcat sm_strlcat ssh_strlcat uv_strlcat vi_strlcat wmii_strlcatxstrlcat zbx_strlcat Replacement copies seem to be quite popular. Some of the names hint at who is doing this, but we can search by these functions to see which packages are defining them: bogofilter bro clamav cntlm cups-filters darktable dkim-milter ffmpeg flac fltk freeciv fte glib2 gtk-gnutella htmldoc iodbc ircd-ratbox isc-bind isc-dhcp ksh93 leafnode libixp libstatgrab link-grammar linkchecker llvm mathomatic mcs mono mowgli mupdf mysql node pmacct postgresql pulseaudio rlwrap samhain sdl2 tcpreplay transmission visitors wmii wpa_supplicant xfe xpilot zabbix So 73 (2% or 3535) of packages define either of these for themselves under a new name. This may seem like a small list, but look it contains monsters like glib2, postgresql, and mysql. In particular, those monster contain libraries.. this will become more obvious a bit further on. (3) What about software which substitutes their own, when they don't find ours? This is harder to determine in the OpenBSD ports tree because our libc functions will always be found. However, we can see if a
Intel drm resume
Looking at ATI resume, I pushed Mark to explore using DVACT_WAKEUP instead, because the code looks super hairy and might sleep... The same applies to Intel. Since there are many more people who rely on such machines, can we get some test reports of this on all machines, for suspend/resume and hibernate? Index: dev/pci/drm/i915/i915_drv.c === RCS file: /cvs/src/sys/dev/pci/drm/i915/i915_drv.c,v retrieving revision 1.55 diff -u -p -u -r1.55 i915_drv.c --- dev/pci/drm/i915/i915_drv.c 6 Dec 2013 11:17:20 - 1.55 +++ dev/pci/drm/i915/i915_drv.c 20 Jan 2014 09:36:07 - @@ -1126,7 +1126,7 @@ inteldrm_activate(struct device *arg, in break; case DVACT_SUSPEND: break; - case DVACT_RESUME: + case DVACT_WAKEUP: i915_drm_thaw(dev); intel_fb_restore_mode(dev); break;
Vax
I would like to thank the members of the community who deliver four vax machines to the project. Vax builds have started again!
openssl's *strlcy
Small demonstration of the kinds of things we'll have to mop up for weeks more. >From OpenSSL CHANGES file: *) Introduce safe string copy and catenation functions (BUF_strlcpy() and BUF_strlcat()). [Ben Laurie (CHATS) and Richard Levitte] That's from back in 2002. These functions work just like ours in OpenBSD. The return values indicate overflow. We've been advising people to check for overflow from the start, which is why we designed these like snprintf. Good move, but then let's see how they used them: ./apps/ca.c:BUF_strlcpy(tofree, s, len); ./apps/ca.c:BUF_strlcat(tofree, "/", len); ./apps/ca.c:BUF_strlcat(tofree, CONFIG_FILE, len); ./apps/ca.c:BUF_strlcat(buf[2], "/", sizeof(buf[2])); ./apps/ca.c:BUF_strlcpy(row[DB_file], "unknown", 8); ./apps/ca.c:BUF_strlcpy(row[DB_file], "unknown", 8); ./apps/ca.c:BUF_strlcpy(str, (char *) revtm->data, i); ./apps/ca.c:BUF_strlcat(str, ",", i); ./apps/ca.c:BUF_strlcat(str, reason, i); ./apps/ca.c:BUF_strlcat(str, ",", i); ./apps/ca.c:BUF_strlcat(str, other, i); ./apps/apps.c: BUF_strlcpy(out, p, size); ./apps/apps.c: BUF_strlcpy(buf[0], serialfile, BSIZE); ./apps/engine.c:BUF_strlcat(*buf, ", ", *size); ./apps/engine.c:BUF_strlcat(*buf, s, *size); ./apps/pkcs12.c:BUF_strlcpy(macpass, pass, sizeof macpass); ./apps/pkcs12.c:BUF_strlcpy(macpass, pass, sizeof macpass); ./apps/req.c: BUF_strlcpy(buf, value, sizeof buf); ./apps/req.c: BUF_strlcat(buf, "\n", sizeof buf); ./apps/req.c: BUF_strlcpy(buf, def, sizeof buf); ./apps/req.c: BUF_strlcat(buf, "\n", sizeof buf); ./apps/req.c: BUF_strlcpy(buf, value, sizeof buf); ./apps/req.c: BUF_strlcat(buf, "\n", sizeof buf); ./apps/req.c: BUF_strlcpy(buf, def, sizeof buf); ./apps/req.c: BUF_strlcat(buf, "\n", sizeof buf); ./apps/s_socket.c: BUF_strlcpy(*host, h1->h_name, strlen(h1->h_name) + 1); ./apps/srp.c: BUF_strlcpy(tofree, s, len); ./apps/srp.c: BUF_strlcat(tofree, "/", len); ./apps/srp.c: BUF_strlcat(tofree, CONFIG_FILE, len); ./apps/x509.c: BUF_strlcpy(buf, CAfile, len); ./apps/x509.c: BUF_strlcat(buf, POSTFIX, len); ./apps/x509.c: BUF_strlcpy(buf, serialfile, len); ./crypto/asn1/a_time.c: if (t->data[0] >= '5') BUF_strlcpy(str, "19", newlen); ./crypto/asn1/a_time.c: else BUF_strlcpy(str, "20", newlen); ./crypto/asn1/a_time.c: BUF_strlcat(str, (char *)t->data, newlen); ./crypto/bio/b_dump.c: BUF_strlcpy(buf, str, sizeof buf); ./crypto/bio/b_dump.c: BUF_strlcat(buf, tmp, sizeof buf); ./crypto/bio/b_dump.c: BUF_strlcat(buf, " ", sizeof buf); ./crypto/bio/b_dump.c: BUF_strlcat(buf, tmp, sizeof buf); ./crypto/bio/b_dump.c: BUF_strlcat(buf, " ", sizeof buf); ./crypto/bio/b_dump.c: BUF_strlcat(buf, tmp, sizeof buf); ./crypto/bio/b_dump.c: BUF_strlcat(buf, "\n", sizeof buf); ./crypto/bio/bss_file.c:BUF_strlcpy(p, "a+", sizeof p); ./crypto/bio/bss_file.c:elseBUF_strlcpy(p, "a", sizeof p); ./crypto/bio/bss_file.c:BUF_strlcpy(p, "r+", sizeof p); ./crypto/bio/bss_file.c:BUF_strlcpy(p, "w", sizeof p); ./crypto/bio/bss_file.c:BUF_strlcpy(p, "r", sizeof p); ./crypto/buffer/buf_str.c:BUF_strlcpy(char *dst, const char *src, size_t size) ./crypto/buffer/buf_str.c:BUF_strlcat(char *dst, const char *src, size_t size) ./crypto/buffer/buffer.h:size_t BUF_strlcpy(char *dst, const char *src, size_t siz); ./crypto/buffer/buffer.h:size_t BUF_strlcat(char *dst, const char *src, size_t siz); ./crypto/conf/conf_def.c: BUF_strlcpy(section,"default",10); ./crypto/conf/conf_def.c: BUF_strlcpy(v->name,pname,strlen(pname)+1); ./crypto/dso/dso_lib.c: BUF_strlcpy(copied, filename, strlen(filename) + 1); ./crypto/dso/dso_lib.c: BUF_strlcpy(result, filename, strlen(filename) + 1); ./crypto/err/err.c: BUF_strlcat(str,a,(size_t)s+1); ./crypto/evp/evp_pbe.c: if (!pbe_obj) BUF_strlcpy (obj_tmp, "NULL", sizeof obj_tmp); ./crypto/objects/obj_dat.c: BUF_strlcpy(buf,s,buf_len); ./crypto/objects/obj_dat.c: BUF_strlcpy(buf,bndec,buf_len); ./crypto/objects/obj_dat.c: BUF_strlcpy(buf,tbuf,buf_len); ./crypto/pem/pem_lib.c: BUF_strlcat(buf,"Proc-Type: 4,",PEM_BUFSIZE); ./crypto/pem/pem_lib.c: BUF_strlcat(buf,str,PEM_BUFSIZE); ./crypto/pem/pem_lib.c: BUF_strlcat(buf,"\n",PEM_BUFSIZE); ./crypto/pem/pem_lib.c: BUF_strlcat(buf,"DEK-Info: ",PEM_BUFSIZE); ./crypto/pem/pem_lib.c: BUF_strlcat(buf,type,P
Thanks to M:tier for package signing infrastucture
Now that the 5.5 release is out, I'd like to remind everyone that (unlike previous releases) the distribution is signed. Also, snapshots are signed on a continuous basis. I would like to thank M:tier for supplying more than half of the signing infrastructure. The remainder came from the OpenBSD Foundation. Thanks guys.
new OpenSSL flaws
We are sorry that the errata for these libssl security issues are not up yet. The majority of these issues are in our ssl library as well. Most other operating system vendors have patches available, but that is because they were (obviously) given a heads up to prepare them over the last few days. OpenBSD / LibreSSL did not receive any heads-up from OpenSSL. So hold on, we'll try to have errata out in a few hours.
mallocarray() in sys/dev, first pass
This is the first pass of mallocarray() in sys/dev. Please proofread.
Index: rd.c
===
RCS file: /cvs/src/sys/dev/rd.c,v
retrieving revision 1.7
diff -u -p -u -r1.7 rd.c
--- rd.c12 Jul 2014 18:48:51 - 1.7
+++ rd.c13 Jul 2014 15:49:17 -
@@ -88,7 +88,7 @@ rdattach(int num)
cf.cf_driver = &rd_cd;
rd_cd.cd_ndevs = num;
- rd_cd.cd_devs = malloc(num * sizeof(void *), M_DEVBUF, M_NOWAIT);
+ rd_cd.cd_devs = mallocarray(num, sizeof(void *), M_DEVBUF, M_NOWAIT);
if (rd_cd.cd_devs == NULL)
panic("rdattach: out of memory");
Index: softraid.c
===
RCS file: /cvs/src/sys/dev/softraid.c,v
retrieving revision 1.334
diff -u -p -u -r1.334 softraid.c
--- softraid.c 12 Jul 2014 18:48:51 - 1.334
+++ softraid.c 13 Jul 2014 15:48:56 -
@@ -252,7 +252,7 @@ sr_meta_attach(struct sr_discipline *sd,
/* we have a valid list now create an array index */
cl = &sd->sd_vol.sv_chunk_list;
- sd->sd_vol.sv_chunks = malloc(sizeof(struct sr_chunk *) * chunk_no,
+ sd->sd_vol.sv_chunks = mallocarray(chunk_no, sizeof(struct sr_chunk *),
M_DEVBUF, M_WAITOK | M_ZERO);
/* fill out chunk array */
@@ -1284,13 +1284,13 @@ sr_boot_assembly(struct sr_softc *sc)
}
/* Allocate memory for device and ondisk version arrays. */
- devs = malloc(BIOC_CRMAXLEN * sizeof(dev_t), M_DEVBUF,
+ devs = mallocarray(BIOC_CRMAXLEN, sizeof(dev_t), M_DEVBUF,
M_NOWAIT | M_CANFAIL);
if (devs == NULL) {
printf("%s: failed to allocate device array\n", DEVNAME(sc));
goto unwind;
}
- ondisk = malloc(BIOC_CRMAXLEN * sizeof(u_int64_t), M_DEVBUF,
+ ondisk = mallocarray(BIOC_CRMAXLEN, sizeof(u_int64_t), M_DEVBUF,
M_NOWAIT | M_CANFAIL);
if (ondisk == NULL) {
printf("%s: failed to allocate ondisk array\n", DEVNAME(sc));
@@ -1934,8 +1934,9 @@ sr_ccb_alloc(struct sr_discipline *sd)
if (sd->sd_ccb)
return (1);
- sd->sd_ccb = malloc(sizeof(struct sr_ccb) *
- sd->sd_max_wu * sd->sd_max_ccb_per_wu, M_DEVBUF, M_WAITOK | M_ZERO);
+ sd->sd_ccb = mallocarray(sd->sd_max_wu,
+ sd->sd_max_ccb_per_wu * sizeof(struct sr_ccb),
+ M_DEVBUF, M_WAITOK | M_ZERO);
TAILQ_INIT(&sd->sd_ccb_freeq);
for (i = 0; i < sd->sd_max_wu * sd->sd_max_ccb_per_wu; i++) {
ccb = &sd->sd_ccb[i];
Index: systrace.c
===
RCS file: /cvs/src/sys/dev/systrace.c,v
retrieving revision 1.70
diff -u -p -u -r1.70 systrace.c
--- systrace.c 12 Jul 2014 18:48:51 - 1.70
+++ systrace.c 13 Jul 2014 15:43:25 -
@@ -1639,7 +1639,7 @@ systrace_newpolicy(struct fsystrace *fst
DPRINTF(("%s: allocating %d -> %lu\n", __func__,
maxents, (u_long)maxents * sizeof(int)));
- pol->sysent = (u_char *)malloc(maxents * sizeof(u_char),
+ pol->sysent = mallocarray(maxents, sizeof(u_char),
M_XDATA, M_WAITOK);
pol->nsysent = maxents;
for (i = 0; i < maxents; i++)
Index: ic/ahci.c
===
RCS file: /cvs/src/sys/dev/ic/ahci.c,v
retrieving revision 1.15
diff -u -p -u -r1.15 ahci.c
--- ic/ahci.c 12 Jul 2014 18:48:17 - 1.15
+++ ic/ahci.c 13 Jul 2014 16:25:21 -
@@ -527,8 +527,8 @@ ahci_port_alloc(struct ahci_softc *sc, u
}
/* Allocate a CCB for each command slot */
- ap->ap_ccbs = malloc(sizeof(struct ahci_ccb) * sc->sc_ncmds, M_DEVBUF,
- M_NOWAIT | M_ZERO);
+ ap->ap_ccbs = mallocarray(sc->sc_ncmds, sizeof(struct ahci_ccb),
+ M_DEVBUF, M_NOWAIT | M_ZERO);
if (ap->ap_ccbs == NULL) {
printf("%s: unable to allocate command list for port %d\n",
DEVNAME(sc), port);
Index: ic/aic79xx.c
===
RCS file: /cvs/src/sys/dev/ic/aic79xx.c,v
retrieving revision 1.53
diff -u -p -u -r1.53 aic79xx.c
--- ic/aic79xx.c12 Jul 2014 18:48:17 - 1.53
+++ ic/aic79xx.c13 Jul 2014 16:25:06 -
@@ -6177,8 +6177,8 @@ ahd_init(struct ahd_softc *ahd)
AHD_ASSERT_MODES(ahd, AHD_MODE_SCSI_MSK, AHD_MODE_SCSI_MSK);
ahd->stack_size = ahd_probe_stack_size(ahd);
- ahd->saved_stack = malloc(ahd->stack_size * sizeof(uint16_t), M_DEVBUF,
- M_NOWAIT | M_ZERO);
+ ahd->saved_stack = mallocarray(ahd->stack_size, sizeof(uint16_t),
+ M_DEVBUF, M_NOWAIT | M_ZERO);
if (ahd->saved_stack == NULL)
return (ENOMEM);
Index: ic/aic7xxx.c
===
RCS file: /cvs/src/sys/dev/ic/aic7xxx.c,
libkvm page size handling
libkvm already figures out the pagesize of the machine in _kvm_open(),
and then allows the machine-dependent _kvm_initvtop() to override it
if need be (thereby, handling sparc). Thus we can avoid the PAGE_SIZE,
PAGE_SHIFT, ... variables.
Seems to be working ... wonder if I missed some relevant testing.
Index: kvm_alpha.c
===
RCS file: /cvs/src/lib/libkvm/kvm_alpha.c,v
retrieving revision 1.14
diff -u -p -u -r1.14 kvm_alpha.c
--- kvm_alpha.c 20 Mar 2006 15:11:48 - 1.14
+++ kvm_alpha.c 23 Mar 2013 16:39:53 -
@@ -113,10 +113,6 @@ _kvm_kvatop(kvm_t *kd, u_long va, paddr_
vm = kd->vmst;
page_off = va & (cpu_kh->page_size - 1);
-#ifndef PAGE_SHIFT
-#definePAGE_SHIFT vm->page_shift
-#endif
-
if (va >= ALPHA_K0SEG_BASE && va <= ALPHA_K0SEG_END) {
/*
* Direct-mapped address: just convert it.
Index: kvm_amd64.c
===
RCS file: /cvs/src/lib/libkvm/kvm_amd64.c,v
retrieving revision 1.9
diff -u -p -u -r1.9 kvm_amd64.c
--- kvm_amd64.c 5 Dec 2012 23:20:02 - 1.9
+++ kvm_amd64.c 23 Mar 2013 16:39:25 -
@@ -96,11 +96,11 @@ _kvm_kvatop(kvm_t *kd, u_long va, paddr_
return (0);
}
- page_off = va & PAGE_MASK;
+ page_off = va & (kd->nbpg - 1);
if (va >= PMAP_DIRECT_BASE && va <= PMAP_DIRECT_END) {
*pa = va - PMAP_DIRECT_BASE;
- return (int)(PAGE_SIZE - page_off);
+ return (int)(kd->nbpg - page_off);
}
cpu_kh = kd->cpu_data;
@@ -177,7 +177,7 @@ _kvm_kvatop(kvm_t *kd, u_long va, paddr_
goto lose;
}
*pa = (pte & PG_FRAME) + page_off;
- return (int)(PAGE_SIZE - page_off);
+ return (int)(kd->nbpg - page_off);
lose:
*pa = (u_long)~0L;
Index: kvm_arm.c
===
RCS file: /cvs/src/lib/libkvm/kvm_arm.c,v
retrieving revision 1.7
diff -u -p -u -r1.7 kvm_arm.c
--- kvm_arm.c 20 Mar 2013 14:46:45 - 1.7
+++ kvm_arm.c 23 Mar 2013 16:39:19 -
@@ -106,7 +106,7 @@ _kvm_kvatop(kvm_t *kd, u_long va, paddr_
va < cpup->kernelbase + cpup->kerneloffs + cpup->staticsize) {
*pa = (va - cpup->kernelbase) +
(paddr_t)cpup->ram_segs[0].start;
- return (int)(PAGE_SIZE - (va & PAGE_MASK));
+ return (int)(kd->nbpg - (va & (kd->nbpg - 1)));
}
_kvm_err(kd, 0, "kvm_vatop: va %x unreachable", va);
Index: kvm_hppa.c
===
RCS file: /cvs/src/lib/libkvm/kvm_hppa.c,v
retrieving revision 1.7
diff -u -p -u -r1.7 kvm_hppa.c
--- kvm_hppa.c 28 Jul 2009 12:56:25 - 1.7
+++ kvm_hppa.c 23 Mar 2013 16:39:13 -
@@ -63,7 +63,7 @@ _kvm_kvatop(kvm_t *kd, u_long va, paddr_
/* XXX this only really works for the kernel image only */
*pa = va;
- return (PAGE_SIZE - (va & PAGE_MASK));
+ return (kd->nbpg - (va & (kd->nbpg - 1)));
}
/*
Index: kvm_hppa64.c
===
RCS file: /cvs/src/lib/libkvm/kvm_hppa64.c,v
retrieving revision 1.1
diff -u -p -u -r1.1 kvm_hppa64.c
--- kvm_hppa64.c9 Jul 2011 00:29:59 - 1.1
+++ kvm_hppa64.c23 Mar 2013 16:39:05 -
@@ -63,7 +63,7 @@ _kvm_kvatop(kvm_t *kd, u_long va, paddr_
/* XXX this only really works for the kernel image only */
*pa = va;
- return (PAGE_SIZE - (va & PAGE_MASK));
+ return (kd->nbpg - (va & (kd->nbpg - 1)));
}
/*
Index: kvm_i386.c
===
RCS file: /cvs/src/lib/libkvm/kvm_i386.c,v
retrieving revision 1.22
diff -u -p -u -r1.22 kvm_i386.c
--- kvm_i386.c 9 Jul 2012 08:43:10 - 1.22
+++ kvm_i386.c 23 Mar 2013 16:38:56 -
@@ -101,10 +101,10 @@ _kvm_initvtop(kvm_t *kd)
(off_t)_kvm_pa2off(kd, nl[0].n_value - KERNBASE)) != sizeof pa)
goto invalid;
- vm->PTD = (pd_entry_t *)_kvm_malloc(kd, PAGE_SIZE);
+ vm->PTD = (pd_entry_t *)_kvm_malloc(kd, kd->nbpg);
- if (_kvm_pread(kd, kd->pmfd, vm->PTD, PAGE_SIZE,
- (off_t)_kvm_pa2off(kd, pa)) != PAGE_SIZE)
+ if (_kvm_pread(kd, kd->pmfd, vm->PTD, kd->nbpg,
+ (off_t)_kvm_pa2off(kd, pa)) != kd->nbpg)
goto invalid;
return (0);
@@ -138,7 +138,7 @@ _kvm_kvatop(kvm_t *kd, u_long va, paddr_
}
vm = kd->vmst;
- offset = va & PAGE_MASK;
+ offset = va & (kd->nbpg - 1);
/*
* If we are initializing (kernel page table descriptor pointer
@@ -146,7 +146,7 @@ _kvm_kvatop(kvm_t *kd, u_long va, paddr_
*/
if (vm->PTD == NULL) {
*pa = va;
- return (PAGE_SIZE - (int)offset);
+
