As mentioned before on bugs@ [1], the custom local prefix feature of vmd broke with some of the recent changes. Since fixing this touches a lot of parts of vmd, I'm breaking it up to make it reviewable by others and hopefully less daunting.
This diff fixes an existing issue (predating recent changes) with how the vm.conf "local prefix" and "local inet6 prefix" are validated by re-using the runtime validation logic at config parse time to catch issues early at startup or reload. Prior to this, someone could provide a local prefix in vm.conf like: local prefix 10.10.0.0/10 But they would only get an error when starting a vm with a local interface instead of vm.conf parse time. The beauty of this change is not only do we keep the validation centralized, but the refactor of passing pointers to struct address simplifies the next diff :) OK? -dv [1] https://marc.info/?l=openbsd-bugs&m=168377086611788&w=2 diff 6a9ebb2fe80ab5d5e37c3efa9a90826539df8d1b 2561c36aebbb7e0d4759a9185eb003d5c960cf99 commit - 6a9ebb2fe80ab5d5e37c3efa9a90826539df8d1b commit + 2561c36aebbb7e0d4759a9185eb003d5c960cf99 blob - 3b8744ffe50f24d24eb51085c5291a91cd8ea983 blob + 8fbd084a0632d8ca9f2c8dba3100540de60c232c --- usr.sbin/vmd/dhcp.c +++ usr.sbin/vmd/dhcp.c @@ -147,7 +147,7 @@ dhcp_request(struct virtio_dev *dev, char *buf, size_t } if ((client_addr.s_addr = - vm_priv_addr(&env->vmd_cfg, + vm_priv_addr(&env->vmd_cfg.cfg_localprefix, dev->vm_vmid, vionet->idx, 1)) == 0) return (-1); memcpy(&resp.yiaddr, &client_addr, @@ -156,8 +156,8 @@ dhcp_request(struct virtio_dev *dev, char *buf, size_t sizeof(client_addr)); ss2sin(&pc.pc_dst)->sin_port = htons(CLIENT_PORT); - if ((server_addr.s_addr = vm_priv_addr(&env->vmd_cfg, dev->vm_vmid, - vionet->idx, 0)) == 0) + if ((server_addr.s_addr = vm_priv_addr(&env->vmd_cfg.cfg_localprefix, + dev->vm_vmid, vionet->idx, 0)) == 0) return (-1); memcpy(&resp.siaddr, &server_addr, sizeof(server_addr)); memcpy(&ss2sin(&pc.pc_src)->sin_addr, &server_addr, blob - 09468e3fe2c9f4f9193710c65667132f79a90df3 blob + 1970016c324a512a0c825745a5f04033555339ef --- usr.sbin/vmd/parse.y +++ usr.sbin/vmd/parse.y @@ -192,29 +192,28 @@ main : LOCAL INET6 { struct address h; if (host($4, &h) == -1 || - h.ss.ss_family != AF_INET6 || - h.prefixlen > 64 || h.prefixlen < 0) { + vm_priv_addr6(&h, 0, 0, 1, NULL) == 0) { yyerror("invalid local inet6 prefix: %s", $4); - free($4); YYERROR; + } else { + env->vmd_cfg.cfg_flags |= VMD_CFG_INET6; + env->vmd_cfg.cfg_flags &= ~VMD_CFG_AUTOINET6; + memcpy(&env->vmd_cfg.cfg_localprefix6, &h, + sizeof(h)); } - - env->vmd_cfg.cfg_flags |= VMD_CFG_INET6; - env->vmd_cfg.cfg_flags &= ~VMD_CFG_AUTOINET6; - memcpy(&env->vmd_cfg.cfg_localprefix6, &h, sizeof(h)); + free($4); } | LOCAL PREFIX STRING { struct address h; if (host($3, &h) == -1 || - h.ss.ss_family != AF_INET || - h.prefixlen > 32 || h.prefixlen < 0) { + vm_priv_addr(&h, 0, 0, 1) == 0) { yyerror("invalid local prefix: %s", $3); - free($3); YYERROR; - } - - memcpy(&env->vmd_cfg.cfg_localprefix, &h, sizeof(h)); + } else + memcpy(&env->vmd_cfg.cfg_localprefix, &h, + sizeof(h)); + free($3); } | SOCKET OWNER owner_id { env->vmd_ps.ps_csock.cs_uid = $3.uid; blob - a7a7a2bc2280ea85ed486d65a45ef77b2d04a8b5 blob + 6d852813dfcbc47934b25f207ec8b322f7e58014 --- usr.sbin/vmd/priv.c +++ usr.sbin/vmd/priv.c @@ -466,7 +466,7 @@ vm_priv_ifconfig(struct privsep *ps, struct vmd_vm *vm sin4->sin_family = AF_INET; sin4->sin_len = sizeof(*sin4); if ((sin4->sin_addr.s_addr = - vm_priv_addr(&env->vmd_cfg, + vm_priv_addr(&env->vmd_cfg.cfg_localprefix, vm->vm_vmid, i, 0)) == 0) return (-1); @@ -493,7 +493,7 @@ vm_priv_ifconfig(struct privsep *ps, struct vmd_vm *vm sin6 = ss2sin6(&vfr.vfr_addr); sin6->sin6_family = AF_INET6; sin6->sin6_len = sizeof(*sin6); - if (vm_priv_addr6(&env->vmd_cfg, + if (vm_priv_addr6(&env->vmd_cfg.cfg_localprefix6, vm->vm_vmid, i, 0, &sin6->sin6_addr) == -1) return (-1); @@ -565,9 +565,8 @@ vm_priv_addr(struct vmd_config *cfg, uint32_t vmid, in } uint32_t -vm_priv_addr(struct vmd_config *cfg, uint32_t vmid, int idx, int isvm) +vm_priv_addr(struct address *h, uint32_t vmid, int idx, int isvm) { - struct address *h = &cfg->cfg_localprefix; in_addr_t prefix, mask, addr; /* @@ -602,7 +601,7 @@ vm_priv_addr(struct vmd_config *cfg, uint32_t vmid, in * - up to 126 interfaces can be encoded per VM. */ if (prefix != (addr & mask) || idx >= 0x7f) { - log_warnx("%s: dhcp address range exceeded," + log_debug("%s: dhcp address range exceeded," " vm id %u interface %d", __func__, vmid, idx); return (0); } @@ -611,10 +610,9 @@ vm_priv_addr6(struct vmd_config *cfg, uint32_t vmid, } int -vm_priv_addr6(struct vmd_config *cfg, uint32_t vmid, +vm_priv_addr6(struct address *h, uint32_t vmid, int idx, int isvm, struct in6_addr *in6_addr) { - struct address *h = &cfg->cfg_localprefix6; struct in6_addr addr, mask; uint32_t addr4; @@ -626,7 +624,7 @@ vm_priv_addr6(struct vmd_config *cfg, uint32_t vmid, prefixlen2mask6(h->prefixlen, &mask); /* 2. Encode the VM IPv4 address as subnet, fd00::NN:NN:0:0/96. */ - if ((addr4 = vm_priv_addr(cfg, vmid, idx, 1)) == 0) + if ((addr4 = vm_priv_addr(h, vmid, idx, 1)) == 0) return (0); memcpy(&addr.s6_addr[8], &addr4, sizeof(addr4)); @@ -640,7 +638,8 @@ vm_priv_addr6(struct vmd_config *cfg, uint32_t vmid, else addr.s6_addr[15] = 2; - memcpy(in6_addr, &addr, sizeof(*in6_addr)); + if (in6_addr != NULL) + memcpy(in6_addr, &addr, sizeof(*in6_addr)); return (0); } blob - 9c25b0c92ade2e1a1d3a1f67548becfc3d4eca7b blob + bf071105b7a16d4a9b447f2ab39823613e6251ea --- usr.sbin/vmd/vmd.h +++ usr.sbin/vmd/vmd.h @@ -473,8 +473,8 @@ uint32_t vm_priv_addr(struct vmd_config *, uint32_t, i int priv_validgroup(const char *); int vm_priv_ifconfig(struct privsep *, struct vmd_vm *); int vm_priv_brconfig(struct privsep *, struct vmd_switch *); -uint32_t vm_priv_addr(struct vmd_config *, uint32_t, int, int); -int vm_priv_addr6(struct vmd_config *, uint32_t, int, int, +uint32_t vm_priv_addr(struct address *, uint32_t, int, int); +int vm_priv_addr6(struct address *, uint32_t, int, int, struct in6_addr *); /* vmm.c */
