On Fri, 16 Nov 2018 16:15:14 +0100, Reyk Floeter <r...@openbsd.org> wrote:
> On Sat, Oct 27, 2018 at 02:53:16PM -0700, Ori Bernstein wrote:
> > On Fri, 26 Oct 2018 01:57:15 +0200, Reyk Floeter <r...@openbsd.org> wrote:
> >
> > > On Tue, Oct 23, 2018 at 10:21:08PM -0700, Ori Bernstein wrote:
> > > > On Mon, 8 Oct 2018 07:59:15 -0700, Bob Beck <b...@openbsd.org> wrote:
> > > >
> > > > > works here and I like it. but probably for after unlock
> > > > >
> > > >
> > > > It's after unlock -- pinging for OKs.
> > > >
> > >
> > > Not yet. Please include the VM's uid in the claim, e.g.
> > >
> > > claim_vmid(const char *name, uid_t uid)
> > >
> > > It is not a strong protection, but it doesn't make sense that other
> > > users can run a VM with the same name and get the claimed Id.
> > >
> > > Reyk
> > >
> >
> > Updated.
> >
> > Ok?
> >
>
> Sorry for the delay...
>
> Two minor things:
>
> - I think config_purge() should clear env->vmd_known as it does with
> vmd_vms and vmd_switches.
>
> - The use of static functions (static uint32_t vm_claimid) is disputable
> but somewhat uncommon in OpenBSD. But even static functions do need a
> prototype ("All functions are prototyped somewhere." - style(9)).
Done. Anyone want to give a second ok?
diff --git usr.sbin/vmd/config.c usr.sbin/vmd/config.c
index 20a16f85442..c662d329eb4 100644
--- usr.sbin/vmd/config.c
+++ usr.sbin/vmd/config.c
@@ -87,7 +87,10 @@ config_init(struct vmd *env)
if (what & CONFIG_VMS) {
if ((env->vmd_vms = calloc(1, sizeof(*env->vmd_vms))) == NULL)
return (-1);
+ if ((env->vmd_known = calloc(1, sizeof(*env->vmd_known))) ==
NULL)
+ return (-1);
TAILQ_INIT(env->vmd_vms);
+ TAILQ_INIT(env->vmd_known);
}
if (what & CONFIG_SWITCHES) {
if ((env->vmd_switches = calloc(1,
@@ -109,6 +112,7 @@ void
config_purge(struct vmd *env, unsigned int reset)
{
struct privsep *ps = &env->vmd_ps;
+ struct name2id *n2i;
struct vmd_vm *vm;
struct vmd_switch *vsw;
unsigned int what;
@@ -125,6 +129,10 @@ config_purge(struct vmd *env, unsigned int reset)
while ((vm = TAILQ_FIRST(env->vmd_vms)) != NULL) {
vm_remove(vm, __func__);
}
+ while ((n2i = TAILQ_FIRST(env->vmd_known)) != NULL) {
+ TAILQ_REMOVE(env->vmd_known, n2i, entry);
+ free(n2i);
+ }
env->vmd_nvm = 0;
}
if (what & CONFIG_SWITCHES && env->vmd_switches != NULL) {
diff --git usr.sbin/vmd/vmd.c usr.sbin/vmd/vmd.c
index 9423081df1e..5bb751511d0 100644
--- usr.sbin/vmd/vmd.c
+++ usr.sbin/vmd/vmd.c
@@ -62,6 +62,7 @@ int vmd_check_vmh(struct vm_dump_header *);
int vm_instance(struct privsep *, struct vmd_vm **,
struct vmop_create_params *, uid_t);
int vm_checkinsflag(struct vmop_create_params *, unsigned int, uid_t);
+uint32_t vm_claimid(const char *, int);
struct vmd *env;
@@ -1169,6 +1170,28 @@ vm_remove(struct vmd_vm *vm, const char *caller)
free(vm);
}
+uint32_t
+vm_claimid(const char *name, int uid)
+{
+ struct name2id *n2i = NULL;
+
+ TAILQ_FOREACH(n2i, env->vmd_known, entry)
+ if (strcmp(n2i->name, name) == 0 && n2i->uid == uid)
+ return n2i->id;
+
+ if (++env->vmd_nvm == 0)
+ fatalx("too many vms");
+ if ((n2i = calloc(1, sizeof(struct name2id))) == NULL)
+ fatalx("could not alloc vm name");
+ n2i->id = env->vmd_nvm;
+ n2i->uid = uid;
+ if (strlcpy(n2i->name, name, sizeof(n2i->name)) >= sizeof(n2i->name))
+ fatalx("overlong vm name");
+ TAILQ_INSERT_TAIL(env->vmd_known, n2i, entry);
+
+ return n2i->id;
+}
+
int
vm_register(struct privsep *ps, struct vmop_create_params *vmc,
struct vmd_vm **ret_vm, uint32_t id, uid_t uid)
@@ -1300,11 +1323,8 @@ vm_register(struct privsep *ps, struct
vmop_create_params *vmc,
vm->vm_cdrom = -1;
vm->vm_iev.ibuf.fd = -1;
- if (++env->vmd_nvm == 0)
- fatalx("too many vms");
-
/* Assign a new internal Id if not specified */
- vm->vm_vmid = id == 0 ? env->vmd_nvm : id;
+ vm->vm_vmid = (id == 0) ? vm_claimid(vcp->vcp_name, uid) : id;
log_debug("%s: registering vm %d", __func__, vm->vm_vmid);
TAILQ_INSERT_TAIL(env->vmd_vms, vm, vm_entry);
diff --git usr.sbin/vmd/vmd.h usr.sbin/vmd/vmd.h
index de87e4337ee..6000dd6d63f 100644
--- usr.sbin/vmd/vmd.h
+++ usr.sbin/vmd/vmd.h
@@ -290,6 +290,14 @@ struct vmd_user {
};
TAILQ_HEAD(userlist, vmd_user);
+struct name2id {
+ char name[VMM_MAX_NAME_LEN];
+ int uid;
+ int32_t id;
+ TAILQ_ENTRY(name2id) entry;
+};
+TAILQ_HEAD(name2idlist, name2id);
+
struct address {
struct sockaddr_storage ss;
int prefixlen;
@@ -319,6 +327,7 @@ struct vmd {
uint32_t vmd_nvm;
struct vmlist *vmd_vms;
+ struct name2idlist *vmd_known;
uint32_t vmd_nswitches;
struct switchlist *vmd_switches;
struct userlist *vmd_users;
--
Ori Bernstein
