mpsafe malloc(9)

[David Gwynne]

the following adds a mutex to malloc and free to protect their
internal state. this should be enough to make the api mpsafe,
assuming the way they interact with uvm is mpsafe.

this only uses a single mutex around the entire malloc subsystem,
but shuffles the code slightly to avoid holding it around calls
into uvm. the shuffling is mostly to account for an allocation (ie,
bump memuse early) before releasing the mutex to try and allocate
it from uvm. if uvm fails, it rolls this back.

this is modelled on how the item accounting and locking is done in
pools.

can anyone comment on the uvm safety?

also, it is obvious that the malloc debug code has started to rot
a bit. ive fixed it up a bit, but now there's also a possible mp
race when items are moved from the used list to the free list, and
when the item is unmapped between that. id like to remove the malloc
debug code and improve the robustness of malloc itself. pools have
shows they can be fast and strict at the same time.

Index: kern_malloc.c
===
RCS file: /cvs/src/sys/kern/kern_malloc.c,v
retrieving revision 1.129
diff -u -p -r1.129 kern_malloc.c
--- kern_malloc.c   7 Jun 2017 13:30:36 -   1.129
+++ kern_malloc.c   3 Jul 2017 10:14:49 -
@@ -39,6 +39,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 
 #include 
@@ -94,6 +95,7 @@ u_int nkmempages_min = 0;
 #endif
 u_int  nkmempages_max = 0;
 
+struct mutex malloc_mtx = MUTEX_INITIALIZER(IPL_VM);
 struct kmembuckets bucket[MINBUCKET + 16];
 #ifdef KMEMSTATS
 struct kmemstats kmemstats[M_LAST];
@@ -151,8 +153,8 @@ malloc(size_t size, int type, int flags)
struct kmemusage *kup;
struct kmem_freelist *freep;
long indx, npg, allocsize;
-   int s;
caddr_t va, cp;
+   int s;
 #ifdef DIAGNOSTIC
int freshalloc;
char *savedtype;
@@ -164,9 +166,6 @@ malloc(size_t size, int type, int flags)
panic("malloc: bogus type %d", type);
 #endif
 
-   if (!cold)
-   KERNEL_ASSERT_LOCKED();
-
KASSERT(flags & (M_WAITOK | M_NOWAIT));
 
if ((flags & M_NOWAIT) == 0) {
@@ -176,10 +175,6 @@ malloc(size_t size, int type, int flags)
if (pool_debug == 2)
yield();
 #endif
-   if (!cold && pool_debug) {
-   KERNEL_UNLOCK();
-   KERNEL_LOCK();
-   }
}
 
 #ifdef MALLOC_DEBUG
