VMD: Fix Failure Start

[Carlos Cardenas]

Howdy.

Attached is a patch to fix the following condition:
When attempting to start a VM from vm.conf that fails (can't allocate
resources, etc...), do not remove vm from vm list.

Reported to bugs@ by mpi@.

Comments? Ok?

+--+
Carlos
Index: config.c
===
RCS file: /home/los/cvs/src/usr.sbin/vmd/config.c,v
retrieving revision 1.40
diff -u -p -a -u -r1.40 config.c
--- config.c5 Jan 2018 13:34:52 -   1.40
+++ config.c25 Jan 2018 03:25:23 -
@@ -416,8 +416,13 @@ config_setvm(struct privsep *ps, struct 
free(tapfds);
}
 
-   log_debug("%s: calling vm_remove", __func__);
-   vm_remove(vm);
+   if (vm->vm_from_config) {
+   log_debug("%s: calling stop vm %d", __func__, vm->vm_vmid);
+   vm_stop(vm, 0);
+   } else {
+   log_debug("%s: calling remove vm %d", __func__, vm->vm_vmid);
+   vm_remove(vm);
+   }
errno = saved_errno;
if (errno == 0)
errno = EINVAL;

Re: bridge(4): protected interface (port)

[Tom Smyth]

Hello, Martin, Remi, All
Im very excited about this feature, Thanks Martin,
Please see Comments inline below

On 23 January 2018 at 18:06, Remi Locherer  wrote:
> On Mon, Jan 22, 2018 at 04:23:59PM +0100, Martin Pieuchot wrote:
>> Diff below adds a new feature to bridge(4), similar to Cisco's Protected
>> Port but with more possibilities.
>>
>> The idea is to prevent traffic to flow between some members of a bridge(4).
>> For example:
>>   - you want to prevent some of your servers to talk to each others, or
>>   - you want employees/students/clients to have access to internet and a
>> printer but not to each other, or
>>   - you want VMs to have access to an uplink but not to each other
>
> I think the last example is really useful and makes sense.
Agreed
>
> For the other cases there would most likely be a switch between the other
> host and the OpenBSD box. Then you need to enforce the policy on that switch.
Other examples where it would be useful if the OpenBSD box is terminating
alot of Tunnels and you want to limit broadcast domains.
>
>> With the diff below it is now possible to defined "protected groups".
>> Interface that are part of such groups cannot send/receive traffic
>> to/from any other member of the group.
>
> Why several protected groups and not just a single one?
Having multiple protected groups can be useful in the following circumstances:
Where you want to isolate clients / vms from each other,
and where you have 2 redundant up-link ports in the bridge that could other wise
create a loop.
(where spanning tree cannot be deployed (due to incompatible or
limited switches)
e.g
a) loop prevention in 2 up link ports on a bridge
so if the 2 up-link  ports are added to the same protected group they cannot
form a loop as the 2 up-links are isolated from each other
Uplink ports would be em1 and em2 and we would add them to protected group1
em1 and em2 cannot communicate with each other and cant create a loop
b)preventing vms from talking to each other by adding them to another protected
group
e.g.
tap1 and tap2  would be added to protected group 2
so tap1 and tap2 would be isolated from each other, and because they are members
of a different protected group to em1 and em2 in the same bridge they
can communicate
freely with either em1 or em2

I have used this technique on certain Layer 2 networks where STP was
not possible or
desirable,, and it would give the administrator granular control
of traffic flow  in the bridge
Other Added benefits
(control broadcast domains etc)
increase security  for access networks (prevent rogue dhcp Servers)

I hope this helps and Im really excited with this diff :)
Thanks Guys


