Re: libc malloc poison
On Thu, Jul 04, 2013 at 05:24:20PM +0200, Mark Kettenis wrote: > > From: Theo de Raadt > > Date: Thu, 04 Jul 2013 09:04:54 -0600 > > > > I suspect the best approach would be a hybrid value. The upper half > > of the address should try to land in an unmapped zone, or into the zero > > page, or into some address space hole, ir into super high memory above > > the stack which is gauranteed unmapped. > > Don't forget strict alignment architectures, where it is beneficial > to have the lowest bit set to trigger alignment traps. You also want the highest bit set. This makes sure signed indexes get interpreted as negative, which should more often detect problems than positive ones. There are not only pointers in the heap. I very much prefer the values te be easily recognizable in a debugger and to keep a clear distinction between "uninitialized" and "freed" chunks. Too much random is not good thing in this case. I picked 0xdf a few years ago to mark free'd memory. The f is supposed to help you remember that when you see it in a debugger. Maybe put a fixed pattern in the low nibbles and a random in the high nibble. Together with the lowest and highest bit set this would go like: 0x8e0d0e0f || 0xr0r0r0r0 (pick another lhs for free'ed mem) Wondering if that would produce easily recognizable patterns that still trigger enough faults. -Otto
Re: Removing -Wno-format from kernel makefiles, 06/16
On Thu, Jul 04, 2013 at 06:39:03PM +0200, Stefan Fritsch wrote:
> diff --git sys/netinet/ip_output.c sys/netinet/ip_output.c
> index b59accf..43a0551 100644
> --- sys/netinet/ip_output.c
> +++ sys/netinet/ip_output.c
> @@ -267,7 +267,7 @@ reroute:
> if (mtag != NULL) {
> #ifdef DIAGNOSTIC
> if (mtag->m_tag_len != sizeof (struct tdb_ident))
> - panic("ip_output: tag of length %d (should be %d",
> + panic("ip_output: tag of length %hu (should be %zu",
> mtag->m_tag_len, sizeof (struct tdb_ident));
> #endif
> tdbi = (struct tdb_ident *)(mtag + 1);
> diff --git sys/netinet6/ip6_forward.c sys/netinet6/ip6_forward.c
> index 6d7f971..1dff149 100644
> --- sys/netinet6/ip6_forward.c
> +++ sys/netinet6/ip6_forward.c
> @@ -160,7 +160,7 @@ reroute:
> if (mtag != NULL) {
> #ifdef DIAGNOSTIC
> if (mtag->m_tag_len != sizeof (struct tdb_ident))
> - panic("ip6_forward: tag of length %d (should be %d",
> + panic("ip6_forward: tag of length %hu (should be %zu",
> mtag->m_tag_len, sizeof (struct tdb_ident));
> #endif
> tdbi = (struct tdb_ident *)(mtag + 1);
> diff --git sys/netinet6/ip6_output.c sys/netinet6/ip6_output.c
> index f405b31..baf4103 100644
> --- sys/netinet6/ip6_output.c
> +++ sys/netinet6/ip6_output.c
> @@ -232,7 +232,7 @@ ip6_output(struct mbuf *m0, struct ip6_pktopts *opt,
> struct route_in6 *ro,
> if (mtag != NULL) {
> #ifdef DIAGNOSTIC
> if (mtag->m_tag_len != sizeof (struct tdb_ident))
> - panic("ip6_output: tag of length %d (should be %d",
> + panic("ip6_output: tag of length %hu (should be %zu",
> mtag->m_tag_len, sizeof (struct tdb_ident));
> #endif
> tdbi = (struct tdb_ident *)(mtag + 1);
netinet and netinet6 is OK bluhm@
> diff --git sys/nfs/nfs_socket.c sys/nfs/nfs_socket.c
> index e0f28e4..e1f7b3d 100644
> --- sys/nfs/nfs_socket.c
> +++ sys/nfs/nfs_socket.c
> @@ -600,7 +600,7 @@ tryagain:
> } while (error == EWOULDBLOCK);
> if (!error && auio.uio_resid > 0) {
> log(LOG_INFO,
> - "short receive (%d/%d) from nfs server %s\n",
> + "short receive (%zd/%zd) from nfs server %s\n",
>sizeof(u_int32_t) - auio.uio_resid,
>sizeof(u_int32_t),
>rep->r_nmp->nm_mountp->mnt_stat.f_mntfromname);
This should be %zu/%zu
> @@ -631,7 +631,7 @@ tryagain:
>error == ERESTART);
> if (!error && auio.uio_resid > 0) {
> log(LOG_INFO,
> - "short receive (%d/%d) from nfs server %s\n",
> + "short receive (%zu/%u) from nfs server %s\n",
> len - auio.uio_resid, len,
> rep->r_nmp->nm_mountp->mnt_stat.f_mntfromname);
> error = EPIPE;
> --
> 1.7.6
Note that in nfs is another len that should be %u.
log(LOG_ERR, "%s (%d) from nfs server %s\n",
"impossible packet length",
len,
rep->r_nmp->nm_mountp->mnt_stat.f_mntfromname);
bluhm
Re: Removing -Wno-format from kernel makefiles, 4/16
> Date: Thu, 4 Jul 2013 18:42:50 +0200 (CEST)
> From: Stefan Fritsch
>
> On Wed, 3 Jul 2013, Mark Kettenis wrote:
> > > diff --git sys/arch/i386/i386/esm.c sys/arch/i386/i386/esm.c
> > > index c90b2c4..3dff69e 100644
> > > --- sys/arch/i386/i386/esm.c
> > > +++ sys/arch/i386/i386/esm.c
> > > @@ -880,7 +880,7 @@ esm_make_sensors(struct esm_softc *sc, struct
> > > esm_devmap *devmap,
> > > }
> > >
> > > for (j = 0; j < nsensors; j++) {
> > > - snprintf(s[j].desc, sizeof(s[j].desc), "%s %d",
> > > + snprintf(s[j].desc, sizeof(s[j].desc), "%s %ld",
> > > sensor_map[i].name, sensor_map[i].arg + j);
> > > }
> > > break;
> >
> > Looking at this one, it makes more sense to make the "arg" member of
> > "struct esm_sensor_map" an int. That will result in some space
> > savings if we'd ever bring this driver to amd64.
>
go for it
> --- sys/arch/i386/i386/esm.c
> +++ sys/arch/i386/i386/esm.c
> @@ -87,7 +87,7 @@ enum sensor_type esm_typemap[] = {
>
> struct esm_sensor_map {
> enum esm_sensor_typetype;
> - longarg;
> + int arg;
> const char *name;
> };
>
>
>
>
Re: Removing -Wno-format from kernel makefiles, 3/16
> Date: Thu, 4 Jul 2013 18:41:30 +0200 (CEST)
> From: Stefan Fritsch
>
> On Wed, 3 Jul 2013, Mark Kettenis wrote:
> > > diff --git sys/arch/i386/i386/db_interface.c
> > > sys/arch/i386/i386/db_interface.c
> > > index 85c1ff5..c75fd89 100644
> > > --- sys/arch/i386/i386/db_interface.c
> > > +++ sys/arch/i386/i386/db_interface.c
> > > @@ -197,11 +197,11 @@ db_sysregs_cmd(db_expr_t addr, int have_addr,
> > > db_expr_t count, char *modif)
> > > uint16_t ldtr, tr;
> > >
> > > __asm__ __volatile__("sidt %0" : "=m" (idtr));
> > > - db_printf("idtr: 0x%08x/%04x\n",
> > > + db_printf("idtr: 0x%08x/%04llx\n",
> > > (unsigned int)(idtr >> 16), idtr & 0x);
> > >
> > > __asm__ __volatile__("sgdt %0" : "=m" (gdtr));
> > > - db_printf("gdtr: 0x%08x/%04x\n",
> > > + db_printf("gdtr: 0x%08x/%04llx\n",
> > > (unsigned int)(gdtr >> 16), gdtr & 0x);
> >
> > This is a tad bit inconsistent. I'd either use %llx for both values
> > and get rid of the cast, or use %x and use a cast in both cases.
>
> Like this?
ok kettenis@
> --- sys/arch/i386/i386/db_interface.c
> +++ sys/arch/i386/i386/db_interface.c
> @@ -197,12 +197,10 @@ db_sysregs_cmd(db_expr_t addr, int have_addr, db_expr_t
> count, char *modif)
> uint16_t ldtr, tr;
>
> __asm__ __volatile__("sidt %0" : "=m" (idtr));
> - db_printf("idtr: 0x%08x/%04x\n",
> - (unsigned int)(idtr >> 16), idtr & 0x);
> + db_printf("idtr: 0x%08llx/%04llx\n", idtr >> 16, idtr & 0x);
>
> __asm__ __volatile__("sgdt %0" : "=m" (gdtr));
> - db_printf("gdtr: 0x%08x/%04x\n",
> - (unsigned int)(gdtr >> 16), gdtr & 0x);
> + db_printf("gdtr: 0x%08llx/%04llx\n", gdtr >> 16, gdtr & 0x);
>
> __asm__ __volatile__("sldt %0" : "=g" (ldtr));
> db_printf("ldtr: 0x%04x\n", ldtr);
>
Re: Removing -Wno-format from kernel makefiles, 07/16
On Jul 4, 2013, at 6:43 PM, Stefan Fritsch wrote:
> fix: %x instead of %p for int
>
> ---
> sys/dev/pci/musycc_obsd.c |2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git sys/dev/pci/musycc_obsd.c sys/dev/pci/musycc_obsd.c
> index 25a58d8..0844136 100644
> --- sys/dev/pci/musycc_obsd.c
> +++ sys/dev/pci/musycc_obsd.c
> @@ -215,7 +215,7 @@ musycc_ebus_attach(struct device *parent, struct
> musycc_softc *esc,
> #endif
>
> if (ebus_attach_device(&rom, sc, 0, 0x400) != 0) {
> - printf(": failed to map rom @ %05p\n", 0);
> + printf(": failed to map rom @ %05x\n", 0);
Makes me wonder what benefit this has other than avoiding to type four
more zeros in the format string manually? ;)
> goto failed;
> }
>
> --
> 1.7.6
>
Re: Removing -Wno-format from kernel makefiles, V2
On Wednesday 03 July 2013, Stefan Fritsch wrote: > I haven't done any signedness fixes so far, only the size fixes. > But I will look over the patches again and try to fix the > signedness issues in the touched code lines, too. There are quite > a few I guess. For people prefering to review by source files, an updated patch is here: http://www.sfritsch.de/~stf/openbsd-format-string-v3/combined.patch
Removing -Wno-format from kernel makefiles, 08/16
%hu/%hd for uint16_t, %u/%d/%x for uint32_t
despite the name, ntohl returns uint32_t
also fix some %d into %u
---
sys/arch/i386/i386/bios.c|4 ++--
sys/arch/i386/i386/ioapic.c |2 +-
sys/arch/i386/pci/pcibios.c |4 ++--
sys/kern/vfs_cluster.c |2 +-
sys/msdosfs/msdosfs_conv.c |2 +-
sys/msdosfs/msdosfs_denode.c |2 +-
sys/msdosfs/msdosfs_vnops.c |2 +-
sys/net/if_spppsubr.c|2 +-
8 files changed, 10 insertions(+), 10 deletions(-)
diff --git sys/arch/i386/i386/bios.c sys/arch/i386/i386/bios.c
index 6e46e6a..75dab38 100644
--- sys/arch/i386/i386/bios.c
+++ sys/arch/i386/i386/bios.c
@@ -238,7 +238,7 @@ biosattach(struct device *parent, struct device *self, void
*aux)
bios32_entry.segment = GSEL(GCODE_SEL, SEL_KPL);
bios32_entry.offset =
(u_int32_t)ISA_HOLE_VADDR(h->entry);
- printf(", BIOS32 rev. %d @ 0x%lx", h->rev, h->entry);
+ printf(", BIOS32 rev. %d @ 0x%x", h->rev, h->entry);
break;
}
}
@@ -285,7 +285,7 @@ biosattach(struct device *parent, struct device *self, void
*aux)
for (; pa < end; pa+= NBPG, eva+= NBPG)
pmap_kenter_pa(eva, pa, VM_PROT_READ);
- printf(", SMBIOS rev. %d.%d @ 0x%lx (%d entries)",
+ printf(", SMBIOS rev. %d.%d @ 0x%x (%hd entries)",
sh->majrev, sh->minrev, sh->addr, sh->count);
/*
* Unbelievably the SMBIOS version number
diff --git sys/arch/i386/i386/ioapic.c sys/arch/i386/i386/ioapic.c
index 0b0c6b8..4197553 100644
--- sys/arch/i386/i386/ioapic.c
+++ sys/arch/i386/i386/ioapic.c
@@ -300,7 +300,7 @@ ioapic_attach(struct device *parent, struct device *self,
void *aux)
ioapic_add(sc);
- printf(" pa 0x%lx", aaa->apic_address);
+ printf(" pa 0x%x", aaa->apic_address);
if (bus_mem_add_mapping(aaa->apic_address, PAGE_SIZE, 0, &bh) != 0) {
printf(", map failed\n");
diff --git sys/arch/i386/pci/pcibios.c sys/arch/i386/pci/pcibios.c
index 372ac51..bc8a848 100644
--- sys/arch/i386/pci/pcibios.c
+++ sys/arch/i386/pci/pcibios.c
@@ -151,7 +151,7 @@ pcibiosprobe(struct device *parent, void *match, void *aux)
rv = bios32_service(PCIBIOS_SIGNATURE, &pcibios_entry,
&pcibios_entry_info);
- PCIBIOS_PRINTV(("pcibiosprobe: 0x%lx:0x%lx at 0x%lx[0x%lx]\n",
+ PCIBIOS_PRINTV(("pcibiosprobe: 0x%hx:0x%x at 0x%x[0x%x]\n",
pcibios_entry.segment, pcibios_entry.offset,
pcibios_entry_info.bei_base, pcibios_entry_info.bei_size));
@@ -172,7 +172,7 @@ pcibiosattach(struct device *parent, struct device *self,
void *aux)
&rev_min, &mech1, &mech2,
&scmech1, &scmech2, &sc->max_bus);
- printf(": rev %d.%d @ 0x%lx/0x%lx\n",
+ printf(": rev %d.%d @ 0x%x/0x%x\n",
rev_maj, rev_min >> 4, pcibios_entry_info.bei_base,
pcibios_entry_info.bei_size);
diff --git sys/kern/vfs_cluster.c sys/kern/vfs_cluster.c
index f9df40e..118a411 100644
--- sys/kern/vfs_cluster.c
+++ sys/kern/vfs_cluster.c
@@ -175,7 +175,7 @@ cluster_wbuild(struct vnode *vp, struct buf *last_bp, long
size,
#ifdef DIAGNOSTIC
if (size != vp->v_mount->mnt_stat.f_iosize)
- panic("cluster_wbuild: size %ld != filesize %ld",
+ panic("cluster_wbuild: size %ld != filesize %u",
size, vp->v_mount->mnt_stat.f_iosize);
#endif
redo:
diff --git sys/msdosfs/msdosfs_conv.c sys/msdosfs/msdosfs_conv.c
index 727acd2..538aa1c 100644
--- sys/msdosfs/msdosfs_conv.c
+++ sys/msdosfs/msdosfs_conv.c
@@ -209,7 +209,7 @@ dos2unixtime(u_int dd, u_int dt, u_int dh, struct timespec
*tsp)
*/
month = (dd & DD_MONTH_MASK) >> DD_MONTH_SHIFT;
if (month == 0) {
- printf("dos2unixtime(): month value out of range
(%ld)\n",
+ printf("dos2unixtime(): month value out of range
(%u)\n",
month);
month = 1;
}
diff --git sys/msdosfs/msdosfs_denode.c sys/msdosfs/msdosfs_denode.c
index d384a8b..29f2e57 100644
--- sys/msdosfs/msdosfs_denode.c
+++ sys/msdosfs/msdosfs_denode.c
@@ -387,7 +387,7 @@ detrunc(struct denode *dep, uint32_t length, int flags,
struct ucred *cred,
* directory's life.
*/
if ((DETOV(dep)->v_flag & VROOT) && !FAT32(pmp)) {
- printf("detrunc(): can't truncate root directory, clust %ld,
offset %ld\n",
+ printf("detrunc(): can't truncate root directory, clust %u,
offset %u\n",
dep->de_dirclust, dep->de_diroffset);
return (EINVAL);
}
diff --git sys/msdosfs/msdosfs_vnops.c sys/msdosfs/msdosfs_vnops.c
index 79f611c..25d7030 1006
Removing -Wno-format from kernel makefiles, 09/16
paddr_t is long
---
sys/arch/i386/i386/machdep.c |4 ++--
sys/arch/i386/pci/pci_addr_fixup.c |2 +-
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git sys/arch/i386/i386/machdep.c sys/arch/i386/i386/machdep.c
index 7924386..1a26e7d 100644
--- sys/arch/i386/i386/machdep.c
+++ sys/arch/i386/i386/machdep.c
@@ -2774,7 +2774,7 @@ dumpsys()
/* Print out how many MBs we have more to go. */
if (dbtob(blkno - dumplo) % (1024 * 1024) < NBPG)
- printf("%d ",
+ printf("%ld ",
(ptoa(dumpsize) - maddr) / (1024 * 1024));
#if 0
printf("(%x %lld) ", maddr, blkno);
@@ -3224,7 +3224,7 @@ init386(paddr_t first_avail)
if (extent_alloc_region(iomem_ex, a, e - a, EX_NOWAIT))
/* XXX What should we do? */
- printf("\nWARNING: CAN'T ALLOCATE RAM (%x-%x)"
+ printf("\nWARNING: CAN'T ALLOCATE RAM (%lx-%lx)"
" FROM IOMEM EXTENT MAP!\n", a, e);
physmem += atop(e - a);
diff --git sys/arch/i386/pci/pci_addr_fixup.c sys/arch/i386/pci/pci_addr_fixup.c
index e4eb1b4..59880eb 100644
--- sys/arch/i386/pci/pci_addr_fixup.c
+++ sys/arch/i386/pci/pci_addr_fixup.c
@@ -136,7 +136,7 @@ pci_addr_fixup(struct pcibios_softc *sc, pci_chipset_tag_t
pc, int maxbus)
start = PCIADDR_ISAMEM_RESERVE;
sc->mem_alloc_start = (start + 0x10 + 1) & ~(0x10 - 1);
sc->port_alloc_start = PCIADDR_ISAPORT_RESERVE;
- PCIBIOS_PRINTV((" Physical memory end: 0x%08x\n PCI memory mapped I/O "
+ PCIBIOS_PRINTV((" Physical memory end: 0x%08lx\n PCI memory mapped I/O "
"space start: 0x%08x\n", avail_end, sc->mem_alloc_start));
/*
--
1.7.6
Removing -Wno-format from kernel makefiles, 10/16
fix: pointer to long
---
sys/arch/i386/pci/pci_addr_fixup.c |2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git sys/arch/i386/pci/pci_addr_fixup.c sys/arch/i386/pci/pci_addr_fixup.c
index 59880eb..40f7918 100644
--- sys/arch/i386/pci/pci_addr_fixup.c
+++ sys/arch/i386/pci/pci_addr_fixup.c
@@ -354,7 +354,7 @@ pciaddr_do_resource_reserve_disabled(struct pcibios_softc
*sc,
(val & PCI_COMMAND_IO_ENABLE) == PCI_COMMAND_IO_ENABLE)
return (0);
- PCIBIOS_PRINTV(("disabled %s space at addr 0x%x size 0x%x\n",
+ PCIBIOS_PRINTV(("disabled %s space at addr 0x%lx size 0x%x\n",
type == PCI_MAPREG_TYPE_MEM ? "mem" : "io", *addr, size));
error = extent_alloc_region(ex, *addr, size, EX_NOWAIT | EX_MALLOCOK);
--
1.7.6
Removing -Wno-format from kernel makefiles, 07/16
fix: %x instead of %p for int
---
sys/dev/pci/musycc_obsd.c |2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git sys/dev/pci/musycc_obsd.c sys/dev/pci/musycc_obsd.c
index 25a58d8..0844136 100644
--- sys/dev/pci/musycc_obsd.c
+++ sys/dev/pci/musycc_obsd.c
@@ -215,7 +215,7 @@ musycc_ebus_attach(struct device *parent, struct
musycc_softc *esc,
#endif
if (ebus_attach_device(&rom, sc, 0, 0x400) != 0) {
- printf(": failed to map rom @ %05p\n", 0);
+ printf(": failed to map rom @ %05x\n", 0);
goto failed;
}
--
1.7.6
Re: Removing -Wno-format from kernel makefiles, 4/16
On Wed, 3 Jul 2013, Mark Kettenis wrote:
> > diff --git sys/arch/i386/i386/esm.c sys/arch/i386/i386/esm.c
> > index c90b2c4..3dff69e 100644
> > --- sys/arch/i386/i386/esm.c
> > +++ sys/arch/i386/i386/esm.c
> > @@ -880,7 +880,7 @@ esm_make_sensors(struct esm_softc *sc, struct
> > esm_devmap *devmap,
> > }
> >
> > for (j = 0; j < nsensors; j++) {
> > - snprintf(s[j].desc, sizeof(s[j].desc), "%s %d",
> > + snprintf(s[j].desc, sizeof(s[j].desc), "%s %ld",
> > sensor_map[i].name, sensor_map[i].arg + j);
> > }
> > break;
>
> Looking at this one, it makes more sense to make the "arg" member of
> "struct esm_sensor_map" an int. That will result in some space
> savings if we'd ever bring this driver to amd64.
--- sys/arch/i386/i386/esm.c
+++ sys/arch/i386/i386/esm.c
@@ -87,7 +87,7 @@ enum sensor_type esm_typemap[] = {
struct esm_sensor_map {
enum esm_sensor_typetype;
- longarg;
+ int arg;
const char *name;
};
Re: Removing -Wno-format from kernel makefiles, 3/16
On Wed, 3 Jul 2013, Mark Kettenis wrote:
> > diff --git sys/arch/i386/i386/db_interface.c
> > sys/arch/i386/i386/db_interface.c
> > index 85c1ff5..c75fd89 100644
> > --- sys/arch/i386/i386/db_interface.c
> > +++ sys/arch/i386/i386/db_interface.c
> > @@ -197,11 +197,11 @@ db_sysregs_cmd(db_expr_t addr, int have_addr,
> > db_expr_t count, char *modif)
> > uint16_t ldtr, tr;
> >
> > __asm__ __volatile__("sidt %0" : "=m" (idtr));
> > - db_printf("idtr: 0x%08x/%04x\n",
> > + db_printf("idtr: 0x%08x/%04llx\n",
> > (unsigned int)(idtr >> 16), idtr & 0x);
> >
> > __asm__ __volatile__("sgdt %0" : "=m" (gdtr));
> > - db_printf("gdtr: 0x%08x/%04x\n",
> > + db_printf("gdtr: 0x%08x/%04llx\n",
> > (unsigned int)(gdtr >> 16), gdtr & 0x);
>
> This is a tad bit inconsistent. I'd either use %llx for both values
> and get rid of the cast, or use %x and use a cast in both cases.
Like this?
--- sys/arch/i386/i386/db_interface.c
+++ sys/arch/i386/i386/db_interface.c
@@ -197,12 +197,10 @@ db_sysregs_cmd(db_expr_t addr, int have_addr, db_expr_t
count, char *modif)
uint16_t ldtr, tr;
__asm__ __volatile__("sidt %0" : "=m" (idtr));
- db_printf("idtr: 0x%08x/%04x\n",
- (unsigned int)(idtr >> 16), idtr & 0x);
+ db_printf("idtr: 0x%08llx/%04llx\n", idtr >> 16, idtr & 0x);
__asm__ __volatile__("sgdt %0" : "=m" (gdtr));
- db_printf("gdtr: 0x%08x/%04x\n",
- (unsigned int)(gdtr >> 16), gdtr & 0x);
+ db_printf("gdtr: 0x%08llx/%04llx\n", gdtr >> 16, gdtr & 0x);
__asm__ __volatile__("sldt %0" : "=g" (ldtr));
db_printf("ldtr: 0x%04x\n", ldtr);
Removing -Wno-format from kernel makefiles, 06/16
Use %z* for size_t
while there, fix a few %d into %u
---
sys/arch/i386/pci/pcibios.c |2 +-
sys/dev/ic/bwi.c|4 ++--
sys/dev/ic/pgt.c|6 +++---
sys/dev/ic/ti.c |4 ++--
sys/dev/pci/amdiic.c|4 ++--
sys/dev/pci/amdpm.c |2 +-
sys/dev/pci/if_iwi.c|4 ++--
sys/dev/pci/if_iwn.c| 10 +-
sys/dev/pci/if_lge.c|4 ++--
sys/dev/pci/if_nge.c|4 ++--
sys/dev/pci/if_tl.c |2 +-
sys/dev/pci/if_wb.c |2 +-
sys/dev/pci/if_wpi.c|4 ++--
sys/dev/pci/piixpm.c|2 +-
sys/dev/pci/yds.c |2 +-
sys/dev/usb/if_urndis.c | 12 ++--
sys/netinet/ip_output.c |2 +-
sys/netinet6/ip6_forward.c |2 +-
sys/netinet6/ip6_output.c |2 +-
sys/nfs/nfs_socket.c|4 ++--
20 files changed, 39 insertions(+), 39 deletions(-)
diff --git sys/arch/i386/pci/pcibios.c sys/arch/i386/pci/pcibios.c
index 1b34391..372ac51 100644
--- sys/arch/i386/pci/pcibios.c
+++ sys/arch/i386/pci/pcibios.c
@@ -260,7 +260,7 @@ pcibios_pir_init(struct pcibios_softc *sc)
cksum += p[i];
printf("%s: PCI IRQ Routing Table rev %d.%d @ 0x%lx/%d "
- "(%d entries)\n", sc->sc_dev.dv_xname,
+ "(%zd entries)\n", sc->sc_dev.dv_xname,
pirh->version >> 8, pirh->version & 0xff, pa,
pirh->tablesize, (pirh->tablesize - sizeof(*pirh)) / 16);
diff --git sys/dev/ic/bwi.c sys/dev/ic/bwi.c
index 6295172..934a6ac 100644
--- sys/dev/ic/bwi.c
+++ sys/dev/ic/bwi.c
@@ -1673,7 +1673,7 @@ bwi_fwimage_is_valid(struct bwi_softc *sc, uint8_t *fw,
size_t fw_len,
const struct bwi_fwhdr *hdr;
if (fw_len < sizeof(*hdr)) {
- printf("%s: invalid firmware (%s): invalid size %u\n",
+ printf("%s: invalid firmware (%s): invalid size %zu\n",
sc->sc_dev.dv_xname, fw_name, fw_len);
return (1);
}
@@ -1686,7 +1686,7 @@ bwi_fwimage_is_valid(struct bwi_softc *sc, uint8_t *fw,
size_t fw_len,
*/
if (betoh32(hdr->fw_size) != fw_len - sizeof(*hdr)) {
printf("%s: invalid firmware (%s): size mismatch, "
- "fw %u, real %u\n",
+ "fw %u, real %zu\n",
sc->sc_dev.dv_xname,
fw_name,
betoh32(hdr->fw_size),
diff --git sys/dev/ic/pgt.c sys/dev/ic/pgt.c
index 692b72c..6f6a5d6 100644
--- sys/dev/ic/pgt.c
+++ sys/dev/ic/pgt.c
@@ -3193,7 +3193,7 @@ pgt_dma_alloc_queue(struct pgt_softc *sc, enum pgt_queue
pq)
error = bus_dmamem_alloc(sc->sc_dmat, PGT_FRAG_SIZE, PAGE_SIZE,
0, &pd->pd_dmas, 1, &nsegs, BUS_DMA_WAITOK);
if (error != 0) {
- printf("%s: error alloc frag %u on queue %u\n",
+ printf("%s: error alloc frag %zu on queue %u\n",
sc->sc_dev.dv_xname, i, pq);
free(pd, M_DEVBUF);
break;
@@ -3202,7 +3202,7 @@ pgt_dma_alloc_queue(struct pgt_softc *sc, enum pgt_queue
pq)
error = bus_dmamem_map(sc->sc_dmat, &pd->pd_dmas, nsegs,
PGT_FRAG_SIZE, (caddr_t *)&pd->pd_mem, BUS_DMA_WAITOK);
if (error != 0) {
- printf("%s: error map frag %u on queue %u\n",
+ printf("%s: error map frag %zu on queue %u\n",
sc->sc_dev.dv_xname, i, pq);
free(pd, M_DEVBUF);
break;
@@ -3212,7 +3212,7 @@ pgt_dma_alloc_queue(struct pgt_softc *sc, enum pgt_queue
pq)
error = bus_dmamap_load(sc->sc_dmat, pd->pd_dmam,
pd->pd_mem, PGT_FRAG_SIZE, NULL, BUS_DMA_NOWAIT);
if (error != 0) {
- printf("%s: error load frag %u on queue %u\n",
+ printf("%s: error load frag %zu on queue %u\n",
sc->sc_dev.dv_xname, i, pq);
bus_dmamem_free(sc->sc_dmat, &pd->pd_dmas,
nsegs);
diff --git sys/dev/ic/ti.c sys/dev/ic/ti.c
index 85e3a2a..902591f 100644
--- sys/dev/ic/ti.c
+++ sys/dev/ic/ti.c
@@ -612,7 +612,7 @@ ti_alloc_jumbo_mem(struct ti_softc *sc)
state = 1;
if (bus_dmamem_map(sc->sc_dmatag, &seg, rseg, TI_JMEM, &kva,
BUS_DMA_NOWAIT)) {
- printf("%s: can't map dma buffers (%d bytes)\n",
+ printf("%s: can't map dma buffers (%zu bytes)\n",
sc->sc_dv.dv_xname, TI_JMEM);
error = ENOBUFS;
goto out;
@@ -1600,7 +1600,7 @@ ti_attach(struct ti_softc *sc)
}
if (bus_dmamem_map(sc->
Re: libc malloc poison
> > From: Theo de Raadt > > Date: Thu, 04 Jul 2013 09:04:54 -0600 > > > > I suspect the best approach would be a hybrid value. The upper half > > of the address should try to land in an unmapped zone, or into the zero > > page, or into some address space hole, ir into super high memory above > > the stack which is gauranteed unmapped. > > Don't forget strict alignment architectures, where it is beneficial > to have the lowest bit set to trigger alignment traps. That's why I vaguely mentioned the idea of a sysctl or MD defines, which would declare a fixed component, plus a mask on top of random. That fixed component need not just be high bits, it can also cover the lowest bit (or two) for instance.
Re: libc malloc poison
> From: Theo de Raadt > Date: Thu, 04 Jul 2013 09:04:54 -0600 > > I suspect the best approach would be a hybrid value. The upper half > of the address should try to land in an unmapped zone, or into the zero > page, or into some address space hole, ir into super high memory above > the stack which is gauranteed unmapped. Don't forget strict alignment architectures, where it is beneficial to have the lowest bit set to trigger alignment traps.
Re: libc malloc poison
> On Wed, Jul 03, 2013 at 17:21, Theo de Raadt wrote: > >> + int pval = 0xd0d0caca; > > > > Can you explain the choice of this? >> > I thought it sounded clever. Ok, because there's more to the picture. Inside the kernel, we tend to use 0xdeadbeef, or the DEADBEEF0/DEADBEEF1 values. Reason for the latter is that we can try to put this value into memory which the kernel does not manage. Basically on half of our kernel architectures if that is loaded into a pointer, it will hit unmanaged kernel memory, exposing the bug. Furthermore, the idea is that the 0xdeadbeef value should have a high mix of set bits versus clear bits, for when it lands in a flag variable. A lot of bits are set; some paper I read years ago discussed that on average "flag bits set" tends to traverse. Furthermore, each of the sub-fields tend to be odd, which in other use after cases regarding offsets/indexes can lead to more unaligned access, once again triggering and exposing a bug. Those are all theoretical ideas to try to expose the bugs as early as possible. I think the use of 0xd0d0caca in userland might not be the most suitable choice, especially in a MI fashion. > > There are arguments to make this MI; other arguments to make it MD; > > and other arguments to introduce a bit of randomness. > > > > I'd like to know which arguments you have > > Since libc doesn't do free list integrity checking, I'm currently > leaning towards a random value. (even with random, we could still > check that all words of a free chunk are the same.) > > Somebody also noticed that we don't have separate values for allocated > and freed memory. I suppose this makes debugging harder since you > can't obviously identify freed memory? I lean towards prioritizing > finding more bugs, which implies we need more variability, since any > one value may allow a program to work where a different value would > not. I agree with this last sentence. I suspect the best approach would be a hybrid value. The upper half of the address should try to land in an unmapped zone, or into the zero page, or into some address space hole, ir into super high memory above the stack which is gauranteed unmapped. The 64-bit machines require a bit more consideration as well; we want two 32-bit values to combine into a nice trashy address. Should we use a kernel sysctl to "recommend" the high word?, and a mask against random? Of course, that could also be done using MD includes.
old unused leftover
Hi,
old has been old and unused for some time already.
- Artturi
Index: kern_timeout.c
===
RCS file: /cvs/src/sys/kern/kern_timeout.c,v
retrieving revision 1.35
diff -u -p -u -p -r1.35 kern_timeout.c
--- kern_timeout.c2 Jun 2012 00:11:16 -1.35
+++ kern_timeout.c3 Jul 2013 22:17:17 -
@@ -350,7 +350,7 @@ timeout_adjust_ticks(int adj)
{
struct timeout *to;
struct circq *p;
-int new_ticks, b, old;
+int new_ticks, b;
/* adjusting the monotonic clock backwards would be a Bad Thing */
if (adj <= 0)
@@ -363,8 +363,6 @@ timeout_adjust_ticks(int adj)
while (p != &timeout_wheel[b]) {
to = (struct timeout *)p; /* XXX */
p = CIRCQ_FIRST(p);
-
-old = to->to_time;
/* when moving a timeout forward need to reinsert it */
if (to->to_time - ticks < adj)
Re: smtpd: if?
On Thu, Jul 04, 2013 at 08:48:16AM +0200, Maxime Villard wrote: > Hi, > > - - - src/usr.sbin/smtpd/smtpd.c l.1326 > > if (! bsnprintf(key, sizeof key, > "profiling.imsg.%s.%s.%s", > proc_name(smtpd_process), > proc_name(p->proc), > imsg_to_str(imsg->hdr.type)));<-- ';' > stat_set(key, stat_timespec(&dt)); > > > What's the goal of the 'if' right there? > I just commited a fix, it was reported by someone else in private too ;) And for the record: No need to worry much about it: it is a bug in profiling code which gets bypassed unless you start smtpd in profiling mode. If you did, you would still not hit it because the values we currently have do not exceed that key buffer. And if they did, it wouldn't really be an issue anyway, this is a key intended to carry informative value to the developer, it is not clear if it's preferable to store a truncated key with its value, or not to store any value at all :-/ -- Gilles Chehade https://www.poolp.org @poolpOrg
