Re: vmd close stdin
On Mon, Sep 10, 2018 at 01:25:24AM +0200, Alexander Bluhm wrote:
> Hi,
>
> vmd(8) may close file descriptor 0 as not all fd fields are properly
> initialized with -1. While there avoid closing -1.
>
> ok?
>
ok mlarkin
> bluhm
>
> Index: usr.sbin/vmd/vmd.c
> ===
> RCS file: /data/mirror/openbsd/cvs/src/usr.sbin/vmd/vmd.c,v
> retrieving revision 1.98
> diff -u -p -r1.98 vmd.c
> --- usr.sbin/vmd/vmd.c15 Jul 2018 14:36:54 - 1.98
> +++ usr.sbin/vmd/vmd.c9 Sep 2018 23:13:50 -
> @@ -1248,11 +1249,11 @@ vm_register(struct privsep *ps, struct v
> vm->vm_paused = 0;
> vm->vm_user = usr;
>
> - for (i = 0; i < vcp->vcp_ndisks; i++)
> + for (i = 0; i < VMM_MAX_DISKS_PER_VM; i++)
> vm->vm_disks[i] = -1;
> - for (i = 0; i < vcp->vcp_nnics; i++) {
> + for (i = 0; i < VMM_MAX_NICS_PER_VM; i++)
> vm->vm_ifs[i].vif_fd = -1;
> -
> + for (i = 0; i < vcp->vcp_nnics; i++) {
> if ((sw = switch_getbyname(vmc->vmc_ifswitch[i])) != NULL) {
> /* inherit per-interface flags from the switch */
> vmc->vmc_ifflags[i] |= (sw->sw_flags & VMIFF_OPTMASK);
> Index: usr.sbin/vmd/vmm.c
> ===
> RCS file: /data/mirror/openbsd/cvs/src/usr.sbin/vmd/vmm.c,v
> retrieving revision 1.88
> diff -u -p -r1.88 vmm.c
> --- usr.sbin/vmd/vmm.c13 Jul 2018 08:42:49 - 1.88
> +++ usr.sbin/vmd/vmm.c9 Sep 2018 23:13:04 -
> @@ -646,20 +646,22 @@ vmm_start_vm(struct imsg *imsg, uint32_t
> close(vm->vm_disks[i]);
> vm->vm_disks[i] = -1;
> }
> -
> for (i = 0 ; i < vcp->vcp_nnics; i++) {
> close(vm->vm_ifs[i].vif_fd);
> vm->vm_ifs[i].vif_fd = -1;
> }
> -
> - close(vm->vm_kernel);
> - vm->vm_kernel = -1;
> -
> - close(vm->vm_cdrom);
> - vm->vm_cdrom = -1;
> -
> - close(vm->vm_tty);
> - vm->vm_tty = -1;
> + if (vm->vm_kernel != -1) {
> + close(vm->vm_kernel);
> + vm->vm_kernel = -1;
> + }
> + if (vm->vm_cdrom != -1) {
> + close(vm->vm_cdrom);
> + vm->vm_cdrom = -1;
> + }
> + if (vm->vm_tty != -1) {
> + close(vm->vm_tty);
> + vm->vm_tty = -1;
> + }
>
> /* read back the kernel-generated vm id from the child */
> if (read(fds[0], >vcp_id, sizeof(vcp->vcp_id)) !=
>
Re: if_cloners list is poulated at system boot only
Hello,
On Sun, Sep 09, 2018 at 10:50:08AM +0200, Alexander Bluhm wrote:
> On Sun, Sep 09, 2018 at 08:41:07AM +0200, Alexandr Nedvedicky wrote:
> > void
> > if_clone_attach(struct if_clone *ifc)
> > {
> > - rw_enter_write(_cloners_lock);
> > LIST_INSERT_HEAD(_cloners, ifc, ifc_list);
> > if_cloners_count++;
> > - rw_exit_write(_cloners_lock);
> > -}
>
> Could you put comment here, that the function is only called during
> boot so no lock is needed? I would prefer an assert to enforce
> that, but I don't know any that is suited.
>
> With comment, OK bluhm@
patch below adds assert... I still need to add a comment.
8<---8<---8<--8<
diff --git a/sys/kern/init_main.c b/sys/kern/init_main.c
index aa4c0a83ef7..a4c7efbd5ef 100644
--- a/sys/kern/init_main.c
+++ b/sys/kern/init_main.c
@@ -174,6 +174,9 @@ struct emul emul_native = {
sigcoderet
};
+#ifdef DIAGNOSTIC
+int pdevinit_done = 0;
+#endif
/*
* System startup; initialize the world, create process 0, mount root
@@ -401,6 +404,9 @@ main(void *framep)
for (pdev = pdevinit; pdev->pdev_attach != NULL; pdev++)
if (pdev->pdev_count > 0)
(*pdev->pdev_attach)(pdev->pdev_count);
+#ifdef DIAGNOSTIC
+ pdevinit_done = 1;
+#endif
#ifdef CRYPTO
crypto_init();
diff --git a/sys/net/if.c b/sys/net/if.c
index 0a6c4157ff9..d0228f73bcb 100644
--- a/sys/net/if.c
+++ b/sys/net/if.c
@@ -129,6 +129,8 @@
#include
#endif
+#include
+
void if_attachsetup(struct ifnet *);
void if_attachdomain(struct ifnet *);
void if_attach_common(struct ifnet *);
@@ -219,8 +221,6 @@ voidif_idxmap_remove(struct ifnet *);
TAILQ_HEAD(, ifg_group) ifg_head = TAILQ_HEAD_INITIALIZER(ifg_head);
-/* Serialize access to _cloners and if_cloners_count */
-struct rwlock if_cloners_lock = RWLOCK_INITIALIZER("ifclonerslk");
LIST_HEAD(, if_clone) if_cloners = LIST_HEAD_INITIALIZER(if_cloners);
int if_cloners_count;
@@ -1252,13 +1252,11 @@ if_clone_lookup(const char *name, int *unitp)
if (cp - name < IFNAMSIZ-1 && *cp == '0' && cp[1] != '\0')
return (NULL); /* unit number 0 padded */
- rw_enter_read(_cloners_lock);
LIST_FOREACH(ifc, _cloners, ifc_list) {
if (strlen(ifc->ifc_name) == cp - name &&
!strncmp(name, ifc->ifc_name, cp - name))
break;
}
- rw_exit_read(_cloners_lock);
if (ifc == NULL)
return (NULL);
@@ -1284,22 +1282,9 @@ if_clone_lookup(const char *name, int *unitp)
void
if_clone_attach(struct if_clone *ifc)
{
- rw_enter_write(_cloners_lock);
+ KASSERT(pdevinit_done == 0);
LIST_INSERT_HEAD(_cloners, ifc, ifc_list);
if_cloners_count++;
- rw_exit_write(_cloners_lock);
-}
-
-/*
- * Unregister a network interface cloner.
- */
-void
-if_clone_detach(struct if_clone *ifc)
-{
- rw_enter_write(_cloners_lock);
- LIST_REMOVE(ifc, ifc_list);
- if_cloners_count--;
- rw_exit_write(_cloners_lock);
}
/*
@@ -1314,17 +1299,13 @@ if_clone_list(struct if_clonereq *ifcr)
if ((dst = ifcr->ifcr_buffer) == NULL) {
/* Just asking how many there are. */
- rw_enter_read(_cloners_lock);
ifcr->ifcr_total = if_cloners_count;
- rw_exit_read(_cloners_lock);
return (0);
}
if (ifcr->ifcr_count < 0)
return (EINVAL);
- rw_enter_read(_cloners_lock);
-
ifcr->ifcr_total = if_cloners_count;
count = MIN(if_cloners_count, ifcr->ifcr_count);
@@ -1340,7 +1321,6 @@ if_clone_list(struct if_clonereq *ifcr)
dst += IFNAMSIZ;
}
- rw_exit_read(_cloners_lock);
return (error);
}
diff --git a/sys/net/if_var.h b/sys/net/if_var.h
index e9f69c964cb..e59f3ba9701 100644
--- a/sys/net/if_var.h
+++ b/sys/net/if_var.h
@@ -328,7 +328,6 @@ voidifafree(struct ifaddr *);
intif_isconnected(const struct ifnet *, unsigned int);
void if_clone_attach(struct if_clone *);
-void if_clone_detach(struct if_clone *);
intif_clone_create(const char *, int);
intif_clone_destroy(const char *);
diff --git a/sys/sys/device.h b/sys/sys/device.h
index 00a1f6ad2a6..56ea6fdafa9 100644
--- a/sys/sys/device.h
+++ b/sys/sys/device.h
@@ -164,6 +164,11 @@ struct pdevinit {
};
#ifdef _KERNEL
+
+#ifdef DIAGNOSTIC
+extern int pdevinit_done;
+#endif
+
extern struct devicelist alldevs; /* list of all devices */
extern int autoconf_verbose;
vmd close stdin
Hi,
vmd(8) may close file descriptor 0 as not all fd fields are properly
initialized with -1. While there avoid closing -1.
ok?
bluhm
Index: usr.sbin/vmd/vmd.c
===
RCS file: /data/mirror/openbsd/cvs/src/usr.sbin/vmd/vmd.c,v
retrieving revision 1.98
diff -u -p -r1.98 vmd.c
--- usr.sbin/vmd/vmd.c 15 Jul 2018 14:36:54 - 1.98
+++ usr.sbin/vmd/vmd.c 9 Sep 2018 23:13:50 -
@@ -1248,11 +1249,11 @@ vm_register(struct privsep *ps, struct v
vm->vm_paused = 0;
vm->vm_user = usr;
- for (i = 0; i < vcp->vcp_ndisks; i++)
+ for (i = 0; i < VMM_MAX_DISKS_PER_VM; i++)
vm->vm_disks[i] = -1;
- for (i = 0; i < vcp->vcp_nnics; i++) {
+ for (i = 0; i < VMM_MAX_NICS_PER_VM; i++)
vm->vm_ifs[i].vif_fd = -1;
-
+ for (i = 0; i < vcp->vcp_nnics; i++) {
if ((sw = switch_getbyname(vmc->vmc_ifswitch[i])) != NULL) {
/* inherit per-interface flags from the switch */
vmc->vmc_ifflags[i] |= (sw->sw_flags & VMIFF_OPTMASK);
Index: usr.sbin/vmd/vmm.c
===
RCS file: /data/mirror/openbsd/cvs/src/usr.sbin/vmd/vmm.c,v
retrieving revision 1.88
diff -u -p -r1.88 vmm.c
--- usr.sbin/vmd/vmm.c 13 Jul 2018 08:42:49 - 1.88
+++ usr.sbin/vmd/vmm.c 9 Sep 2018 23:13:04 -
@@ -646,20 +646,22 @@ vmm_start_vm(struct imsg *imsg, uint32_t
close(vm->vm_disks[i]);
vm->vm_disks[i] = -1;
}
-
for (i = 0 ; i < vcp->vcp_nnics; i++) {
close(vm->vm_ifs[i].vif_fd);
vm->vm_ifs[i].vif_fd = -1;
}
-
- close(vm->vm_kernel);
- vm->vm_kernel = -1;
-
- close(vm->vm_cdrom);
- vm->vm_cdrom = -1;
-
- close(vm->vm_tty);
- vm->vm_tty = -1;
+ if (vm->vm_kernel != -1) {
+ close(vm->vm_kernel);
+ vm->vm_kernel = -1;
+ }
+ if (vm->vm_cdrom != -1) {
+ close(vm->vm_cdrom);
+ vm->vm_cdrom = -1;
+ }
+ if (vm->vm_tty != -1) {
+ close(vm->vm_tty);
+ vm->vm_tty = -1;
+ }
/* read back the kernel-generated vm id from the child */
if (read(fds[0], >vcp_id, sizeof(vcp->vcp_id)) !=
pfctl: merge host_v{4,6}() into simpler host_ip()
Reduce duplicate code, make similar paths such as the memcpy() calls
more uniform to simplify upcoming diffs and tidy up a bit.
Feedback? OK?
Index: pfctl_parser.c
===
RCS file: /cvs/src/sbin/pfctl/pfctl_parser.c,v
retrieving revision 1.332
diff -u -p -r1.332 pfctl_parser.c
--- pfctl_parser.c 7 Sep 2018 21:37:03 - 1.332
+++ pfctl_parser.c 9 Sep 2018 22:39:46 -
@@ -74,8 +74,7 @@ intifa_skip_if(const char *filter, st
struct node_host *ifa_grouplookup(const char *, int);
struct node_host *host_if(const char *, int);
-struct node_host *host_v4(const char *, int);
-struct node_host *host_v6(const char *, int);
+struct node_host *host_ip(const char *, int);
struct node_host *host_dns(const char *, int, int);
const char *tcpflags = "FSRPAUEW";
@@ -1649,8 +1648,7 @@ host(const char *s, int opts)
r = ps;
if ((h = host_if(ps, mask)) == NULL &&
- (h = host_v4(r, mask)) == NULL &&
- (h = host_v6(ps, mask)) == NULL &&
+ (h = host_ip(ps, mask)) == NULL &&
(h = host_dns(ps, mask, (opts & PF_OPT_NODNS))) == NULL) {
fprintf(stderr, "no IP address found for %s\n", s);
goto error;
@@ -1717,59 +1715,48 @@ error:
}
struct node_host *
-host_v4(const char *s, int mask)
-{
- struct node_host*h = NULL;
- struct in_addr ina;
-
- memset(, 0, sizeof(ina));
- if (mask > -1) {
- if (inet_net_pton(AF_INET, s, , sizeof(ina)) == -1)
- return (NULL);
- } else {
- if (inet_pton(AF_INET, s, ) != 1)
- return (NULL);
- }
-
- h = calloc(1, sizeof(struct node_host));
- if (h == NULL)
- err(1, "address: calloc");
- h->ifname = NULL;
- h->af = AF_INET;
- h->addr.v.a.addr.addr32[0] = ina.s_addr;
- set_ipmask(h, mask);
- h->next = NULL;
- h->tail = h;
-
- return (h);
-}
-
-struct node_host *
-host_v6(const char *s, int mask)
+host_ip(const char *s, int mask)
{
struct addrinfo hints, *res;
+ struct in_addr ina;
struct node_host*h = NULL;
memset(, 0, sizeof(hints));
- hints.ai_family = AF_INET6;
+ hints.ai_family = AF_UNSPEC;
hints.ai_socktype = SOCK_DGRAM; /*dummy*/
hints.ai_flags = AI_NUMERICHOST;
- if (getaddrinfo(s, "0", , ) == 0) {
- h = calloc(1, sizeof(struct node_host));
+ if (getaddrinfo(s, NULL, , ) == 0) {
+ h = calloc(1, sizeof(*h));
if (h == NULL)
- err(1, "address: calloc");
- h->ifname = NULL;
- h->af = AF_INET6;
- memcpy(>addr.v.a.addr,
- &((struct sockaddr_in6 *)res->ai_addr)->sin6_addr,
- sizeof(h->addr.v.a.addr));
- h->ifindex =
- ((struct sockaddr_in6 *)res->ai_addr)->sin6_scope_id;
- set_ipmask(h, mask);
+ err(1, "%s: calloc", __func__);
+ h->af = res->ai_family;
+ if (h->af == AF_INET6) {
+ memcpy(>addr.v.a.addr.v6,
+ &((struct sockaddr_in6 *)res->ai_addr)->sin6_addr,
+ sizeof(h->addr.v.a.addr.v6));
+ h->ifindex =
+ ((struct sockaddr_in6
*)res->ai_addr)->sin6_scope_id;
+ } else
+ memcpy(>addr.v.a.addr.v4,
+ &((struct sockaddr_in *)res->ai_addr)->sin_addr,
+ sizeof(h->addr.v.a.addr.v4));
freeaddrinfo(res);
- h->next = NULL;
- h->tail = h;
+ } else {/* ie. for 10/8 parsing */
+ if (mask == -1)
+ return (NULL);
+ memset(, 0, sizeof(ina));
+ if (inet_net_pton(AF_INET, s, , sizeof(ina)) == -1)
+ return (NULL);
+ h = calloc(1, sizeof(*h));
+ if (h == NULL)
+ err(1, "%s: calloc", __func__);
+ h->af = AF_INET;
+ memcpy(>addr.v.a.addr.v4, , sizeof(h->addr.v.a.addr.v4));
}
+ set_ipmask(h, mask);
+ h->ifname = NULL;
+ h->next = NULL;
+ h->tail = h;
return (h);
}
===
Stats: --- 47 lines 1182 chars
Stats: +++ 34 lines 1095 chars
Stats: -13 lines
Stats: -87 chars
Re: vmd stdio /dev/null
On Sun, Sep 09, 2018 at 11:45:07PM +0200, Alexander Bluhm wrote:
> Hi,
>
> Like the other proc.c daemons, vmd(8) children do not detach from
> the terminal properly.
>
> ok?
>
Reads ok to me but reyk@ should approve.
> bluhm
>
> Index: src/usr.sbin/vmd/proc.c
> ===
> RCS file: /data/mirror/openbsd/cvs/src/usr.sbin/vmd/proc.c,v
> retrieving revision 1.17
> diff -u -p -r1.17 proc.c
> --- src/usr.sbin/vmd/proc.c 5 Aug 2018 08:20:54 - 1.17
> +++ src/usr.sbin/vmd/proc.c 9 Sep 2018 21:26:32 -
> @@ -29,13 +29,14 @@
> #include
> #include
> #include
> +#include
> #include
> #include
> #include
>
> #include "proc.h"
>
> -void proc_exec(struct privsep *, struct privsep_proc *, unsigned int,
> +void proc_exec(struct privsep *, struct privsep_proc *, unsigned int, int,
> int, char **);
> void proc_setup(struct privsep *, struct privsep_proc *, unsigned int);
> void proc_open(struct privsep *, int, int);
> @@ -80,7 +81,7 @@ proc_getid(struct privsep_proc *procs, u
>
> void
> proc_exec(struct privsep *ps, struct privsep_proc *procs, unsigned int nproc,
> -int argc, char **argv)
> +int debug, int argc, char **argv)
> {
> unsigned int proc, nargc, i, proc_i;
> char**nargv;
> @@ -141,6 +142,16 @@ proc_exec(struct privsep *ps, struct pri
> } else if (fcntl(fd, F_SETFD, 0) == -1)
> fatal("fcntl");
>
> + /* Daemons detach from terminal. */
> + if (!debug && (fd =
> + open(_PATH_DEVNULL, O_RDWR, 0)) != -1) {
> + (void)dup2(fd, STDIN_FILENO);
> + (void)dup2(fd, STDOUT_FILENO);
> + (void)dup2(fd, STDERR_FILENO);
> + if (fd > 2)
> + (void)close(fd);
> + }
> +
> execvp(argv[0], nargv);
> fatal("%s: execvp", __func__);
> break;
> @@ -191,7 +202,7 @@ proc_connect(struct privsep *ps)
>
> void
> proc_init(struct privsep *ps, struct privsep_proc *procs, unsigned int nproc,
> -int argc, char **argv, enum privsep_procid proc_id)
> +int debug, int argc, char **argv, enum privsep_procid proc_id)
> {
> struct privsep_proc *p = NULL;
> struct privsep_pipes*pa, *pb;
> @@ -231,7 +242,7 @@ proc_init(struct privsep *ps, struct pri
> }
>
> /* Engage! */
> - proc_exec(ps, procs, nproc, argc, argv);
> + proc_exec(ps, procs, nproc, debug, argc, argv);
> return;
> }
>
> Index: src/usr.sbin/vmd/proc.h
> ===
> RCS file: /data/mirror/openbsd/cvs/src/usr.sbin/vmd/proc.h,v
> retrieving revision 1.15
> diff -u -p -r1.15 proc.h
> --- src/usr.sbin/vmd/proc.h 5 Aug 2018 08:20:54 - 1.15
> +++ src/usr.sbin/vmd/proc.h 9 Sep 2018 21:15:50 -
> @@ -156,7 +156,7 @@ struct privsep_fd {
> #define PROC_MAX_INSTANCES 32
>
> /* proc.c */
> -void proc_init(struct privsep *, struct privsep_proc *, unsigned int,
> +void proc_init(struct privsep *, struct privsep_proc *, unsigned int, int,
> int, char **, enum privsep_procid);
> void proc_kill(struct privsep *);
> void proc_connect(struct privsep *ps);
> Index: src/usr.sbin/vmd/vmd.c
> ===
> RCS file: /data/mirror/openbsd/cvs/src/usr.sbin/vmd/vmd.c,v
> retrieving revision 1.98
> diff -u -p -r1.98 vmd.c
> --- src/usr.sbin/vmd/vmd.c15 Jul 2018 14:36:54 - 1.98
> +++ src/usr.sbin/vmd/vmd.c9 Sep 2018 21:25:57 -
> @@ -792,7 +792,8 @@ main(int argc, char **argv)
> ps->ps_title[proc_id] = title;
>
> /* only the parent returns */
> - proc_init(ps, procs, nitems(procs), argc0, argv, proc_id);
> + proc_init(ps, procs, nitems(procs), env->vmd_debug, argc0, argv,
> + proc_id);
>
> log_procinit("parent");
> if (!env->vmd_debug && daemon(0, 0) == -1)
>
vmd stdio /dev/null
Hi,
Like the other proc.c daemons, vmd(8) children do not detach from
the terminal properly.
ok?
bluhm
Index: src/usr.sbin/vmd/proc.c
===
RCS file: /data/mirror/openbsd/cvs/src/usr.sbin/vmd/proc.c,v
retrieving revision 1.17
diff -u -p -r1.17 proc.c
--- src/usr.sbin/vmd/proc.c 5 Aug 2018 08:20:54 - 1.17
+++ src/usr.sbin/vmd/proc.c 9 Sep 2018 21:26:32 -
@@ -29,13 +29,14 @@
#include
#include
#include
+#include
#include
#include
#include
#include "proc.h"
-voidproc_exec(struct privsep *, struct privsep_proc *, unsigned int,
+voidproc_exec(struct privsep *, struct privsep_proc *, unsigned int, int,
int, char **);
voidproc_setup(struct privsep *, struct privsep_proc *, unsigned int);
voidproc_open(struct privsep *, int, int);
@@ -80,7 +81,7 @@ proc_getid(struct privsep_proc *procs, u
void
proc_exec(struct privsep *ps, struct privsep_proc *procs, unsigned int nproc,
-int argc, char **argv)
+int debug, int argc, char **argv)
{
unsigned int proc, nargc, i, proc_i;
char**nargv;
@@ -141,6 +142,16 @@ proc_exec(struct privsep *ps, struct pri
} else if (fcntl(fd, F_SETFD, 0) == -1)
fatal("fcntl");
+ /* Daemons detach from terminal. */
+ if (!debug && (fd =
+ open(_PATH_DEVNULL, O_RDWR, 0)) != -1) {
+ (void)dup2(fd, STDIN_FILENO);
+ (void)dup2(fd, STDOUT_FILENO);
+ (void)dup2(fd, STDERR_FILENO);
+ if (fd > 2)
+ (void)close(fd);
+ }
+
execvp(argv[0], nargv);
fatal("%s: execvp", __func__);
break;
@@ -191,7 +202,7 @@ proc_connect(struct privsep *ps)
void
proc_init(struct privsep *ps, struct privsep_proc *procs, unsigned int nproc,
-int argc, char **argv, enum privsep_procid proc_id)
+int debug, int argc, char **argv, enum privsep_procid proc_id)
{
struct privsep_proc *p = NULL;
struct privsep_pipes*pa, *pb;
@@ -231,7 +242,7 @@ proc_init(struct privsep *ps, struct pri
}
/* Engage! */
- proc_exec(ps, procs, nproc, argc, argv);
+ proc_exec(ps, procs, nproc, debug, argc, argv);
return;
}
Index: src/usr.sbin/vmd/proc.h
===
RCS file: /data/mirror/openbsd/cvs/src/usr.sbin/vmd/proc.h,v
retrieving revision 1.15
diff -u -p -r1.15 proc.h
--- src/usr.sbin/vmd/proc.h 5 Aug 2018 08:20:54 - 1.15
+++ src/usr.sbin/vmd/proc.h 9 Sep 2018 21:15:50 -
@@ -156,7 +156,7 @@ struct privsep_fd {
#define PROC_MAX_INSTANCES 32
/* proc.c */
-voidproc_init(struct privsep *, struct privsep_proc *, unsigned int,
+voidproc_init(struct privsep *, struct privsep_proc *, unsigned int, int,
int, char **, enum privsep_procid);
voidproc_kill(struct privsep *);
voidproc_connect(struct privsep *ps);
Index: src/usr.sbin/vmd/vmd.c
===
RCS file: /data/mirror/openbsd/cvs/src/usr.sbin/vmd/vmd.c,v
retrieving revision 1.98
diff -u -p -r1.98 vmd.c
--- src/usr.sbin/vmd/vmd.c 15 Jul 2018 14:36:54 - 1.98
+++ src/usr.sbin/vmd/vmd.c 9 Sep 2018 21:25:57 -
@@ -792,7 +792,8 @@ main(int argc, char **argv)
ps->ps_title[proc_id] = title;
/* only the parent returns */
- proc_init(ps, procs, nitems(procs), argc0, argv, proc_id);
+ proc_init(ps, procs, nitems(procs), env->vmd_debug, argc0, argv,
+ proc_id);
log_procinit("parent");
if (!env->vmd_debug && daemon(0, 0) == -1)
pcb inet6ctlerrmap
Hi,
My goal is to get in6_pcb and in_pcb in sync.
Let's make both inetctlerrmap and inet6ctlerrmap u_char. That is
what FreeBSD does. There it is also in in6?_input.c.
FreeBSD and NetBSD have the declaration in in6?_var.h, we have it
in in6?.h, but I don't bother enough to move it.
ok?
bluhm
Index: netinet/in.h
===
RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/in.h,v
retrieving revision 1.131
diff -u -p -r1.131 in.h
--- netinet/in.h10 Jul 2018 11:34:12 - 1.131
+++ netinet/in.h9 Sep 2018 15:39:32 -
@@ -795,8 +795,8 @@ __END_DECLS
#endif /* !_KERNEL */
#ifdef _KERNEL
-externconst int inetctlerrmap[];
-externconst struct in_addr zeroin_addr;
+extern const u_char inetctlerrmap[];
+extern const struct in_addr zeroin_addr;
struct mbuf;
struct sockaddr;
Index: netinet/in_pcb.c
===
RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/in_pcb.c,v
retrieving revision 1.241
diff -u -p -r1.241 in_pcb.c
--- netinet/in_pcb.c7 Sep 2018 10:55:35 - 1.241
+++ netinet/in_pcb.c9 Sep 2018 15:21:24 -
@@ -98,7 +98,6 @@
#ifdef INET6
#include
-#include
#endif /* INET6 */
#ifdef IPSEC
#include
Index: netinet/ip_input.c
===
RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/ip_input.c,v
retrieving revision 1.338
diff -u -p -r1.338 ip_input.c
--- netinet/ip_input.c 10 Jul 2018 11:34:12 - 1.338
+++ netinet/ip_input.c 9 Sep 2018 15:40:38 -
@@ -1384,7 +1384,7 @@ ip_stripoptions(struct mbuf *m)
ip->ip_len = htons(ntohs(ip->ip_len) - olen);
}
-const int inetctlerrmap[PRC_NCMDS] = {
+const u_char inetctlerrmap[PRC_NCMDS] = {
0, 0, 0, 0,
0, EMSGSIZE, EHOSTDOWN, EHOSTUNREACH,
EHOSTUNREACH, EHOSTUNREACH, ECONNREFUSED, ECONNREFUSED,
Index: netinet6/in6.h
===
RCS file: /data/mirror/openbsd/cvs/src/sys/netinet6/in6.h,v
retrieving revision 1.102
diff -u -p -r1.102 in6.h
--- netinet6/in6.h 7 Jun 2018 08:46:24 - 1.102
+++ netinet6/in6.h 9 Sep 2018 15:39:16 -
@@ -404,8 +404,8 @@ typedef __socklen_t socklen_t; /* length
#endif /* __BSD_VISIBLE */
#ifdef _KERNEL
-extern const u_char inet6ctlerrmap[];
-extern const struct in6_addr zeroin6_addr;
+extern const u_char inet6ctlerrmap[];
+extern const struct in6_addr zeroin6_addr;
struct mbuf;
struct ifnet;
Index: netinet6/in6_pcb.c
===
RCS file: /data/mirror/openbsd/cvs/src/sys/netinet6/in6_pcb.c,v
retrieving revision 1.104
diff -u -p -r1.104 in6_pcb.c
--- netinet6/in6_pcb.c 14 Jun 2018 17:00:58 - 1.104
+++ netinet6/in6_pcb.c 9 Sep 2018 15:39:58 -
@@ -124,31 +124,7 @@
#include
#include
-/*
- * External globals
- */
-
-/*
- * Globals
- */
-
const struct in6_addr zeroin6_addr;
-
-/*
- * Keep separate inet6ctlerrmap, because I may remap some of these.
- * I also put it here, because, quite frankly, it belongs here, not in
- * ip{v6,}_input().
- */
-#if 0
-u_char inet6ctlerrmap[PRC_NCMDS] = {
- 0, 0, 0, 0,
- 0, EMSGSIZE, EHOSTDOWN, EHOSTUNREACH,
- EHOSTUNREACH, EHOSTUNREACH, ECONNREFUSED, ECONNREFUSED,
- EMSGSIZE, EHOSTUNREACH, 0, 0,
- 0, 0, 0, 0,
- ENOPROTOOPT
-};
-#endif
int
in6_pcbaddrisavail(struct inpcb *inp, struct sockaddr_in6 *sin6, int wild,
Re: switchd(8): don't set output port to OFP*_PORT_ANY for input == output
On Sun, Sep 09, 2018 at 03:17:19AM -0700, Ayaka Koshibe wrote:
> Hi,
>
> This is a new version of a previous diff that I had for making switchd(8)
> ignore PACKET_IN messages generated from looped traffic. Currently, it will
> respond to the PACKET_IN with an invalid PACKET_OUT onto OFP*_PORT_ANY,
> resulting in the switch responding with an error that makes switchd disconnect
> the switch.
>
> I didn't follow through with the previous diff since it seemed to chew up CPU,
> but saw the cause elsewhere after looking at it further. This just improves on
> switchd's behavior.
>
> OK?
Reasoning makes sense so OK claudio@
> Thanks,
> Ayaka
>
>
> Index: usr.sbin/switchd/ofp10.c
> ===
> RCS file: /cvs/src/usr.sbin/switchd/ofp10.c,v
> retrieving revision 1.19
> diff -u -p -u -r1.19 ofp10.c
> --- usr.sbin/switchd/ofp10.c 2 Dec 2016 14:39:46 - 1.19
> +++ usr.sbin/switchd/ofp10.c 9 Sep 2018 09:36:33 -
> @@ -393,7 +393,8 @@ ofp10_packet_in(struct switchd *sc, stru
>* silently drop looping packet
>* (don't use OFP10_PORT_INPUT here)
>*/
> - dstport = OFP10_PORT_ANY;
> + ret = 0;
> + goto done;
> } else {
> addflow = 1;
> }
> Index: usr.sbin/switchd/ofp13.c
> ===
> RCS file: /cvs/src/usr.sbin/switchd/ofp13.c,v
> retrieving revision 1.43
> diff -u -p -u -r1.43 ofp13.c
> --- usr.sbin/switchd/ofp13.c 17 Jan 2017 09:21:50 - 1.43
> +++ usr.sbin/switchd/ofp13.c 9 Sep 2018 09:36:33 -
> @@ -1082,7 +1082,8 @@ ofp13_packet_in(struct switchd *sc, stru
>* silently drop looping packet
>* (don't use OFP_PORT_INPUT here)
>*/
> - dstport = OFP_PORT_ANY;
> + ret = 0;
> + goto done;
> } else {
> addflow = 1;
> }
>
--
:wq Claudio
switchd(8): don't set output port to OFP*_PORT_ANY for input == output
Hi,
This is a new version of a previous diff that I had for making switchd(8)
ignore PACKET_IN messages generated from looped traffic. Currently, it will
respond to the PACKET_IN with an invalid PACKET_OUT onto OFP*_PORT_ANY,
resulting in the switch responding with an error that makes switchd disconnect
the switch.
I didn't follow through with the previous diff since it seemed to chew up CPU,
but saw the cause elsewhere after looking at it further. This just improves on
switchd's behavior.
OK?
Thanks,
Ayaka
Index: usr.sbin/switchd/ofp10.c
===
RCS file: /cvs/src/usr.sbin/switchd/ofp10.c,v
retrieving revision 1.19
diff -u -p -u -r1.19 ofp10.c
--- usr.sbin/switchd/ofp10.c2 Dec 2016 14:39:46 - 1.19
+++ usr.sbin/switchd/ofp10.c9 Sep 2018 09:36:33 -
@@ -393,7 +393,8 @@ ofp10_packet_in(struct switchd *sc, stru
* silently drop looping packet
* (don't use OFP10_PORT_INPUT here)
*/
- dstport = OFP10_PORT_ANY;
+ ret = 0;
+ goto done;
} else {
addflow = 1;
}
Index: usr.sbin/switchd/ofp13.c
===
RCS file: /cvs/src/usr.sbin/switchd/ofp13.c,v
retrieving revision 1.43
diff -u -p -u -r1.43 ofp13.c
--- usr.sbin/switchd/ofp13.c17 Jan 2017 09:21:50 - 1.43
+++ usr.sbin/switchd/ofp13.c9 Sep 2018 09:36:33 -
@@ -1082,7 +1082,8 @@ ofp13_packet_in(struct switchd *sc, stru
* silently drop looping packet
* (don't use OFP_PORT_INPUT here)
*/
- dstport = OFP_PORT_ANY;
+ ret = 0;
+ goto done;
} else {
addflow = 1;
}
Re: smtp core dump
Your bug report which ought to go to bugs@ is incomplete, please see https://www.openbsd.org/report.html. On Sat, Sep 08, 2018 at 11:35:47PM -0700, Jungle Boogie wrote: > The backtrace is pretty uneventful, there's no debugging: /usr/bin/smtp lacks debug symbols. Rebuild from source: $ cd /usr/src/usr.bin/smtp $ make obj $ make DEBUG=-g3 Then either use the existing binary and core dump with your new symbols $ egdb -e /usr/bin/smtp -s /usr/obj/usr.bin/smtp/smtp \ -c ./smtp.core -batch -ex bt or reproduce the issue using the new binary (and possibly turning off optimizations) $ make clean $ make DEBUG='-O0 -g3' $ ./obj/smtp ... $ egdb -se ./obj/smtp -c ./smtp.core -batch -ex bt
Re: if_cloners list is poulated at system boot only
On Sun, Sep 09, 2018 at 10:50:08AM +0200, Alexander Bluhm wrote: > Could you put comment here, that the function is only called during > boot so no lock is needed? I would prefer an assert to enforce > that, but I don't know any that is suited. > > With comment, OK bluhm@ OK kn
Re: if_cloners list is poulated at system boot only
On Sun, Sep 09, 2018 at 08:41:07AM +0200, Alexandr Nedvedicky wrote:
> void
> if_clone_attach(struct if_clone *ifc)
> {
> - rw_enter_write(_cloners_lock);
> LIST_INSERT_HEAD(_cloners, ifc, ifc_list);
> if_cloners_count++;
> - rw_exit_write(_cloners_lock);
> -}
Could you put comment here, that the function is only called during
boot so no lock is needed? I would prefer an assert to enforce
that, but I don't know any that is suited.
With comment, OK bluhm@
Re: if_cloners list is poulated at system boot only
On Sun, Sep 09, 2018 at 08:41:07AM +0200, Alexandr Nedvedicky wrote:
> Hello,
>
> while poking around 'XXXSMP' comments in net/if.c, I've noticed
> the 'if_cloners_lock' can be removed. The thing is the list of
> cloners gets populated/modified at system boot only, while kernel
> attaches device drivers.
>
> And because cloners are never removed, the if_clone_detach()
> function can go, no one calls it.
>
> OK ?
OK claudio@
> thanks and
> regards
> sashan
>
> 8<---8<---8<--8<
> diff --git a/sys/net/if.c b/sys/net/if.c
> index 7097eb278ef..93def034a9e 100644
> --- a/sys/net/if.c
> +++ b/sys/net/if.c
> @@ -219,8 +219,6 @@ void if_idxmap_remove(struct ifnet *);
>
> TAILQ_HEAD(, ifg_group) ifg_head = TAILQ_HEAD_INITIALIZER(ifg_head);
>
> -/* Serialize access to _cloners and if_cloners_count */
> -struct rwlock if_cloners_lock = RWLOCK_INITIALIZER("ifclonerslk");
> LIST_HEAD(, if_clone) if_cloners = LIST_HEAD_INITIALIZER(if_cloners);
> int if_cloners_count;
>
> @@ -1252,13 +1250,11 @@ if_clone_lookup(const char *name, int *unitp)
> if (cp - name < IFNAMSIZ-1 && *cp == '0' && cp[1] != '\0')
> return (NULL); /* unit number 0 padded */
>
> - rw_enter_read(_cloners_lock);
> LIST_FOREACH(ifc, _cloners, ifc_list) {
> if (strlen(ifc->ifc_name) == cp - name &&
> !strncmp(name, ifc->ifc_name, cp - name))
> break;
> }
> - rw_exit_read(_cloners_lock);
>
> if (ifc == NULL)
> return (NULL);
> @@ -1284,22 +1280,8 @@ if_clone_lookup(const char *name, int *unitp)
> void
> if_clone_attach(struct if_clone *ifc)
> {
> - rw_enter_write(_cloners_lock);
> LIST_INSERT_HEAD(_cloners, ifc, ifc_list);
> if_cloners_count++;
> - rw_exit_write(_cloners_lock);
> -}
> -
> -/*
> - * Unregister a network interface cloner.
> - */
> -void
> -if_clone_detach(struct if_clone *ifc)
> -{
> - rw_enter_write(_cloners_lock);
> - LIST_REMOVE(ifc, ifc_list);
> - if_cloners_count--;
> - rw_exit_write(_cloners_lock);
> }
>
> /*
> @@ -1314,17 +1296,13 @@ if_clone_list(struct if_clonereq *ifcr)
>
> if ((dst = ifcr->ifcr_buffer) == NULL) {
> /* Just asking how many there are. */
> - rw_enter_read(_cloners_lock);
> ifcr->ifcr_total = if_cloners_count;
> - rw_exit_read(_cloners_lock);
> return (0);
> }
>
> if (ifcr->ifcr_count < 0)
> return (EINVAL);
>
> - rw_enter_read(_cloners_lock);
> -
> ifcr->ifcr_total = if_cloners_count;
> count = MIN(if_cloners_count, ifcr->ifcr_count);
>
> @@ -1340,7 +1318,6 @@ if_clone_list(struct if_clonereq *ifcr)
> dst += IFNAMSIZ;
> }
>
> - rw_exit_read(_cloners_lock);
> return (error);
> }
>
> diff --git a/sys/net/if_var.h b/sys/net/if_var.h
> index e9f69c964cb..e59f3ba9701 100644
> --- a/sys/net/if_var.h
> +++ b/sys/net/if_var.h
> @@ -328,7 +328,6 @@ void ifafree(struct ifaddr *);
> int if_isconnected(const struct ifnet *, unsigned int);
>
> void if_clone_attach(struct if_clone *);
> -void if_clone_detach(struct if_clone *);
>
> int if_clone_create(const char *, int);
> int if_clone_destroy(const char *);
>
--
:wq Claudio
if_cloners list is poulated at system boot only
Hello,
while poking around 'XXXSMP' comments in net/if.c, I've noticed
the 'if_cloners_lock' can be removed. The thing is the list of
cloners gets populated/modified at system boot only, while kernel
attaches device drivers.
And because cloners are never removed, the if_clone_detach()
function can go, no one calls it.
OK ?
thanks and
regards
sashan
8<---8<---8<--8<
diff --git a/sys/net/if.c b/sys/net/if.c
index 7097eb278ef..93def034a9e 100644
--- a/sys/net/if.c
+++ b/sys/net/if.c
@@ -219,8 +219,6 @@ voidif_idxmap_remove(struct ifnet *);
TAILQ_HEAD(, ifg_group) ifg_head = TAILQ_HEAD_INITIALIZER(ifg_head);
-/* Serialize access to _cloners and if_cloners_count */
-struct rwlock if_cloners_lock = RWLOCK_INITIALIZER("ifclonerslk");
LIST_HEAD(, if_clone) if_cloners = LIST_HEAD_INITIALIZER(if_cloners);
int if_cloners_count;
@@ -1252,13 +1250,11 @@ if_clone_lookup(const char *name, int *unitp)
if (cp - name < IFNAMSIZ-1 && *cp == '0' && cp[1] != '\0')
return (NULL); /* unit number 0 padded */
- rw_enter_read(_cloners_lock);
LIST_FOREACH(ifc, _cloners, ifc_list) {
if (strlen(ifc->ifc_name) == cp - name &&
!strncmp(name, ifc->ifc_name, cp - name))
break;
}
- rw_exit_read(_cloners_lock);
if (ifc == NULL)
return (NULL);
@@ -1284,22 +1280,8 @@ if_clone_lookup(const char *name, int *unitp)
void
if_clone_attach(struct if_clone *ifc)
{
- rw_enter_write(_cloners_lock);
LIST_INSERT_HEAD(_cloners, ifc, ifc_list);
if_cloners_count++;
- rw_exit_write(_cloners_lock);
-}
-
-/*
- * Unregister a network interface cloner.
- */
-void
-if_clone_detach(struct if_clone *ifc)
-{
- rw_enter_write(_cloners_lock);
- LIST_REMOVE(ifc, ifc_list);
- if_cloners_count--;
- rw_exit_write(_cloners_lock);
}
/*
@@ -1314,17 +1296,13 @@ if_clone_list(struct if_clonereq *ifcr)
if ((dst = ifcr->ifcr_buffer) == NULL) {
/* Just asking how many there are. */
- rw_enter_read(_cloners_lock);
ifcr->ifcr_total = if_cloners_count;
- rw_exit_read(_cloners_lock);
return (0);
}
if (ifcr->ifcr_count < 0)
return (EINVAL);
- rw_enter_read(_cloners_lock);
-
ifcr->ifcr_total = if_cloners_count;
count = MIN(if_cloners_count, ifcr->ifcr_count);
@@ -1340,7 +1318,6 @@ if_clone_list(struct if_clonereq *ifcr)
dst += IFNAMSIZ;
}
- rw_exit_read(_cloners_lock);
return (error);
}
diff --git a/sys/net/if_var.h b/sys/net/if_var.h
index e9f69c964cb..e59f3ba9701 100644
--- a/sys/net/if_var.h
+++ b/sys/net/if_var.h
@@ -328,7 +328,6 @@ voidifafree(struct ifaddr *);
intif_isconnected(const struct ifnet *, unsigned int);
void if_clone_attach(struct if_clone *);
-void if_clone_detach(struct if_clone *);
intif_clone_create(const char *, int);
intif_clone_destroy(const char *);
smtp core dump
Hi All, I'm only playing around with smtp(1), so my use case may be wrong. I'm getting a coredump with it on OpenBSD snapshot from today (8 Sept). My string to cause the core dump: smtp -C -vv -F jungleboog...@gmail.com -s smtp://user:p...@imap.gmail.com:587 jungleboog...@gmail.com hello trying host 74.125.195.108 port 587... 0x12f009456800: INIT -> BANNER 0x12f009456800: <<< 220 smtp.gmail.com ESMTP r23-v6sm25386962pfj.5 - gsmtp 0x12f009456800: BANNER -> EHLO mta: 0x12f009456800: >>> EHLO puffer.in.mydomain.net 0x12f009456800: <<< 250-smtp.gmail.com at your service, [70.173.220.152] 0x12f009456800: <<< 250-SIZE 35882577 0x12f009456800: <<< 250-8BITMIME 0x12f009456800: <<< 250-STARTTLS 0x12f009456800: <<< 250-ENHANCEDSTATUSCODES 0x12f009456800: <<< 250-PIPELINING 0x12f009456800: <<< 250-CHUNKING 0x12f009456800: <<< 250 SMTPUTF8 0x12f009456800: EHLO -> STARTTLS mta: 0x12f009456800: >>> STARTTLS 0x12f009456800: <<< 220 2.0.0 Ready to start TLS validating server certificate... 0x12f009456800: STARTTLS -> EHLO mta: 0x12f009456800: >>> EHLO puffer.in.mydomain.net 0x12f009456800: <<< 250-smtp.gmail.com at your service, [70.173.220.152] 0x12f009456800: <<< 250-SIZE 35882577 0x12f009456800: <<< 250-8BITMIME 0x12f009456800: <<< 250-AUTH LOGIN PLAIN XOAUTH2 PLAIN-CLIENTTOKEN OAUTHBEARER XOAUTH 0x12f009456800: <<< 250-ENHANCEDSTATUSCODES 0x12f009456800: <<< 250-PIPELINING 0x12f009456800: <<< 250-CHUNKING 0x12f009456800: <<< 250 SMTPUTF8 0x12f009456800: EHLO -> STARTTLS 0x12f009456800: STARTTLS -> AUTH 0x12f009456800: AUTH -> AUTH_PLAIN mta: 0x12f009456800: >>> AUTH PLAIN user:pass 0x12f009456800: <<< 501 5.5.2 Cannot Decode response r23-v6sm25386962pfj.5 - gsmtp rejected by server: 501 5.5.2 Cannot Decode response r23-v6sm25386962pfj.5 - gsmtp 0x12f009456800: AUTH_PLAIN -> QUIT mta: 0x12f009456800: >>> QUIT 0x12f009456800: <<< 221 2.0.0 closing connection r23-v6sm25386962pfj.5 - gsmtp connection closed... trying host 74.125.195.109 port 587... 0x12f009456c00: INIT -> BANNER 0x12f009456c00: <<< 220 smtp.gmail.com ESMTP z63-v6sm14785681pgd.69 - gsmtp 0x12f009456c00: BANNER -> EHLO mta: 0x12f009456c00: >>> EHLO puffer.in.mydomain.net 0x12f009456c00: <<< 250-smtp.gmail.com at your service, [70.173.220.152] 0x12f009456c00: <<< 250-SIZE 35882577 0x12f009456c00: <<< 250-8BITMIME 0x12f009456c00: <<< 250-STARTTLS 0x12f009456c00: <<< 250-ENHANCEDSTATUSCODES 0x12f009456c00: <<< 250-PIPELINING 0x12f009456c00: <<< 250-CHUNKING 0x12f009456c00: <<< 250 SMTPUTF8 0x12f009456c00: EHLO -> STARTTLS mta: 0x12f009456c00: >>> STARTTLS 0x12f009456c00: <<< 220 2.0.0 Ready to start TLS Bus error (core dumped) The backtrace is pretty uneventful, there's no debugging: gdb) bt #0 SSL_set_fd (s=0x12f06818ff00, fd=5) at /usr/src/lib/libssl/ssl_lib.c:580 #1 0x12edb7d035b6 in ?? () from /usr/bin/smtp #2 0x12edb7d04e18 in ?? () from /usr/bin/smtp #3 0x12edb7d0318d in ?? () from /usr/bin/smtp #4 0x12effd9aeb6d in event_base_loop (base=0x12f068189800, flags=0) at /usr/src/lib/libevent/event.c:350 #5 0x12edb7d063ef in ?? () from /usr/bin/smtp #6 0x12edb7d061f7 in ?? () from /usr/bin/smtp #7 0x12edb7d00c56 in ?? () from /usr/bin/smtp #8 0x in ?? () So what's going on with this?