>
>>
>> In the example below, em1 and em2 can only send/receive traffic through
>> em0 because they are both in the protected group 2.
>>
>> bridge0: flags=41
>> index 9 llprio 3
>> groups: bridge
>> priority 32768 hellotime 2 fwddelay 15 maxage 20 holdcnt 6 proto rstp
>> designated: id 00:00:00:00:00:00 priority 0
>> em0 flags=3
>> port 6 ifpriority 0 ifcost 0
>> em1 flags=3
>> port 7 ifpriority 0 ifcost 0 protected 2
>> em2 flags=3
>> port 8 ifpriority 0 ifcost 0 protected 2
>>
>> Adding an interface to a protected group is as simple as:
>>
>>   # ifconfig bridge0 protected em1 2
>>
>> Removing it:
>>
>>   # ifconfig bridge0 -protected em1
>>
>> Comments?  Oks?
>>
>> Index: sys/net/if_bridge.c
>> ===
>> RCS file: /cvs/src/sys/net/if_bridge.c,v
>> retrieving revision 1.301
>> diff -u -p -r1.301 if_bridge.c
>> --- sys/net/if_bridge.c   10 Jan 2018 23:50:39 -  1.301
>> +++ sys/net/if_bridge.c   22 Jan 2018 14:27:41 -
>> @@ -409,6 +409,7 @@ bridge_ioctl(struct ifnet *ifp, u_long c
>>   }
>>   req->ifbr_ifsflags = p->bif_flags;
>>   req->ifbr_portno = p->ifp->if_index & 0xfff;
>> + req->ifbr_protected = p->bif_protected;
>>   if (p->bif_flags & IFBIF_STP) {
>>   bp = p->bif_stp;
>>   req->ifbr_state = bstp_getstate(bs, bp);
>> @@ -496,6 +497,19 @@ bridge_ioctl(struct ifnet *ifp, u_long c
>>   brop->ifbop_last_tc_time.tv_sec = bs->bs_last_tc_time.tv_sec;
>>   brop->ifbop_last_tc_time.tv_usec = bs->bs_last_tc_time.tv_usec;
>>   break;
>> + case SIOCBRDGSIFPROT:
>> + ifs = ifunit(req->ifbr_ifsname);
>> + if (ifs == NULL) {
>> + error = ENOENT;
>> + break;
>> + }
>> + p = (struct bridge_iflist *)ifs->if_bridgeport;
>> + if (p == NULL || p->bridge_sc != sc) {
>> + error = ESRCH;
>> + break;
>> + }
>> + p->bif_protected = 

Re: awk: better support for \v and \a escapes

[Jeremie Courreges-Anglas]

On Tue, Jan 23 2018, "Todd C. Miller"  wrote:
> POSIX says awk supports \v and \a escapes but ours does not.
> I used \007 for BEL since that is what awk's lex.c uses, though we
> could safely use '\a' there instead.

ok jca@

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

Re: malloc optimizations wrt junking and freezero

[Otto Moerbeek]

On Sun, Jan 21, 2018 at 11:50:24AM +0100, Otto Moerbeek wrote:

> Hi,
> 
> chunk pages do not need to be junked, all allocated chunks are already
> junked on free. Same hold for a few error paths.
> 
> Also, for freezero(), only clear the size as reqeusted instead of the
> whole allocation.
> 
> Please review/test,

No feedback at all

Anyone? Otherwise this goes to "test by commit" mode. Still it would
be a shame if I made a mistake in the freezero part and bytes that
should be won't be cleared anymore.

-Otto

> 
> Index: malloc.c
> ===
> RCS file: /cvs/src/lib/libc/stdlib/malloc.c,v
> retrieving revision 1.241
> diff -u -p -r1.241 malloc.c
> --- malloc.c  18 Jan 2018 20:06:16 -  1.241
> +++ malloc.c  20 Jan 2018 20:46:57 -
> @@ -633,7 +633,7 @@ delete(struct dir_info *d, struct region
>   * cache are in MALLOC_PAGESIZE units.
>   */
>  static void
> -unmap(struct dir_info *d, void *p, size_t sz, int clear)
> +unmap(struct dir_info *d, void *p, size_t sz, size_t clear, int junk)
>  {
>   size_t psz = sz >> MALLOC_PAGESHIFT;
>   size_t rsz;
> @@ -650,7 +650,7 @@ unmap(struct dir_info *d, void *p, size_
>* to unmap is larger than the cache size or we're clearing and the
>* cache is full, just munmap
>*/
> - if (psz > mopts.malloc_cache || (clear && rsz == 0)) {
> + if (psz > mopts.malloc_cache || (clear > 0 && rsz == 0)) {
>   i = munmap(p, sz);
>   if (i)
>   wrterror(d, "munmap %p", p);
> @@ -686,11 +686,10 @@ unmap(struct dir_info *d, void *p, size_
>   for (i = 0; ; i++) {
>   r = >free_regions[(i + offset) & mask];
>   if (r->p == NULL) {
> - if (clear)
> - memset(p, 0, sz - mopts.malloc_guard);
> - if (mopts.malloc_junk && !mopts.malloc_freeunmap) {
> - size_t amt = mopts.malloc_junk == 1 ?
> - MALLOC_MAXCHUNK : sz;
> + if (clear > 0)
> + memset(p, 0, clear);
> + if (junk && !mopts.malloc_freeunmap) {
> + size_t amt = junk == 1 ?  MALLOC_MAXCHUNK : sz;
>   memset(p, SOME_FREEJUNK, amt);
>   }
>   if (mopts.malloc_freeunmap)
> @@ -888,7 +887,7 @@ omalloc_make_chunks(struct dir_info *d, 
>   return bp;
>  
>  err:
> - unmap(d, pp, MALLOC_PAGESIZE, 0);
> + unmap(d, pp, MALLOC_PAGESIZE, 0, mopts.malloc_junk);
>   return NULL;
>  }
>  
> @@ -1087,7 +1086,7 @@ free_bytes(struct dir_info *d, struct re
>  
>   if (info->size == 0 && !mopts.malloc_freeunmap)
>   mprotect(info->page, MALLOC_PAGESIZE, PROT_READ | PROT_WRITE);
> - unmap(d, info->page, MALLOC_PAGESIZE, 0);
> + unmap(d, info->page, MALLOC_PAGESIZE, 0, 0);
>  
>   delete(d, r);
>   if (info->size != 0)
> @@ -1118,7 +1117,7 @@ omalloc(struct dir_info *pool, size_t sz
>   return NULL;
>   }
>   if (insert(pool, p, sz, f)) {
> - unmap(pool, p, psz, 0);
> + unmap(pool, p, psz, 0, 0);
>   errno = ENOMEM;
>   return NULL;
>   }
> @@ -1309,8 +1308,7 @@ ofree(struct dir_info *argpool, void *p,
>   uint32_t chunknum =
>   find_chunknum(pool, info, p, 0);
>  
> - if (info->bits[info->offset + chunknum] <
> - argsz)
> + if (info->bits[info->offset + chunknum] < argsz)
>   wrterror(pool, "recorded size %hu"
>   " < %zu",
>   info->bits[info->offset + chunknum],
> @@ -1350,7 +1348,8 @@ ofree(struct dir_info *argpool, void *p,
>   }
>   STATS_SUB(pool->malloc_guarded, mopts.malloc_guard);
>   }
> - unmap(pool, p, PAGEROUND(sz), clear);
> + unmap(pool, p, PAGEROUND(sz), clear ? argsz : 0,
> + mopts.malloc_junk);
>   delete(pool, r);
>   } else {
>   /* Validate and optionally canary check */
> @@ -1376,8 +1375,8 @@ ofree(struct dir_info *argpool, void *p,
>   pool->delayed_chunks[i] = tmp;
>   if (mopts.malloc_junk)
>   validate_junk(pool, p);
> - } else if (sz > 0)
> - memset(p, 0, sz);
> + } else if (argsz > 0)
> + memset(p, 0, argsz);
>   if (p != NULL) {
>   r = find(pool, p);
>   if (r == NULL)
> @@ -1575,7 +1574,8 @@ gotit:
>

iwn/iwm: ignore missed beacons during background scan

[Stefan Sperling]

I just observed iwn(4) firmware reporting a missed beacon event
during a background scan:

Jan 24 18:24:50 laptop /bsd: iwn0: begin background scan
Jan 24 18:24:57 laptop /bsd: iwn0: sending probe_req to xx:xx:xx:xx:xx:xx on 
channel 10 mode 11g
Jan 24 18:24:59 laptop /bsd: iwn0: end background scan

Under normal conditions, a missed beacon event triggers a probe request
being sent to our current AP. If the AP does respond, all is good. If it
does not respond, we assume the AP has gone away and we go to SCAN state.

But in the above case the driver lost link and didn't recover (i.e. it
never came back out of SCAN state).

One obvious solution is to ignore missed beacon events while a
background scan is in progress. This should prevent both of these
mechanisms from interacting with each other in unfortunate ways.

ok?

Index: if_iwm.c
===
RCS file: /cvs/src/sys/dev/pci/if_iwm.c,v
retrieving revision 1.223
diff -u -p -r1.223 if_iwm.c
--- if_iwm.c14 Jan 2018 11:51:34 -  1.223
+++ if_iwm.c24 Jan 2018 17:27:08 -
@@ -3673,6 +3673,9 @@ iwm_rx_bmiss(struct iwm_softc *sc, struc
(ic->ic_state != IEEE80211_S_RUN))
return;
 
+   if (sc->sc_flags & IWM_FLAG_BGSCAN)
+   return;
+
bus_dmamap_sync(sc->sc_dmat, data->map, sizeof(*pkt),
sizeof(*mbn), BUS_DMASYNC_POSTREAD);
 
Index: if_iwn.c
===
RCS file: /cvs/src/sys/dev/pci/if_iwn.c,v
retrieving revision 1.198
diff -u -p -r1.198 if_iwn.c
--- if_iwn.c9 Jan 2018 10:00:12 -   1.198
+++ if_iwn.c24 Jan 2018 17:26:24 -
@@ -2530,6 +2530,9 @@ iwn_notif_intr(struct iwn_softc *sc)
(ic->ic_state != IEEE80211_S_RUN))
break;
 
+   if (ic->ic_flags & IWN_FLAG_BGSCAN)
+   break;
+
bus_dmamap_sync(sc->sc_dmat, data->map, sizeof (*desc),
sizeof (*miss), BUS_DMASYNC_POSTREAD);
missed = letoh32(miss->consecutive);

Re: ftp: use closefrom instead of close(pid)

[Theo de Raadt]

Sure.

But always be careful that the fd loop isn't bounded for a reason,
for instance some fd at > 20 that it wanted left open.

> This variable reuse has a funny smell.
> 
> Index: usr.bin/ftp/cmds.c
> ===
> RCS file: /var/cvs/src/usr.bin/ftp/cmds.c,v
> retrieving revision 1.79
> diff -u -p -r1.79 cmds.c
> --- usr.bin/ftp/cmds.c21 Jan 2017 08:33:07 -  1.79
> +++ usr.bin/ftp/cmds.c23 Jan 2018 08:40:32 -
> @@ -987,8 +987,7 @@ shell(int argc, char *argv[])
>   old1 = signal (SIGINT, SIG_IGN);
>   old2 = signal (SIGQUIT, SIG_IGN);
>   if ((pid = fork()) == 0) {
> - for (pid = 3; pid < 20; pid++)
> - (void)close(pid);
> + (void)closefrom(3);
>   (void)signal(SIGINT, SIG_DFL);
>   (void)signal(SIGQUIT, SIG_DFL);
>   shellp = getenv("SHELL");
> 

ksh: bug with quoted parameter expansion

[mail]

I found a bug in ksh's parameter expansion on an edge case:

true $(true "${USER#'"'}")

/home/sh[4]: no closing quote

The problem seems to occurs when all of these conditions are present:

1. On ${var#pattern} or ${var%pattern} parameter expansion
2. When the pattern contains a singly quoted double quote: '"'
3. While expansion occurs withing $(...), (but not `...`)
4. While the expansion is quoted: "${var#pattern}".

true can be replaced by other commands or var=...
if '"' gets replaced by \", the issue disappear

The bin/ksh/lex.c seems to use a big switch+goto table with one label
per grammar context.  Maybe by jumping from context to context in a
specific pattern like above is producing the issue.

I did try to figure out where, but I lack time for now.

I hope the problem does not come from my way to test it and that it
have not been fixed.

$ uname -a  # also tested with /bin/ksh
OpenBSD t470s 6.2 GENERIC.MP#134 amd64

$ cvs co bin/ksh/; cd bin/ksh; make
[...]

$ ./ksh ~/sh
/home/sh[4]: no closing quote

Re: ftp: use closefrom instead of close(pid)

[Jeremie Courreges-Anglas]

On Wed, Jan 24 2018, Theo Buehler  wrote:
> This variable reuse has a funny smell.

ok

> Index: usr.bin/ftp/cmds.c
> ===
> RCS file: /var/cvs/src/usr.bin/ftp/cmds.c,v
> retrieving revision 1.79
> diff -u -p -r1.79 cmds.c
> --- usr.bin/ftp/cmds.c21 Jan 2017 08:33:07 -  1.79
> +++ usr.bin/ftp/cmds.c23 Jan 2018 08:40:32 -
> @@ -987,8 +987,7 @@ shell(int argc, char *argv[])
>   old1 = signal (SIGINT, SIG_IGN);
>   old2 = signal (SIGQUIT, SIG_IGN);
>   if ((pid = fork()) == 0) {
> - for (pid = 3; pid < 20; pid++)
> - (void)close(pid);
> + (void)closefrom(3);
>   (void)signal(SIGINT, SIG_DFL);
>   (void)signal(SIGQUIT, SIG_DFL);
>   shellp = getenv("SHELL");
>

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

ftp: use closefrom instead of close(pid)

[Theo Buehler]

This variable reuse has a funny smell.

Index: usr.bin/ftp/cmds.c
===
RCS file: /var/cvs/src/usr.bin/ftp/cmds.c,v
retrieving revision 1.79
diff -u -p -r1.79 cmds.c
--- usr.bin/ftp/cmds.c  21 Jan 2017 08:33:07 -  1.79
+++ usr.bin/ftp/cmds.c  23 Jan 2018 08:40:32 -
@@ -987,8 +987,7 @@ shell(int argc, char *argv[])
old1 = signal (SIGINT, SIG_IGN);
old2 = signal (SIGQUIT, SIG_IGN);
if ((pid = fork()) == 0) {
-   for (pid = 3; pid < 20; pid++)
-   (void)close(pid);
+   (void)closefrom(3);
(void)signal(SIGINT, SIG_DFL);
(void)signal(SIGQUIT, SIG_DFL);
shellp = getenv("SHELL");

Re: carp_ourether() tweak

[Alexander Bluhm]

On Mon, Jan 22, 2018 at 11:58:30AM +0100, Martin Pieuchot wrote:
> Check if `if_carp' is empty inside carp_ourether() instead of outside. 
> 
> ok?

Maybe I am confused by the ! and && but I think this diff changes the
logic.

Old code:

- if_carp is empty
- SRPL_EMPTY_LOCKED(>ifp->if_carp) is true
- !SRPL_EMPTY_LOCKED(>ifp->if_carp) is false
- (!SRPL_EMPTY_LOCKED(>ifp->if_carp) &&
  !carp_ourether(ifl->ifp, eh->ether_dhost)) is false
- if block is not executed

New Code:
- if_carp is empty
- SRPL_EMPTY_LOCKED(>ifp->if_carp) is true
- carp_ourether() returns 0
- carp_ourether(ifl->ifp, eh->ether_dhost) is false
- !carp_ourether(ifl->ifp, eh->ether_dhost) is true
- if block is executed

Is this modification of behavior intensional?

bluhm

> Index: net/if_bridge.c
> ===
> RCS file: /cvs/src/sys/net/if_bridge.c,v
> retrieving revision 1.301
> diff -u -p -r1.301 if_bridge.c
> --- net/if_bridge.c   10 Jan 2018 23:50:39 -  1.301
> +++ net/if_bridge.c   22 Jan 2018 10:54:48 -
> @@ -1108,8 +1108,7 @@ bridge_process(struct ifnet *ifp, struct
>   ac = (struct arpcom *)ifl->ifp;
>   if (bcmp(ac->ac_enaddr, eh->ether_dhost, ETHER_ADDR_LEN) == 0
>  #if NCARP > 0
> - || (!SRPL_EMPTY_LOCKED(>ifp->if_carp) &&
> - !carp_ourether(ifl->ifp, eh->ether_dhost))
> + || !carp_ourether(ifl->ifp, eh->ether_dhost)
>  #endif
>   ) {
>   if (srcifl->bif_flags & IFBIF_LEARNING)
> @@ -1131,8 +1130,7 @@ bridge_process(struct ifnet *ifp, struct
>   }
>   if (bcmp(ac->ac_enaddr, eh->ether_shost, ETHER_ADDR_LEN) == 0
>  #if NCARP > 0
> - || (!SRPL_EMPTY_LOCKED(>ifp->if_carp) &&
> - !carp_ourether(ifl->ifp, eh->ether_shost))
> + || !carp_ourether(ifl->ifp, eh->ether_shost)
>  #endif
>   ) {
>   m_freem(m);
> Index: netinet/ip_carp.c
> ===
> RCS file: /cvs/src/sys/netinet/ip_carp.c,v
> retrieving revision 1.327
> diff -u -p -r1.327 ip_carp.c
> --- netinet/ip_carp.c 12 Jan 2018 23:47:24 -  1.327
> +++ netinet/ip_carp.c 22 Jan 2018 10:53:35 -
> @@ -1339,12 +1339,15 @@ carp_iamatch(struct ifnet *ifp)
>  int
>  carp_ourether(struct ifnet *ifp, u_int8_t *ena)
>  {
> - struct srpl *cif;
> + struct srpl *cif = >if_carp;
>   struct carp_softc *vh;
>  
>   KERNEL_ASSERT_LOCKED(); /* touching if_carp + carp_vhosts */
> +
> + if (SRPL_EMPTY_LOCKED(cif))
> + return (0);
> +
>   KASSERT(ifp->if_type == IFT_ETHER);
> - cif = >if_carp;
>  
>   SRPL_FOREACH_LOCKED(vh, cif, sc_list) {
>   struct carp_vhost_entry *vhe;