@@ -193,6 +188,7 @@ malloc(size_t size, int type, int flags)
if (size > 65535 * PAGE_SIZE) {
if (flags & M_CANFAIL) {
 #ifndef SMALL_KERNEL
+   /* XXX lock */
if (ratecheck(_lasterr, _errintvl))
printf("malloc(): allocation too large, "
"type = %d, size = %lu\n", type, size);
@@ -204,33 +200,38 @@ malloc(size_t size, int type, int flags)
}
 
indx = BUCKETINDX(size);
+   if (size > MAXALLOCSAVE)
+   allocsize = round_page(size);
+   else
+   allocsize = 1 << indx;
kbp = [indx];
-   s = splvm();
+   mtx_enter(_mtx);
 #ifdef KMEMSTATS
while (ksp->ks_memuse >= ksp->ks_limit) {
if (flags & M_NOWAIT) {
-   splx(s);
+   mtx_leave(_mtx);
return (NULL);
}
if (ksp->ks_limblocks < 65535)
ksp->ks_limblocks++;
-   tsleep(ksp, PSWP+2, memname[type], 0);
+   msleep(ksp, _mtx, PSWP+2, memname[type], 0);
}
+   ksp->ks_memuse += allocsize; /* account for this early */
ksp->ks_size |= 1 << indx;
 #endif
-   if (size > MAXALLOCSAVE)
-   allocsize = round_page(size);
-   else
-   allocsize = 1 << indx;
if (XSIMPLEQ_FIRST(>kb_freelist) == NULL) {
+   mtx_leave(_mtx);
npg = atop(round_page(allocsize));
+   s = splvm();
va = (caddr_t)uvm_km_kmemalloc_pla(kmem_map, NULL,
(vsize_t)ptoa(npg), 0,
((flags & M_NOWAIT) ? UVM_KMF_NOWAIT : 0) |
((flags & M_CANFAIL) ? UVM_KMF_CANFAIL : 0),
no_constraint.ucr_low, no_constraint.ucr_high,
0, 0, 0);
+   splx(s);
if (va == NULL) {
+   long wake;
/*
 * Kmem_malloc() can return NULL, even if it can
 * wait, if there is no map space available, because
@@ -241,9 +242,18 @@ malloc(size_t size, int type, int flags)
 */
if ((flags & (M_NOWAIT|M_CANFAIL)) == 0)
panic("malloc: out of space in kmem_map");
-   splx(s);
+
+   mtx_enter(_mtx);
+ 

Re: ktrace: Following symlinks

[Klemens Nanni]

On Thu, Jun 29, 2017 at 11:33:36PM -0700, Philip Guenther wrote:

On Thu, Jun 29, 2017 at 10:42 PM, Klemens Nanni  wrote:

On Thu, Jun 29, 2017 at 09:50:25PM -0700, Philip Guenther wrote:


On Thu, Jun 22, 2017 at 7:17 PM, Klemens Nanni  wrote:


So I just wrapped my head around vfs(9) with regard to making ktrace
following symlinks again, however I don't quite understand what problems
may occur when doing so. May anyone enlighten me on this?



IMHO, it makes more sense to add fktrace(2) aka NetBSD where an open
fd is passed in.


To have a more generic interface?


Yes.  ktrace -f - some_command | kdump

kdump expects ktrace.out here and complains.


Hmm, I wonder what happens if the fd involved is a pipe to the process
being traced and if that will deadlock the kernel.

Uh, hmm, could that happen as well with your suggestion to support
FIFO if the traced process is the only reader, ala:
  mkfifio kt
  ktrace -f kt kdump -f /dev/stdin < kt

That one's interesting: kdump won't see data from kt on stdin since
you're redirecting into ktrace. Here's an equivalent inocation that's
more obvious:

$ < kt ktrace -f kt -- kdump -f /dev/stdin
^Cksh: cannot open kt: Interrupted system call

Neither ktrace nor kdump will actually be executed since the shell
already chokes on it. You can check for that while it's still running
(doing nothing):

$ pgrep -fl k[td]
$

Here's some more fun, trace yourself endlessly while eating CPU:

$ ktrace -f - kdump -f -

58275 kle 54 0 648K 684K onproc/3 - 0:20 56.54% kdump -f -

Or let ktrace wait forever (without -a the FIFO gets replaced and you'll
see dumps just like above):

$ mkfifo kt
$ ktrace -af kt -- kdump -f kt

95073 root 2 0 128K 160K idle fifow 0:00 0.00% ktrace -af kt -- kdump 
-f kt


?  It's okay if that just blocks, but it's not okay if it blocks
processes that aren't being traced or if it eats the CPU.

Playing around with it hasn't blocked other programs or crashed the
kernel so far but I'll further look into this.


(VREG vnodes are exactly what the kernel can write to without having
to worry about looping internally or userspace blocking it for
arbitrary lengths of time.  Well, at least if we ignore FUSE, which is
basically ignored for this sort of discussion anyway, being a security
nightmare.  Anyone tried to ktrace a fuse-serving process, directing
the ktrace to the fuse'd filesystem?  Same question applies to acct()
to a fuse'd filesystem, but at least that's root-only.)



Why not letting ktrace(2) handle this just like it already does for regular 
files?


*If* if it's safe (see above for an *example* consideration), then fd
/ struct file base access is much more general than filename / vnode
based access.

ktrace(1) *always* open()s the target filename, so would arguably
remove a TOCTOU.

Re: armv7 small bootstrap improvement/simplification

[Artturi Alm]

On Wed, Jul 05, 2017 at 02:27:46AM +0300, Artturi Alm wrote:
> Hi,
> 
> instead of messing w/bs_tags, use the fact pmap_kernel()->pm_refs is going
> to be 0 until pmap_bootstrap() has ran. tmp_bs_tag was unused, and
> bootstrap_bs_map doesn't need/use the void *t-arg when being ran indirectly
> via armv7_bs_map().
> 
> the whole existence of bootstrap_bs_map is another story, and the comment in
> /* Now, map the FDT area. */ is somewhat an stupid excuse, it's already mapped
> before initarm() w/VA=PA, and could well be _init()&_get_size()'d & memcpy'ed
> somewhere in reach within bootstrap KVA, guess diff might follow for that,
> too, if anyone has time for these simplifications.
> 

meh, i forgot where i was going with the above, the rant was supposed to let
ppl know we have this thing known as uvm_page.c, which doesn't just load the
physmem in, meaning, we could very well get the info out of FDT _early_ and
uvm_page_physload() it before all the memory 'stealing' address messing
does begin, and utilize uvm_page_physget() fwiw., initarm() doesn't need to
look and feel like netbsd of the past decade, imo.

-Artturi

> the diff got tested/booted on cubie2, and i don't see how the result could
> differ on any other SoC.
> -Artturi
> 

armv7 small bootstrap improvement/simplification

[Artturi Alm]

Hi,

instead of messing w/bs_tags, use the fact pmap_kernel()->pm_refs is going
to be 0 until pmap_bootstrap() has ran. tmp_bs_tag was unused, and
bootstrap_bs_map doesn't need/use the void *t-arg when being ran indirectly
via armv7_bs_map().

the whole existence of bootstrap_bs_map is another story, and the comment in
/* Now, map the FDT area. */ is somewhat an stupid excuse, it's already mapped
before initarm() w/VA=PA, and could well be _init()&_get_size()'d & memcpy'ed
somewhere in reach within bootstrap KVA, guess diff might follow for that,
too, if anyone has time for these simplifications.

the diff got tested/booted on cubie2, and i don't see how the result could
differ on any other SoC.
-Artturi



diff --git a/sys/arch/arm/armv7/armv7_space.c b/sys/arch/arm/armv7/armv7_space.c
index c7e9b686b8f..08aa551e014 100644
--- a/sys/arch/arm/armv7/armv7_space.c
+++ b/sys/arch/arm/armv7/armv7_space.c
@@ -170,7 +170,12 @@ armv7_bs_map(void *t, uint64_t bpa, bus_size_t size,
 {
u_long startpa, endpa, pa;
vaddr_t va;
-   int pmap_flags = PMAP_DEVICE;
+   int pmap_flags;
+   extern int bootstrap_bs_map(uint64_t, bus_size_t, int,
+   bus_space_handle_t *);
+
+   if (pmap_kernel()->pm_refs == 0)
+   return bootstrap_bs_map(bpa, size, flags, bshp);
 
startpa = trunc_page(bpa);
endpa = round_page(bpa + size);
@@ -183,8 +188,7 @@ armv7_bs_map(void *t, uint64_t bpa, bus_size_t size,
 
*bshp = (bus_space_handle_t)(va + (bpa - startpa));
 
-   if (flags & BUS_SPACE_MAP_CACHEABLE)
-   pmap_flags = 0;
+   pmap_flags = (flags & BUS_SPACE_MAP_CACHEABLE) ? 0 : PMAP_DEVICE;
 
for (pa = startpa; pa < endpa; pa += PAGE_SIZE, va += PAGE_SIZE)
pmap_kenter_pa(va, pa | pmap_flags, PROT_READ | PROT_WRITE);
diff --git a/sys/arch/armv7/armv7/armv7_machdep.c 
b/sys/arch/armv7/armv7/armv7_machdep.c
index aa1c549b29b..f33f7dff818 100644
--- a/sys/arch/armv7/armv7/armv7_machdep.c
+++ b/sys/arch/armv7/armv7/armv7_machdep.c
@@ -199,8 +199,7 @@ int   safepri = 0;
 /* Prototypes */
 
 char   bootargs[MAX_BOOT_STRING];
-intbootstrap_bs_map(void *, uint64_t, bus_size_t, int,
-bus_space_handle_t *);
+intbootstrap_bs_map(uint64_t, bus_size_t, int, bus_space_handle_t *);
 void   process_kernel_args(char *);
 void   consinit(void);
 
@@ -311,8 +310,8 @@ read_ttb(void)
 static vaddr_t section_free = 0xfd00; /* XXX - huh */
 
 int
-bootstrap_bs_map(void *t, uint64_t bpa, bus_size_t size,
-int flags, bus_space_handle_t *bshp)
+bootstrap_bs_map(uint64_t bpa, bus_size_t size, int flags,
+bus_space_handle_t *bshp)
 {
u_long startpa, pa, endpa;
vaddr_t va;
@@ -384,11 +383,6 @@ initarm(void *arg0, void *arg1, void *arg2, paddr_t 
loadaddr)
void *node;
extern uint32_t esym; /* &_end if no symbols are loaded */
 
-   /* early bus_space_map support */
-   struct bus_space tmp_bs_tag;
-   int (*map_func_save)(void *, uint64_t, bus_size_t, int,
-   bus_space_handle_t *);
-
if (arg0)
esym = (uint32_t)arg0;
 
@@ -407,19 +401,6 @@ initarm(void *arg0, void *arg1, void *arg2, paddr_t 
loadaddr)
panic("cpu not recognized!");
 
/*
-* Temporarily replace bus_space_map() functions so that
-* console devices can get mapped.
-*
-* Note that this relies upon the fact that both regular
-* and a4x bus_space tags use the same map function.
-*/
-   tmp_bs_tag = armv7_bs_tag;
-   map_func_save = armv7_bs_tag.bs_map;
-   armv7_bs_tag.bs_map = bootstrap_bs_map;
-   armv7_a4x_bs_tag.bs_map = bootstrap_bs_map;
-   tmp_bs_tag.bs_map = bootstrap_bs_map;
-
-   /*
 * Now, map the FDT area.
 *
 * As we don't know the size of a possible FDT, map the size of a
@@ -429,7 +410,7 @@ initarm(void *arg0, void *arg1, void *arg2, paddr_t 
loadaddr)
 * XXX: There's (currently) no way to unmap a bootstrap mapping, so
 * we might lose a bit of the bootstrap address space.
 */
-   bootstrap_bs_map(NULL, (bus_addr_t)arg2, L1_S_SIZE, 0,
+   bootstrap_bs_map((bus_addr_t)arg2, L1_S_SIZE, 0,
(bus_space_handle_t *));
 
if (!fdt_init(config) || fdt_get_size(config) == 0)
@@ -772,12 +753,6 @@ initarm(void *arg0, void *arg1, void *arg2, paddr_t 
loadaddr)
 
vector_page_setprot(PROT_READ | PROT_EXEC);
 
-   /*
-* Restore proper bus_space operation, now that pmap is initialized.
-*/
-   armv7_bs_tag.bs_map = map_func_save;
-   armv7_a4x_bs_tag.bs_map = map_func_save;
-
 #ifdef DDB
db_machine_init();
 

Re: patch: fix inteldrm for Intel P4000 graphics

[Mark Kettenis]

> Date: Tue, 4 Jul 2017 17:24:27 -0400
> From: "Joe Gidi" 
> 
> I have a machine with a Xeon E3-1225v2 CPU, which includes integrated
> Intel P4000 graphics. This required a patch back in 2015 to avoid matching
> on the mythical "Intel Quanta Transcode" device, which kettenis@ committed
> here:
> 
> http://marc.info/?l=openbsd-cvs=144342287923444=2
> 
> The recent update to inteldrm broke X on this system, because this patch
> got lost.
> 
> The patch below has been tested and restores inteldrm functionality with
> P4000 graphics.

Thanks.  Fixed this in a different way such that it hopefully doesn't
happen again.  Can you let me know if the problem persists?

> Index: i915_drv.c
> ===
> RCS file: /cvs/src/sys/dev/pci/drm/i915/i915_drv.c,v
> retrieving revision 1.103
> diff -u -p -u -r1.103 i915_drv.c
> --- i915_drv.c1 Jul 2017 16:14:10 -   1.103
> +++ i915_drv.c4 Jul 2017 21:13:48 -
> @@ -424,7 +424,6 @@ static const struct intel_device_info in
>   INTEL_IRONLAKE_M_IDS(_ironlake_m_info),   \
>   INTEL_SNB_D_IDS(_sandybridge_d_info), \
>   INTEL_SNB_M_IDS(_sandybridge_m_info), \
> - INTEL_IVB_Q_IDS(_ivybridge_q_info), /* must be first IVB */ \
>   INTEL_IVB_M_IDS(_ivybridge_m_info),   \
>   INTEL_IVB_D_IDS(_ivybridge_d_info),   \
>   INTEL_HSW_D_IDS(_haswell_d_info), \
> 
> 
> 
> 

Re: armv7 alignment faults

[Artturi Alm]

On Tue, Jul 04, 2017 at 05:14:00AM +0200, Jeremie Courreges-Anglas wrote:
> Artturi Alm  writes:
> 
> > Hi,
> >
> > i think i've noted about this before, around 13months ago freebsd
> > first disabled alignment faults, and they haven't enabled them since.
> > deja vu, or not, i don't recall if the last diff like below did go
> > anywhere, nor if it got discussed about, so i'm sorry in advance,
> > if i'm banging my head to the wall with this one.
> 
> AFAIK we're rather headed towards strict alignement rules whenever
> possible.
> 

Yep, kind of presumed so, and should of have studied the issue further,
to find out that it was just the git, which i needed most at the time.

> > and indeed, even git is unusable without this (on latest snapshot w/git
> > installed from packages), 'git branch' did happen to be the only command
> > of the few i tried,
> > that didn't end up into SIGBUS, but even it fails reliably w/-v..
> 
> They messed up their new sha1 implementation.  Should be fixed with
> git-2.13.1p0, git-2.13.2 works fine here.
> 

Thanks for the info, diff withdrawn, and it's sysctl-knob-diff version rm'd.

> -- 
> jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF  DDCC 0DFA 74AE 1524 E7EE


I understand why this alignment correctness is wanted, and appreciate it's
performance winnings, exposure to bugs etc., i just got fooled by having not
been user of armv7 for around an year or so, and git was the first and only
package i had installed when i sent out the diff while being under false
impression of current. sheer bad luck w/regards timing to hit the bug in git.

-Artturi

Re: ping: Style fixes/cleanups

[Klemens Nanni]

Purely cosmetic/style(9) fixes: Remove unneeded indent in prototypes,
add space after return keyword.

Feedback/OK?

Index: ping.c
===
RCS file: /cvs/src/sbin/ping/ping.c,v
retrieving revision 1.220
diff -u -p -r1.220 ping.c
--- ping.c  4 Jul 2017 15:55:22 -   1.220
+++ ping.c  4 Jul 2017 21:43:12 -
@@ -208,34 +208,34 @@ volatile sig_atomic_t seenalrm;
volatile sig_atomic_t seenint;
volatile sig_atomic_t seeninfo;

-voidfill(char *, char *);
-voidsummary(void);
-voidonsignal(int);
-voidretransmit(int);
-int pinger(int);
-const char *pr_addr(struct sockaddr *, socklen_t);
-voidpr_pack(u_char *, int, struct msghdr *);
-__dead void usage(void);
+voidfill(char *, char *);
+voidsummary(void);
+voidonsignal(int);
+voidretransmit(int);
+int pinger(int);
+const char *pr_addr(struct sockaddr *, socklen_t);
+voidpr_pack(u_char *, int, struct msghdr *);
+__dead void usage(void);

/* IPv4 specific functions */
-voidpr_ipopt(int, u_char *);
-int in_cksum(u_short *, int);
-voidpr_icmph(struct icmp *);
-voidpr_retip(struct ip *);
-voidpr_iph(struct ip *);
+voidpr_ipopt(int, u_char *);
+int in_cksum(u_short *, int);
+voidpr_icmph(struct icmp *);
+voidpr_retip(struct ip *);
+voidpr_iph(struct ip *);
#ifndef SMALL
-int map_tos(char *, int *);
+int map_tos(char *, int *);
#endif  /* SMALL */

/* IPv6 specific functions */
-int get_hoplim(struct msghdr *);
-int get_pathmtu(struct msghdr *, struct sockaddr_in6 *);
-voidpr_icmph6(struct icmp6_hdr *, u_char *);
-voidpr_iph6(struct ip6_hdr *);
-voidpr_exthdrs(struct msghdr *);
-voidpr_ip6opt(void *);
-voidpr_rthdr(void *);
-voidpr_retip6(struct ip6_hdr *, u_char *);
+int get_hoplim(struct msghdr *);
+int get_pathmtu(struct msghdr *, struct sockaddr_in6 *);
+voidpr_icmph6(struct icmp6_hdr *, u_char *);
+voidpr_iph6(struct ip6_hdr *);
+voidpr_exthdrs(struct msghdr *);
+voidpr_ip6opt(void *);
+voidpr_rthdr(void *);
+voidpr_retip6(struct ip6_hdr *, u_char *);

int
main(int argc, char *argv[])
@@ -572,7 +572,7 @@ main(int argc, char *argv[])
timing = 1;

if (v6flag) {
-   /* in F_VERBOSE case, we may get non-echoreply packets*/
+   /* in F_VERBOSE case, we may get non-echoreply packets */
if (options & F_VERBOSE && datalen < 2048) /* XXX 2048? */
packlen = 2048 + IP6LEN + ECHOLEN + EXTRA;
else
@@ -965,7 +965,7 @@ pr_addr(struct sockaddr *addr, socklen_t
if (getnameinfo(addr, addrlen, buf, sizeof(buf), NULL, 0, flag) == 0)
return (buf);
else
-   return "?";
+   return ("?");
}

/*
@@ -1023,7 +1023,7 @@ pinger(int s)
u_int16_t seq;

if (npackets && ntransmitted >= npackets)
-   return(-1); /* no more transmission */
+   return (-1);/* no more transmission */

seq = htons(ntransmitted++);

@@ -1459,7 +1459,7 @@ in_cksum(u_short *addr, int len)
sum = (sum >> 16) + (sum & 0x);   /* add hi 16 to low 16 */
sum += (sum >> 16);   /* add carry */
answer = ~sum;  /* truncate to 16 bits */
-   return(answer);
+   return (answer);
}

/*
@@ -1849,15 +1849,15 @@ get_hoplim(struct msghdr *mhdr)
for (cm = (struct cmsghdr *)CMSG_FIRSTHDR(mhdr); cm;
 cm = (struct cmsghdr *)CMSG_NXTHDR(mhdr, cm)) {
if (cm->cmsg_len == 0)
-   return(-1);
+   return (-1);

if (cm->cmsg_level == IPPROTO_IPV6 &&
cm->cmsg_type == IPV6_HOPLIMIT &&
cm->cmsg_len == CMSG_LEN(sizeof(int)))
-   return(*(int *)CMSG_DATA(cm));
+   return (*(int *)CMSG_DATA(cm));
}

-   return(-1);
+   return (-1);
}

int
@@ -1869,7 +1869,7 @@ get_pathmtu(struct msghdr *mhdr, struct 
	for (cm = (struct cmsghdr *)CMSG_FIRSTHDR(mhdr); cm;

cm = (struct cmsghdr *)CMSG_NXTHDR(mhdr, cm)) {
if (cm->cmsg_len == 0)
-   return(0);
+   return (0);

if (cm->cmsg_level == IPPROTO_IPV6 &&
cm->cmsg_type == IPV6_PATHMTU &&
@@ -1897,7 +1897,7 @@ 

Re: ping: Style fixes/cleanups

[Klemens Nanni]

On Tue, Jul 04, 2017 at 03:58:13PM +, Florian Obser wrote:

I like most (all? of the correct ones), can you have another go
at trying to fix the pointed out changes in behaviour?


Thanks natano for pointing out these stupid mistakes.

Unify option checking, check for F_RROUTE only if F_HDRINCL is not set
already (they're incompatible and checked together already beforehand).

Feedback/OK?

Index: ping.c
===
RCS file: /cvs/src/sbin/ping/ping.c,v
retrieving revision 1.220
diff -u -p -r1.220 ping.c
--- ping.c  4 Jul 2017 15:55:22 -   1.220
+++ ping.c  4 Jul 2017 21:18:36 -
@@ -562,10 +562,10 @@ main(int argc, char *argv[])
(void)setsockopt(s, SOL_SOCKET, SO_DEBUG, ,
sizeof(optval));

-   if ((options & F_FLOOD) && (options & F_INTERVAL))
+   if (options & F_FLOOD && options & F_INTERVAL)
errx(1, "-f and -i options are incompatible");

-   if ((options & F_FLOOD) && (options & (F_AUD_RECV | F_AUD_MISS)))
+   if (options & F_FLOOD && options & (F_AUD_RECV | F_AUD_MISS))
warnx("No audible output for flood pings");

if (datalen >= sizeof(struct payload))   /* can we time transfer */
@@ -621,7 +621,7 @@ main(int argc, char *argv[])
 * let the kernel pass extension headers of incoming packets,
 * for privileged socket options
 */
-   if ((options & F_VERBOSE) != 0) {
+   if (options & F_VERBOSE) {
int opton = 1;

if (setsockopt(s, IPPROTO_IPV6, IPV6_RECVHOPOPTS,
@@ -717,10 +717,8 @@ main(int argc, char *argv[])
else
ip->ip_src.s_addr = INADDR_ANY;
ip->ip_dst = dst4.sin_addr;
-   }
-
/* record route option */
-   if (options & F_RROUTE) {
+   } else if (options & F_RROUTE) {
if (IN_MULTICAST(ntohl(dst4.sin_addr.s_addr)))
errx(1, "record route not valid to multicast"
" destinations");
@@ -771,7 +769,7 @@ main(int argc, char *argv[])
(void)signal(SIGINT, onsignal);
(void)signal(SIGINFO, onsignal);

-   if ((options & F_FLOOD) == 0) {
+   if (!(options & F_FLOOD)) {
(void)signal(SIGALRM, onsignal);
itimer.it_interval = interval;
itimer.it_value = interval;
@@ -805,7 +803,7 @@ main(int argc, char *argv[])
if (ntransmitted - nreceived - 1 > nmissedmax) {
nmissedmax = ntransmitted - nreceived - 1;
if (!(options & F_FLOOD) &&
-   (options & F_AUD_MISS))
+   options & F_AUD_MISS)
(void)fputc('\a', stderr);
}
continue;
@@ -859,7 +857,7 @@ main(int argc, char *argv[])
 * a path MTU notification.)
 */
if ((mtu = get_pathmtu(, )) > 0) {
-   if ((options & F_VERBOSE) != 0) {
+   if (options & F_VERBOSE) {
printf("new path MTU (%d) is "
"notified\n", mtu);
}
@@ -959,7 +957,7 @@ pr_addr(struct sockaddr *addr, socklen_t
static char buf[NI_MAXHOST];
int flag = 0;

-   if ((options & F_HOSTNAME) == 0)
+   if (!(options & F_HOSTNAME))
flag |= NI_NUMERICHOST;

if (getnameinfo(addr, addrlen, buf, sizeof(buf), NULL, 0, flag) == 0)
@@ -1890,7 +1888,7 @@ get_pathmtu(struct msghdr *mhdr, struct 
			dst->sin6_scope_id &&

mtuctl->ip6m_addr.sin6_scope_id !=
dst->sin6_scope_id)) {
-   if ((options & F_VERBOSE) != 0) {
+   if (options & F_VERBOSE) {
printf("path MTU for %s is notified. "
"(ignored)\n",
pr_addr((struct sockaddr *)

patch: fix inteldrm for Intel P4000 graphics

[Joe Gidi]

I have a machine with a Xeon E3-1225v2 CPU, which includes integrated
Intel P4000 graphics. This required a patch back in 2015 to avoid matching
on the mythical "Intel Quanta Transcode" device, which kettenis@ committed
here:

http://marc.info/?l=openbsd-cvs=144342287923444=2

The recent update to inteldrm broke X on this system, because this patch
got lost.

The patch below has been tested and restores inteldrm functionality with
P4000 graphics.

Thanks,

-- 

Joe Gidi
j...@entropicblur.com



Index: i915_drv.c
===
RCS file: /cvs/src/sys/dev/pci/drm/i915/i915_drv.c,v
retrieving revision 1.103
diff -u -p -u -r1.103 i915_drv.c
--- i915_drv.c  1 Jul 2017 16:14:10 -   1.103
+++ i915_drv.c  4 Jul 2017 21:13:48 -
@@ -424,7 +424,6 @@ static const struct intel_device_info in
INTEL_IRONLAKE_M_IDS(_ironlake_m_info),   \
INTEL_SNB_D_IDS(_sandybridge_d_info), \
INTEL_SNB_M_IDS(_sandybridge_m_info), \
-   INTEL_IVB_Q_IDS(_ivybridge_q_info), /* must be first IVB */ \
INTEL_IVB_M_IDS(_ivybridge_m_info),   \
INTEL_IVB_D_IDS(_ivybridge_d_info),   \
INTEL_HSW_D_IDS(_haswell_d_info), \

Re: ifstated diff rename variables to avoid state confusion

[Sebastian Benoit]

Rob Pierce(r...@2keys.ca) on 2017.07.04 07:34:27 -0400:
> On Mon, Jul 03, 2017 at 04:24:30PM -0400, Rob Pierce wrote:
> > ifstated monitors interface state and the return state of invoked commands,
> > and takes action accordingly, all of which is managed with the help of a
> > finite state machine. That makes for a lot of "state" references in the 
> > code.
> > 
> > The following diff renames variables to make a distinction between link 
> > status
> > and command execution status (now referenced as "status"), and the various 
> > states
> > within the state machine (referred to as "state").
> > 
> > As an example of the current confusion, the link_states() function in 
> > parse.y
> > actually links states within the finite state machine (as oppose to having
> > anything to do with interface link states).
> > 
> > This diff helps to make my head hurt less.
> > 
> > For your consideration.
> > 
> > Regards,
> > 
> > Rob
> 
> Ok, that was overkill. I would be happy with the following diff instead.

I agree, commited this one.

Thanks.

 
> Rob
> 
> Index: ifstated.c
> ===
> RCS file: /cvs/src/usr.sbin/ifstated/ifstated.c,v
> retrieving revision 1.49
> diff -u -p -r1.49 ifstated.c
> --- ifstated.c3 Jul 2017 18:45:34 -   1.49
> +++ ifstated.c4 Jul 2017 11:31:08 -
> @@ -64,7 +64,7 @@ voidcheck_external_status(struct ifsd_
>  void external_evtimer_setup(struct ifsd_state *, int);
>  void scan_ifstate(int, int, int);
>  int  scan_ifstate_single(int, int, struct ifsd_state *);
> -void fetch_state(void);
> +void fetch_ifstate(void);
>  __dead void  usage(void);
>  void adjust_expressions(struct ifsd_expression_list *, int);
>  void adjust_external_expressions(struct ifsd_state *);
> @@ -208,7 +208,7 @@ load_config(void)
>   clear_config(conf);
>   conf = newconf;
>   conf->initstate.entered = time(NULL);
> - fetch_state();
> + fetch_ifstate();
>   external_evtimer_setup(>initstate, IFSD_EVTIMER_ADD);
>   adjust_external_expressions(>initstate);
>   eval_state(>initstate);
> @@ -597,7 +597,7 @@ do_action(struct ifsd_action *action)
>   * Fetch the current link states.
>   */
>  void
> -fetch_state(void)
> +fetch_ifstate(void)
>  {
>   struct ifaddrs *ifap, *ifa;
>   char *oname = NULL;
> 

Re: ospfd: add IMSG_IFADDRADD to deal with "sh /etc/netstart if"

[Remi Locherer]

On Sun, Jun 25, 2017 at 11:47:09PM +0200, Remi Locherer wrote:
> Hi,
> 
> ospfd does not react nicely when running "sh /etc/netstart if".
> 
> This is because adding the same address again do an interface results
> in RTM_DELADDR and RTM_NEWADDR. ospfd handles the former but the later.
> If this happens ospfd says "interface vether0:192.168.250.1 gone".
> Adjacencies on that interface are down and ospfd can not recover.
> 
> The below patch adds IMSG_IFADDRADD to deal with that. With it ospfd
> logs the following after "ifconfig vether0 192.168.250.1/24" (same address
> as active before):
> 
> orig_rtr_lsa: area 0.0.0.0
> orig_rtr_lsa: stub net, interface iwm0
> orig_rtr_lsa: stub net, interface vether0
> orig_rtr_lsa: transit net, interface pair0
> if_fsm: event DOWN resulted in action RESET and changing state for interface 
> vether0 from DR to DOWN
> interface vether0:192.168.250.1 gone
> orig_rtr_lsa: area 0.0.0.0
> orig_rtr_lsa: stub net, interface iwm0
> orig_rtr_lsa: stub net, interface vether0
> orig_rtr_lsa: transit net, interface pair0
> if_fsm: event UP resulted in action START and changing state for interface 
> vether0 from DOWN to WAIT
> interface vether0:192.168.250.1 returned
> 
> 
> In addition the patch also deals with a changed netmask if the address
> stays the same.
> 
> A complete new address still needs a change in ospfd.conf and a reload.
> 
> Remi
> 

ping.

Any comments, opinions or improvements about this patch?

> 
> Index: kroute.c
> ===
> RCS file: /cvs/src/usr.sbin/ospfd/kroute.c,v
> retrieving revision 1.107
> diff -u -p -r1.107 kroute.c
> --- kroute.c  27 Dec 2016 09:15:16 -  1.107
> +++ kroute.c  25 Jun 2017 20:58:33 -
> @@ -1046,6 +1046,7 @@ if_newaddr(u_short ifindex, struct socka
>  {
>   struct kif_node *kif;
>   struct kif_addr *ka;
> + struct ifaddrnew ifn;
>  
>   if (ifa == NULL || ifa->sin_family != AF_INET)
>   return;
> @@ -1066,6 +1067,12 @@ if_newaddr(u_short ifindex, struct socka
>   ka->dstbrd.s_addr = INADDR_NONE;
>  
>   TAILQ_INSERT_TAIL(>addrs, ka, entry);
> +
> + ifn.addr = ka->addr;
> + ifn.mask = ka->mask;
> + ifn.dst = ka->dstbrd;
> + ifn.ifindex = ifindex;
> + main_imsg_compose_ospfe(IMSG_IFADDRADD, 0, , sizeof(ifn));
>  }
>  
>  void
> Index: ospfd.h
> ===
> RCS file: /cvs/src/usr.sbin/ospfd/ospfd.h,v
> retrieving revision 1.97
> diff -u -p -r1.97 ospfd.h
> --- ospfd.h   24 Jan 2017 04:24:25 -  1.97
> +++ ospfd.h   25 Jun 2017 20:58:33 -
> @@ -132,6 +132,7 @@ enum imsg_type {
>   IMSG_RECONF_REDIST,
>   IMSG_RECONF_END,
>   IMSG_DEMOTE,
> + IMSG_IFADDRADD,
>   IMSG_IFADDRDEL
>  };
>  
> @@ -358,6 +359,13 @@ struct iface {
>   u_int8_t linkstate;
>   u_int8_t priority;
>   u_int8_t passive;
> +};
> +
> +struct ifaddrnew {
> + struct in_addr  addr;
> + struct in_addr  mask;
> + struct in_addr  dst;
> + unsigned intifindex;
>  };
>  
>  struct ifaddrdel {
> Index: ospfe.c
> ===
> RCS file: /cvs/src/usr.sbin/ospfd/ospfe.c,v
> retrieving revision 1.99
> diff -u -p -r1.99 ospfe.c
> --- ospfe.c   24 Jan 2017 04:24:25 -  1.99
> +++ ospfe.c   25 Jun 2017 20:58:33 -
> @@ -278,6 +278,7 @@ ospfe_dispatch_main(int fd, short event,
>  {
>   static struct area  *narea;
>   static struct iface *niface;
> + struct ifaddrnew*ifn;
>   struct ifaddrdel*ifc;
>   struct imsg  imsg;
>   struct imsgev   *iev = bula;
> @@ -345,6 +346,28 @@ ospfe_dispatch_main(int fd, short event,
>   " down",
>   iface->name);
>   }
> + }
> + }
> + }
> + break;
> + case IMSG_IFADDRADD:
> + if (imsg.hdr.len != IMSG_HEADER_SIZE +
> + sizeof(struct ifaddrnew))
> + fatalx("IFADDRADD imsg with wrong len");
> + ifn = imsg.data;
> +
> + LIST_FOREACH(area, >area_list, entry) {
> + LIST_FOREACH(iface, >iface_list, entry) {
> + if (ifn->ifindex == iface->ifindex &&
> + ifn->addr.s_addr ==
> + iface->addr.s_addr) {
> + iface->mask = ifn->mask;
> + iface->dst = ifn->dst;
> + 

Re: relayd ipv6 ttl check_icmp / check_tcp

[Sebastian Benoit]

Florian Obser(flor...@openbsd.org) on 2017.07.04 19:27:15 +:
> On Fri, Jun 23, 2017 at 01:52:52PM +0300, Kapetanakis Giannis wrote:
> > Hi,
> > 
> > Using relayd's redirect/forward on ipv6 addresses I discovered problems 
> > relating to setting TTL.
> > 
> > There is no check for address family and setsockopt tries to apply IP_TTL 
> > always.
> > 
> > Without ip ttl on ipv6 table, check_icmp gives
> > send_icmp: getsockopt: Invalid argument
> > 
> > With ip ttl on ipv6 table, check_tcp gives
> > hce_notify_done: fdaa:10:1:9::11 (tcp socket option)
> > 
> > is the following diff valid?
> 
> the check_tcp hunk looks good and is OK florian@

commited, thanks!
 
> > I've removed the IP_IPDEFTTL check. Was this ok?
> 
> Nope, relayd reuses the raw socket between config reloads (I think),
> if the ttl gets removed from the config we need to reset to the
> default. Don't think there is a getsockopt for v6, you can take a look

i think jca@ once had a diff for somethin called IPV6_MINHOPLIMIT? Unsure if
thats what we need here though.

> at the sysctl(3) song and dance in traceroute(8) how to do this
> somewhat AF independet.
> 
> Also please make sure to not exceed 80 cols
> 
> > 
> > regards,
> > 
> > Giannis
> > 
> > Index: check_icmp.c
> > ===
> > RCS file: /cvs/src/usr.sbin/relayd/check_icmp.c,v
> > retrieving revision 1.45
> > diff -u -p -r1.45 check_icmp.c
> > --- check_icmp.c28 May 2017 10:39:15 -  1.45
> > +++ check_icmp.c23 Jun 2017 10:42:30 -
> > @@ -165,7 +165,7 @@ send_icmp(int s, short event, void *arg)
> > struct icmp6_hdr*icp6;
> > ssize_t  r;
> > u_char   packet[ICMP_BUF_SIZE];
> > -   socklen_tslen, len;
> > +   socklen_tslen;
> > int  i = 0, ttl;
> > u_int32_tid;
> >  
> > @@ -221,18 +221,18 @@ send_icmp(int s, short event, void *arg)
> > }
> >  
> > if ((ttl = host->conf.ttl) > 0)
> > -   (void)setsockopt(s, IPPROTO_IP, IP_TTL,
> > -   >conf.ttl, sizeof(int));
> > -   else {
> > -   /* Revert to default TTL */
> > -   len = sizeof(ttl);
> > -   if (getsockopt(s, IPPROTO_IP, IP_IPDEFTTL,
> > -   , ) == 0)
> > -   (void)setsockopt(s, IPPROTO_IP, IP_TTL,
> > -   , len);
> > -   else
> > -   log_warn("%s: getsockopt",__func__);
> > -   }
> > +   switch(cie->af) {
> > +   case AF_INET:
> > +   if (setsockopt(s, IPPROTO_IP, IP_TTL,
> > +   >conf.ttl, sizeof(int)) == -1)
> > +   log_warn("%s: 
> > setsockopt",__func__);
> > +   break;
> > +   case AF_INET6:
> > +   if (setsockopt(s, IPPROTO_IPV6, 
> > IPV6_UNICAST_HOPS,
> > +   >conf.ttl, sizeof(int)) == -1)
> > +   log_warn("%s: 
> > setsockopt",__func__);
> > +   break;
> > +   }
> >  
> > r = sendto(s, packet, sizeof(packet), 0, to, slen);
> > if (r == -1) {
> > Index: check_tcp.c
> > ===
> > RCS file: /cvs/src/usr.sbin/relayd/check_tcp.c,v
> > retrieving revision 1.54
> > diff -u -p -r1.54 check_tcp.c
> > --- check_tcp.c 28 May 2017 10:39:15 -  1.54
> > +++ check_tcp.c 23 Jun 2017 10:42:30 -
> > @@ -82,11 +82,19 @@ check_tcp(struct ctl_tcp_event *cte)
> > if (setsockopt(s, SOL_SOCKET, SO_LINGER, , sizeof(lng)) == -1)
> > goto bad;
> >  
> > -   if (cte->host->conf.ttl > 0) {
> > -   if (setsockopt(s, IPPROTO_IP, IP_TTL,
> > -   >host->conf.ttl, sizeof(int)) == -1)
> > -   goto bad;
> > -   }
> > +   if (cte->host->conf.ttl > 0)
> > +   switch (cte->host->conf.ss.ss_family) {
> > +   case AF_INET:
> > +   if (setsockopt(s, IPPROTO_IP, IP_TTL,
> > +   >host->conf.ttl, sizeof(int)) == -1)
> > +   goto bad;
> > +   break;
> > +   case AF_INET6:
> > +   if (setsockopt(s, IPPROTO_IPV6, IPV6_UNICAST_HOPS,
> > +   >host->conf.ttl, sizeof(int)) == -1)
> > +   goto bad;
> > +   break;
> > +   }
> >  
> > bcopy(>table->conf.timeout, , sizeof(tv));
> > if (connect(s, (struct 

Re: install.sub: ieee80211_{scan,config}: Allow quoted SSIDs

[Klemens Nanni]

On Mon, Jul 03, 2017 at 12:45:32AM +0200, Klemens Nanni wrote:

Thanks for looking into it.

On Sun, Jul 02, 2017 at 04:32:43PM +, Robert Peichaer wrote:

ieee80211_scan()
- Extract the needed information (nwid, bssid) using a very specific
  sed expression. Any line, not matching this expr is ignored.

- Remove leading and trailing double-quotes in case of nwids with
  spaces.

I had the ugly case of an empty SSID in reach while testing this so I
intentionally left double quotes in place within WLANLIST so that
the list presented to the user wouldn't look broken, e.g.
"my wifi" chan 1 bssid ...
"" chan 2 bssid ...
as opposed to
my wifi chan 1 bssid ...
 chan 2 bssid ...

I'd also leave unqouting to the routine that actually requires it
instead of the function that just provides the list.


- Write nwid and bssid into WLANLIST as '()'.

Writing the simple format directly to cache seems like a good idea
instead of just cutting ^nwid first here and .*$ somewhere else.


ieee80211_config()
- just print WLANLIST using ieee80211_scan() if the user chooses
  '?' which has the right format already

- in case the user selects an entry from WLANLIST using a number,
  remove the '()' part from the line, resulting in
  the nwid (without double-quotes)

- using the quote() function with the ifconfig command ensures,
  that the nwid is quoted properly with single-quotes in case it
  contains spaces

This is not needed as "$_nwid" will even work if _nwid='my "wifi'.


- using the quote() function when writing the nwid to the hostname.if
  files ensures that the nwid is quoted properly with single-quotes
  in case it contains spaces

The parse_hn_line() function in netstart does handle quoted nwids
properly when processing the hostname.if config lines as far as I
can see.

Yes, it does. But it chokes on SSIDs containing a literal " for example.


Here is an updated diff taking above considerations into account.

Note how ([[:xdigit:]:]*)$ when picking the answer must not be
simplified to (.*)$ as this would fail on SSIDs like "my (hidden) wifi".

Feedback/OK?

That patch was mangled, sorry. Here it goes again.

Index: install.sub
===
RCS file: /cvs/src/distrib/miniroot/install.sub,v
retrieving revision 1.1019
diff -u -p -r1.1019 install.sub
--- install.sub 2 Jul 2017 12:45:43 -   1.1019
+++ install.sub 4 Jul 2017 20:43:56 -
@@ -1060,10 +1060,9 @@ v6_config() {
# Perform an 802.11 network scan on interface $1.
# The result is cached in $WLANLIST.
ieee80211_scan() {
-   # N.B. Skipping quoted nwid's for now.
[[ -f $WLANLIST ]] ||
ifconfig $1 scan |
-   sed -n 's/^ nwid \([^"]\)/\1/p' >$WLANLIST
+   sed -n 's/^[[:space:]]*nwid \(.*\) chan [0-9]* bssid 
\([[:xdigit:]:]*\).*/\1 (\2)/p' >$WLANLIST
cat $WLANLIST
}

@@ -1082,12 +1081,12 @@ ieee80211_config() {
ask_until "Access point? (ESSID, 'any', list# or '?')" "any"
case "$resp" in
+([0-9]))
-   _nwid=$(ieee80211_scan $_if | sed -n "${resp}s/ .*//p")
+   _nwid=$(ieee80211_scan $_if |
+   sed -n ${resp}'{s/ ([[:xdigit:]:]*)$//p;q;}')
[[ -z $_nwid ]] && echo "There is no line $resp."
+   [[ $_nwid = \"*\" ]] && _nwid=${_nwid#\"} 
_nwid=${_nwid%\"}
;;
-   \?) ieee80211_scan $_if |
-   sed -n 's/^\([^ ]*\) chan .* bssid \([^ ]*\) 
.*$/   \1 (\2)/p' |
-   cat -n | more -c
+   \?) ieee80211_scan $_if | cat -n | more -c
;;
*)  _nwid=$resp
;;

Re: elf.h

[Karel Gardas]

On Tue, 4 Jul 2017 11:48:27 +0200
Martin Pieuchot  wrote:

> Hello,
>
> I think that moving towards  is a good thing.  However are you
> sure that  provides all the definitions required by
> ?

Not yet. At least a lot of machine related definitions are missing, but they 
are not required if neither base nor ports need them. In patch below I'm adding 
just EM_PPC64 which I know is needed and worked around by GHC. I'm also killing 
comment as I've verified those or fixed/removed those not in correct way 
alligned with spec.

> Are you sure it doesn't provide any other definition that
> would make code written on OpenBSD non portable?

PT_OPENBSD_* and NT_OPENBSD_* are there. The first I moved to ifdef 
ELF_OPENBSD_EXTENSION. The second is used only by kernel hence _KERNEL. 
Otherwise a lot of _DEFINED_ from sys/types.h leak in, but the question 
is if this hurts or not.

> What would it take to convert base programs to ?

Base is quite clean except few exceptions. I used ELF_OPENBSD_EXTENSION for 
those. Anyway, both toolchains (bintils+gcc/llvm+clang) seem to live their own 
ELF lifes without even attempting to include system elf.h/elf_abi.h so they are 
clean and not touched.

> If base starts to provide  you also need to make sure it doesn't
> break any port.

Both elf.h/elf_abi.h so far points to the exactly same definitions, hence if 
there is port which is broken, then it'll be broken at compile time. Quite 
easily discoverable by building all ports. Not yet done on my side though.

> I would welcome such move since it would make easier to port programs
> from OpenBSD to other platforms.  However it's not as easy as including
> a header in another.  Are you ready to tackle the above mentioned
> issues?

Don't know about complexity of this. Anyway, below is another step which fixes 
machine definition (obvious issues), adds mentioned EM_PPC64 and _KERNEL ifdefs 
NT_OPENBSD and also uses ELF_OPENBSD_EXTENSION to filter out PT_OPENBSD. 
However later is questionable since PT_GNU is there without any #ifdef. So I 
can see several ways how to improve on this. For example:

- move #define ELF_OPENBSD_EXTENSION to elf_abi.h and remove from base 
progs/libs. This way elf.h will be portable and elf_abi.h OBSD specific with an 
option to rename to elf_obsd.h or so in the future.
- if you don't like ELF_OPENBSD_EXTENSION I'm free to rename to whatever is 
preferred.
- if you don't like ELF_OBSD_EXT at all and prefer separate file then I'll need 
to proceed with way described above with differenting elf.h and elf_abi.h -- or 
so.

Comments welcome! Thanks,

Karel

diff --git a/sys/arch/amd64/stand/libsa/elf32.c 
b/sys/arch/amd64/stand/libsa/elf32.c
index f943067411c..896cd68bcb0 100644
--- a/sys/arch/amd64/stand/libsa/elf32.c
+++ b/sys/arch/amd64/stand/libsa/elf32.c
@@ -27,6 +27,8 @@
 #undef ELFSIZE
 #define ELFSIZE  32
 
+#define ELF_OPENBSD_EXTENSION
 #include 
+#undef ELF_OPENBSD_EXTENSION
 
 #include "../../../../lib/libsa/loadfile_elf.c"
diff --git a/sys/arch/amd64/stand/libsa/elf64.c 
b/sys/arch/amd64/stand/libsa/elf64.c
index d7d11c843c4..49bdaa62fed 100644
--- a/sys/arch/amd64/stand/libsa/elf64.c
+++ b/sys/arch/amd64/stand/libsa/elf64.c
@@ -27,6 +27,8 @@
 #undef ELFSIZE
 #define ELFSIZE  64
 
+#define ELF_OPENBSD_EXTENSION
 #include 
+#undef ELF_OPENBSD_EXTENSION
 
 #include "../../../../lib/libsa/loadfile_elf.c"
diff --git a/sys/sys/exec_elf.h b/sys/sys/exec_elf.h
index 77c13a0372c..95f21d9d44a 100644
--- a/sys/sys/exec_elf.h
+++ b/sys/sys/exec_elf.h
@@ -177,16 +177,12 @@ typedef struct {
 #define EM_486 6   /* Intel 80486 - unused? */
 #define EM_860 7   /* Intel 80860 */
 #define EM_MIPS8   /* MIPS R3000 Big-Endian only */
-/*
- * Don't know if EM_MIPS_RS4_BE,
- * EM_SPARC64, EM_PARISC,
- * or EM_PPC are ABI compliant
- */
-#define EM_MIPS_RS4_BE 10  /* MIPS R4000 Big-Endian */
-#define EM_SPARC64 11  /* SPARC v9 64-bit unofficial */
+#define EM_MIPS_RS3_LE 10  /* MIPS R3000 Little-Endian */
+   /* 11 - 14 reserved for future use */
 #define EM_PARISC  15  /* HPPA */
 #define EM_SPARC32PLUS 18  /* Enhanced instruction set SPARC */
 #define EM_PPC 20  /* PowerPC */
+#define EM_PPC64   21  /* PowerPC 64 */
 #define EM_ARM 40  /* Advanced RISC Machines ARM */
 #define EM_ALPHA   41  /* DEC ALPHA */
 #defineEM_SH   42  /* Hitachi/Renesas Super-H */
@@ -431,10 +427,14 @@ typedef struct {
 #define PT_GNU_EH_FRAME0x6474e550  /* Exception handling 
info */
 #define PT_GNU_RELRO   0x6474e552  /* Read-only after relocation */
 
+#if defined(_KERNEL) || defined(_DYN_LOADER) || defined(ELF_OPENBSD_EXTENSION)
+
 #define PT_OPENBSD_RANDOMIZE   0x65a3dbe6  /* fill with random data */
 #define PT_OPENBSD_WXNEEDED0x65a3dbe7

Re: ping: Style fixes/cleanups

[Klemens Nanni]

On Tue, Jul 04, 2017 at 04:00:43PM +, Florian Obser wrote:

yeah, this is arse backwards, I'm willing to commit the oposite though,
i.e. get rid of the void casts for printf


Casts removed, cosecutive calls merged where suitable.

Feedback/OK?

Index: ping.c
===
RCS file: /cvs/src/sbin/ping/ping.c,v
retrieving revision 1.220
diff -u -p -r1.220 ping.c
--- ping.c  4 Jul 2017 15:55:22 -   1.220
+++ ping.c  4 Jul 2017 19:39:35 -
@@ -914,19 +914,19 @@ fill(char *bp, char *patp)
for (jj = 0; jj < ii; ++jj)
bp[jj + kk] = pat[jj];
if (!(options & F_QUIET)) {
-   (void)printf("PATTERN: 0x");
+   printf("PATTERN: 0x");
for (jj = 0; jj < ii; ++jj)
-   (void)printf("%02x", bp[jj] & 0xFF);
-   (void)printf("\n");
+   printf("%02x", bp[jj] & 0xFF);
+   printf("\n");
}
}

void
summary(void)
{
-   printf("\n--- %s ping statistics ---\n", hostname);
-   printf("%lld packets transmitted, ", ntransmitted);
-   printf("%lld packets received, ", nreceived);
+   printf("\n--- %s ping statistics ---\n"
+   "%lld packets transmitted, %lld packets received, ",
+   hostname, ntransmitted, nreceived);

if (nrepeats)
printf("%lld duplicates, ", nrepeats);
@@ -944,8 +944,8 @@ summary(void)
double num = nreceived + nrepeats;
double avg = tsum / num;
double dev = sqrt(fmax(0, tsumsq / num - avg * avg));
-   printf("round-trip min/avg/max/std-dev = %.3f/%.3f/%.3f/%.3f 
ms\n",
-   tmin, avg, tmax, dev);
+   printf("round-trip min/avg/max/std-dev = "
+   "%.3f/%.3f/%.3f/%.3f ms\n", tmin, avg, tmax, dev);
}
}

@@ -1209,7 +1209,7 @@ pr_pack(u_char *buf, int cc, struct msgh

if (timingsafe_memcmp(mac, ,
sizeof(mac)) != 0) {
-   (void)printf("signature mismatch!\n");
+   printf("signature mismatch!\n");
return;
}
timinginfo=1;
@@ -1245,19 +1245,19 @@ pr_pack(u_char *buf, int cc, struct msgh
if (options & F_FLOOD)
(void)write(STDOUT_FILENO, , 1);
else {
-   (void)printf("%d bytes from %s: icmp_seq=%u", cc,
+   printf("%d bytes from %s: icmp_seq=%u", cc,
pr_addr(from, fromlen), ntohs(seq));
if (v6flag)
-   (void)printf(" hlim=%d", hoplim);
+   printf(" hlim=%d", hoplim);
else
-   (void)printf(" ttl=%d", ip->ip_ttl);
+   printf(" ttl=%d", ip->ip_ttl);
if (cc >= ECHOLEN + ECHOTMLEN)
-   (void)printf(" time=%.3f ms", triptime);
+   printf(" time=%.3f ms", triptime);
if (dupflag)
-   (void)printf(" (DUP!)");
+   printf(" (DUP!)");
/* check the data */
if (cc - ECHOLEN < datalen)
-   (void)printf(" (TRUNC!)");
+   printf(" (TRUNC!)");
if (v6flag)
cp = buf + ECHOLEN + ECHOTMLEN;
else
@@ -1267,7 +1267,7 @@ pr_pack(u_char *buf, int cc, struct msgh
i < cc && i < datalen;
++i, ++cp, ++dp) {
if (*cp != *dp) {
-   (void)printf("\nwrong data byte #%d "
+   printf("\nwrong data byte #%d "
"should be 0x%x but was 0x%x",
i - ECHOLEN, *dp, *cp);
if (v6flag)
@@ -1278,8 +1278,8 @@ pr_pack(u_char *buf, int cc, struct msgh
for (i = ECHOLEN; i < cc && i < datalen;
++i, ++cp) {
if ((i % 32) == 8)
-   (void)printf("\n\t");
-   (void)printf("%x ", *cp);
+   printf("\n\t");
+   printf("%x ", *cp);
}
break;
}
@@ -1289,7 +1289,7 @@ 

Re: CVS: cvs.openbsd.org: src

[J Doe]

Hi,

On Jul 4, 2017, at 7:40 AM, Mark Kettenis  wrote:

>> From: Frank Groeneveld 
>> Date: Tue, 04 Jul 2017 09:38:18 +0200
>> 
>>> On Mon, Jul 3, 2017, at 08:30, Martijn van Duren wrote:
>>> This change *STILL* breaks my $DAYJOB machine.
>>> 
>>> dmesg with DRMDEBUG enabled
>> 
>> Maybe you shouldn't chose Apple hardware ;-)

I've actually had really positive experiences with running OpenBSD with Apple 
hardware.  I had an iMac that I used as an experimental box and the only issue 
with it was retrieving the blob for the video card.  Likewise, I had a 
firewall/NIDS running on a headless Mac Mini with no issues.  Even more 
impressive, I had zero-configuration support for an Apple USB 10/100 NIC, which 
I expected would be a bit more of a niche device.

I note that you mention ThinkPads and I know they are highly regarded within 
the OpenBSD community . . . I just thought I'd throw in my two cents on Apple 
hardware!

Cheers,

- J

Re: fix relayd dns protocol

[Florian Obser]

OK florian@

On Thu, Jun 29, 2017 at 09:26:16PM +, Rivo Nurges wrote:
> Hi!
> 
> config_setrelay>relay_privinit>relay_udp_privinit doesn't set env
> since env isn't set in relay.c yet, causing dns relay to SIGSEGV
> in relay_udp_server. Move setting env to relay_udp_init.
> 
> Rivo
> 
> Index: usr.sbin/relayd/relay.c
> ===
> RCS file: /cvs/src/usr.sbin/relayd/relay.c,v
> retrieving revision 1.221
> diff -u -p -r1.221 relay.c
> --- usr.sbin/relayd/relay.c   28 May 2017 10:39:15 -  1.221
> +++ usr.sbin/relayd/relay.c   29 Jun 2017 20:58:08 -
> @@ -285,7 +285,7 @@ relay_privinit(struct relay *rlay)
>  
>   switch (rlay->rl_proto->type) {
>   case RELAY_PROTO_DNS:
> - relay_udp_privinit(env, rlay);
> + relay_udp_privinit(rlay);
>   break;
>   case RELAY_PROTO_TCP:
>   break;
> @@ -445,7 +445,7 @@ relay_launch(void)
>  
>   switch (rlay->rl_proto->type) {
>   case RELAY_PROTO_DNS:
> - relay_udp_init(rlay);
> + relay_udp_init(env, rlay);
>   break;
>   case RELAY_PROTO_TCP:
>   case RELAY_PROTO_HTTP:
> Index: usr.sbin/relayd/relay_udp.c
> ===
> RCS file: /cvs/src/usr.sbin/relayd/relay_udp.c,v
> retrieving revision 1.46
> diff -u -p -r1.46 relay_udp.c
> --- usr.sbin/relayd/relay_udp.c   28 May 2017 10:39:15 -  1.46
> +++ usr.sbin/relayd/relay_udp.c   29 Jun 2017 20:58:08 -
> @@ -58,20 +58,20 @@ void   relay_dns_result(struct rsession 
>  int   relay_dns_cmp(struct rsession *, struct rsession *);
>  
>  void
> -relay_udp_privinit(struct relayd *x_env, struct relay *rlay)
> +relay_udp_privinit(struct relay *rlay)
>  {
> - if (env == NULL)
> - env = x_env;
> -
>   if (rlay->rl_conf.flags & F_TLS)
>   fatalx("tls over udp is not supported");
>   rlay->rl_conf.flags |= F_UDP;
>  }
>  
>  void
> -relay_udp_init(struct relay *rlay)
> +relay_udp_init(struct relayd *x_env, struct relay *rlay)
>  {
>   struct protocol *proto = rlay->rl_proto;
> +
> + if (env == NULL)
> + env = x_env;
>  
>   switch (proto->type) {
>   case RELAY_PROTO_DNS:
> Index: usr.sbin/relayd/relayd.h
> ===
> RCS file: /cvs/src/usr.sbin/relayd/relayd.h,v
> retrieving revision 1.240
> diff -u -p -r1.240 relayd.h
> --- usr.sbin/relayd/relayd.h  27 May 2017 08:33:25 -  1.240
> +++ usr.sbin/relayd/relayd.h  29 Jun 2017 20:58:08 -
> @@ -1218,8 +1218,8 @@ int  relay_httpdesc_init(struct ctl_rela
>  ssize_t   relay_http_time(time_t, char *, size_t);
>  
>  /* relay_udp.c */
> -void  relay_udp_privinit(struct relayd *, struct relay *);
> -void  relay_udp_init(struct relay *);
> +void  relay_udp_privinit(struct relay *);
> +void  relay_udp_init(struct relayd *, struct relay *);
>  int   relay_udp_bind(struct sockaddr_storage *, in_port_t,
>   struct protocol *);
>  void  relay_udp_server(int, short, void *);
> 
> 

-- 
I'm not entirely sure you are real.

Re: relayd ipv6 ttl check_icmp / check_tcp

[Florian Obser]

On Fri, Jun 23, 2017 at 01:52:52PM +0300, Kapetanakis Giannis wrote:
> Hi,
> 
> Using relayd's redirect/forward on ipv6 addresses I discovered problems 
> relating to setting TTL.
> 
> There is no check for address family and setsockopt tries to apply IP_TTL 
> always.
> 
> Without ip ttl on ipv6 table, check_icmp gives
> send_icmp: getsockopt: Invalid argument
> 
> With ip ttl on ipv6 table, check_tcp gives
> hce_notify_done: fdaa:10:1:9::11 (tcp socket option)
> 
> is the following diff valid?

the check_tcp hunk looks good and is OK florian@

> I've removed the IP_IPDEFTTL check. Was this ok?

Nope, relayd reuses the raw socket between config reloads (I think),
if the ttl gets removed from the config we need to reset to the
default. Don't think there is a getsockopt for v6, you can take a look
at the sysctl(3) song and dance in traceroute(8) how to do this
somewhat AF independet.

Also please make sure to not exceed 80 cols

> 
> regards,
> 
> Giannis
> 
> Index: check_icmp.c
> ===
> RCS file: /cvs/src/usr.sbin/relayd/check_icmp.c,v
> retrieving revision 1.45
> diff -u -p -r1.45 check_icmp.c
> --- check_icmp.c  28 May 2017 10:39:15 -  1.45
> +++ check_icmp.c  23 Jun 2017 10:42:30 -
> @@ -165,7 +165,7 @@ send_icmp(int s, short event, void *arg)
>   struct icmp6_hdr*icp6;
>   ssize_t  r;
>   u_char   packet[ICMP_BUF_SIZE];
> - socklen_tslen, len;
> + socklen_tslen;
>   int  i = 0, ttl;
>   u_int32_tid;
>  
> @@ -221,18 +221,18 @@ send_icmp(int s, short event, void *arg)
>   }
>  
>   if ((ttl = host->conf.ttl) > 0)
> - (void)setsockopt(s, IPPROTO_IP, IP_TTL,
> - >conf.ttl, sizeof(int));
> - else {
> - /* Revert to default TTL */
> - len = sizeof(ttl);
> - if (getsockopt(s, IPPROTO_IP, IP_IPDEFTTL,
> - , ) == 0)
> - (void)setsockopt(s, IPPROTO_IP, IP_TTL,
> - , len);
> - else
> - log_warn("%s: getsockopt",__func__);
> - }
> + switch(cie->af) {
> + case AF_INET:
> + if (setsockopt(s, IPPROTO_IP, IP_TTL,
> + >conf.ttl, sizeof(int)) == -1)
> + log_warn("%s: 
> setsockopt",__func__);
> + break;
> + case AF_INET6:
> + if (setsockopt(s, IPPROTO_IPV6, 
> IPV6_UNICAST_HOPS,
> + >conf.ttl, sizeof(int)) == -1)
> + log_warn("%s: 
> setsockopt",__func__);
> + break;
> + }
>  
>   r = sendto(s, packet, sizeof(packet), 0, to, slen);
>   if (r == -1) {
> Index: check_tcp.c
> ===
> RCS file: /cvs/src/usr.sbin/relayd/check_tcp.c,v
> retrieving revision 1.54
> diff -u -p -r1.54 check_tcp.c
> --- check_tcp.c   28 May 2017 10:39:15 -  1.54
> +++ check_tcp.c   23 Jun 2017 10:42:30 -
> @@ -82,11 +82,19 @@ check_tcp(struct ctl_tcp_event *cte)
>   if (setsockopt(s, SOL_SOCKET, SO_LINGER, , sizeof(lng)) == -1)
>   goto bad;
>  
> - if (cte->host->conf.ttl > 0) {
> - if (setsockopt(s, IPPROTO_IP, IP_TTL,
> - >host->conf.ttl, sizeof(int)) == -1)
> - goto bad;
> - }
> + if (cte->host->conf.ttl > 0)
> + switch (cte->host->conf.ss.ss_family) {
> + case AF_INET:
> + if (setsockopt(s, IPPROTO_IP, IP_TTL,
> + >host->conf.ttl, sizeof(int)) == -1)
> + goto bad;
> + break;
> + case AF_INET6:
> + if (setsockopt(s, IPPROTO_IPV6, IPV6_UNICAST_HOPS,
> + >host->conf.ttl, sizeof(int)) == -1)
> + goto bad;
> + break;
> + }
>  
>   bcopy(>table->conf.timeout, , sizeof(tv));
>   if (connect(s, (struct sockaddr *)>host->conf.ss, len) == -1) {
> 

-- 
I'm not entirely sure you are real.

Re: Missed ifconfig [[-]txpower dBm] option for 802.11

[Ted Unangst]

Denis wrote:
> Looking for ifconfig '[[-]txpower dBm]' option which was present in
> OpenBSD 5.4 amd64. Try to find 'txpower' on 6.0 amd64 but seems it
> missed out.
> 
> Actively using it to match power for 802.11 card and it's RF recipient
> (post amp). What mechanism of output power matching is provided
> currently since 5.4 amd64?

txpower was removed because only the wi driver supported it and the relevance
of the wi driver has faded.

Re: ping: Style fixes/cleanups

[Florian Obser]

yeah, this is arse backwards, I'm willing to commit the oposite though,
i.e. get rid of the void casts for printf

On Tue, Jun 13, 2017 at 08:12:47PM +0200, Klemens Nanni wrote:
> printf's return value is ignored allmost all the times so do the same
> for the rest of them as well for consistency.
> 
> Subsequent calls got merged where I think is appropiate and readability
> improves.
> 
> The usage format string now looks like it's actual put.
> 
> Index: ping.c
> ===
> RCS file: /cvs/src/sbin/ping/ping.c,v
> retrieving revision 1.218
> diff -u -p -r1.218 ping.c
> --- ping.c22 Feb 2017 13:43:35 -  1.218
> +++ ping.c13 Jun 2017 18:08:42 -
> @@ -756,10 +756,10 @@ main(int argc, char *argv[])
>   arc4random_buf(_offset, sizeof(tv64_offset));
>   arc4random_buf(_key, sizeof(mac_key));
> 
> - printf("PING %s (", hostname);
> + (void)printf("PING %s (", hostname);
>   if (options & F_VERBOSE)
> - printf("%s --> ", pr_addr(from, from->sa_len));
> - printf("%s): %d data bytes\n", pr_addr(dst, dst->sa_len), datalen);
> + (void)printf("%s --> ", pr_addr(from, from->sa_len));
> + (void)printf("%s): %d data bytes\n", pr_addr(dst, dst->sa_len), 
> datalen);
> 
>   smsghdr.msg_name = dst;
>   smsghdr.msg_namelen = dst->sa_len;
> @@ -862,7 +862,7 @@ main(int argc, char *argv[])
>*/
>   if ((mtu = get_pathmtu(, )) > 0) {
>   if ((options & F_VERBOSE) != 0) {
> - printf("new path MTU (%d) is "
> + (void)printf("new path MTU (%d) is "
>   "notified\n", mtu);
>   }
>   }
> @@ -926,28 +926,28 @@ fill(char *bp, char *patp)
> void
> summary(void)
> {
> - printf("\n--- %s ping statistics ---\n", hostname);
> - printf("%lld packets transmitted, ", ntransmitted);
> - printf("%lld packets received, ", nreceived);
> + (void)printf("\n--- %s ping statistics ---\n"
> + "%lld packets transmitted, %lld packets received, ",
> + hostname, ntransmitted, nreceived);
> 
>   if (nrepeats)
> - printf("%lld duplicates, ", nrepeats);
> + (void)printf("%lld duplicates, ", nrepeats);
>   if (ntransmitted) {
>   if (nreceived > ntransmitted)
> - printf("-- somebody's duplicating packets!");
> + (void)printf("-- somebody's duplicating packets!");
>   else
> - printf("%.1f%% packet loss",
> - double)ntransmitted - nreceived) * 100) /
> - ntransmitted));
> + (void)printf("%.1f%% packet loss",
> + (double)(ntransmitted - nreceived)
> + * 100 / ntransmitted);
>   }
> - printf("\n");
> + (void)printf("\n");
>   if (timinginfo) {
>   /* Only display average to microseconds */
>   double num = nreceived + nrepeats;
>   double avg = tsum / num;
>   double dev = sqrt(fmax(0, tsumsq / num - avg * avg));
> - printf("round-trip min/avg/max/std-dev = %.3f/%.3f/%.3f/%.3f 
> ms\n",
> - tmin, avg, tmax, dev);
> + (void)printf("round-trip min/avg/max/std-dev = "
> + "%.3f/%.3f/%.3f/%.3f ms\n", tmin, avg, tmax, dev);
>   }
> }
> 
> @@ -1092,7 +1092,7 @@ pinger(int s)
>   if (i < 0 || i != cc) {
>   if (i < 0)
>   warn("sendmsg");
> - printf("ping: wrote %s %d chars, ret=%d\n", hostname, cc, i);
> + (void)printf("ping: wrote %s %d chars, ret=%d\n", hostname, cc, 
> i);
>   }
>   if (!(options & F_QUIET) && options & F_FLOOD)
>   (void)write(STDOUT_FILENO, , 1);
> @@ -1278,11 +1278,9 @@ pr_pack(u_char *buf, int cc, struct msgh
>   cp = (u_char *)
>   >icmp_data[0];
>   for (i = ECHOLEN; i < cc && i < datalen;
> - ++i, ++cp) {
> - if ((i % 32) == 8)
> - (void)printf("\n\t");
> - (void)printf("%x ", *cp);
> - }
> + ++i, ++cp)
> + (void)printf(i % 32 == 8 ?
> + "\n\t%x " : "%x ", *cp);
>   break;
>   }
>   }
> @@ -1560,8 +1558,7 @@ pr_icmph(struct icmp *icp)
>   

Re: ping: Style fixes/cleanups

[Florian Obser]

I like most (all? of the correct ones), can you have another go
at trying to fix the pointed out changes in behaviour?

On Tue, Jun 13, 2017 at 08:02:13PM +0200, Klemens Nanni wrote:
> Unify option checking and simply logic.
> 
> F_HDRINCL and F_ROUTE are mutually exclusive, thus check the latter only
> if the former one is not set.
> 
> Index: ping.c
> ===
> RCS file: /cvs/src/sbin/ping/ping.c,v
> retrieving revision 1.218
> diff -u -p -r1.218 ping.c
> --- ping.c22 Feb 2017 13:43:35 -  1.218
> +++ ping.c13 Jun 2017 17:38:22 -
> @@ -562,17 +562,17 @@ main(int argc, char *argv[])
>   (void)setsockopt(s, SOL_SOCKET, SO_DEBUG, ,
>   sizeof(optval));
> 
> - if ((options & F_FLOOD) && (options & F_INTERVAL))
> + if (options & (F_FLOOD | F_INTERVAL))
>   errx(1, "-f and -i options are incompatible");
> 
> - if ((options & F_FLOOD) && (options & (F_AUD_RECV | F_AUD_MISS)))
> + if (options & (F_FLOOD | F_AUD_RECV | F_AUD_MISS))
>   warnx("No audible output for flood pings");
> 
>   if (datalen >= sizeof(struct payload))  /* can we time transfer */
>   timing = 1;
> 
>   if (v6flag) {
> - /* in F_VERBOSE case, we may get non-echoreply packets*/
> + /* in F_VERBOSE case, we may get non-echoreply packets */
>   if (options & F_VERBOSE && datalen < 2048) /* XXX 2048? */
>   packlen = 2048 + IP6LEN + ECHOLEN + EXTRA;
>   else
> @@ -621,7 +621,7 @@ main(int argc, char *argv[])
>* let the kernel pass extension headers of incoming packets,
>* for privileged socket options
>*/
> - if ((options & F_VERBOSE) != 0) {
> + if (options & F_VERBOSE) {
>   int opton = 1;
> 
>   if (setsockopt(s, IPPROTO_IPV6, IPV6_RECVHOPOPTS,
> @@ -696,7 +696,7 @@ main(int argc, char *argv[])
>   options |= F_HDRINCL;
>   }
> 
> - if (options & F_RROUTE && options & F_HDRINCL)
> + if (options & (F_RROUTE | F_HDRINCL))
>   errx(1, "-R option and -D or -T, or -t to unicast"
>   " destinations are incompatible");
> 
> @@ -717,10 +717,8 @@ main(int argc, char *argv[])
>   else
>   ip->ip_src.s_addr = INADDR_ANY;
>   ip->ip_dst = dst4.sin_addr;
> - }
> -
>   /* record route option */
> - if (options & F_RROUTE) {
> + } else if (options & F_RROUTE) {
>   if (IN_MULTICAST(ntohl(dst4.sin_addr.s_addr)))
>   errx(1, "record route not valid to multicast"
>   " destinations");
> @@ -773,7 +771,7 @@ main(int argc, char *argv[])
>   (void)signal(SIGINT, onsignal);
>   (void)signal(SIGINFO, onsignal);
> 
> - if ((options & F_FLOOD) == 0) {
> + if (!(options & F_FLOOD)) {
>   (void)signal(SIGALRM, onsignal);
>   itimer.it_interval = interval;
>   itimer.it_value = interval;
> @@ -861,8 +859,8 @@ main(int argc, char *argv[])
>* a path MTU notification.)
>*/
>   if ((mtu = get_pathmtu(, )) > 0) {
> - if ((options & F_VERBOSE) != 0) {
> - printf("new path MTU (%d) is "
> + if (options & F_VERBOSE) {
> + (void)printf("new path MTU (%d) is "
>   "notified\n", mtu);
>   }
>   }
> @@ -961,7 +959,7 @@ pr_addr(struct sockaddr *addr, socklen_t
>   static char buf[NI_MAXHOST];
>   int flag = 0;
> 
> - if ((options & F_HOSTNAME) == 0)
> + if (!(options & F_HOSTNAME))
>   flag |= NI_NUMERICHOST;
> 
>   if (getnameinfo(addr, addrlen, buf, sizeof(buf), NULL, 0, flag) == 0)
> @@ -1892,7 +1890,7 @@ get_pathmtu(struct msghdr *mhdr, struct 
> 
> dst->sin6_scope_id &&
>   mtuctl->ip6m_addr.sin6_scope_id !=
>   dst->sin6_scope_id)) {
> - if ((options & F_VERBOSE) != 0) {
> + if (options & F_VERBOSE) {
>   printf("path MTU for %s is notified. "
>   "(ignored)\n",
>   pr_addr((struct sockaddr *)
> 

-- 
I'm not entirely sure you are real.

Re: ping: Style fixes/cleanups

[Florian Obser]

commited, thanks

On Tue, Jun 13, 2017 at 07:56:32PM +0200, Klemens Nanni wrote:
> The intent here is to get the highest multiple of four smaller or equal
> than i + 3. Instead of relying on integer division to get rid of the
> remainder just to "undo" everything, simply clear the lowest two bits
> (0b11 = 3) leaving multiples of four.
> 
> Index: ping.c
> ===
> RCS file: /cvs/src/sbin/ping/ping.c,v
> retrieving revision 1.218
> diff -u -p -r1.218 ping.c
> --- ping.c22 Feb 2017 13:43:35 -  1.218
> +++ ping.c13 Jun 2017 17:29:19 -
> @@ -1379,7 +1379,7 @@ pr_ipopt(int hlen, u_char *buf)
>   !memcmp(cp, old_rr, i) &&
>   !(options & F_FLOOD)) {
>   (void)printf("\t(same route)");
> - i = ((i + 3) / 4) * 4;
> + i = (i + 3) & ~0x3;
>   hlen -= i;
>   cp += i;
>   break;
> 


-- 
I'm not entirely sure you are real.

Re: ping: Style fixes/cleanups

[Florian Obser]

Meh, personal taste. Also the idea is to keep AF switches to a minimum
and use them as an indication that something important is going on.
I'd rather just read over the memset as something boring.

Maybe we are even lucky and compilers are smart enough to figure this
out on their own?

On Tue, Jun 13, 2017 at 07:51:00PM +0200, Klemens Nanni wrote:
> Do not clear the unneeded dst[46] structure.
> 
> Index: ping.c
> ===
> RCS file: /cvs/src/sbin/ping/ping.c,v
> retrieving revision 1.218
> diff -u -p -r1.218 ping.c
> --- ping.c22 Feb 2017 13:43:35 -  1.218
> +++ ping.c13 Jun 2017 17:32:13 -
> @@ -434,8 +434,10 @@ main(int argc, char *argv[])
>   if (argc != 1)
>   usage();
> 
> - memset(, 0, sizeof(dst4));
> - memset(, 0, sizeof(dst6));
> + if (v6flag)
> + memset(, 0, sizeof(dst6));
> + else
> + memset(, 0, sizeof(dst4));
> 
>   target = *argv;
> 
> 

-- 
I'm not entirely sure you are real.

Re: ping: Style fixes/cleanups

[Florian Obser]

commited, thanks!

On Tue, Jun 13, 2017 at 07:48:27PM +0200, Klemens Nanni wrote:
> On Tue, Jun 13, 2017 at 10:45:57AM -0600, Theo de Raadt wrote:
> >Sorry, but that type of diff is a no-go.  You've made a large variety
> >of different decisions on your own and mixed them up with ones which
> >are personal taste, and then touched so many lines of the code that
> >future study of historical changes in the code will be confusing.
> Points taken.
> 
> >The right approach is seperate changes into seperate catagories with
> >seperate explanations.  So I suggest you start with the ones of most
> >importance, and don't mix it with fluff (that's what we call it).
> Each of the following patches applies cleanly on its own.
> 
> Starting with this one replacing perror(3) with err(1).
> 
> 
> Index: ping.c
> ===
> RCS file: /cvs/src/sbin/ping/ping.c,v
> retrieving revision 1.218
> diff -u -p -r1.218 ping.c
> --- ping.c22 Feb 2017 13:43:35 -  1.218
> +++ ping.c13 Jun 2017 17:28:46 -
> @@ -729,10 +729,8 @@ main(int argc, char *argv[])
>   rspace[IPOPT_OLEN] = sizeof(rspace)-1;
>   rspace[IPOPT_OFFSET] = IPOPT_MINOFF;
>   if (setsockopt(s, IPPROTO_IP, IP_OPTIONS, rspace,
> - sizeof(rspace)) < 0) {
> - perror("ping: record route");
> - exit(1);
> - }
> + sizeof(rspace)) < 0)
> + err(1, "record route");
>   }
> 
>   if ((moptions & MULTICAST_NOLOOP) &&
> 


-- 
I'm not entirely sure you are real.

Re: CVS: cvs.openbsd.org: src

[Mark Kettenis]

> From: Frank Groeneveld 
> Date: Tue, 04 Jul 2017 09:38:18 +0200
> 
> On Mon, Jul 3, 2017, at 08:30, Martijn van Duren wrote:
> > This change *STILL* breaks my $DAYJOB machine.
> > 
> > dmesg with DRMDEBUG enabled
> 
> Maybe you shouldn't chose Apple hardware ;-)

Well.  That machine used to work.  Some change in the generic drm code
or my changes to radeondrm(4) made it stop working.  Hopefully the
problem can be reproduced and we can get some clues what broke.

Re: ifstated diff rename variables to avoid state confusion

[Rob Pierce]

On Mon, Jul 03, 2017 at 04:24:30PM -0400, Rob Pierce wrote:
> ifstated monitors interface state and the return state of invoked commands,
> and takes action accordingly, all of which is managed with the help of a
> finite state machine. That makes for a lot of "state" references in the code.
> 
> The following diff renames variables to make a distinction between link status
> and command execution status (now referenced as "status"), and the various 
> states
> within the state machine (referred to as "state").
> 
> As an example of the current confusion, the link_states() function in parse.y
> actually links states within the finite state machine (as oppose to having
> anything to do with interface link states).
> 
> This diff helps to make my head hurt less.
> 
> For your consideration.
> 
> Regards,
> 
> Rob

Ok, that was overkill. I would be happy with the following diff instead.

Rob

Index: ifstated.c
===
RCS file: /cvs/src/usr.sbin/ifstated/ifstated.c,v
retrieving revision 1.49
diff -u -p -r1.49 ifstated.c
--- ifstated.c  3 Jul 2017 18:45:34 -   1.49
+++ ifstated.c  4 Jul 2017 11:31:08 -
@@ -64,7 +64,7 @@ void  check_external_status(struct ifsd_
 void   external_evtimer_setup(struct ifsd_state *, int);
 void   scan_ifstate(int, int, int);
 intscan_ifstate_single(int, int, struct ifsd_state *);
-void   fetch_state(void);
+void   fetch_ifstate(void);
 __dead voidusage(void);
 void   adjust_expressions(struct ifsd_expression_list *, int);
 void   adjust_external_expressions(struct ifsd_state *);
@@ -208,7 +208,7 @@ load_config(void)
clear_config(conf);
conf = newconf;
conf->initstate.entered = time(NULL);
-   fetch_state();
+   fetch_ifstate();
external_evtimer_setup(>initstate, IFSD_EVTIMER_ADD);
adjust_external_expressions(>initstate);
eval_state(>initstate);
@@ -597,7 +597,7 @@ do_action(struct ifsd_action *action)
  * Fetch the current link states.
  */
 void
-fetch_state(void)
+fetch_ifstate(void)
 {
struct ifaddrs *ifap, *ifa;
char *oname = NULL;

Re: elf.h

[Martin Pieuchot]

Hello,

On 03/07/17(Mon) 18:58, Karel Gardas wrote:
> I'm curious if it's possible to provide /usr/include/elf.h file on OpenBSD to 
> improve its niceness to software porting from other Unixes. Following patch 
> adds this for me and is tested with GHC where I'd like to kill code like:
> 
> #if !defined(openbsd_HOST_OS)
> #  include 
> #else
> /* openbsd elf has things in different places, with diff names */
> #  include 
> #endif
> 
> I assume other apps dealing with ELF needs to do the similar thing for 
> OpenBSD support. I've checked and /usr/include/elf.h is available on Solaris 
> 10, Solaris 11, various Linuxes, NetBSD 7.x and FreeBSD 10.x. In patch below, 
> elf.h is a result of copy of elf_abi.h with small edit.

I think that moving towards  is a good thing.  However are you
sure that  provides all the definitions required by
?  Are you sure it doesn't provide any other definition that
would make code written on OpenBSD non portable?

What would it take to convert base programs to ?

If base starts to provide  you also need to make sure it doesn't
break any port.

I would welcome such move since it would make easier to port programs
from OpenBSD to other platforms.  However it's not as easy as including
a header in another.  Are you ready to tackle the above mentioned
issues?

ifstated parse.y removed unused tokens

[Rob Pierce]

These tokens have existed since version 1.1 but have never been used.

Can we delete them?

Rob

Index: parse.y
===
RCS file: /cvs/src/usr.sbin/ifstated/parse.y,v
retrieving revision 1.43
diff -u -p -r1.43 parse.y
--- parse.y 2 Jul 2017 15:28:26 -   1.43
+++ parse.y 4 Jul 2017 09:33:17 -
@@ -106,7 +106,7 @@ typedef struct {
 %}
 
 %token STATE INITSTATE
-%token LINK UP DOWN UNKNOWN ADDED REMOVED
+%token LINK UP DOWN UNKNOWN
 %token IF RUN SETSTATE EVERY INIT
 %left  AND OR
 %left  UNARY
@@ -390,14 +390,12 @@ lookup(char *s)
/* this has to be sorted always */
static const struct keywords keywords[] = {
{ "&&", AND},
-   { "added",  ADDED},
{ "down",   DOWN},
{ "every",  EVERY},
{ "if", IF},
{ "init",   INIT},
{ "init-state", INITSTATE},
{ "link",   LINK},
-   { "removed",REMOVED},
{ "run",RUN},
{ "set-state",  SETSTATE},
{ "state",  STATE},

Re: CVS: cvs.openbsd.org: src

[Frank Groeneveld]

On Mon, Jul 3, 2017, at 08:30, Martijn van Duren wrote:
> This change *STILL* breaks my $DAYJOB machine.
> 
> dmesg with DRMDEBUG enabled

Maybe you shouldn't chose Apple hardware ;-)

Works great here on a Thinkpad X260 Mark, thank you very much!

Frank