Re: Qcow2: Clean up logging/error handling
> On Tue, Oct 30, 2018 at 10:32:37PM -0700, Ori Bernstein wrote:
>> On Tue, 30 Oct 2018 22:29:01 -0700, Mike Larkin wrote:
>>
>> > > -if (disk->base->clustersz != disk->clustersz) {
>> > > -log_warn("%s: all disks must share clustersize",
>> > > +fatal("%s: could not open %s", basepath,
>> > > __func__);
>> >
>> > is this right?
>> >
>> > > +if (qc2_open(disk->base, fds + 1, nfd - 1) == -1)
>> > > +fatalx("%s: could not open %s", basepath,
>> > > __func__);
>> >
>> > same
>> >
>> > > +if (disk->base->clustersz != disk->clustersz)
>> > > +fatalx("%s: all disk parts must share
>> > > clustersize",
>> > > __func__);
>> > > -goto error;
>> > > -}
>> > > -}
>>
>> Good question. I think from earlier discussion, we wanted to fail if we could
>> not open a disk image -- especially since these ones are actually *parts* of
>> a disk image, meaning that we've got a corrupt image.
>>
>> I can easily change these back to warns + error returns.
>>
>> --
>> Ori Bernstein
>
> I was referring to the argument order.
Oh, that's obviously wrong. Updated.
diff --git usr.sbin/vmd/config.c usr.sbin/vmd/config.c
index 6e7ed279d6b..0860ad0e7d2 100644
--- usr.sbin/vmd/config.c
+++ usr.sbin/vmd/config.c
@@ -284,8 +284,7 @@ config_setvm(struct privsep *ps, struct vmd_vm *vm,
uint32_t peerid, uid_t uid)
*/
if (kernfd == -1 && !vmboot &&
(kernfd = open(VM_DEFAULT_BIOS, O_RDONLY)) == -1) {
- log_warn("%s: can't open %s", __func__,
- VM_DEFAULT_BIOS);
+ log_warn("can't open %s", VM_DEFAULT_BIOS);
errno = VMD_BIOS_MISSING;
goto fail;
}
@@ -305,8 +304,7 @@ config_setvm(struct privsep *ps, struct vmd_vm *vm,
uint32_t peerid, uid_t uid)
/* Stat cdrom to ensure it is a regular file */
if ((cdromfd =
open(vcp->vcp_cdrom, O_RDONLY)) == -1) {
- log_warn("%s: can't open cdrom %s", __func__,
- vcp->vcp_cdrom);
+ log_warn("can't open cdrom %s", vcp->vcp_cdrom);
errno = VMD_CDROM_MISSING;
goto fail;
}
@@ -325,14 +323,14 @@ config_setvm(struct privsep *ps, struct vmd_vm *vm,
uint32_t peerid, uid_t uid)
for (i = 0 ; i < vcp->vcp_ndisks; i++) {
if (strlcpy(path, vcp->vcp_disks[i], sizeof(path))
>= sizeof(path))
- log_warnx("%s, disk path too long", __func__);
+ log_warnx("disk path %s too long", vcp->vcp_disks[i]);
memset(vmc->vmc_diskbases, 0, sizeof(vmc->vmc_diskbases));
oflags = O_RDWR|O_EXLOCK|O_NONBLOCK;
aflags = R_OK|W_OK;
for (j = 0; j < VM_MAX_BASE_PER_DISK; j++) {
/* Stat disk[i] to ensure it is a regular file */
if ((diskfds[i][j] = open(path, oflags)) == -1) {
- log_warn("%s: can't open disk %s", __func__,
+ log_warn("can't open disk %s",
vcp->vcp_disks[i]);
errno = VMD_DISK_MISSING;
goto fail;
@@ -487,7 +485,7 @@ config_setvm(struct privsep *ps, struct vmd_vm *vm,
uint32_t peerid, uid_t uid)
fail:
saved_errno = errno;
- log_warnx("%s: failed to start vm %s", __func__, vcp->vcp_name);
+ log_warnx("failed to start vm %s", vcp->vcp_name);
if (kernfd != -1)
close(kernfd);
diff --git usr.sbin/vmd/vioqcow2.c usr.sbin/vmd/vioqcow2.c
index 28b087df39a..af762fcdf45 100644
--- usr.sbin/vmd/vioqcow2.c
+++ usr.sbin/vmd/vioqcow2.c
@@ -102,9 +102,9 @@ struct qcdisk {
extern char *__progname;
static off_t xlate(struct qcdisk *, off_t, int *);
-static int copy_cluster(struct qcdisk *, struct qcdisk *, off_t, off_t);
+static void copy_cluster(struct qcdisk *, struct qcdisk *, off_t, off_t);
+static void inc_refs(struct qcdisk *, off_t, int);
static off_t mkcluster(struct qcdisk *, struct qcdisk *, off_t, off_t);
-static int inc_refs(struct qcdisk *, off_t, int);
static int qc2_open(struct qcdisk *, int *, size_t);
static ssize_t qc2_pread(void *, char *, size_t, off_t);
static ssize_t qc2_pwrite(void *, char *, size_t, off_t);
@@ -126,7 +126,7 @@ virtio_init_qcow2(struct virtio_backing *file, off_t *szp,
int *fd, size_t nfd)
if (diskp == NULL)
return -1;
if (qc2_open(diskp, fd, nfd) == -1) {
- log_warnx("%s: could not open qcow2 disk", __func__);
+ log_warnx("could not open qcow2
Re: Qcow2: Clean up logging/error handling
On Tue, Oct 30, 2018 at 10:32:37PM -0700, Ori Bernstein wrote:
> On Tue, 30 Oct 2018 22:29:01 -0700, Mike Larkin wrote:
>
> > > - if (disk->base->clustersz != disk->clustersz) {
> > > - log_warn("%s: all disks must share clustersize",
> > > + fatal("%s: could not open %s", basepath, __func__);
> >
> > is this right?
> >
> > > + if (qc2_open(disk->base, fds + 1, nfd - 1) == -1)
> > > + fatalx("%s: could not open %s", basepath, __func__);
> >
> > same
> >
> > > + if (disk->base->clustersz != disk->clustersz)
> > > + fatalx("%s: all disk parts must share clustersize",
> > > __func__);
> > > - goto error;
> > > - }
> > > - }
>
> Good question. I think from earlier discussion, we wanted to fail if we could
> not open a disk image -- especially since these ones are actually *parts* of
> a disk image, meaning that we've got a corrupt image.
>
> I can easily change these back to warns + error returns.
>
> --
> Ori Bernstein
I was referring to the argument order.
Re: Qcow2: Clean up logging/error handling
On Tue, 30 Oct 2018 22:29:01 -0700, Mike Larkin wrote:
> > - if (disk->base->clustersz != disk->clustersz) {
> > - log_warn("%s: all disks must share clustersize",
> > + fatal("%s: could not open %s", basepath, __func__);
>
> is this right?
>
> > + if (qc2_open(disk->base, fds + 1, nfd - 1) == -1)
> > + fatalx("%s: could not open %s", basepath, __func__);
>
> same
>
> > + if (disk->base->clustersz != disk->clustersz)
> > + fatalx("%s: all disk parts must share clustersize",
> > __func__);
> > - goto error;
> > - }
> > - }
Good question. I think from earlier discussion, we wanted to fail if we could
not open a disk image -- especially since these ones are actually *parts* of
a disk image, meaning that we've got a corrupt image.
I can easily change these back to warns + error returns.
--
Ori Bernstein
Re: Qcow2: Clean up logging/error handling
On Tue, Oct 30, 2018 at 10:01:08PM -0700, Ori Bernstein wrote:
> On Sun, 28 Oct 2018 00:58:57 +0200, Reyk Floeter wrote:
>
> > Most of these are fatal and log_debug. Keep the __func__ there! But we___ll
> > remove it from other logging functions where it was never intended to be
> > used and potentially reword the warnings nicely.
> >
> > Reyk
> >
>
> Alright, updated accordingly, with __func__ on fatals, and without it
> on __warn__s. Also touched a few places where I noticed it in config.c.
>
> Ok?
>
see below.
>
> diff --git usr.sbin/vmd/config.c usr.sbin/vmd/config.c
> index 6e7ed279d6b..7dc2b266dfe 100644
> --- usr.sbin/vmd/config.c
> +++ usr.sbin/vmd/config.c
> @@ -284,8 +284,7 @@ config_setvm(struct privsep *ps, struct vmd_vm *vm,
> uint32_t peerid, uid_t uid)
>*/
> if (kernfd == -1 && !vmboot &&
> (kernfd = open(VM_DEFAULT_BIOS, O_RDONLY)) == -1) {
> - log_warn("%s: can't open %s", __func__,
> - VM_DEFAULT_BIOS);
> + log_warn("can't open %s", VM_DEFAULT_BIOS);
> errno = VMD_BIOS_MISSING;
> goto fail;
> }
> @@ -305,8 +304,7 @@ config_setvm(struct privsep *ps, struct vmd_vm *vm,
> uint32_t peerid, uid_t uid)
> /* Stat cdrom to ensure it is a regular file */
> if ((cdromfd =
> open(vcp->vcp_cdrom, O_RDONLY)) == -1) {
> - log_warn("%s: can't open cdrom %s", __func__,
> - vcp->vcp_cdrom);
> + log_warn("can't open cdrom %s", vcp->vcp_cdrom);
> errno = VMD_CDROM_MISSING;
> goto fail;
> }
> @@ -325,14 +323,14 @@ config_setvm(struct privsep *ps, struct vmd_vm *vm,
> uint32_t peerid, uid_t uid)
> for (i = 0 ; i < vcp->vcp_ndisks; i++) {
> if (strlcpy(path, vcp->vcp_disks[i], sizeof(path))
> >= sizeof(path))
> - log_warnx("%s, disk path too long", __func__);
> + log_warnx("disk path %s too long", vcp->vcp_disks[i]);
> memset(vmc->vmc_diskbases, 0, sizeof(vmc->vmc_diskbases));
> oflags = O_RDWR|O_EXLOCK|O_NONBLOCK;
> aflags = R_OK|W_OK;
> for (j = 0; j < VM_MAX_BASE_PER_DISK; j++) {
> /* Stat disk[i] to ensure it is a regular file */
> if ((diskfds[i][j] = open(path, oflags)) == -1) {
> - log_warn("%s: can't open disk %s", __func__,
> + log_warn("can't open disk %s",
> vcp->vcp_disks[i]);
> errno = VMD_DISK_MISSING;
> goto fail;
> @@ -487,7 +485,7 @@ config_setvm(struct privsep *ps, struct vmd_vm *vm,
> uint32_t peerid, uid_t uid)
>
> fail:
> saved_errno = errno;
> - log_warnx("%s: failed to start vm %s", __func__, vcp->vcp_name);
> + log_warnx("failed to start vm %s", __func__, vcp->vcp_name);
>
> if (kernfd != -1)
> close(kernfd);
> diff --git usr.sbin/vmd/vioqcow2.c usr.sbin/vmd/vioqcow2.c
> index 28b087df39a..de9068827c5 100644
> --- usr.sbin/vmd/vioqcow2.c
> +++ usr.sbin/vmd/vioqcow2.c
> @@ -102,9 +102,9 @@ struct qcdisk {
> extern char *__progname;
>
> static off_t xlate(struct qcdisk *, off_t, int *);
> -static int copy_cluster(struct qcdisk *, struct qcdisk *, off_t, off_t);
> +static void copy_cluster(struct qcdisk *, struct qcdisk *, off_t, off_t);
> +static void inc_refs(struct qcdisk *, off_t, int);
> static off_t mkcluster(struct qcdisk *, struct qcdisk *, off_t, off_t);
> -static int inc_refs(struct qcdisk *, off_t, int);
> static int qc2_open(struct qcdisk *, int *, size_t);
> static ssize_t qc2_pread(void *, char *, size_t, off_t);
> static ssize_t qc2_pwrite(void *, char *, size_t, off_t);
> @@ -126,7 +126,7 @@ virtio_init_qcow2(struct virtio_backing *file, off_t
> *szp, int *fd, size_t nfd)
> if (diskp == NULL)
> return -1;
> if (qc2_open(diskp, fd, nfd) == -1) {
> - log_warnx("%s: could not open qcow2 disk", __func__);
> + log_warnx("could not open qcow2 disk");
> return -1;
> }
> file->p = diskp;
> @@ -137,6 +137,10 @@ virtio_init_qcow2(struct virtio_backing *file, off_t
> *szp, int *fd, size_t nfd)
> return 0;
> }
>
> +/*
> + * Return the path to the base image given a disk image.
> + * Called from vmctl.
> + */
> ssize_t
> virtio_qcow2_get_base(int fd, char *path, size_t npath, const char *dpath)
> {
> @@ -147,11 +151,11 @@ virtio_qcow2_get_base(int fd, char *path, size_t npath,
> const char *dpath)
> char *s = NULL;
>
> if (pread(fd, , sizeof(header), 0) != sizeof(header)) {
> - log_warnx("%s: short read on header", __func__);
> + log_warnx("short
Re: kernel page fault in vm_teardown
On Tue, Oct 30, 2018 at 09:17:19PM -0700, Greg Steuck wrote:
> My syzkaller machine running a recent snapshot just crashed. The value
> 0x415efd243b54d319 passed into uvm_map_deallocate looks quite fishy to me.
>
Known issue. And the parameters in the list aren't right (there needs to be
something added to clang/llvm to support reading the params properly).
-ml
> Some hopefully useful info below.
>
> ddb{4}> trace
> uvm_unmap_remove(c05f7f8cd1633180,ff036f57f5a8,80b85f00,ff036f57f598,8000222b8040,0)
> at uvm_unmap_remove+0x212
> uvm_map_deallocate(415efd243b54d319) at uvm_map_deallocate+0x5e
> vm_teardown(ff036f57f3d8) at vm_teardown+0xf0
> vm_run(a186e3e68e0c8d2d) at vm_run+0x226
> VOP_IOCTL(d3bfd0b457c4b224,ff03c9c6f5f0,32269d81b8d394bf,8000222b4968,ff043f7ca420,3)
> at VOP_IOCTL+0x5a
> vn_ioctl(d3bfd0b4579725f3,ff03ca9e15b0,8000222b4968,20) at
> vn_ioctl+0x6b
> sys_ioctl(7867d986861f8ba2,360,8000222b4968) at sys_ioctl+0x3ec
> syscall(3871e5d148df7b3d) at syscall+0x32a
> Xsyscall(0,36,0,36,1fc2fafb52d0,1fc2faf35000) at Xsyscall+0x128
> end of kernel
> end trace frame: 0x1fc5a67a25b0, count: -9
> ddb{4}> show proc
> PROC (vmd) pid=51765 stat=onproc
> flags process=100010 proc=400
> pri=86, usrpri=86, nice=20
> forw=0x, list=0x8000222b5520,0x8000222b4270
> process=0x8000fffecfc8 user=0x80002237d000,
> vmspace=0xff03c12e9
> c70
> estcpu=36, cpticks=110340, pctcpu=13.31
> user=0, sys=110290, intr=0
> ddb{4}> show registers
> rdi 0x313679acpi_pdirpa+0x2ff4e1
> rsi 0x20656874203a7374
> rbp 0x800022382510
> rbx 0x8000223824d0
> rdx 0x11f010acpi_pdirpa+0x10ae78
> rcx0
> rax 0xff01189c9c80
> r8 0x3
> r9 0xaacpi_pdirpa+0x8be68
> r10 0x843d1fe10f0343b5
> r11 0x871ebb2341e37234
> r12 0xff036df6f800
> r13 0x80b85f00
> r14 0xff036df6f560
> r15 0x2000
> rip 0x81253ea2uvm_unmap_remove+0x212
> cs 0x8
> rflags 0x10246__ALIGN_SIZE+0xf246
> rsp 0x8000223824c0
> ss 0x10
> uvm_unmap_remove+0x212: movq0x100(%r13),%r8
> ddb{4}> ps
>PID TID PPIDUID S FLAGS WAIT COMMAND
> 17768 177047 33715 1000 30x100082 netio vmctl
> 29298 159270 33715 1000 30x100082 selectssh
> 64908 229787 65965107 30x100090 fsleepvmd
> *64908 51765 65965107 7 0x4100010vmd
> 64908 303902 65965107 3 0x4100090 kqreadvmd
> 13897 386612 33715 1000 30x100082 kqreadcu
> 73064 419314 33715 1000 30x100082 selectssh
> 4542 45446 33715 1000 30x100082 selectssh
> 68055 103187 65965107 30x100090 fsleepvmd
> 68055 234837 65965107 7 0x4100010vmd
> 68055 264629 65965107 3 0x4100090 kqreadvmd
> 52273 63673 33715 1000 30x100082 kqreadcu
> 66423 519194 65965107 30x100090 fsleepvmd
> 66423 290968 65965107 7 0x4100010vmd
> 66423 87324 65965107 3 0x4100090 kqreadvmd
> 99721 216090 33715 1000 30x100082 kqreadcu
> 94925 180901 59444 1000 30x100083 ttyin ksh
> 59444 245156 97608 1000 30x90 selectsshd
> 97608 190596 7060 0 30x92 poll sshd
> 33715 486116 47331 1000 30x82 thrsleep syz-manager
> 33715 476656 47331 1000 3 0x482 nanosleep syz-manager
> 33715 250648 47331 1000 3 0x482 thrsleep syz-manager
> 33715 416559 47331 1000 3 0x482 thrsleep syz-manager
> 33715 446496 47331 1000 3 0x482 thrsleep syz-manager
> 33715 28430 47331 1000 3 0x482 wait syz-manager
> 33715 416959 47331 1000 3 0x482 thrsleep syz-manager
> 33715 35863 47331 1000 3 0x482 thrsleep syz-manager
> 33715 12026 47331 1000 3 0x482 thrsleep syz-manager
> 33715 50683 47331 1000 3 0x482 thrsleep syz-manager
> 33715 263314 47331 1000 3 0x482 thrsleep syz-manager
> 33715 270714 47331 1000 3 0x482 thrsleep syz-manager
> 33715 504545 47331 1000 3 0x482 thrsleep syz-manager
> 33715 37212 47331 1000 3 0x482 thrsleep syz-manager
> 33715 487285 47331 1000 3 0x482 kqreadsyz-manager
> 33715 367916 47331 1000 3 0x482 thrsleep syz-manager
Re: Qcow2: Clean up logging/error handling
On Sun, 28 Oct 2018 00:58:57 +0200, Reyk Floeter wrote:
> Most of these are fatal and log_debug. Keep the __func__ there! But we___ll
> remove it from other logging functions where it was never intended to be
> used and potentially reword the warnings nicely.
>
> Reyk
>
Alright, updated accordingly, with __func__ on fatals, and without it
on __warn__s. Also touched a few places where I noticed it in config.c.
Ok?
diff --git usr.sbin/vmd/config.c usr.sbin/vmd/config.c
index 6e7ed279d6b..7dc2b266dfe 100644
--- usr.sbin/vmd/config.c
+++ usr.sbin/vmd/config.c
@@ -284,8 +284,7 @@ config_setvm(struct privsep *ps, struct vmd_vm *vm,
uint32_t peerid, uid_t uid)
*/
if (kernfd == -1 && !vmboot &&
(kernfd = open(VM_DEFAULT_BIOS, O_RDONLY)) == -1) {
- log_warn("%s: can't open %s", __func__,
- VM_DEFAULT_BIOS);
+ log_warn("can't open %s", VM_DEFAULT_BIOS);
errno = VMD_BIOS_MISSING;
goto fail;
}
@@ -305,8 +304,7 @@ config_setvm(struct privsep *ps, struct vmd_vm *vm,
uint32_t peerid, uid_t uid)
/* Stat cdrom to ensure it is a regular file */
if ((cdromfd =
open(vcp->vcp_cdrom, O_RDONLY)) == -1) {
- log_warn("%s: can't open cdrom %s", __func__,
- vcp->vcp_cdrom);
+ log_warn("can't open cdrom %s", vcp->vcp_cdrom);
errno = VMD_CDROM_MISSING;
goto fail;
}
@@ -325,14 +323,14 @@ config_setvm(struct privsep *ps, struct vmd_vm *vm,
uint32_t peerid, uid_t uid)
for (i = 0 ; i < vcp->vcp_ndisks; i++) {
if (strlcpy(path, vcp->vcp_disks[i], sizeof(path))
>= sizeof(path))
- log_warnx("%s, disk path too long", __func__);
+ log_warnx("disk path %s too long", vcp->vcp_disks[i]);
memset(vmc->vmc_diskbases, 0, sizeof(vmc->vmc_diskbases));
oflags = O_RDWR|O_EXLOCK|O_NONBLOCK;
aflags = R_OK|W_OK;
for (j = 0; j < VM_MAX_BASE_PER_DISK; j++) {
/* Stat disk[i] to ensure it is a regular file */
if ((diskfds[i][j] = open(path, oflags)) == -1) {
- log_warn("%s: can't open disk %s", __func__,
+ log_warn("can't open disk %s",
vcp->vcp_disks[i]);
errno = VMD_DISK_MISSING;
goto fail;
@@ -487,7 +485,7 @@ config_setvm(struct privsep *ps, struct vmd_vm *vm,
uint32_t peerid, uid_t uid)
fail:
saved_errno = errno;
- log_warnx("%s: failed to start vm %s", __func__, vcp->vcp_name);
+ log_warnx("failed to start vm %s", __func__, vcp->vcp_name);
if (kernfd != -1)
close(kernfd);
diff --git usr.sbin/vmd/vioqcow2.c usr.sbin/vmd/vioqcow2.c
index 28b087df39a..de9068827c5 100644
--- usr.sbin/vmd/vioqcow2.c
+++ usr.sbin/vmd/vioqcow2.c
@@ -102,9 +102,9 @@ struct qcdisk {
extern char *__progname;
static off_t xlate(struct qcdisk *, off_t, int *);
-static int copy_cluster(struct qcdisk *, struct qcdisk *, off_t, off_t);
+static void copy_cluster(struct qcdisk *, struct qcdisk *, off_t, off_t);
+static void inc_refs(struct qcdisk *, off_t, int);
static off_t mkcluster(struct qcdisk *, struct qcdisk *, off_t, off_t);
-static int inc_refs(struct qcdisk *, off_t, int);
static int qc2_open(struct qcdisk *, int *, size_t);
static ssize_t qc2_pread(void *, char *, size_t, off_t);
static ssize_t qc2_pwrite(void *, char *, size_t, off_t);
@@ -126,7 +126,7 @@ virtio_init_qcow2(struct virtio_backing *file, off_t *szp,
int *fd, size_t nfd)
if (diskp == NULL)
return -1;
if (qc2_open(diskp, fd, nfd) == -1) {
- log_warnx("%s: could not open qcow2 disk", __func__);
+ log_warnx("could not open qcow2 disk");
return -1;
}
file->p = diskp;
@@ -137,6 +137,10 @@ virtio_init_qcow2(struct virtio_backing *file, off_t *szp,
int *fd, size_t nfd)
return 0;
}
+/*
+ * Return the path to the base image given a disk image.
+ * Called from vmctl.
+ */
ssize_t
virtio_qcow2_get_base(int fd, char *path, size_t npath, const char *dpath)
{
@@ -147,11 +151,11 @@ virtio_qcow2_get_base(int fd, char *path, size_t npath,
const char *dpath)
char *s = NULL;
if (pread(fd, , sizeof(header), 0) != sizeof(header)) {
- log_warnx("%s: short read on header", __func__);
+ log_warnx("short read on header");
return -1;
}
if (strncmp(header.magic, VM_MAGIC_QCOW, strlen(VM_MAGIC_QCOW)) != 0) {
- log_warn("%s: invalid magic numbers", __func__);
Re: inteldrm(4): uvm write-combining support
On Sat, Oct 27, 2018 at 12:31:48PM +0200, Mark Kettenis wrote:
> > Date: Sat, 27 Oct 2018 17:16:57 +1100
> > From: Jonathan Gray
> >
> > On Fri, Oct 26, 2018 at 09:47:51PM +0200, Mark Kettenis wrote:
> > > Diff below adds support to uvm to create wtite-combining mappings and
> > > uses this to implement support for the I915_MMAP_WC flag in
> > > inteldrm(4). With this diff I can re-enable support for the
> > > I915_PARAM_MMAP_VERSION.
> > >
> > > Please test.
> >
> > Defining PMAP_WC like that in uvm headers will make
> > pgprot_writecombine() in dev/pci/drm/drm_linux.h take the wrong path
> > on archs without PMAP_WC.
> >
> > static inline pgprot_t
> > pgprot_writecombine(pgprot_t prot)
> > {
> > #ifdef PMAP_WC
> > return prot | PMAP_WC;
> > #else
> > return prot | PMAP_NOCACHE;
> > #endif
> > }
> >
> > Should it instead be #if defined(PMAP_WC) && PMAP_WC > 0
> > or just #if PMAP_WC > 0 here?
>
> Good catch. Maybe at some point we should revisit the naming of these
> flags and make them more uniform across architectures. Write-combing
> is a bit of an x86-specific thing, but other architectures have
> similar concepts.
>
> I went with #if PMAP_WC != 0 and did the same for PMAP_DEVICE.
ok jsg@
>
> Index: dev/pci/drm/drm_linux.h
> ===
> RCS file: /cvs/src/sys/dev/pci/drm/drm_linux.h,v
> retrieving revision 1.91
> diff -u -p -r1.91 drm_linux.h
> --- dev/pci/drm/drm_linux.h 20 Aug 2018 19:33:31 - 1.91
> +++ dev/pci/drm/drm_linux.h 27 Oct 2018 10:24:50 -
> @@ -2123,7 +2123,7 @@ typedef int pgprot_t;
> static inline pgprot_t
> pgprot_writecombine(pgprot_t prot)
> {
> -#ifdef PMAP_WC
> +#if PMAP_WC != 0
> return prot | PMAP_WC;
> #else
> return prot | PMAP_NOCACHE;
> @@ -2133,7 +2133,7 @@ pgprot_writecombine(pgprot_t prot)
> static inline pgprot_t
> pgprot_noncached(pgprot_t prot)
> {
> -#ifdef PMAP_DEVICE
> +#if PMAP_DEVICE != 0
> return prot | PMAP_DEVICE;
> #else
> return prot | PMAP_NOCACHE;
> Index: dev/pci/drm/i915/i915_dma.c
> ===
> RCS file: /cvs/src/sys/dev/pci/drm/i915/i915_dma.c,v
> retrieving revision 1.27
> diff -u -p -r1.27 i915_dma.c
> --- dev/pci/drm/i915/i915_dma.c 25 Oct 2018 20:29:38 - 1.27
> +++ dev/pci/drm/i915/i915_dma.c 27 Oct 2018 10:24:50 -
> @@ -156,11 +156,9 @@ static int i915_getparam(struct drm_devi
> case I915_PARAM_HAS_COHERENT_PHYS_GTT:
> value = 1;
> break;
> -#ifdef notyet
> case I915_PARAM_MMAP_VERSION:
> value = 1;
> break;
> -#endif
> case I915_PARAM_SUBSLICE_TOTAL:
> value = INTEL_INFO(dev)->subslice_total;
> if (!value)
> Index: dev/pci/drm/i915/i915_gem.c
> ===
> RCS file: /cvs/src/sys/dev/pci/drm/i915/i915_gem.c,v
> retrieving revision 1.113
> diff -u -p -r1.113 i915_gem.c
> --- dev/pci/drm/i915/i915_gem.c 13 Sep 2018 03:38:15 - 1.113
> +++ dev/pci/drm/i915/i915_gem.c 27 Oct 2018 10:24:50 -
> @@ -1987,7 +1987,8 @@ i915_gem_mmap_ioctl(struct drm_device *d
> addr = 0;
> ret = -uvm_map(>p_vmspace->vm_map, , size,
> obj->uao, args->offset, 0, UVM_MAPFLAG(PROT_READ | PROT_WRITE,
> - PROT_READ | PROT_WRITE, MAP_INHERIT_SHARE, MADV_RANDOM, 0));
> + PROT_READ | PROT_WRITE, MAP_INHERIT_SHARE, MADV_RANDOM,
> + (args->flags & I915_MMAP_WC) ? UVM_FLAG_WC : 0));
> if (ret == 0)
> uao_reference(obj->uao);
> drm_gem_object_unreference_unlocked(obj);
> Index: uvm/uvm.h
> ===
> RCS file: /cvs/src/sys/uvm/uvm.h,v
> retrieving revision 1.62
> diff -u -p -r1.62 uvm.h
> --- uvm/uvm.h 12 Apr 2018 17:13:44 - 1.62
> +++ uvm/uvm.h 27 Oct 2018 10:24:50 -
> @@ -82,14 +82,15 @@ struct uvm {
> * vm_map_entry etype bits:
> */
>
> -#define UVM_ET_OBJ 0x01/* it is a uvm_object */
> -#define UVM_ET_SUBMAP0x02/* it is a vm_map submap */
> -#define UVM_ET_COPYONWRITE 0x04/* copy_on_write */
> -#define UVM_ET_NEEDSCOPY 0x08/* needs_copy */
> -#define UVM_ET_HOLE 0x10/* no backend */
> -#define UVM_ET_NOFAULT 0x20/* don't fault */
> -#define UVM_ET_STACK 0x40/* this is a stack */
> -#define UVM_ET_FREEMAPPED0x80/* map entry is on free list (DEBUG) */
> +#define UVM_ET_OBJ 0x0001 /* it is a uvm_object */
> +#define UVM_ET_SUBMAP0x0002 /* it is a vm_map submap */
> +#define UVM_ET_COPYONWRITE 0x0004 /* copy_on_write */
> +#define UVM_ET_NEEDSCOPY 0x0008 /* needs_copy */
> +#define UVM_ET_HOLE 0x0010 /* no backend */
> +#define UVM_ET_NOFAULT 0x0020 /* don't fault */
> +#define UVM_ET_STACK
kernel page fault in vm_teardown
My syzkaller machine running a recent snapshot just crashed. The value
0x415efd243b54d319 passed into uvm_map_deallocate looks quite fishy to me.
Some hopefully useful info below.
ddb{4}> trace
uvm_unmap_remove(c05f7f8cd1633180,ff036f57f5a8,80b85f00,ff036f57f598,8000222b8040,0)
at uvm_unmap_remove+0x212
uvm_map_deallocate(415efd243b54d319) at uvm_map_deallocate+0x5e
vm_teardown(ff036f57f3d8) at vm_teardown+0xf0
vm_run(a186e3e68e0c8d2d) at vm_run+0x226
VOP_IOCTL(d3bfd0b457c4b224,ff03c9c6f5f0,32269d81b8d394bf,8000222b4968,ff043f7ca420,3)
at VOP_IOCTL+0x5a
vn_ioctl(d3bfd0b4579725f3,ff03ca9e15b0,8000222b4968,20) at
vn_ioctl+0x6b
sys_ioctl(7867d986861f8ba2,360,8000222b4968) at sys_ioctl+0x3ec
syscall(3871e5d148df7b3d) at syscall+0x32a
Xsyscall(0,36,0,36,1fc2fafb52d0,1fc2faf35000) at Xsyscall+0x128
end of kernel
end trace frame: 0x1fc5a67a25b0, count: -9
ddb{4}> show proc
PROC (vmd) pid=51765 stat=onproc
flags process=100010 proc=400
pri=86, usrpri=86, nice=20
forw=0x, list=0x8000222b5520,0x8000222b4270
process=0x8000fffecfc8 user=0x80002237d000,
vmspace=0xff03c12e9
c70
estcpu=36, cpticks=110340, pctcpu=13.31
user=0, sys=110290, intr=0
ddb{4}> show registers
rdi 0x313679acpi_pdirpa+0x2ff4e1
rsi 0x20656874203a7374
rbp 0x800022382510
rbx 0x8000223824d0
rdx 0x11f010acpi_pdirpa+0x10ae78
rcx0
rax 0xff01189c9c80
r8 0x3
r9 0xaacpi_pdirpa+0x8be68
r10 0x843d1fe10f0343b5
r11 0x871ebb2341e37234
r12 0xff036df6f800
r13 0x80b85f00
r14 0xff036df6f560
r15 0x2000
rip 0x81253ea2uvm_unmap_remove+0x212
cs 0x8
rflags 0x10246__ALIGN_SIZE+0xf246
rsp 0x8000223824c0
ss 0x10
uvm_unmap_remove+0x212: movq0x100(%r13),%r8
ddb{4}> ps
PID TID PPIDUID S FLAGS WAIT COMMAND
17768 177047 33715 1000 30x100082 netio vmctl
29298 159270 33715 1000 30x100082 selectssh
64908 229787 65965107 30x100090 fsleepvmd
*64908 51765 65965107 7 0x4100010vmd
64908 303902 65965107 3 0x4100090 kqreadvmd
13897 386612 33715 1000 30x100082 kqreadcu
73064 419314 33715 1000 30x100082 selectssh
4542 45446 33715 1000 30x100082 selectssh
68055 103187 65965107 30x100090 fsleepvmd
68055 234837 65965107 7 0x4100010vmd
68055 264629 65965107 3 0x4100090 kqreadvmd
52273 63673 33715 1000 30x100082 kqreadcu
66423 519194 65965107 30x100090 fsleepvmd
66423 290968 65965107 7 0x4100010vmd
66423 87324 65965107 3 0x4100090 kqreadvmd
99721 216090 33715 1000 30x100082 kqreadcu
94925 180901 59444 1000 30x100083 ttyin ksh
59444 245156 97608 1000 30x90 selectsshd
97608 190596 7060 0 30x92 poll sshd
33715 486116 47331 1000 30x82 thrsleep syz-manager
33715 476656 47331 1000 3 0x482 nanosleep syz-manager
33715 250648 47331 1000 3 0x482 thrsleep syz-manager
33715 416559 47331 1000 3 0x482 thrsleep syz-manager
33715 446496 47331 1000 3 0x482 thrsleep syz-manager
33715 28430 47331 1000 3 0x482 wait syz-manager
33715 416959 47331 1000 3 0x482 thrsleep syz-manager
33715 35863 47331 1000 3 0x482 thrsleep syz-manager
33715 12026 47331 1000 3 0x482 thrsleep syz-manager
33715 50683 47331 1000 3 0x482 thrsleep syz-manager
33715 263314 47331 1000 3 0x482 thrsleep syz-manager
33715 270714 47331 1000 3 0x482 thrsleep syz-manager
33715 504545 47331 1000 3 0x482 thrsleep syz-manager
33715 37212 47331 1000 3 0x482 thrsleep syz-manager
33715 487285 47331 1000 3 0x482 kqreadsyz-manager
33715 367916 47331 1000 3 0x482 thrsleep syz-manager
33715 365101 47331 1000 3 0x482 thrsleep syz-manager
33715 175614 47331 1000 3 0x482 thrsleep syz-manager
33715 86128 47331 1000 3 0x482 thrsleep syz-manager
33715 243048 47331 1000 3 0x482 thrsleep syz-manager
33715 65128 47331 1000 3 0x482 thrsleep syz-manager
7782 391573 1 0 3
Re: disable fs access on ripd
Sebastian Benoit wrote:
> Florian Obser(flor...@openbsd.org) on 2018.10.30 18:32:15 +0100:
> > On Tue, Oct 30, 2018 at 10:54:10AM -0600, Theo de Raadt wrote:
> > > Remi Locherer wrote:
> > >
> > > > On Tue, Oct 30, 2018 at 03:20:35PM +, Ricardo Mestre wrote:
> > > > > Hi,
> > > > >
> > > > > After all files are opened ripd(8) can have the fs access disabled
> > > > > just before
> > > > > each process main loop. Its 2 childs already run under chroot, but
> > > > > since they
> > > > > are still not pledged at least they have no way to read/write/create
> > > > > files within
> > > > > the chroot. No loads or reloads of the config file happen through any
> > > > > signal,
> > > > > nor can we do it via ripctl(8).
> > > > >
> > > > > I was able to run a simple daemon with the example file. Comments? OK?
> > > >
> > > > control_cleanup() unlinks the control socket on exit. I think you should
> > > > either unveil(conf->csock, "c") or remove control_cleanup().
> > >
> > > I don't understand this latter comment, let me ask.
> > >
> > > You think it is smart to leave these sockets lying around?
> > >
> >
> > Back in the summer the consensus was that it is fine to leave the
> > sockets lying around. Furthermore the consensus was that it is worth
> > to do so if we can drop the cpath pledge in daemons. (IIRC at least benno,
> > claudio, deraadt, florian and mestre where in the loop.)
> >
> > mestre went on a rampage and removed control_cleanup and cpath pledge
> > in a bunch of daemons. I think he stopped when he couldn't find any
> > more daemons where the parent process was pledged.
> >
> > This was before the
> > unveil("/", "r");
> > unveil(control_socket, "rwc");
> > pattern was discovered which is also very powerful.
> >
> > > I suspect there are a few oddball programs where it makes senes, but as
> > > a general rule I think it is a bad idea; as stated in other threads it
> > > means control programs and restart sequences have a bunch more oddball
> > > behaviours which will be inconsistant.
> >
> > I want consistency, if we go with the unveil pattern, fine, then we
> > should restore the control_cleanup code in a bunch of daemons.
> >
> > However:
> > - the control program behaviour change had been discussed in the
> > summer and the only difference discovered then was "file not found" vs.
> > "connection refused".
> > - benno's comment about only one instance of ospfd running is not
> > effected by leaving the socket behind, it checks if the socket has a
> > listener. a dead socket is no problem.
> > - daemons/routers might crash and leave a socket behind anyway. they
> > must be able to deal with this and I'm pretty sure they all do. I just
> > checked ospfd, bgpd and rad.
>
> i agree with that. In the summer i first thought it would be better to clean
> up, but the discussion back then convinced me otherwise, and florian summed
> it up nicely.
Fine, but let's try to make them consistant?
Re: faq16: remove unneeded vether(4) from bridging example
On Tue, Oct 30, 2018 at 08:43:01PM +0100, Klemens Nanni wrote: > Prompted by a question on the internet, the following diff removes the > useless virtual ethernet interface from > "Option 4 - VMs as real hosts on the same network" where guests are > bridged directly into the host network. > > vether(4) serves no purpose in this setup, the egress interface can/will > be used to communicate with guests. > > As florian@ noted, this is probably a copy/pasta left-over from > "Option 3". > > Feedback? Objections? OK? > Ok ccardenas@ +--+ Carlos
Re: disable fs access on ripd
>From my point of view, and my sole opinion, the process running as root
from a long standing daemon, and which is not under a chroot, should not have
the hability to write and/or create files (even better if it cannot even
read). But we should reach a consensus once again, and if control_cleanup()
should be brought back to the daemons I removed it from I won't be against it.
The combination of unveil/pledge, or even just by using unveil is very
powerful and if we by all means can limit the access the most we can to the
filesystem is really great against this type of attack vector.
On 21:53 Tue 30 Oct , Sebastian Benoit wrote:
> Florian Obser(flor...@openbsd.org) on 2018.10.30 18:32:15 +0100:
> > On Tue, Oct 30, 2018 at 10:54:10AM -0600, Theo de Raadt wrote:
> > > Remi Locherer wrote:
> > >
> > > > On Tue, Oct 30, 2018 at 03:20:35PM +, Ricardo Mestre wrote:
> > > > > Hi,
> > > > >
> > > > > After all files are opened ripd(8) can have the fs access disabled
> > > > > just before
> > > > > each process main loop. Its 2 childs already run under chroot, but
> > > > > since they
> > > > > are still not pledged at least they have no way to read/write/create
> > > > > files within
> > > > > the chroot. No loads or reloads of the config file happen through any
> > > > > signal,
> > > > > nor can we do it via ripctl(8).
> > > > >
> > > > > I was able to run a simple daemon with the example file. Comments? OK?
> > > >
> > > > control_cleanup() unlinks the control socket on exit. I think you should
> > > > either unveil(conf->csock, "c") or remove control_cleanup().
> > >
> > > I don't understand this latter comment, let me ask.
> > >
> > > You think it is smart to leave these sockets lying around?
> > >
> >
> > Back in the summer the consensus was that it is fine to leave the
> > sockets lying around. Furthermore the consensus was that it is worth
> > to do so if we can drop the cpath pledge in daemons. (IIRC at least benno,
> > claudio, deraadt, florian and mestre where in the loop.)
> >
> > mestre went on a rampage and removed control_cleanup and cpath pledge
> > in a bunch of daemons. I think he stopped when he couldn't find any
> > more daemons where the parent process was pledged.
> >
> > This was before the
> > unveil("/", "r");
> > unveil(control_socket, "rwc");
> > pattern was discovered which is also very powerful.
> >
> > > I suspect there are a few oddball programs where it makes senes, but as
> > > a general rule I think it is a bad idea; as stated in other threads it
> > > means control programs and restart sequences have a bunch more oddball
> > > behaviours which will be inconsistant.
> >
> > I want consistency, if we go with the unveil pattern, fine, then we
> > should restore the control_cleanup code in a bunch of daemons.
> >
> > However:
> > - the control program behaviour change had been discussed in the
> > summer and the only difference discovered then was "file not found" vs.
> > "connection refused".
> > - benno's comment about only one instance of ospfd running is not
> > effected by leaving the socket behind, it checks if the socket has a
> > listener. a dead socket is no problem.
> > - daemons/routers might crash and leave a socket behind anyway. they
> > must be able to deal with this and I'm pretty sure they all do. I just
> > checked ospfd, bgpd and rad.
>
> i agree with that. In the summer i first thought it would be better to clean
> up, but the discussion back then convinced me otherwise, and florian summed
> it up nicely.
Re: disable fs access on ripd
Florian Obser(flor...@openbsd.org) on 2018.10.30 18:32:15 +0100:
> On Tue, Oct 30, 2018 at 10:54:10AM -0600, Theo de Raadt wrote:
> > Remi Locherer wrote:
> >
> > > On Tue, Oct 30, 2018 at 03:20:35PM +, Ricardo Mestre wrote:
> > > > Hi,
> > > >
> > > > After all files are opened ripd(8) can have the fs access disabled just
> > > > before
> > > > each process main loop. Its 2 childs already run under chroot, but
> > > > since they
> > > > are still not pledged at least they have no way to read/write/create
> > > > files within
> > > > the chroot. No loads or reloads of the config file happen through any
> > > > signal,
> > > > nor can we do it via ripctl(8).
> > > >
> > > > I was able to run a simple daemon with the example file. Comments? OK?
> > >
> > > control_cleanup() unlinks the control socket on exit. I think you should
> > > either unveil(conf->csock, "c") or remove control_cleanup().
> >
> > I don't understand this latter comment, let me ask.
> >
> > You think it is smart to leave these sockets lying around?
> >
>
> Back in the summer the consensus was that it is fine to leave the
> sockets lying around. Furthermore the consensus was that it is worth
> to do so if we can drop the cpath pledge in daemons. (IIRC at least benno,
> claudio, deraadt, florian and mestre where in the loop.)
>
> mestre went on a rampage and removed control_cleanup and cpath pledge
> in a bunch of daemons. I think he stopped when he couldn't find any
> more daemons where the parent process was pledged.
>
> This was before the
> unveil("/", "r");
> unveil(control_socket, "rwc");
> pattern was discovered which is also very powerful.
>
> > I suspect there are a few oddball programs where it makes senes, but as
> > a general rule I think it is a bad idea; as stated in other threads it
> > means control programs and restart sequences have a bunch more oddball
> > behaviours which will be inconsistant.
>
> I want consistency, if we go with the unveil pattern, fine, then we
> should restore the control_cleanup code in a bunch of daemons.
>
> However:
> - the control program behaviour change had been discussed in the
> summer and the only difference discovered then was "file not found" vs.
> "connection refused".
> - benno's comment about only one instance of ospfd running is not
> effected by leaving the socket behind, it checks if the socket has a
> listener. a dead socket is no problem.
> - daemons/routers might crash and leave a socket behind anyway. they
> must be able to deal with this and I'm pretty sure they all do. I just
> checked ospfd, bgpd and rad.
i agree with that. In the summer i first thought it would be better to clean
up, but the discussion back then convinced me otherwise, and florian summed
it up nicely.
Re: top: merge duplicate code into helper functions
On Tue, Oct 30, 2018 at 02:24:46PM -0600, Todd C. Miller wrote:
> I really dislike the side-effect in filteruser(), see below.
> > +static int
> > +filteruser(char buf[])
> > +{
> > + char *bufp = buf;
> > + uid_t *uidp;
> > +
> > + if (bufp[0] == '-') {
> > + bufp++[0] = ' ';
>
> Why is this needed, can't you just do "bufp++"?
Because we cannot increment the function argument directly:
int
rundisplay(void)
{
static char tempbuf[TEMPBUFSIZE];
...
case CMD_user:
...
} else if (filteruser(tempbuf) == -1) {
Hence filteruser() uses a pointer to it. Replacing the prefixed dash
with a space it not neccessary for parsing, it merely keeps error
messages intact.
With just `bufp++' the command "u-kn" would print " -foo: unknown user".
Re: top: merge duplicate code into helper functions
On Tue, 30 Oct 2018 21:09:29 +0100, Klemens Nanni wrote:
> This introduces filteruser() and filterpid(), where the first one can
> now easily be extended to support matching numeric UIDs. The latter one
> is now used by the `highlight` command as well.
I really dislike the side-effect in filteruser(), see below.
Otherwise looks good.
- todd
> Index: top.c
> ===
> RCS file: /cvs/src/usr.bin/top/top.c,v
> retrieving revision 1.94
> diff -u -p -r1.94 top.c
> --- top.c 5 Oct 2018 18:56:57 - 1.94
> +++ top.c 30 Oct 2018 19:56:07 -
> @@ -83,7 +83,7 @@ int no_command = Yes;
> int old_system = No;
> int old_threads = No;
> int show_args = No;
> -pid_t hlpid = -1;
> +pid_t hlpid = (pid_t)-1;
> int combine_cpus = 0;
>
> #if Default_TOPN == Infinity
> @@ -131,6 +131,46 @@ usage(void)
> __progname);
> }
>
> +static int
> +filteruser(char buf[])
> +{
> + char *bufp = buf;
> + uid_t *uidp;
> +
> + if (bufp[0] == '-') {
> + bufp++[0] = ' ';
Why is this needed, can't you just do "bufp++"?
> + uidp =
> + ps.uid = (pid_t)-1;
> + } else {
> + uidp =
> + ps.huid = (pid_t)-1;
> + }
> +
> + return uid_from_user(bufp, uidp);
> +}
> +
> +static int
> +filterpid(char buf[], int hl)
> +{
> + const char *errstr;
> + int pid;
> +
> + pid = strtonum(buf, 0, INT_MAX, );
> + if (errstr != NULL || !find_pid(pid))
> + return -1;
> +
> + if (hl == Yes)
> + hlpid = (pid_t)pid;
> + else {
> + if (ps.system == No)
> + old_system = No;
> + ps.pid = (pid_t)pid;
> + ps.system = Yes;
> + }
> +
> + return 0;
> +}
> +
> static void
> parseargs(int ac, char **av)
> {
diff - set ipv6 tclass
Hello -
ftp, ntpd, and telnet set IPTOS_LOWDELAY/IPTOS_THROUGHPUT for IPv4
sockets. Should we do it for IPv6 as well? I used
IPTOS_DCSP_AF21/IPTOS_DSCP_CS1 to match the values ssh uses by default.
Thoughts?
Index: usr.bin/ftp/ftp.c
===
RCS file: /cvs/src/usr.bin/ftp/ftp.c,v
retrieving revision 1.100
diff -u -p -r1.100 ftp.c
--- usr.bin/ftp/ftp.c 22 Aug 2016 16:27:00 - 1.100
+++ usr.bin/ftp/ftp.c 30 Oct 2018 19:31:34 -
@@ -257,10 +257,15 @@ hookup(char *host, char *port)
code = -1;
goto bad;
}
- if (hisctladdr.sa.sa_family == AF_INET) {
+ switch (hisctladdr.sa.sa_family) {
+ case AF_INET:
tos = IPTOS_LOWDELAY;
- if (setsockopt(s, IPPROTO_IP, IP_TOS, (char *),
sizeof(int)) < 0)
- warn("setsockopt TOS (ignored)");
+ if (setsockopt(s, IPPROTO_IP, IP_TOS, , sizeof(tos)) < 0)
+ warn("setsockopt IPTOS_LOWDELAY");
+ case AF_INET6:
+ tos = IPTOS_DSCP_AF21;
+ if (setsockopt(s, IPPROTO_IPV6, IPV6_TCLASS, , sizeof(tos))
< 0)
+ warn("setsockopt IPTOS_DSCP_AF21");
}
cin = fdopen(s, "r");
cout = fdopen(s, "w");
@@ -1524,11 +1529,17 @@ reinit:
warn("connect");
goto bad;
}
- if (data_addr.sa.sa_family == AF_INET) {
+ switch (data_addr.sa.sa_family) {
+ case AF_INET:
on = IPTOS_THROUGHPUT;
- if (setsockopt(data, IPPROTO_IP, IP_TOS, (char *),
- sizeof(int)) < 0)
- warn("setsockopt TOS (ignored)");
+ if (setsockopt(data, IPPROTO_IP, IP_TOS, ,
+ sizeof(on)) < 0)
+ warn("setsockopt IPTOS_THROUGHPUT");
+ case AF_INET6:
+ on = IPTOS_DSCP_CS1;
+ if (setsockopt(data, IPPROTO_IPV6, IPV6_TCLASS, ,
+ sizeof(on)) < 0)
+ warn("setsockopt IPTOS_DSCP_CS1");
}
return (0);
}
@@ -1682,7 +1693,7 @@ dataconn(const char *lmode)
{
union sockaddr_union from;
socklen_t fromlen = myctladdr.sa.sa_len;
- int s;
+ int s, tos;
if (passivemode)
return (fdopen(data, lmode));
@@ -1695,12 +1706,16 @@ dataconn(const char *lmode)
}
(void)close(data);
data = s;
- if (from.sa.sa_family == AF_INET) {
- int tos = IPTOS_THROUGHPUT;
- if (setsockopt(s, IPPROTO_IP, IP_TOS, (char *),
- sizeof(int)) < 0) {
- warn("setsockopt TOS (ignored)");
- }
+ switch (from.sa.sa_family) {
+ case AF_INET:
+ tos = IPTOS_THROUGHPUT;
+ if (setsockopt(s, IPPROTO_IP, IP_TOS, , sizeof(tos)) < 0)
+ warn("setsockopt IPTOS_THROUGHPUT");
+ case AF_INET6:
+ tos = IPTOS_DSCP_CS1;
+ if (setsockopt(s, IPPROTO_IPV6, IPV6_TCLASS, ,
+ sizeof(tos)) < 0)
+ warn("setsockopt IPTOS_DSCP_CS1");
}
return (fdopen(data, lmode));
}
Index: usr.bin/telnet/commands.c
===
RCS file: /cvs/src/usr.bin/telnet/commands.c,v
retrieving revision 1.86
diff -u -p -r1.86 commands.c
--- usr.bin/telnet/commands.c 30 Sep 2018 14:35:32 - 1.86
+++ usr.bin/telnet/commands.c 30 Oct 2018 19:31:35 -
@@ -1742,8 +1742,8 @@ tn(int argc, char *argv[])
{
struct addrinfo hints, *res, *res0;
char *cmd, *hostp = 0, *portp = 0, *user = 0, *aliasp = 0;
-int error, retry;
-const int niflags = NI_NUMERICHOST, tos = IPTOS_LOWDELAY;
+int error, retry, tos;
+const int niflags = NI_NUMERICHOST;
if (connected) {
printf("?Already connected to %s\r\n", hostname);
@@ -1870,11 +1870,13 @@ tn(int argc, char *argv[])
switch (res->ai_family) {
case AF_INET:
+ tos = IPTOS_LOWDELAY;
if (setsockopt(net, IPPROTO_IP, IP_TOS, , sizeof(tos)) < 0
&& errno != ENOPROTOOPT)
perror("telnet: setsockopt (IP_TOS) (ignored)");
break;
case AF_INET6:
+ tos = IPTOS_DSCP_AF21;
if (setsockopt(net, IPPROTO_IPV6, IPV6_TCLASS, ,
sizeof(tos)) < 0 && errno != ENOPROTOOPT)
perror("telnet: setsockopt (IPV6_TCLASS) (ignored)");
Index: usr.sbin/ntpd/client.c
===
RCS file: /cvs/src/usr.sbin/ntpd/client.c,v
retrieving revision 1.105
diff
Re: top: merge duplicate code into helper functions
Another approach to merging code from options and interactive commands.
This introduces filteruser() and filterpid(), where the first one can
now easily be extended to support matching numeric UIDs. The latter one
is now used by the `highlight` command as well.
Feedback? OK?
Index: top.c
===
RCS file: /cvs/src/usr.bin/top/top.c,v
retrieving revision 1.94
diff -u -p -r1.94 top.c
--- top.c 5 Oct 2018 18:56:57 - 1.94
+++ top.c 30 Oct 2018 19:56:07 -
@@ -83,7 +83,7 @@ int no_command = Yes;
int old_system = No;
int old_threads = No;
int show_args = No;
-pid_t hlpid = -1;
+pid_t hlpid = (pid_t)-1;
int combine_cpus = 0;
#if Default_TOPN == Infinity
@@ -131,6 +131,46 @@ usage(void)
__progname);
}
+static int
+filteruser(char buf[])
+{
+ char *bufp = buf;
+ uid_t *uidp;
+
+ if (bufp[0] == '-') {
+ bufp++[0] = ' ';
+ uidp =
+ ps.uid = (pid_t)-1;
+ } else {
+ uidp =
+ ps.huid = (pid_t)-1;
+ }
+
+ return uid_from_user(bufp, uidp);
+}
+
+static int
+filterpid(char buf[], int hl)
+{
+ const char *errstr;
+ int pid;
+
+ pid = strtonum(buf, 0, INT_MAX, );
+ if (errstr != NULL || !find_pid(pid))
+ return -1;
+
+ if (hl == Yes)
+ hlpid = (pid_t)pid;
+ else {
+ if (ps.system == No)
+ old_system = No;
+ ps.pid = (pid_t)pid;
+ ps.system = Yes;
+ }
+
+ return 0;
+}
+
static void
parseargs(int ac, char **av)
{
@@ -150,32 +190,16 @@ parseargs(int ac, char **av)
break;
case 'U': /* display only username's processes */
- if (optarg[0] == '-') {
- if (uid_from_user(optarg+1, ) == -1)
- new_message(MT_delayed,
- "%s: unknown user", optarg);
- else
- ps.uid = (uid_t)-1;
- } else if (uid_from_user(optarg, ) == -1)
+ if (filteruser(optarg) == -1)
new_message(MT_delayed, "%s: unknown user",
optarg);
- else
- ps.huid = (uid_t)-1;
break;
- case 'p': { /* display only process id */
- const char *errstr;
-
- i = strtonum(optarg, 0, INT_MAX, );
- if (errstr != NULL || !find_pid(i))
+ case 'p': /* display only process id */
+ if (filterpid(optarg, No) == -1)
new_message(MT_delayed, "%s: unknown pid",
optarg);
- else {
- ps.pid = (pid_t)i;
- ps.system = Yes;
- }
break;
- }
case 'S': /* show system processes */
ps.system = !ps.system;
@@ -802,20 +826,10 @@ rundisplay(void)
tempbuf[1] == '\0') {
ps.uid = (uid_t)-1;
ps.huid = (uid_t)-1;
- } else if (tempbuf[0] == '-') {
- if (uid_from_user(tempbuf+1, )
== -1) {
- new_message(MT_standout,
- " %s: unknown user",
tempbuf+1);
- no_command = Yes;
- } else {
- ps.uid = (uid_t)-1;
- }
- } else if (uid_from_user(tempbuf, ) ==
-1) {
- new_message(MT_standout,
- " %s: unknown user",
tempbuf);
- no_command = Yes;
- } else {
- ps.huid = (uid_t)-1;
+ } else if (filteruser(tempbuf) == -1) {
+ new_message(MT_standout,
+ "%s: unknown user", tempbuf);
+ no_command = Yes;
}
putr();
} else
@@ -854,23 +868,10 @@ rundisplay(void)
tempbuf[1] == '\0') {
ps.pid =
faq16: remove unneeded vether(4) from bridging example
Prompted by a question on the internet, the following diff removes the useless virtual ethernet interface from "Option 4 - VMs as real hosts on the same network" where guests are bridged directly into the host network. vether(4) serves no purpose in this setup, the egress interface can/will be used to communicate with guests. As florian@ noted, this is probably a copy/pasta left-over from "Option 3". Feedback? Objections? OK? Index: faq16.html === RCS file: /cvs/www/faq/faq16.html,v retrieving revision 1.6 diff -u -p -r1.6 faq16.html --- faq16.html 30 Oct 2018 12:10:34 - 1.6 +++ faq16.html 30 Oct 2018 19:31:01 - @@ -282,25 +282,13 @@ This option only works for Ethernet-base prevents wireless interfaces from participating in network bridges. -Create the vether0 interface: - - -# echo 'up' > /etc/hostname.vether0 -# sh /etc/netstart vether0 - - -The vether0 interface will be used as the local interface for the -host that is managed via DHCP on the host network. - - Create the bridge0 interface with the host network interface as a -bridge port, along with the newly created vether0 interface. +bridge port. In this example, the host network interface is em0 - you should substitute the interface name that you wish to connect the VM to: # echo 'add em0' > /etc/hostname.bridge0 -# echo 'add vether0' >> /etc/hostname.bridge0 # sh /etc/netstart bridge0
Re: disable fs access on ripd
On Tue, Oct 30, 2018 at 10:54:10AM -0600, Theo de Raadt wrote:
> Remi Locherer wrote:
>
> > On Tue, Oct 30, 2018 at 03:20:35PM +, Ricardo Mestre wrote:
> > > Hi,
> > >
> > > After all files are opened ripd(8) can have the fs access disabled just
> > > before
> > > each process main loop. Its 2 childs already run under chroot, but since
> > > they
> > > are still not pledged at least they have no way to read/write/create
> > > files within
> > > the chroot. No loads or reloads of the config file happen through any
> > > signal,
> > > nor can we do it via ripctl(8).
> > >
> > > I was able to run a simple daemon with the example file. Comments? OK?
> >
> > control_cleanup() unlinks the control socket on exit. I think you should
> > either unveil(conf->csock, "c") or remove control_cleanup().
>
> I don't understand this latter comment, let me ask.
>
> You think it is smart to leave these sockets lying around?
>
Back in the summer the consensus was that it is fine to leave the
sockets lying around. Furthermore the consensus was that it is worth
to do so if we can drop the cpath pledge in daemons. (IIRC at least benno,
claudio, deraadt, florian and mestre where in the loop.)
mestre went on a rampage and removed control_cleanup and cpath pledge
in a bunch of daemons. I think he stopped when he couldn't find any
more daemons where the parent process was pledged.
This was before the
unveil("/", "r");
unveil(control_socket, "rwc");
pattern was discovered which is also very powerful.
> I suspect there are a few oddball programs where it makes senes, but as
> a general rule I think it is a bad idea; as stated in other threads it
> means control programs and restart sequences have a bunch more oddball
> behaviours which will be inconsistant.
I want consistency, if we go with the unveil pattern, fine, then we
should restore the control_cleanup code in a bunch of daemons.
However:
- the control program behaviour change had been discussed in the
summer and the only difference discovered then was "file not found" vs.
"connection refused".
- benno's comment about only one instance of ospfd running is not
effected by leaving the socket behind, it checks if the socket has a
listener. a dead socket is no problem.
- daemons/routers might crash and leave a socket behind anyway. they
must be able to deal with this and I'm pretty sure they all do. I just
checked ospfd, bgpd and rad.
--
I'm not entirely sure you are real.
Re: disable fs access on ripd
clearly an oversight due to looking at too many daemons at the same
time. since the only thing ripd needs to do is unlink the socket I think
we can remove control_cleanup, even though I'd rather do this
introducing pledge, but for now it's a great compromise.
OK?
Index: control.c
===
RCS file: /cvs/src/usr.sbin/ripd/control.c,v
retrieving revision 1.25
diff -u -p -u -r1.25 control.c
--- control.c 17 Jan 2017 22:10:56 - 1.25
+++ control.c 30 Oct 2018 17:24:44 -
@@ -100,14 +100,6 @@ control_listen(void)
return (0);
}
-void
-control_cleanup(char *path)
-{
- event_del(_state.ev);
- event_del(_state.evt);
- unlink(path);
-}
-
/* ARGSUSED */
void
control_accept(int listenfd, short event, void *bula)
Index: control.h
===
RCS file: /cvs/src/usr.sbin/ripd/control.h,v
retrieving revision 1.5
diff -u -p -u -r1.5 control.h
--- control.h 2 Aug 2016 16:05:32 - 1.5
+++ control.h 30 Oct 2018 17:24:44 -
@@ -39,6 +39,5 @@ int control_listen(void);
void control_accept(int, short, void *);
void control_dispatch_imsg(int, short, void *);
intcontrol_imsg_relay(struct imsg *);
-void control_cleanup(char *);
#endif /* _CONTROL_H_ */
Index: rde.c
===
RCS file: /cvs/src/usr.sbin/ripd/rde.c,v
retrieving revision 1.21
diff -u -p -u -r1.21 rde.c
--- rde.c 3 Sep 2016 10:28:08 - 1.21
+++ rde.c 30 Oct 2018 17:24:44 -
@@ -151,6 +151,11 @@ rde(struct ripd_conf *xconf, int pipe_pa
free(r);
}
+ if (unveil("/", "") == -1)
+ fatal("unveil");
+ if (unveil(NULL, NULL) == -1)
+ fatal("unveil");
+
event_dispatch();
rde_shutdown();
Index: ripd.c
===
RCS file: /cvs/src/usr.sbin/ripd/ripd.c,v
retrieving revision 1.30
diff -u -p -u -r1.30 ripd.c
--- ripd.c 3 Sep 2016 10:28:08 - 1.30
+++ ripd.c 30 Oct 2018 17:24:45 -
@@ -251,6 +251,11 @@ main(int argc, char *argv[])
conf->rdomain) == -1)
fatalx("kr_init failed");
+ if (unveil("/", "") == -1)
+ fatal("unveil");
+ if (unveil(NULL, NULL) == -1)
+ fatal("unveil");
+
event_dispatch();
ripd_shutdown();
@@ -276,7 +281,6 @@ ripd_shutdown(void)
if_del(i);
}
- control_cleanup(conf->csock);
kr_shutdown();
log_debug("waiting for children to terminate");
Index: ripe.c
===
RCS file: /cvs/src/usr.sbin/ripd/ripe.c,v
retrieving revision 1.22
diff -u -p -u -r1.22 ripe.c
--- ripe.c 3 Sep 2016 10:28:08 - 1.22
+++ ripe.c 30 Oct 2018 17:24:45 -
@@ -201,6 +201,11 @@ ripe(struct ripd_conf *xconf, int pipe_p
ripe_imsg_compose_rde(IMSG_FULL_REQUEST, 0, 0, NULL, 0);
+ if (unveil("/", "") == -1)
+ fatal("unveil");
+ if (unveil(NULL, NULL) == -1)
+ fatal("unveil");
+
event_dispatch();
ripe_shutdown();
On 11:15 Tue 30 Oct , Theo de Raadt wrote:
> Remi Locherer wrote:
>
> > On Tue, Oct 30, 2018 at 10:54:10AM -0600, Theo de Raadt wrote:
> > > Remi Locherer wrote:
> > >
> > > > On Tue, Oct 30, 2018 at 03:20:35PM +, Ricardo Mestre wrote:
> > > > > Hi,
> > > > >
> > > > > After all files are opened ripd(8) can have the fs access disabled
> > > > > just before
> > > > > each process main loop. Its 2 childs already run under chroot, but
> > > > > since they
> > > > > are still not pledged at least they have no way to read/write/create
> > > > > files within
> > > > > the chroot. No loads or reloads of the config file happen through any
> > > > > signal,
> > > > > nor can we do it via ripctl(8).
> > > > >
> > > > > I was able to run a simple daemon with the example file. Comments? OK?
> > > >
> > > > control_cleanup() unlinks the control socket on exit. I think you should
> > > > either unveil(conf->csock, "c") or remove control_cleanup().
> > >
> > > I don't understand this latter comment, let me ask.
> > >
> > > You think it is smart to leave these sockets lying around?
> > >
> > > I suspect there are a few oddball programs where it makes senes, but as
> > > a general rule I think it is a bad idea; as stated in other threads it
> > > means control programs and restart sequences have a bunch more oddball
> > > behaviours which will be inconsistant.
> > >
> >
> > I prefer if sockets get removed on exit. But I was not sure if this is
> > just my personal taste.
>
> I didn't speak about taste, I spoke about behaviour relative to the
> control program. Why do you propose potentially changing behaviour
> without checking first?
>
Re: disable fs access on ripd
Remi Locherer wrote: > On Tue, Oct 30, 2018 at 10:54:10AM -0600, Theo de Raadt wrote: > > Remi Locherer wrote: > > > > > On Tue, Oct 30, 2018 at 03:20:35PM +, Ricardo Mestre wrote: > > > > Hi, > > > > > > > > After all files are opened ripd(8) can have the fs access disabled just > > > > before > > > > each process main loop. Its 2 childs already run under chroot, but > > > > since they > > > > are still not pledged at least they have no way to read/write/create > > > > files within > > > > the chroot. No loads or reloads of the config file happen through any > > > > signal, > > > > nor can we do it via ripctl(8). > > > > > > > > I was able to run a simple daemon with the example file. Comments? OK? > > > > > > control_cleanup() unlinks the control socket on exit. I think you should > > > either unveil(conf->csock, "c") or remove control_cleanup(). > > > > I don't understand this latter comment, let me ask. > > > > You think it is smart to leave these sockets lying around? > > > > I suspect there are a few oddball programs where it makes senes, but as > > a general rule I think it is a bad idea; as stated in other threads it > > means control programs and restart sequences have a bunch more oddball > > behaviours which will be inconsistant. > > > > I prefer if sockets get removed on exit. But I was not sure if this is > just my personal taste. I didn't speak about taste, I spoke about behaviour relative to the control program. Why do you propose potentially changing behaviour without checking first?
Re: disable fs access on ripd
On Tue, Oct 30, 2018 at 10:54:10AM -0600, Theo de Raadt wrote: > Remi Locherer wrote: > > > On Tue, Oct 30, 2018 at 03:20:35PM +, Ricardo Mestre wrote: > > > Hi, > > > > > > After all files are opened ripd(8) can have the fs access disabled just > > > before > > > each process main loop. Its 2 childs already run under chroot, but since > > > they > > > are still not pledged at least they have no way to read/write/create > > > files within > > > the chroot. No loads or reloads of the config file happen through any > > > signal, > > > nor can we do it via ripctl(8). > > > > > > I was able to run a simple daemon with the example file. Comments? OK? > > > > control_cleanup() unlinks the control socket on exit. I think you should > > either unveil(conf->csock, "c") or remove control_cleanup(). > > I don't understand this latter comment, let me ask. > > You think it is smart to leave these sockets lying around? > > I suspect there are a few oddball programs where it makes senes, but as > a general rule I think it is a bad idea; as stated in other threads it > means control programs and restart sequences have a bunch more oddball > behaviours which will be inconsistant. > I prefer if sockets get removed on exit. But I was not sure if this is just my personal taste.
Re: disable fs access on ripd
Remi Locherer wrote: > On Tue, Oct 30, 2018 at 03:20:35PM +, Ricardo Mestre wrote: > > Hi, > > > > After all files are opened ripd(8) can have the fs access disabled just > > before > > each process main loop. Its 2 childs already run under chroot, but since > > they > > are still not pledged at least they have no way to read/write/create files > > within > > the chroot. No loads or reloads of the config file happen through any > > signal, > > nor can we do it via ripctl(8). > > > > I was able to run a simple daemon with the example file. Comments? OK? > > control_cleanup() unlinks the control socket on exit. I think you should > either unveil(conf->csock, "c") or remove control_cleanup(). I don't understand this latter comment, let me ask. You think it is smart to leave these sockets lying around? I suspect there are a few oddball programs where it makes senes, but as a general rule I think it is a bad idea; as stated in other threads it means control programs and restart sequences have a bunch more oddball behaviours which will be inconsistant.
Re: disable fs access on ripd
On Tue, Oct 30, 2018 at 03:20:35PM +, Ricardo Mestre wrote:
> Hi,
>
> After all files are opened ripd(8) can have the fs access disabled just before
> each process main loop. Its 2 childs already run under chroot, but since they
> are still not pledged at least they have no way to read/write/create files
> within
> the chroot. No loads or reloads of the config file happen through any signal,
> nor can we do it via ripctl(8).
>
> I was able to run a simple daemon with the example file. Comments? OK?
control_cleanup() unlinks the control socket on exit. I think you should
either unveil(conf->csock, "c") or remove control_cleanup().
>
> Index: rde.c
> ===
> RCS file: /cvs/src/usr.sbin/ripd/rde.c,v
> retrieving revision 1.21
> diff -u -p -u -r1.21 rde.c
> --- rde.c 3 Sep 2016 10:28:08 - 1.21
> +++ rde.c 30 Oct 2018 15:09:44 -
> @@ -151,6 +151,11 @@ rde(struct ripd_conf *xconf, int pipe_pa
> free(r);
> }
>
> + if (unveil("/", "") == -1)
> + fatal("unveil");
> + if (unveil(NULL, NULL) == -1)
> + fatal("unveil");
> +
> event_dispatch();
>
> rde_shutdown();
> Index: ripd.c
> ===
> RCS file: /cvs/src/usr.sbin/ripd/ripd.c,v
> retrieving revision 1.30
> diff -u -p -u -r1.30 ripd.c
> --- ripd.c3 Sep 2016 10:28:08 - 1.30
> +++ ripd.c30 Oct 2018 15:09:44 -
> @@ -251,6 +251,11 @@ main(int argc, char *argv[])
> conf->rdomain) == -1)
> fatalx("kr_init failed");
>
> + if (unveil("/", "") == -1)
> + fatal("unveil");
> + if (unveil(NULL, NULL) == -1)
> + fatal("unveil");
> +
> event_dispatch();
>
> ripd_shutdown();
> Index: ripe.c
> ===
> RCS file: /cvs/src/usr.sbin/ripd/ripe.c,v
> retrieving revision 1.22
> diff -u -p -u -r1.22 ripe.c
> --- ripe.c3 Sep 2016 10:28:08 - 1.22
> +++ ripe.c30 Oct 2018 15:09:44 -
> @@ -201,6 +201,11 @@ ripe(struct ripd_conf *xconf, int pipe_p
>
> ripe_imsg_compose_rde(IMSG_FULL_REQUEST, 0, 0, NULL, 0);
>
> + if (unveil("/", "") == -1)
> + fatal("unveil");
> + if (unveil(NULL, NULL) == -1)
> + fatal("unveil");
> +
> event_dispatch();
>
> ripe_shutdown();
>
Re: disable fs access on ripd
ok benno@
Ricardo Mestre(ser...@helheim.mooo.com) on 2018.10.30 15:20:35 +:
> Hi,
>
> After all files are opened ripd(8) can have the fs access disabled just before
> each process main loop. Its 2 childs already run under chroot, but since they
> are still not pledged at least they have no way to read/write/create files
> within
> the chroot. No loads or reloads of the config file happen through any signal,
> nor can we do it via ripctl(8).
>
> I was able to run a simple daemon with the example file. Comments? OK?
>
> Index: rde.c
> ===
> RCS file: /cvs/src/usr.sbin/ripd/rde.c,v
> retrieving revision 1.21
> diff -u -p -u -r1.21 rde.c
> --- rde.c 3 Sep 2016 10:28:08 - 1.21
> +++ rde.c 30 Oct 2018 15:09:44 -
> @@ -151,6 +151,11 @@ rde(struct ripd_conf *xconf, int pipe_pa
> free(r);
> }
>
> + if (unveil("/", "") == -1)
> + fatal("unveil");
> + if (unveil(NULL, NULL) == -1)
> + fatal("unveil");
> +
> event_dispatch();
>
> rde_shutdown();
> Index: ripd.c
> ===
> RCS file: /cvs/src/usr.sbin/ripd/ripd.c,v
> retrieving revision 1.30
> diff -u -p -u -r1.30 ripd.c
> --- ripd.c3 Sep 2016 10:28:08 - 1.30
> +++ ripd.c30 Oct 2018 15:09:44 -
> @@ -251,6 +251,11 @@ main(int argc, char *argv[])
> conf->rdomain) == -1)
> fatalx("kr_init failed");
>
> + if (unveil("/", "") == -1)
> + fatal("unveil");
> + if (unveil(NULL, NULL) == -1)
> + fatal("unveil");
> +
> event_dispatch();
>
> ripd_shutdown();
> Index: ripe.c
> ===
> RCS file: /cvs/src/usr.sbin/ripd/ripe.c,v
> retrieving revision 1.22
> diff -u -p -u -r1.22 ripe.c
> --- ripe.c3 Sep 2016 10:28:08 - 1.22
> +++ ripe.c30 Oct 2018 15:09:44 -
> @@ -201,6 +201,11 @@ ripe(struct ripd_conf *xconf, int pipe_p
>
> ripe_imsg_compose_rde(IMSG_FULL_REQUEST, 0, 0, NULL, 0);
>
> + if (unveil("/", "") == -1)
> + fatal("unveil");
> + if (unveil(NULL, NULL) == -1)
> + fatal("unveil");
> +
> event_dispatch();
>
> ripe_shutdown();
>
disable fs access on ripd
Hi,
After all files are opened ripd(8) can have the fs access disabled just before
each process main loop. Its 2 childs already run under chroot, but since they
are still not pledged at least they have no way to read/write/create files
within
the chroot. No loads or reloads of the config file happen through any signal,
nor can we do it via ripctl(8).
I was able to run a simple daemon with the example file. Comments? OK?
Index: rde.c
===
RCS file: /cvs/src/usr.sbin/ripd/rde.c,v
retrieving revision 1.21
diff -u -p -u -r1.21 rde.c
--- rde.c 3 Sep 2016 10:28:08 - 1.21
+++ rde.c 30 Oct 2018 15:09:44 -
@@ -151,6 +151,11 @@ rde(struct ripd_conf *xconf, int pipe_pa
free(r);
}
+ if (unveil("/", "") == -1)
+ fatal("unveil");
+ if (unveil(NULL, NULL) == -1)
+ fatal("unveil");
+
event_dispatch();
rde_shutdown();
Index: ripd.c
===
RCS file: /cvs/src/usr.sbin/ripd/ripd.c,v
retrieving revision 1.30
diff -u -p -u -r1.30 ripd.c
--- ripd.c 3 Sep 2016 10:28:08 - 1.30
+++ ripd.c 30 Oct 2018 15:09:44 -
@@ -251,6 +251,11 @@ main(int argc, char *argv[])
conf->rdomain) == -1)
fatalx("kr_init failed");
+ if (unveil("/", "") == -1)
+ fatal("unveil");
+ if (unveil(NULL, NULL) == -1)
+ fatal("unveil");
+
event_dispatch();
ripd_shutdown();
Index: ripe.c
===
RCS file: /cvs/src/usr.sbin/ripd/ripe.c,v
retrieving revision 1.22
diff -u -p -u -r1.22 ripe.c
--- ripe.c 3 Sep 2016 10:28:08 - 1.22
+++ ripe.c 30 Oct 2018 15:09:44 -
@@ -201,6 +201,11 @@ ripe(struct ripd_conf *xconf, int pipe_p
ripe_imsg_compose_rde(IMSG_FULL_REQUEST, 0, 0, NULL, 0);
+ if (unveil("/", "") == -1)
+ fatal("unveil");
+ if (unveil(NULL, NULL) == -1)
+ fatal("unveil");
+
event_dispatch();
ripe_shutdown();
Re: disable fs access on snmpd
Looks right.
Ricardo Mestre wrote:
> snmpd(8)'s main process needs to open the config file and /dev/pf both with
> read permissions, but once it reaches pledge(2) just before the main loop both
> were already opened. Since snmpd(8) doesn't have a way to load or reload the
> config file, not even through SIGHUP, then rpath promise is not needed.
>
> The snmpe process cannot yet be pledged, but it doesn't need fs access so we
> can disable the access through unveil("/", ""); unveil(NULL, NULL);
>
> The traphandler is already pledged to not access the fs at all.
>
> With both modifications the regress tests still pass. Comments? OK?
>
> Index: snmpd.c
> ===
> RCS file: /cvs/src/usr.sbin/snmpd/snmpd.c,v
> retrieving revision 1.39
> diff -u -p -u -r1.39 snmpd.c
> --- snmpd.c 5 Aug 2018 09:33:13 - 1.39
> +++ snmpd.c 30 Oct 2018 14:03:38 -
> @@ -255,7 +255,7 @@ main(int argc, char *argv[])
>
> proc_connect(ps);
>
> - if (pledge("stdio rpath dns sendfd proc exec id", NULL) == -1)
> + if (pledge("stdio dns sendfd proc exec id", NULL) == -1)
> fatal("pledge");
>
> event_dispatch();
> Index: snmpe.c
> ===
> RCS file: /cvs/src/usr.sbin/snmpd/snmpe.c,v
> retrieving revision 1.54
> diff -u -p -u -r1.54 snmpe.c
> --- snmpe.c 31 Jul 2018 11:01:29 - 1.54
> +++ snmpe.c 30 Oct 2018 14:03:38 -
> @@ -120,6 +120,10 @@ snmpe_init(struct privsep *ps, struct pr
> event_add(>s_ev, NULL);
> }
>
> + if (unveil("/", "") == -1)
> + fatal("unveil");
> + if (unveil(NULL, NULL) == -1)
> + fatal("unveil");
> #if 0
> /*
>* XXX Refactoring required to move illegal ioctls and sysctls.
Re: unveil ifstated
Looks good to me.
Ricardo Mestre wrote:
> ifstated(8) needs to load configfile from within the main loop, but also to
> reload it on SIGHUP so unveil(2) it with read permissions. Additionally all
> commands are exec'ed through /bin/sh instead of directly so we can just
> unveil(2) /bin/sh with x perms. Since /bin/sh is already used on another place
> I used _PATH_BSHELL.
>
> Both regress tests passed. Comments? OK?
>
> Index: ifstated.c
> ===
> RCS file: /cvs/src/usr.sbin/ifstated/ifstated.c,v
> retrieving revision 1.61
> diff -u -p -u -r1.61 ifstated.c
> --- ifstated.c30 Aug 2017 16:14:52 - 1.61
> +++ ifstated.c30 Oct 2018 12:01:20 -
> @@ -31,6 +31,7 @@
> #include
> #include
>
> +#include
> #include
> #include
> #include
> @@ -160,6 +161,10 @@ main(int argc, char *argv[])
> , sizeof(rtfilter)) == -1) /* not fatal */
> log_warn("%s: setsockopt tablefilter", __func__);
>
> + if (unveil(configfile, "r") == -1)
> + fatal("unveil");
> + if (unveil(_PATH_BSHELL, "x") == -1)
> + fatal("unveil");
> if (pledge("stdio rpath route proc exec", NULL) == -1)
> fatal("pledge");
>
> @@ -326,7 +331,7 @@ external_exec(struct ifsd_external *exte
> if (pid < 0) {
> log_warn("fork error");
> } else if (pid == 0) {
> - execv("/bin/sh", argp);
> + execv(_PATH_BSHELL, argp);
> _exit(1);
> /* NOTREACHED */
> } else {
disable fs access on snmpd
Hi,
snmpd(8)'s main process needs to open the config file and /dev/pf both with
read permissions, but once it reaches pledge(2) just before the main loop both
were already opened. Since snmpd(8) doesn't have a way to load or reload the
config file, not even through SIGHUP, then rpath promise is not needed.
The snmpe process cannot yet be pledged, but it doesn't need fs access so we
can disable the access through unveil("/", ""); unveil(NULL, NULL);
The traphandler is already pledged to not access the fs at all.
With both modifications the regress tests still pass. Comments? OK?
Index: snmpd.c
===
RCS file: /cvs/src/usr.sbin/snmpd/snmpd.c,v
retrieving revision 1.39
diff -u -p -u -r1.39 snmpd.c
--- snmpd.c 5 Aug 2018 09:33:13 - 1.39
+++ snmpd.c 30 Oct 2018 14:03:38 -
@@ -255,7 +255,7 @@ main(int argc, char *argv[])
proc_connect(ps);
- if (pledge("stdio rpath dns sendfd proc exec id", NULL) == -1)
+ if (pledge("stdio dns sendfd proc exec id", NULL) == -1)
fatal("pledge");
event_dispatch();
Index: snmpe.c
===
RCS file: /cvs/src/usr.sbin/snmpd/snmpe.c,v
retrieving revision 1.54
diff -u -p -u -r1.54 snmpe.c
--- snmpe.c 31 Jul 2018 11:01:29 - 1.54
+++ snmpe.c 30 Oct 2018 14:03:38 -
@@ -120,6 +120,10 @@ snmpe_init(struct privsep *ps, struct pr
event_add(>s_ev, NULL);
}
+ if (unveil("/", "") == -1)
+ fatal("unveil");
+ if (unveil(NULL, NULL) == -1)
+ fatal("unveil");
#if 0
/*
* XXX Refactoring required to move illegal ioctls and sysctls.
Re: unveil htpasswd
Makes sense to me.
> Hi,
>
> htpasswd(1) when in batch mode (-I) and 1 argument is used, or when not in
> batch mode and 2 arguments are used we know we have to access argv[0] with rwc
> permissions and also to rwc a temporary file in /tmp so we can unveil(2) both
> argv[0] and /tmp with rwc permissions. In order to avoid adding "unveil" to
> pledge(2), just call it after getopt(3).
>
> Remaining code paths already have fs access disabled via pledge(2).
>
> Comments? OK?
>
> Index: htpasswd.c
> ===
> RCS file: /cvs/src/usr.bin/htpasswd/htpasswd.c,v
> retrieving revision 1.16
> diff -u -p -u -r1.16 htpasswd.c
> --- htpasswd.c7 Jun 2017 09:11:52 - 1.16
> +++ htpasswd.c30 Oct 2018 08:55:45 -
> @@ -57,9 +57,6 @@ main(int argc, char** argv)
> ssize_t linelen;
> mode_t old_umask;
>
> - if (pledge("stdio rpath wpath cpath flock tmppath tty", NULL) == -1)
> - err(1, "pledge");
> -
> while ((c = getopt(argc, argv, "I")) != -1) {
> switch (c) {
> case 'I':
> @@ -74,6 +71,15 @@ main(int argc, char** argv)
>
> argc -= optind;
> argv += optind;
> +
> + if ((batch && argc == 1) || (!batch && argc == 2)) {
> + if (unveil(argv[0], "rwc") == -1)
> + err(1, "unveil");
> + if (unveil("/tmp", "rwc") == -1)
> + err(1, "unveil");
> + }
> + if (pledge("stdio rpath wpath cpath flock tmppath tty", NULL) == -1)
> + err(1, "pledge");
>
> if (batch) {
> if (argc == 1)
>
unveil ifstated
Hi,
ifstated(8) needs to load configfile from within the main loop, but also to
reload it on SIGHUP so unveil(2) it with read permissions. Additionally all
commands are exec'ed through /bin/sh instead of directly so we can just
unveil(2) /bin/sh with x perms. Since /bin/sh is already used on another place
I used _PATH_BSHELL.
Both regress tests passed. Comments? OK?
Index: ifstated.c
===
RCS file: /cvs/src/usr.sbin/ifstated/ifstated.c,v
retrieving revision 1.61
diff -u -p -u -r1.61 ifstated.c
--- ifstated.c 30 Aug 2017 16:14:52 - 1.61
+++ ifstated.c 30 Oct 2018 12:01:20 -
@@ -31,6 +31,7 @@
#include
#include
+#include
#include
#include
#include
@@ -160,6 +161,10 @@ main(int argc, char *argv[])
, sizeof(rtfilter)) == -1) /* not fatal */
log_warn("%s: setsockopt tablefilter", __func__);
+ if (unveil(configfile, "r") == -1)
+ fatal("unveil");
+ if (unveil(_PATH_BSHELL, "x") == -1)
+ fatal("unveil");
if (pledge("stdio rpath route proc exec", NULL) == -1)
fatal("pledge");
@@ -326,7 +331,7 @@ external_exec(struct ifsd_external *exte
if (pid < 0) {
log_warn("fork error");
} else if (pid == 0) {
- execv("/bin/sh", argp);
+ execv(_PATH_BSHELL, argp);
_exit(1);
/* NOTREACHED */
} else {
Re: unveil ospf6d's parent proc
OK florian@
On Mon, Oct 29, 2018 at 11:27:15PM +0100, Remi Locherer wrote:
> Hi,
>
> ospf6d does not support reloading so its parent proc does not need
> filesystem access with the exception of the control socket cleanup on
> exit. Once we teach it how to reload the config it is easy to unveil "/"
> readonly as I just did for ospfd.
>
> OK?
>
> Remi
>
>
> cvs diff: Diffing .
> Index: ospf6d.c
> ===
> RCS file: /cvs/src/usr.sbin/ospf6d/ospf6d.c,v
> retrieving revision 1.39
> diff -u -p -r1.39 ospf6d.c
> --- ospf6d.c 1 Sep 2018 19:21:10 - 1.39
> +++ ospf6d.c 29 Oct 2018 22:20:45 -
> @@ -274,6 +274,11 @@ main(int argc, char *argv[])
> fatalx("control socket setup failed");
> main_imsg_compose_ospfe_fd(IMSG_CONTROLFD, 0, control_fd);
>
> + if (unveil(ospfd_conf->csock, "c") == -1)
> + fatal("unveil");
> + if (unveil(NULL, NULL) == -1)
> + fatal("unveil");
> +
> if (kr_init(!(ospfd_conf->flags & OSPFD_FLAG_NO_FIB_UPDATE),
> ospfd_conf->rdomain) == -1)
> fatalx("kr_init failed");
>
--
I'm not entirely sure you are real.
Re: ure(4): VLANs and Jumbo frames
On Tue, Oct 30, 2018 at 10:57:08AM +0100, Patrick Wildt wrote: > On Tue, Oct 30, 2018 at 05:30:55PM +0800, Kevin Lo wrote: > > On Tue, Oct 30, 2018 at 09:43:08AM +0100, Patrick Wildt wrote: > > > + if (sc->ure_flags & URE_FLAG_8152) > > > + ifp->ifp_hardmtu = URE_MAX_FRAMELEN_8152; > > > + else > > > + ifp->ifp_hardmtu = URE_MAX_FRAMELEN_8153; > > > + ifp->ifp_hardmtu -= (ETHER_HDR_LEN + ETHER_VLAN_ENCAP_LEN); > > > > Typo. Should be ifp->if_hardmtu, not ifp->ifp_hardmtu. > > Oops, yes, I sent the wrong diff that still had the typo. Fixed diff > attached which has no other change. ok kevlo@ > > diff --git a/sys/dev/usb/if_ure.c b/sys/dev/usb/if_ure.c > index b6c6c99ef34..9ec0dedceff 100644 > --- a/sys/dev/usb/if_ure.c > +++ b/sys/dev/usb/if_ure.c > @@ -695,6 +695,9 @@ ure_rtl8152_init(struct ure_softc *sc) > > ure_init_fifo(sc); > > + /* Set allowed frame size. */ > + ure_write_2(sc, URE_PLA_RMS, URE_MCU_TYPE_PLA, URE_MAX_FRAMELEN_8152); > + > ure_write_1(sc, URE_USB_TX_AGG, URE_MCU_TYPE_USB, > URE_TX_AGG_MAX_THRESHOLD); > ure_write_4(sc, URE_USB_RX_BUF_TH, URE_MCU_TYPE_USB, URE_RX_THR_HIGH); > @@ -835,6 +838,10 @@ ure_rtl8153_init(struct ure_softc *sc) > > ure_init_fifo(sc); > > + /* Set allowed frame size. */ > + ure_write_2(sc, URE_PLA_RMS, URE_MCU_TYPE_PLA, URE_MAX_FRAMELEN_8153); > + ure_write_2(sc, URE_PLA_MTPS, URE_MCU_TYPE_PLA, URE_MTPS_JUMBO); > + > /* Enable Rx aggregation. */ > ure_write_2(sc, URE_USB_USB_CTRL, URE_MCU_TYPE_USB, > ure_read_2(sc, URE_USB_USB_CTRL, URE_MCU_TYPE_USB) & > @@ -1147,6 +1154,12 @@ ure_attach(struct device *parent, struct device *self, > void *aux) > ifp->if_start = ure_start; > ifp->if_capabilities = 0; > > + if (sc->ure_flags & URE_FLAG_8152) > + ifp->if_hardmtu = URE_MAX_FRAMELEN_8152; > + else > + ifp->if_hardmtu = URE_MAX_FRAMELEN_8153; > + ifp->if_hardmtu -= (ETHER_HDR_LEN + ETHER_VLAN_ENCAP_LEN); > + > mii = >ure_mii; > mii->mii_ifp = ifp; > mii->mii_readreg = ure_miibus_readreg; > diff --git a/sys/dev/usb/if_urereg.h b/sys/dev/usb/if_urereg.h > index 2260ec37890..8963d2753ed 100644 > --- a/sys/dev/usb/if_urereg.h > +++ b/sys/dev/usb/if_urereg.h > @@ -41,7 +41,8 @@ > #define URE_BYTE_EN_BYTE0x11 > #define URE_BYTE_EN_SIX_BYTES 0x3f > > -#define URE_MAX_FRAMELEN(ETHER_MAX_LEN + ETHER_VLAN_ENCAP_LEN) > +#define URE_MAX_FRAMELEN_8152 (ETHER_MAX_LEN + ETHER_VLAN_ENCAP_LEN) > +#define URE_MAX_FRAMELEN_8153 (9 * 1024) > > #define URE_PLA_IDR 0xc000 > #define URE_PLA_RCR 0xc010 > @@ -186,6 +187,10 @@ > /* PLA_TCR1 */ > #define URE_VERSION_MASK0x7cf0 > > +/* PLA_MTPS */ > +#define URE_MTPS_JUMBO (12 * 1024 / 64) > +#define URE_MTPS_DEFAULT (6 * 1024 / 64) > + > /* PLA_CR */ > #define URE_CR_RST 0x10 > #define URE_CR_RE 0x08
Re: unveil htpasswd
OK florian@
On Tue, Oct 30, 2018 at 09:02:48AM +, Ricardo Mestre wrote:
> Hi,
>
> htpasswd(1) when in batch mode (-I) and 1 argument is used, or when not in
> batch mode and 2 arguments are used we know we have to access argv[0] with rwc
> permissions and also to rwc a temporary file in /tmp so we can unveil(2) both
> argv[0] and /tmp with rwc permissions. In order to avoid adding "unveil" to
> pledge(2), just call it after getopt(3).
>
> Remaining code paths already have fs access disabled via pledge(2).
>
> Comments? OK?
>
> Index: htpasswd.c
> ===
> RCS file: /cvs/src/usr.bin/htpasswd/htpasswd.c,v
> retrieving revision 1.16
> diff -u -p -u -r1.16 htpasswd.c
> --- htpasswd.c7 Jun 2017 09:11:52 - 1.16
> +++ htpasswd.c30 Oct 2018 08:55:45 -
> @@ -57,9 +57,6 @@ main(int argc, char** argv)
> ssize_t linelen;
> mode_t old_umask;
>
> - if (pledge("stdio rpath wpath cpath flock tmppath tty", NULL) == -1)
> - err(1, "pledge");
> -
> while ((c = getopt(argc, argv, "I")) != -1) {
> switch (c) {
> case 'I':
> @@ -74,6 +71,15 @@ main(int argc, char** argv)
>
> argc -= optind;
> argv += optind;
> +
> + if ((batch && argc == 1) || (!batch && argc == 2)) {
> + if (unveil(argv[0], "rwc") == -1)
> + err(1, "unveil");
> + if (unveil("/tmp", "rwc") == -1)
> + err(1, "unveil");
> + }
> + if (pledge("stdio rpath wpath cpath flock tmppath tty", NULL) == -1)
> + err(1, "pledge");
>
> if (batch) {
> if (argc == 1)
--
I'm not entirely sure you are real.
Re: faq16: clarify ownership management
Klemens Nanni wrote: > "system" is ambigious, see `owner id` in vm.conf(5). > > OK? > > Index: faq16.html > === > RCS file: /cvs/www/faq/faq16.html,v > retrieving revision 1.2 > diff -u -p -r1.2 faq16.html > --- faq16.html25 Oct 2018 15:43:31 - 1.2 > +++ faq16.html30 Oct 2018 09:59:28 - > @@ -62,7 +62,7 @@ The following features are available: > >serial console access to the virtual machines >https://man.openbsd.org/tap;>tap(4) interfaces > - per-system user/group ownership > + per-VM user/group ownership >privilege separation >raw, qcow2 and qcow2-derived images >dumping and restoring of guest system memory seems fine to me. The original idea was to say it uses system user/group for VM ownership, but in fact the current wording doesn't make sense. ok solene@
faq16: clarify ownership management
"system" is ambigious, see `owner id` in vm.conf(5). OK? Index: faq16.html === RCS file: /cvs/www/faq/faq16.html,v retrieving revision 1.2 diff -u -p -r1.2 faq16.html --- faq16.html 25 Oct 2018 15:43:31 - 1.2 +++ faq16.html 30 Oct 2018 09:59:28 - @@ -62,7 +62,7 @@ The following features are available: serial console access to the virtual machines https://man.openbsd.org/tap;>tap(4) interfaces - per-system user/group ownership + per-VM user/group ownership privilege separation raw, qcow2 and qcow2-derived images dumping and restoring of guest system memory
Re: ure(4): VLANs and Jumbo frames
On Tue, Oct 30, 2018 at 05:30:55PM +0800, Kevin Lo wrote: > On Tue, Oct 30, 2018 at 09:43:08AM +0100, Patrick Wildt wrote: > > + if (sc->ure_flags & URE_FLAG_8152) > > + ifp->ifp_hardmtu = URE_MAX_FRAMELEN_8152; > > + else > > + ifp->ifp_hardmtu = URE_MAX_FRAMELEN_8153; > > + ifp->ifp_hardmtu -= (ETHER_HDR_LEN + ETHER_VLAN_ENCAP_LEN); > > Typo. Should be ifp->if_hardmtu, not ifp->ifp_hardmtu. Oops, yes, I sent the wrong diff that still had the typo. Fixed diff attached which has no other change. diff --git a/sys/dev/usb/if_ure.c b/sys/dev/usb/if_ure.c index b6c6c99ef34..9ec0dedceff 100644 --- a/sys/dev/usb/if_ure.c +++ b/sys/dev/usb/if_ure.c @@ -695,6 +695,9 @@ ure_rtl8152_init(struct ure_softc *sc) ure_init_fifo(sc); + /* Set allowed frame size. */ + ure_write_2(sc, URE_PLA_RMS, URE_MCU_TYPE_PLA, URE_MAX_FRAMELEN_8152); + ure_write_1(sc, URE_USB_TX_AGG, URE_MCU_TYPE_USB, URE_TX_AGG_MAX_THRESHOLD); ure_write_4(sc, URE_USB_RX_BUF_TH, URE_MCU_TYPE_USB, URE_RX_THR_HIGH); @@ -835,6 +838,10 @@ ure_rtl8153_init(struct ure_softc *sc) ure_init_fifo(sc); + /* Set allowed frame size. */ + ure_write_2(sc, URE_PLA_RMS, URE_MCU_TYPE_PLA, URE_MAX_FRAMELEN_8153); + ure_write_2(sc, URE_PLA_MTPS, URE_MCU_TYPE_PLA, URE_MTPS_JUMBO); + /* Enable Rx aggregation. */ ure_write_2(sc, URE_USB_USB_CTRL, URE_MCU_TYPE_USB, ure_read_2(sc, URE_USB_USB_CTRL, URE_MCU_TYPE_USB) & @@ -1147,6 +1154,12 @@ ure_attach(struct device *parent, struct device *self, void *aux) ifp->if_start = ure_start; ifp->if_capabilities = 0; + if (sc->ure_flags & URE_FLAG_8152) + ifp->if_hardmtu = URE_MAX_FRAMELEN_8152; + else + ifp->if_hardmtu = URE_MAX_FRAMELEN_8153; + ifp->if_hardmtu -= (ETHER_HDR_LEN + ETHER_VLAN_ENCAP_LEN); + mii = >ure_mii; mii->mii_ifp = ifp; mii->mii_readreg = ure_miibus_readreg; diff --git a/sys/dev/usb/if_urereg.h b/sys/dev/usb/if_urereg.h index 2260ec37890..8963d2753ed 100644 --- a/sys/dev/usb/if_urereg.h +++ b/sys/dev/usb/if_urereg.h @@ -41,7 +41,8 @@ #defineURE_BYTE_EN_BYTE0x11 #defineURE_BYTE_EN_SIX_BYTES 0x3f -#defineURE_MAX_FRAMELEN(ETHER_MAX_LEN + ETHER_VLAN_ENCAP_LEN) +#defineURE_MAX_FRAMELEN_8152 (ETHER_MAX_LEN + ETHER_VLAN_ENCAP_LEN) +#defineURE_MAX_FRAMELEN_8153 (9 * 1024) #defineURE_PLA_IDR 0xc000 #defineURE_PLA_RCR 0xc010 @@ -186,6 +187,10 @@ /* PLA_TCR1 */ #defineURE_VERSION_MASK0x7cf0 +/* PLA_MTPS */ +#define URE_MTPS_JUMBO (12 * 1024 / 64) +#define URE_MTPS_DEFAULT (6 * 1024 / 64) + /* PLA_CR */ #defineURE_CR_RST 0x10 #defineURE_CR_RE 0x08
Re: ure(4): VLANs and Jumbo frames
On Tue, Oct 30, 2018 at 09:43:08AM +0100, Patrick Wildt wrote: > > Hi, > > I recently wanted to use VLANs on ure(4) but failed. As it turns out, > hardmtu is set to 1500 which apparently does not leave enough space > for the VLAN tag. It looks like the Gigabit Version does even support > Jumbo frames. Thus we can set the maximum framelen on the RX path > to something bigger and enable Jumbo frames. > > I tried this on a Winyao USB1000F. Would be nice if people tested this > change who have those ure(4)s in active use. > > Thanks, > Patrick > > diff --git a/sys/dev/usb/if_ure.c b/sys/dev/usb/if_ure.c > index b6c6c99ef34..637fd5eca5f 100644 > --- a/sys/dev/usb/if_ure.c > +++ b/sys/dev/usb/if_ure.c > @@ -695,6 +695,9 @@ ure_rtl8152_init(struct ure_softc *sc) > > ure_init_fifo(sc); > > + /* Set allowed frame size. */ > + ure_write_2(sc, URE_PLA_RMS, URE_MCU_TYPE_PLA, URE_MAX_FRAMELEN_8152); > + > ure_write_1(sc, URE_USB_TX_AGG, URE_MCU_TYPE_USB, > URE_TX_AGG_MAX_THRESHOLD); > ure_write_4(sc, URE_USB_RX_BUF_TH, URE_MCU_TYPE_USB, URE_RX_THR_HIGH); > @@ -835,6 +838,10 @@ ure_rtl8153_init(struct ure_softc *sc) > > ure_init_fifo(sc); > > + /* Set allowed frame size. */ > + ure_write_2(sc, URE_PLA_RMS, URE_MCU_TYPE_PLA, URE_MAX_FRAMELEN_8153); > + ure_write_2(sc, URE_PLA_MTPS, URE_MCU_TYPE_PLA, URE_MTPS_JUMBO); > + > /* Enable Rx aggregation. */ > ure_write_2(sc, URE_USB_USB_CTRL, URE_MCU_TYPE_USB, > ure_read_2(sc, URE_USB_USB_CTRL, URE_MCU_TYPE_USB) & > @@ -1147,6 +1154,12 @@ ure_attach(struct device *parent, struct device *self, > void *aux) > ifp->if_start = ure_start; > ifp->if_capabilities = 0; > > + if (sc->ure_flags & URE_FLAG_8152) > + ifp->ifp_hardmtu = URE_MAX_FRAMELEN_8152; > + else > + ifp->ifp_hardmtu = URE_MAX_FRAMELEN_8153; > + ifp->ifp_hardmtu -= (ETHER_HDR_LEN + ETHER_VLAN_ENCAP_LEN); Typo. Should be ifp->if_hardmtu, not ifp->ifp_hardmtu. > + > mii = >ure_mii; > mii->mii_ifp = ifp; > mii->mii_readreg = ure_miibus_readreg; > diff --git a/sys/dev/usb/if_urereg.h b/sys/dev/usb/if_urereg.h > index 2260ec37890..8963d2753ed 100644 > --- a/sys/dev/usb/if_urereg.h > +++ b/sys/dev/usb/if_urereg.h > @@ -41,7 +41,8 @@ > #define URE_BYTE_EN_BYTE0x11 > #define URE_BYTE_EN_SIX_BYTES 0x3f > > -#define URE_MAX_FRAMELEN(ETHER_MAX_LEN + ETHER_VLAN_ENCAP_LEN) > +#define URE_MAX_FRAMELEN_8152 (ETHER_MAX_LEN + ETHER_VLAN_ENCAP_LEN) > +#define URE_MAX_FRAMELEN_8153 (9 * 1024) > > #define URE_PLA_IDR 0xc000 > #define URE_PLA_RCR 0xc010 > @@ -186,6 +187,10 @@ > /* PLA_TCR1 */ > #define URE_VERSION_MASK0x7cf0 > > +/* PLA_MTPS */ > +#define URE_MTPS_JUMBO (12 * 1024 / 64) > +#define URE_MTPS_DEFAULT (6 * 1024 / 64) > + > /* PLA_CR */ > #define URE_CR_RST 0x10 > #define URE_CR_RE 0x08 Tested on RTL8152 and RTL8153, seems to be working fine. ok kevlo@
unveil htpasswd
Hi,
htpasswd(1) when in batch mode (-I) and 1 argument is used, or when not in
batch mode and 2 arguments are used we know we have to access argv[0] with rwc
permissions and also to rwc a temporary file in /tmp so we can unveil(2) both
argv[0] and /tmp with rwc permissions. In order to avoid adding "unveil" to
pledge(2), just call it after getopt(3).
Remaining code paths already have fs access disabled via pledge(2).
Comments? OK?
Index: htpasswd.c
===
RCS file: /cvs/src/usr.bin/htpasswd/htpasswd.c,v
retrieving revision 1.16
diff -u -p -u -r1.16 htpasswd.c
--- htpasswd.c 7 Jun 2017 09:11:52 - 1.16
+++ htpasswd.c 30 Oct 2018 08:55:45 -
@@ -57,9 +57,6 @@ main(int argc, char** argv)
ssize_t linelen;
mode_t old_umask;
- if (pledge("stdio rpath wpath cpath flock tmppath tty", NULL) == -1)
- err(1, "pledge");
-
while ((c = getopt(argc, argv, "I")) != -1) {
switch (c) {
case 'I':
@@ -74,6 +71,15 @@ main(int argc, char** argv)
argc -= optind;
argv += optind;
+
+ if ((batch && argc == 1) || (!batch && argc == 2)) {
+ if (unveil(argv[0], "rwc") == -1)
+ err(1, "unveil");
+ if (unveil("/tmp", "rwc") == -1)
+ err(1, "unveil");
+ }
+ if (pledge("stdio rpath wpath cpath flock tmppath tty", NULL) == -1)
+ err(1, "pledge");
if (batch) {
if (argc == 1)
[v2] Use TAILQ functions for slpque
Do not roll our own for-loop, use the proper TAILQ function.
Also use unsleep() instead of unrolling the same functionality here again.
Now there is only one place that resets p_wchan. And the unsleep-part has
the same pattern like in endtsleep().
---
v2:
- TAILQ_FOREACH -> TAILQ_FOREACH_SAFE, since TAILQ_REMOVE might be
called. Silly me.
sys/kern/kern_synch.c | 11 +--
1 file changed, 5 insertions(+), 6 deletions(-)
diff --git a/sys/kern/kern_synch.c b/sys/kern/kern_synch.c
index 684624428db..a1689765ff9 100644
--- a/sys/kern/kern_synch.c
+++ b/sys/kern/kern_synch.c
@@ -436,15 +436,14 @@ unsleep(struct proc *p)
void
wakeup_n(const volatile void *ident, int n)
{
- struct slpque *qp;
struct proc *p;
struct proc *pnext;
int s;
SCHED_LOCK(s);
- qp = [LOOKUP(ident)];
- for (p = TAILQ_FIRST(qp); p != NULL && n != 0; p = pnext) {
- pnext = TAILQ_NEXT(p, p_runq);
+ TAILQ_FOREACH_SAFE(p, [LOOKUP(ident)], p_runq, pnext) {
+ if (n == 0)
+ break;
#ifdef DIAGNOSTIC
/*
* If the rwlock passed to rwsleep() is contended, the
@@ -460,10 +459,10 @@ wakeup_n(const volatile void *ident, int n)
#endif
if (p->p_wchan == ident) {
--n;
- p->p_wchan = 0;
- TAILQ_REMOVE(qp, p, p_runq);
if (p->p_stat == SSLEEP)
setrunnable(p);
+ else
+ unsleep(p);
}
}
SCHED_UNLOCK(s);
--
2.19.1
ure(4): VLANs and Jumbo frames
Hi, I recently wanted to use VLANs on ure(4) but failed. As it turns out, hardmtu is set to 1500 which apparently does not leave enough space for the VLAN tag. It looks like the Gigabit Version does even support Jumbo frames. Thus we can set the maximum framelen on the RX path to something bigger and enable Jumbo frames. I tried this on a Winyao USB1000F. Would be nice if people tested this change who have those ure(4)s in active use. Thanks, Patrick diff --git a/sys/dev/usb/if_ure.c b/sys/dev/usb/if_ure.c index b6c6c99ef34..637fd5eca5f 100644 --- a/sys/dev/usb/if_ure.c +++ b/sys/dev/usb/if_ure.c @@ -695,6 +695,9 @@ ure_rtl8152_init(struct ure_softc *sc) ure_init_fifo(sc); + /* Set allowed frame size. */ + ure_write_2(sc, URE_PLA_RMS, URE_MCU_TYPE_PLA, URE_MAX_FRAMELEN_8152); + ure_write_1(sc, URE_USB_TX_AGG, URE_MCU_TYPE_USB, URE_TX_AGG_MAX_THRESHOLD); ure_write_4(sc, URE_USB_RX_BUF_TH, URE_MCU_TYPE_USB, URE_RX_THR_HIGH); @@ -835,6 +838,10 @@ ure_rtl8153_init(struct ure_softc *sc) ure_init_fifo(sc); + /* Set allowed frame size. */ + ure_write_2(sc, URE_PLA_RMS, URE_MCU_TYPE_PLA, URE_MAX_FRAMELEN_8153); + ure_write_2(sc, URE_PLA_MTPS, URE_MCU_TYPE_PLA, URE_MTPS_JUMBO); + /* Enable Rx aggregation. */ ure_write_2(sc, URE_USB_USB_CTRL, URE_MCU_TYPE_USB, ure_read_2(sc, URE_USB_USB_CTRL, URE_MCU_TYPE_USB) & @@ -1147,6 +1154,12 @@ ure_attach(struct device *parent, struct device *self, void *aux) ifp->if_start = ure_start; ifp->if_capabilities = 0; + if (sc->ure_flags & URE_FLAG_8152) + ifp->ifp_hardmtu = URE_MAX_FRAMELEN_8152; + else + ifp->ifp_hardmtu = URE_MAX_FRAMELEN_8153; + ifp->ifp_hardmtu -= (ETHER_HDR_LEN + ETHER_VLAN_ENCAP_LEN); + mii = >ure_mii; mii->mii_ifp = ifp; mii->mii_readreg = ure_miibus_readreg; diff --git a/sys/dev/usb/if_urereg.h b/sys/dev/usb/if_urereg.h index 2260ec37890..8963d2753ed 100644 --- a/sys/dev/usb/if_urereg.h +++ b/sys/dev/usb/if_urereg.h @@ -41,7 +41,8 @@ #defineURE_BYTE_EN_BYTE0x11 #defineURE_BYTE_EN_SIX_BYTES 0x3f -#defineURE_MAX_FRAMELEN(ETHER_MAX_LEN + ETHER_VLAN_ENCAP_LEN) +#defineURE_MAX_FRAMELEN_8152 (ETHER_MAX_LEN + ETHER_VLAN_ENCAP_LEN) +#defineURE_MAX_FRAMELEN_8153 (9 * 1024) #defineURE_PLA_IDR 0xc000 #defineURE_PLA_RCR 0xc010 @@ -186,6 +187,10 @@ /* PLA_TCR1 */ #defineURE_VERSION_MASK0x7cf0 +/* PLA_MTPS */ +#define URE_MTPS_JUMBO (12 * 1024 / 64) +#define URE_MTPS_DEFAULT (6 * 1024 / 64) + /* PLA_CR */ #defineURE_CR_RST 0x10 #defineURE_CR_RE 0x08
Re: diff: Fix send(2) EACCES mistake
On Mon, Oct 29, 2018 at 11:55:52PM +0100, Jan Klemkow wrote: > On Sun, Oct 28, 2018 at 10:58:34PM +, Jason McIntyre wrote: > > On Sun, Oct 28, 2018 at 09:40:33PM +0100, Jan Klemkow wrote: > > > Unlike the manpage saids or one might think , sendto(2) sets errno to > > > EHOSTUNREACH instead of EACCES in cases of blocking by pf(4) or not > > > enabled broadcasts. Finally I ran into both cases and think, its time > > > to fix this issue. The diff suggests a new explanation that should > > > cover all error cases. > > > > > > Index: /usr/src/lib/libc/sys/send.2 > > > === > > > RCS file: /cvs/src/lib/libc/sys/send.2,v > > > retrieving revision 1.32 > > > diff -u -p -r1.32 send.2 > > > --- /usr/src/lib/libc/sys/send.2 5 Oct 2017 12:30:16 - 1.32 > > > +++ /usr/src/lib/libc/sys/send.2 28 Oct 2018 20:16:20 - > > > @@ -161,13 +161,13 @@ The operation may succeed when buffers b > > > The output queue for a network interface was full. > > > This generally indicates that the interface has stopped sending, > > > but may be caused by transient congestion. > > > -.It Bq Er EACCES > > > -The > > > -.Dv SO_BROADCAST > > > -option is not set on the socket, and a broadcast address > > > -was given as the destination. > > > .It Bq Er EHOSTUNREACH > > > -The destination address specified an unreachable host. > > > +The destination address specified an host that is unreachable, > > > > unless you're going for humour, "a host" rather than "an host". > > > > i can;t comment on the accuracy, but the rest of the diff reads ok. > > No humour (here in Germany), just a spelling mistakes :-) > Fixed version below. > morning. to be fair, some people do still unambiguously use "an" before h* words. so as such it is not wrong (i think). still, our pages do not use that format. anyway, enough of the small stuff. who wants to review this page and actually ok or nok it? jmc > Thanks, > Jan > > Index: /usr/src/lib/libc/sys/send.2 > === > RCS file: /cvs/src/lib/libc/sys/send.2,v > retrieving revision 1.32 > diff -u -p -r1.32 send.2 > --- /usr/src/lib/libc/sys/send.2 5 Oct 2017 12:30:16 - 1.32 > +++ /usr/src/lib/libc/sys/send.2 29 Oct 2018 22:48:24 - > @@ -161,13 +161,13 @@ The operation may succeed when buffers b > The output queue for a network interface was full. > This generally indicates that the interface has stopped sending, > but may be caused by transient congestion. > -.It Bq Er EACCES > -The > -.Dv SO_BROADCAST > -option is not set on the socket, and a broadcast address > -was given as the destination. > .It Bq Er EHOSTUNREACH > -The destination address specified an unreachable host. > +The destination address specified a host that is unreachable, > +blocked by > +.Xr pf 4 > +or is broadcast communication which was not enabled > +via the socket option > +.Dv SO_BROADCAST . > .It Bq Er EINVAL > The > .Fa flags >
