armv7: bus_space_map corner case fix
The bus_space_map() function on armv7 fails in the corner case where you want to allocate the last memory page. Consider a case where bpa = 0xfc00 and size=0x120. startpa becomes 0xf000 while endpa becomes startpa + PAGE_SIZE which is 0x. The following for-loop is currently conditioned to terminate when pa < endpa, but in this case endpa < startpa due to integer overflow, so the bus space is never mapped. Below is a patch that converts the for-loop's termination condition to startpa != endpa, which performs the expected action in this corner case. Index: sys/arch/arm/armv7/armv7_space.c === RCS file: /cvs/src/sys/arch/arm/armv7/armv7_space.c,v retrieving revision 1.10 diff -u -p -r1.10 armv7_space.c --- sys/arch/arm/armv7/armv7_space.c20 Mar 2018 23:04:48 - 1.10 +++ sys/arch/arm/armv7/armv7_space.c8 Mar 2019 01:12:18 - @@ -187,7 +187,7 @@ armv7_bs_map(void *t, uint64_t bpa, bus_ if (flags & BUS_SPACE_MAP_CACHEABLE) pmap_flags = 0; - for (pa = startpa; pa < endpa; pa += PAGE_SIZE, va += PAGE_SIZE) + for (pa = startpa; pa != endpa; pa += PAGE_SIZE, va += PAGE_SIZE) pmap_kenter_pa(va, pa | pmap_flags, PROT_READ | PROT_WRITE); pmap_update(pmap_kernel());
Re: radeon driver argb cursor fixes (was Re: X segmentation fault by chromium)
On Tue, Mar 05, 2019 at 10:24:10PM +0100, Matthieu Herrb wrote: > On Mon, Mar 04, 2019 at 09:14:45AM +0100, Matthieu Herrb wrote: > > On Sat, Mar 02, 2019 at 10:24:22PM +0200, Mihai Popescu wrote: > > > Hello, > > > > > > I am able to generate a segmentation fault on X with chromium help. > > > This is happening each time I visit the web page at [1]. Basically, > > > there is a picture of a product and whenever I hover the mouse over > > > it, X gets a segmentation fault and exits at xenodm login prompt. > > > Sometimes, if I repeat this procedure over and over, I get an offset > > > for the image on my display, making things hard to see. > > > > > > [1] > > > https://computers.woot.com/offers/lenovo-thinkcentre-m78-amd-a4-sff-desktop-1 > > > > > > I have inspected the web page in question with firefox, and I can tell > > > the page loads a special image cursor when I hover on the picture in > > > question. No segmentation fault for X this time. > > > > > > > Hi, > > > > Thanks for the bug report. Can you try the attached patch (from > > upstream) ? > > The patch below is in the most recent snapshots and Mihai reported that it > fixes the issue for him. So if you're using the ati/radeon driver > please test (and pay attention to possible cursor corruption...) > > The relevant upstream commits are: > https://gitlab.freedesktop.org/xorg/driver/xf86-video-ati/commit/0c40a76d1c050d018e6d59bebb5efc9c62be308c > > Detect and fix up non-premultiplied cursor data > > and > https://gitlab.freedesktop.org/xorg/driver/xf86-video-ati/commit/99ac121770da53196124d80375a5c8edbcf827fa > > Skip gamma correction of cursor data if premultiplied R/G/B > alpha > > ok ? As xf86-video-ati 19.0.0 recently released https://lists.x.org/archives/xorg-announce/2019-March/002967.html I'd prefer if we just went to that which includes the commits. Index: ChangeLog === RCS file: /cvs/xenocara/driver/xf86-video-ati/ChangeLog,v retrieving revision 1.16 diff -u -p -r1.16 ChangeLog --- ChangeLog 13 Jan 2019 07:16:48 - 1.16 +++ ChangeLog 7 Mar 2019 22:45:56 - @@ -1,3 +1,440 @@ +commit 0d132d99e0b750896a78f47d73a8639680495d8c +Author: Michel D??nzer +Date: Wed Mar 6 17:48:03 2019 +0100 + +Bump version for 19.0.0 release + +commit c301b8af25d2c2cd49035a4395ebe6c3612df366 +Author: Michel D??nzer +Date: Fri Mar 1 18:28:11 2019 +0100 + +dri2: Call drm_queue_handle_deferred in dri2_deferred_event + +drm_queue_handler just puts the event on the signalled list; without +calling drm_queue_handle_deferred, actual processing of the event may be +delayed indefinitely, e.g. until another event arrives from the kernel. + +This could result in DRI2 clients hanging during DPMS off. + +Fixes: ba83a866af5a "Add radeon_drm_handle_event wrapper for + drmHandleEvent" +(Ported from amdgpu commit 09be74a3d1dd9604336d9a27f98d132b262dcbaf) +Reviewed-by: Alex Deucher + +commit 705020b6247eaa062edc9c88e6ad52f8c5468051 +Author: Michel D??nzer +Date: Fri Mar 1 18:23:30 2019 +0100 + +present: Check that flip and screen pixmap pitches match + +If they don't, flipping will result in corrupted display. + +Test case: + +* Run Xorg at 1920x1080 with no window manager +* glxgears -geometry 2048x1080 + +The Present extension code in xserver 1.21 will check for this. + +(Ported from amdgpu commit a636f42b496b0604ca00a144690ece61d1a88a27) +Reviewed-by: Alex Deucher + +commit 15697ee242c30b9ea6775624e8282e0171a113a7 +Author: Michel D??nzer +Date: Mon Jan 28 18:27:10 2019 +0100 + +Keep waiting for a pending flip if drm_handle_event returns 0 + +drm_wait_pending_flip stopped waiting if drm_handle_event returned 0, +but that might have processed only some unrelated DRM events. As long as +the flip is pending, we have to keep waiting for its completion event. + +Noticed while working on the previous fix. + +(Ported from amdgpu commit 9045fb310f88780e250e60b80431ca153330e61b) + +commit 227123de3d862e691131708b7f55260bee17f2b7 +Author: Michel D??nzer +Date: Mon Jan 28 18:24:41 2019 +0100 + +Call drmHandleEvent again if it was interrupted by a signal + +drmHandleEvent can be interrupted by a signal in read(), in which case +it doesn't process any events but returns -1, which +drm_handle_event propagated to its callers. This could cause the +following failure cascade: + +1. drm_wait_pending_flip stopped waiting for a pending flip. +2. Its caller cleared drmmode_crtc->flip_pending before the flip + completed. +3. Another flip was attempted but got an unexpected EBUSY error because + the previous flip was still pending. +4. TearFree was disabled due to the error. + +The solution is to call drmHandleEvent if it was interrupted by a +signal. We can do that
Re: vmd/vmctl: improve VM name checks/error handling
On Thu, Mar 07, 2019 at 09:00:55PM +0100, Theo Buehler wrote: > On Thu, Mar 07, 2019 at 08:52:45PM +0100, Klemens Nanni wrote: > > vmd(8) does not support numerical names with `start' and `receive'. > > It never worked, the manuals are now clearer about this, but error > > handling can still be improved: > > I'm not sure I understand. This works fine for me with -current from > yesterday (I have vms configured in my vm.conf). > > $ vmctl start 1 > vmctl: started vm 1 successfully, tty /dev/ttyp0 Hm. That works because the first VM defined in your vm.conf(5) has a valid non-numerical name and gets assigned ID 1. With predefined VMs it works as you showed because ctl_start() parses the name as ID really, that is valid IDs will be accepted although the vmctl(8) always said `vmctl start name ...' not `vmctl start id ...'. The problem I faced is creating new VMs on the fly, possibly without any vm.conf present. Here the name passed on the command line is used to create a new VM instead of referencing an existing one. This never worked but the manual said it would be possible. With `receive' it's even more visible, as it never possibly uses the passed name as ID. Here's an example: $ vmctl create /10.img -s 10M vmctl: raw imagefile created # cat >/etc/vm.conf <
Re: vmd/vmctl: improve VM name checks/error handling
On Thu, Mar 07, 2019 at 08:52:45PM +0100, Klemens Nanni wrote:
> vmd(8) does not support numerical names with `start' and `receive'.
> It never worked, the manuals are now clearer about this, but error
> handling can still be improved:
>
> $ vmctl start 60 -t test -d 60.qcow2
> vmctl: start vm command failed: No such file or directory
>
> That's from vm_start_complete() in vmctl.c:254 where vmd(8) unexpectedly
> returns EPERM after it failed to create the VM.
>
> $ vmctl receive 60vmctl: invalid vm name
> vmctl: invalid id: 60
>
> Here, first parse_vmid(), then it's caller ctl_receive() complains.
>
> parse_vmid()'s last argument is `needname', indicating that the id to be
> parsed must not be numerical.
>
> The following diff makes the code check for a letter in the beginning,
> adjusts `start' and `receive' set this argument and print only one error.
This is not right. Each started (or defined if in /etc/vm.conf) VM is assigned
an ID. That ID can be used in place of a name. That behaviour needs to continue
to work.
My advice is to just clean up the man page and error messages, nothing more.
-ml
>
> $ ./obj/vmctl start 60 -t test -d 60.qcow2
> vmctl: invalid VM name
> $ ./obj/vmctl receive 60vmctl: invalid VM name
>
> "vm" -> "VM" for consistency with all other "invalid VM name" occurences
> throughout the sources.
>
> Feedback? OK?
>
> Index: usr.sbin/vmctl/main.c
> ===
> RCS file: /cvs/src/usr.sbin/vmctl/main.c,v
> retrieving revision 1.54
> diff -u -p -r1.54 main.c
> --- usr.sbin/vmctl/main.c 1 Mar 2019 12:47:36 - 1.54
> +++ usr.sbin/vmctl/main.c 7 Mar 2019 19:14:15 -
> @@ -524,7 +524,7 @@ parse_vmid(struct parse_result *res, cha
> id = strtonum(word, 0, UINT32_MAX, );
> if (error == NULL) {
> if (needname) {
> - warnx("invalid vm name");
> + warnx("invalid VM name");
> return (-1);
> } else {
> res->id = id;
> @@ -842,8 +842,8 @@ ctl_start(struct parse_result *res, int
> if (argc < 2)
> ctl_usage(res->ctl);
>
> - if (parse_vmid(res, argv[1], 0) == -1)
> - errx(1, "invalid id: %s", argv[1]);
> + if (parse_vmid(res, argv[1], 1) == -1)
> + exit(1);
>
> argc--;
> argv++;
> @@ -1046,7 +1046,7 @@ ctl_receive(struct parse_result *res, in
> err(1, "pledge");
> if (argc == 2) {
> if (parse_vmid(res, argv[1], 1) == -1)
> - errx(1, "invalid id: %s", argv[1]);
> + exit(1);
> } else if (argc != 2)
> ctl_usage(res->ctl);
>
> Index: usr.sbin/vmctl/vmctl.c
> ===
> RCS file: /cvs/src/usr.sbin/vmctl/vmctl.c,v
> retrieving revision 1.65
> diff -u -p -r1.65 vmctl.c
> --- usr.sbin/vmctl/vmctl.c6 Dec 2018 09:23:15 - 1.65
> +++ usr.sbin/vmctl/vmctl.c7 Mar 2019 19:14:15 -
> @@ -154,12 +154,7 @@ vm_start(uint32_t start_id, const char *
> }
> }
> if (name != NULL) {
> - /*
> - * Allow VMs names with alphanumeric characters, dot, hyphen
> - * and underscore. But disallow dot, hyphen and underscore at
> - * the start.
> - */
> - if (*name == '-' || *name == '.' || *name == '_')
> + if (!isalpha(*name))
> errx(1, "invalid VM name");
>
> for (s = name; *s != '\0'; ++s) {
> Index: usr.sbin/vmd/vmd.c
> ===
> RCS file: /cvs/src/usr.sbin/vmd/vmd.c,v
> retrieving revision 1.108
> diff -u -p -r1.108 vmd.c
> --- usr.sbin/vmd/vmd.c9 Dec 2018 12:26:38 - 1.108
> +++ usr.sbin/vmd/vmd.c7 Mar 2019 19:14:15 -
> @@ -1257,11 +1257,7 @@ vm_register(struct privsep *ps, struct v
> vcp->vcp_ndisks == 0 && strlen(vcp->vcp_cdrom) == 0) {
> log_warnx("no kernel or disk/cdrom specified");
> goto fail;
> - } else if (strlen(vcp->vcp_name) == 0) {
> - log_warnx("invalid VM name");
> - goto fail;
> - } else if (*vcp->vcp_name == '-' || *vcp->vcp_name == '.' ||
> - *vcp->vcp_name == '_') {
> + } else if (strlen(vcp->vcp_name) == 0 || !isalpha(*vcp->vcp_name)) {
> log_warnx("invalid VM name");
> goto fail;
> } else {
> ===
> Stats: --- 15 lines 531 chars
> Stats: +++ 6 lines 185 chars
> Stats: -9 lines
> Stats: -346 chars
>
Re: Spleen kernel fonts improvements
> Date: Thu, 7 Mar 2019 21:22:09 +0100
> From: Frederic Cambus
>
> Hi tech@,
>
> Here is a diff to sync kernel fonts with the latest released Spleen
> version, bringing the following improvements:
>
> - Remove strain pixels on the inner upper part of parentheses for the
> 16x32 version
> - Harmonize the 'v' and 'V' characters across all sizes
> - Remove artefacts from the 'c' in the copyright sign across all sizes
> - Make the upper left corner of the 'R' sharp in the registered sign
> for the 16x32 and 32x64 versions
>
> Comments? OK?
Go for it.
> Index: sys/dev/wsfont/spleen12x24.h
> ===
> RCS file: /cvs/src/sys/dev/wsfont/spleen12x24.h,v
> retrieving revision 1.1
> diff -u -p -r1.1 spleen12x24.h
> --- sys/dev/wsfont/spleen12x24.h 2 Dec 2018 14:44:33 - 1.1
> +++ sys/dev/wsfont/spleen12x24.h 7 Mar 2019 14:13:19 -
> @@ -1,7 +1,7 @@
> /* $OpenBSD: spleen12x24.h,v 1.1 2018/12/02 14:44:33 fcambus Exp $ */
>
> /*
> - * Copyright (c) 2018 Frederic Cambus
> + * Copyright (c) 2018-2019 Frederic Cambus
> * All rights reserved.
> *
> * Redistribution and use in source and binary forms, with or without
> @@ -1408,10 +1408,10 @@ static u_char spleen12x24_data[] = {
> 0x60, 0x60, /* .**..**. */
> 0x60, 0x60, /* .**..**. */
> 0x60, 0x60, /* .**..**. */
> - 0x30, 0xe0, /* ..*****. */
> + 0x30, 0xc0, /* ..****.. */
> 0x30, 0xc0, /* ..****.. */
> 0x19, 0x80, /* ...**..**... */
> - 0x0f, 0x80, /* *... */
> + 0x0f, 0x00, /* */
> 0x06, 0x00, /* .**. */
> 0x00, 0x00, /* */
> 0x00, 0x00, /* */
> @@ -2208,7 +2208,7 @@ static u_char spleen12x24_data[] = {
> 0x60, 0x60, /* .**..**. */
> 0x60, 0x60, /* .**..**. */
> 0x60, 0x60, /* .**..**. */
> - 0x30, 0xe0, /* ..*****. */
> + 0x30, 0xc0, /* ..****.. */
> 0x30, 0xc0, /* ..****.. */
> 0x19, 0x80, /* ...**..**... */
> 0x0f, 0x00, /* */
> @@ -3480,7 +3480,7 @@ static u_char spleen12x24_data[] = {
> 0x30, 0xc0, /* ..****.. */
> 0x60, 0x60, /* .**..**. */
> 0x67, 0x60, /* .**..***.**. */
> - 0x6d, 0x60, /* .**.**.*.**. */
> + 0x6c, 0x60, /* .**.**...**. */
> 0x6c, 0x60, /* .**.**...**. */
> 0x6c, 0x60, /* .**.**...**. */
> 0x6c, 0x60, /* .**.**...**. */
> Index: sys/dev/wsfont/spleen16x32.h
> ===
> RCS file: /cvs/src/sys/dev/wsfont/spleen16x32.h,v
> retrieving revision 1.1
> diff -u -p -r1.1 spleen16x32.h
> --- sys/dev/wsfont/spleen16x32.h 2 Dec 2018 14:44:33 - 1.1
> +++ sys/dev/wsfont/spleen16x32.h 7 Mar 2019 14:13:19 -
> @@ -1,7 +1,7 @@
> /* $OpenBSD: spleen16x32.h,v 1.1 2018/12/02 14:44:33 fcambus Exp $ */
>
> /*
> - * Copyright (c) 2018 Frederic Cambus
> + * Copyright (c) 2018-2019 Frederic Cambus
> * All rights reserved.
> *
> * Redistribution and use in source and binary forms, with or without
> @@ -316,7 +316,7 @@ static u_char spleen16x32_data[] = {
> 0x01, 0xe0, /* .... */
> 0x03, 0x80, /* ..***... */
> 0x07, 0x00, /* .*** */
> - 0x07, 0x00, /* .*** */
> + 0x06, 0x00, /* .**. */
> 0x0e, 0x00, /* ***. */
> 0x0c, 0x00, /* **.. */
> 0x1c, 0x00, /* ...***.. */
> @@ -349,7 +349,7 @@ static u_char spleen16x32_data[] = {
> 0x07, 0x80, /* .... */
> 0x01, 0xc0, /* ...***.. */
> 0x00, 0xe0, /* ***. */
> - 0x00, 0xe0, /* ***. */
> + 0x00, 0x60, /* .**. */
> 0x00, 0x70, /* .*** */
> 0x00, 0x30, /* ..** */
> 0x00, 0x38, /* ..***... */
> @@ -1846,7 +1846,7 @@ static u_char spleen16x32_data[] = {
> 0x30, 0x0c, /* ..****.. */
> 0x30, 0x0c, /* ..****.. */
> 0x30, 0x0c, /* ..****.. */
> - 0x18, 0x18, /* ...**..**... */
> + 0x38, 0x1c, /* ..***..***.. */
> 0x1c, 0x38, /* ...******... */
> 0x0e, 0x70, /* ***..*** */
> 0x07, 0xe0, /* .**. */
> @@ -4581,7 +4581,7 @@ static u_char spleen16x32_data[] = {
> 0x30, 0x0c, /* ..****.. */
> 0x33, 0xec, /* ..**..*.**.. */
> 0x37, 0xec, /* ..**.**.**.. */
> - 0x36, 0x6c, /* ..**.**..**.**.. */
> + 0x36, 0x0c, /* ..**.**.**.. */
>
Spleen kernel fonts improvements
Hi tech@,
Here is a diff to sync kernel fonts with the latest released Spleen
version, bringing the following improvements:
- Remove strain pixels on the inner upper part of parentheses for the
16x32 version
- Harmonize the 'v' and 'V' characters across all sizes
- Remove artefacts from the 'c' in the copyright sign across all sizes
- Make the upper left corner of the 'R' sharp in the registered sign
for the 16x32 and 32x64 versions
Comments? OK?
Index: sys/dev/wsfont/spleen12x24.h
===
RCS file: /cvs/src/sys/dev/wsfont/spleen12x24.h,v
retrieving revision 1.1
diff -u -p -r1.1 spleen12x24.h
--- sys/dev/wsfont/spleen12x24.h2 Dec 2018 14:44:33 - 1.1
+++ sys/dev/wsfont/spleen12x24.h7 Mar 2019 14:13:19 -
@@ -1,7 +1,7 @@
/* $OpenBSD: spleen12x24.h,v 1.1 2018/12/02 14:44:33 fcambus Exp $ */
/*
- * Copyright (c) 2018 Frederic Cambus
+ * Copyright (c) 2018-2019 Frederic Cambus
* All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
@@ -1408,10 +1408,10 @@ static u_char spleen12x24_data[] = {
0x60, 0x60, /* .**..**. */
0x60, 0x60, /* .**..**. */
0x60, 0x60, /* .**..**. */
- 0x30, 0xe0, /* ..*****. */
+ 0x30, 0xc0, /* ..****.. */
0x30, 0xc0, /* ..****.. */
0x19, 0x80, /* ...**..**... */
- 0x0f, 0x80, /* *... */
+ 0x0f, 0x00, /* */
0x06, 0x00, /* .**. */
0x00, 0x00, /* */
0x00, 0x00, /* */
@@ -2208,7 +2208,7 @@ static u_char spleen12x24_data[] = {
0x60, 0x60, /* .**..**. */
0x60, 0x60, /* .**..**. */
0x60, 0x60, /* .**..**. */
- 0x30, 0xe0, /* ..*****. */
+ 0x30, 0xc0, /* ..****.. */
0x30, 0xc0, /* ..****.. */
0x19, 0x80, /* ...**..**... */
0x0f, 0x00, /* */
@@ -3480,7 +3480,7 @@ static u_char spleen12x24_data[] = {
0x30, 0xc0, /* ..****.. */
0x60, 0x60, /* .**..**. */
0x67, 0x60, /* .**..***.**. */
- 0x6d, 0x60, /* .**.**.*.**. */
+ 0x6c, 0x60, /* .**.**...**. */
0x6c, 0x60, /* .**.**...**. */
0x6c, 0x60, /* .**.**...**. */
0x6c, 0x60, /* .**.**...**. */
Index: sys/dev/wsfont/spleen16x32.h
===
RCS file: /cvs/src/sys/dev/wsfont/spleen16x32.h,v
retrieving revision 1.1
diff -u -p -r1.1 spleen16x32.h
--- sys/dev/wsfont/spleen16x32.h2 Dec 2018 14:44:33 - 1.1
+++ sys/dev/wsfont/spleen16x32.h7 Mar 2019 14:13:19 -
@@ -1,7 +1,7 @@
/* $OpenBSD: spleen16x32.h,v 1.1 2018/12/02 14:44:33 fcambus Exp $ */
/*
- * Copyright (c) 2018 Frederic Cambus
+ * Copyright (c) 2018-2019 Frederic Cambus
* All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
@@ -316,7 +316,7 @@ static u_char spleen16x32_data[] = {
0x01, 0xe0, /* .... */
0x03, 0x80, /* ..***... */
0x07, 0x00, /* .*** */
- 0x07, 0x00, /* .*** */
+ 0x06, 0x00, /* .**. */
0x0e, 0x00, /* ***. */
0x0c, 0x00, /* **.. */
0x1c, 0x00, /* ...***.. */
@@ -349,7 +349,7 @@ static u_char spleen16x32_data[] = {
0x07, 0x80, /* .... */
0x01, 0xc0, /* ...***.. */
0x00, 0xe0, /* ***. */
- 0x00, 0xe0, /* ***. */
+ 0x00, 0x60, /* .**. */
0x00, 0x70, /* .*** */
0x00, 0x30, /* ..** */
0x00, 0x38, /* ..***... */
@@ -1846,7 +1846,7 @@ static u_char spleen16x32_data[] = {
0x30, 0x0c, /* ..****.. */
0x30, 0x0c, /* ..****.. */
0x30, 0x0c, /* ..****.. */
- 0x18, 0x18, /* ...**..**... */
+ 0x38, 0x1c, /* ..***..***.. */
0x1c, 0x38, /* ...******... */
0x0e, 0x70, /* ***..*** */
0x07, 0xe0, /* .**. */
@@ -4581,7 +4581,7 @@ static u_char spleen16x32_data[] = {
0x30, 0x0c, /* ..****.. */
0x33, 0xec, /* ..**..*.**.. */
0x37, 0xec, /* ..**.**.**.. */
- 0x36, 0x6c, /* ..**.**..**.**.. */
+ 0x36, 0x0c, /* ..**.**.**.. */
0x36, 0x0c, /* ..**.**.**.. */
0x36, 0x0c, /* ..**.**.**.. */
0x36, 0x0c, /* ..**.**.**.. */
@@ -4742,7 +4742,7 @@ static u_char
Re: vmd/vmctl: improve VM name checks/error handling
On Thu, Mar 07, 2019 at 08:52:45PM +0100, Klemens Nanni wrote: > vmd(8) does not support numerical names with `start' and `receive'. > It never worked, the manuals are now clearer about this, but error > handling can still be improved: I'm not sure I understand. This works fine for me with -current from yesterday (I have vms configured in my vm.conf). $ vmctl start 1 vmctl: started vm 1 successfully, tty /dev/ttyp0 Please don't disallow or break it.
vmd/vmctl: improve VM name checks/error handling
vmd(8) does not support numerical names with `start' and `receive'.
It never worked, the manuals are now clearer about this, but error
handling can still be improved:
$ vmctl start 60 -t test -d 60.qcow2
vmctl: start vm command failed: No such file or directory
That's from vm_start_complete() in vmctl.c:254 where vmd(8) unexpectedly
returns EPERM after it failed to create the VM.
$ vmctl receive 60 "VM" for consistency with all other "invalid VM name" occurences
throughout the sources.
Feedback? OK?
Index: usr.sbin/vmctl/main.c
===
RCS file: /cvs/src/usr.sbin/vmctl/main.c,v
retrieving revision 1.54
diff -u -p -r1.54 main.c
--- usr.sbin/vmctl/main.c 1 Mar 2019 12:47:36 - 1.54
+++ usr.sbin/vmctl/main.c 7 Mar 2019 19:14:15 -
@@ -524,7 +524,7 @@ parse_vmid(struct parse_result *res, cha
id = strtonum(word, 0, UINT32_MAX, );
if (error == NULL) {
if (needname) {
- warnx("invalid vm name");
+ warnx("invalid VM name");
return (-1);
} else {
res->id = id;
@@ -842,8 +842,8 @@ ctl_start(struct parse_result *res, int
if (argc < 2)
ctl_usage(res->ctl);
- if (parse_vmid(res, argv[1], 0) == -1)
- errx(1, "invalid id: %s", argv[1]);
+ if (parse_vmid(res, argv[1], 1) == -1)
+ exit(1);
argc--;
argv++;
@@ -1046,7 +1046,7 @@ ctl_receive(struct parse_result *res, in
err(1, "pledge");
if (argc == 2) {
if (parse_vmid(res, argv[1], 1) == -1)
- errx(1, "invalid id: %s", argv[1]);
+ exit(1);
} else if (argc != 2)
ctl_usage(res->ctl);
Index: usr.sbin/vmctl/vmctl.c
===
RCS file: /cvs/src/usr.sbin/vmctl/vmctl.c,v
retrieving revision 1.65
diff -u -p -r1.65 vmctl.c
--- usr.sbin/vmctl/vmctl.c 6 Dec 2018 09:23:15 - 1.65
+++ usr.sbin/vmctl/vmctl.c 7 Mar 2019 19:14:15 -
@@ -154,12 +154,7 @@ vm_start(uint32_t start_id, const char *
}
}
if (name != NULL) {
- /*
-* Allow VMs names with alphanumeric characters, dot, hyphen
-* and underscore. But disallow dot, hyphen and underscore at
-* the start.
-*/
- if (*name == '-' || *name == '.' || *name == '_')
+ if (!isalpha(*name))
errx(1, "invalid VM name");
for (s = name; *s != '\0'; ++s) {
Index: usr.sbin/vmd/vmd.c
===
RCS file: /cvs/src/usr.sbin/vmd/vmd.c,v
retrieving revision 1.108
diff -u -p -r1.108 vmd.c
--- usr.sbin/vmd/vmd.c 9 Dec 2018 12:26:38 - 1.108
+++ usr.sbin/vmd/vmd.c 7 Mar 2019 19:14:15 -
@@ -1257,11 +1257,7 @@ vm_register(struct privsep *ps, struct v
vcp->vcp_ndisks == 0 && strlen(vcp->vcp_cdrom) == 0) {
log_warnx("no kernel or disk/cdrom specified");
goto fail;
- } else if (strlen(vcp->vcp_name) == 0) {
- log_warnx("invalid VM name");
- goto fail;
- } else if (*vcp->vcp_name == '-' || *vcp->vcp_name == '.' ||
- *vcp->vcp_name == '_') {
+ } else if (strlen(vcp->vcp_name) == 0 || !isalpha(*vcp->vcp_name)) {
log_warnx("invalid VM name");
goto fail;
} else {
===
Stats: --- 15 lines 531 chars
Stats: +++ 6 lines 185 chars
Stats: -9 lines
Stats: -346 chars
Re: acpithinkpad: a fix for the x260
On Thu, Mar 07, 2019 at 01:31:59PM +, Stuart Henderson wrote:
> On 2019/03/06 20:55, joshua stein wrote:
> > sthen found that the HKEY version metric failed on the x260 where it
> > reports version 1 but requires the new ACPI method of changing
> > backlight.
> >
> > This diff tries to do the ACPI method on all machines and falls back
> > to the CMOS method if that fails.
> >
> > Can all those that tried the previous diff (which has been
> > committed) try this one and make sure nothing broke?
> >
> > This also unmasks the microphone mute event which helps mixerctl
> > stay in sync on the x260 (it has no effect on my x1c6, but probably
> > can't hurt).
> >
> >
X280 is still happy with this diff. The microphone mute button doesn't
seem to do anything tho.
> > Index: sys/dev/acpi/acpithinkpad.c
>
> The patch in the mail doesn't apply cleanly for me due to some
> whitespace issues, if anyone else is having problems with that here's
> a regenerated version:
>
>
> Index: acpithinkpad.c
> ===
> RCS file: /cvs/src/sys/dev/acpi/acpithinkpad.c,v
> retrieving revision 1.63
> diff -u -p -r1.63 acpithinkpad.c
> --- acpithinkpad.c6 Mar 2019 15:36:30 - 1.63
> +++ acpithinkpad.c7 Mar 2019 13:29:46 -
> @@ -124,6 +124,7 @@
> #define THINKPAD_ADAPTIVE_MODE_HOME 1
> #define THINKPAD_ADAPTIVE_MODE_FUNCTION 3
>
> +#define THINKPAD_MASK_MIC_MUTE (1 << 14)
> #define THINKPAD_MASK_BRIGHTNESS_UP (1 << 15)
> #define THINKPAD_MASK_BRIGHTNESS_DOWN(1 << 16)
> #define THINKPAD_MASK_KBD_BACKLIGHT (1 << 17)
> @@ -171,8 +172,8 @@ int thinkpad_get_kbd_backlight(struct ws
> int thinkpad_set_kbd_backlight(struct wskbd_backlight *);
> extern int (*wskbd_get_backlight)(struct wskbd_backlight *);
> extern int (*wskbd_set_backlight)(struct wskbd_backlight *);
> -void thinkpad_get_brightness(struct acpithinkpad_softc *);
> -void thinkpad_set_brightness(void *, int);
> +int thinkpad_get_brightness(struct acpithinkpad_softc *);
> +int thinkpad_set_brightness(void *, int);
> int thinkpad_get_param(struct wsdisplay_param *);
> int thinkpad_set_param(struct wsdisplay_param *);
> extern int (*ws_get_param)(struct wsdisplay_param *);
> @@ -345,7 +346,9 @@ thinkpad_enable_events(struct acpithinkp
> }
>
> /* Enable events we need to know about */
> - mask |= (THINKPAD_MASK_BRIGHTNESS_UP | THINKPAD_MASK_BRIGHTNESS_DOWN |
> + mask |= (THINKPAD_MASK_MIC_MUTE |
> + THINKPAD_MASK_BRIGHTNESS_UP |
> + THINKPAD_MASK_BRIGHTNESS_DOWN |
> THINKPAD_MASK_KBD_BACKLIGHT);
>
> DPRINTF(("%s: setting event mask to 0x%llx\n", DEVNAME(sc), mask));
> @@ -555,8 +558,7 @@ thinkpad_brightness_up(struct acpithinkp
> {
> int b;
>
> - if (sc->sc_hkey_version == THINKPAD_HKEY_VERSION2) {
> - thinkpad_get_brightness(sc);
> + if (thinkpad_get_brightness(sc) == 0) {
> b = sc->sc_brightness & 0xff;
> if (b < ((sc->sc_brightness >> 8) & 0xff)) {
> sc->sc_brightness = b + 1;
> @@ -573,8 +575,7 @@ thinkpad_brightness_down(struct acpithin
> {
> int b;
>
> - if (sc->sc_hkey_version == THINKPAD_HKEY_VERSION2) {
> - thinkpad_get_brightness(sc);
> + if (thinkpad_get_brightness(sc) == 0) {
> b = sc->sc_brightness & 0xff;
> if (b > 0) {
> sc->sc_brightness = b - 1;
> @@ -701,30 +702,39 @@ thinkpad_set_kbd_backlight(struct wskbd_
> return 0;
> }
>
> -void
> +int
> thinkpad_get_brightness(struct acpithinkpad_softc *sc)
> {
> - aml_evalinteger(sc->sc_acpi, sc->sc_devnode,
> - "PBLG", 0, NULL, >sc_brightness);
> + int ret;
> +
> + ret = aml_evalinteger(sc->sc_acpi, sc->sc_devnode, "PBLG", 0, NULL,
> + >sc_brightness);
>
> DPRINTF(("%s: %s: 0x%llx\n", DEVNAME(sc), __func__, sc->sc_brightness));
> +
> + return ret;
> }
>
> -void
> +int
> thinkpad_set_brightness(void *arg0, int arg1)
> {
> struct acpithinkpad_softc *sc = arg0;
> struct aml_value arg;
> + int ret;
>
> DPRINTF(("%s: %s: 0x%llx\n", DEVNAME(sc), __func__, sc->sc_brightness));
>
> memset(, 0, sizeof(arg));
> arg.type = AML_OBJTYPE_INTEGER;
> arg.v_integer = sc->sc_brightness & 0xff;
> - aml_evalname(sc->sc_acpi, sc->sc_devnode,
> - "PBLS", 1, , NULL);
> + ret = aml_evalname(sc->sc_acpi, sc->sc_devnode, "PBLS", 1, , NULL);
> +
> + if (ret)
> + return ret;
>
> thinkpad_get_brightness(sc);
> +
> + return 0;
> }
>
> int
> @@ -765,7 +775,8 @@ thinkpad_set_param(struct wsdisplay_para
> dp->curval = maxval;
> sc->sc_brightness &= ~0xff;
> sc->sc_brightness |= dp->curval;
> - acpi_addtask(sc->sc_acpi, thinkpad_set_brightness, sc, 0);
> +
Re: acpithinkpad: a fix for the x260
No problems here.
Tracey
On Wed, Mar 06, 2019 at 08:55:15PM -0600, joshua stein wrote:
> sthen found that the HKEY version metric failed on the x260 where it
> reports version 1 but requires the new ACPI method of changing
> backlight.
>
> This diff tries to do the ACPI method on all machines and falls back
> to the CMOS method if that fails.
>
> Can all those that tried the previous diff (which has been
> committed) try this one and make sure nothing broke?
>
> This also unmasks the microphone mute event which helps mixerctl
> stay in sync on the x260 (it has no effect on my x1c6, but probably
> can't hurt).
>
>
> Index: sys/dev/acpi/acpithinkpad.c
> ===
> RCS file: /cvs/src/sys/dev/acpi/acpithinkpad.c,v
> retrieving revision 1.63
> diff -u -p -u -p -r1.63 acpithinkpad.c
> --- sys/dev/acpi/acpithinkpad.c 6 Mar 2019 15:36:30 - 1.63
> +++ sys/dev/acpi/acpithinkpad.c 7 Mar 2019 02:53:36 -
> @@ -124,6 +124,7 @@
> #define THINKPAD_ADAPTIVE_MODE_HOME 1
> #define THINKPAD_ADAPTIVE_MODE_FUNCTION 3
>
> +#define THINKPAD_MASK_MIC_MUTE (1 << 14)
> #define THINKPAD_MASK_BRIGHTNESS_UP (1 << 15)
> #define THINKPAD_MASK_BRIGHTNESS_DOWN (1 << 16)
> #define THINKPAD_MASK_KBD_BACKLIGHT (1 << 17)
> @@ -171,8 +172,8 @@ int thinkpad_get_kbd_backlight(struct ws
> int thinkpad_set_kbd_backlight(struct wskbd_backlight *);
> extern int (*wskbd_get_backlight)(struct wskbd_backlight *);
> extern int (*wskbd_set_backlight)(struct wskbd_backlight *);
> -void thinkpad_get_brightness(struct acpithinkpad_softc *);
> -void thinkpad_set_brightness(void *, int);
> +int thinkpad_get_brightness(struct acpithinkpad_softc *);
> +int thinkpad_set_brightness(void *, int);
> int thinkpad_get_param(struct wsdisplay_param *);
> int thinkpad_set_param(struct wsdisplay_param *);
> extern int (*ws_get_param)(struct wsdisplay_param *);
> @@ -345,7 +346,9 @@ thinkpad_enable_events(struct acpithinkp
> }
>
> /* Enable events we need to know about */
> - mask |= (THINKPAD_MASK_BRIGHTNESS_UP | THINKPAD_MASK_BRIGHTNESS_DOWN |
> + mask |= (THINKPAD_MASK_MIC_MUTE |
> + THINKPAD_MASK_BRIGHTNESS_UP |
> + THINKPAD_MASK_BRIGHTNESS_DOWN |
> THINKPAD_MASK_KBD_BACKLIGHT);
>
> DPRINTF(("%s: setting event mask to 0x%llx\n", DEVNAME(sc), mask));
> @@ -555,8 +558,7 @@ thinkpad_brightness_up(struct acpithinkp
> {
> int b;
>
> - if (sc->sc_hkey_version == THINKPAD_HKEY_VERSION2) {
> - thinkpad_get_brightness(sc);
> + if (thinkpad_get_brightness(sc) == 0) {
> b = sc->sc_brightness & 0xff;
> if (b < ((sc->sc_brightness >> 8) & 0xff)) {
> sc->sc_brightness = b + 1;
> @@ -573,8 +575,7 @@ thinkpad_brightness_down(struct acpithin
> {
> int b;
>
> - if (sc->sc_hkey_version == THINKPAD_HKEY_VERSION2) {
> - thinkpad_get_brightness(sc);
> + if (thinkpad_get_brightness(sc) == 0) {
> b = sc->sc_brightness & 0xff;
> if (b > 0) {
> sc->sc_brightness = b - 1;
> @@ -701,30 +702,39 @@ thinkpad_set_kbd_backlight(struct wskbd_
> return 0;
> }
>
> -void
> +int
> thinkpad_get_brightness(struct acpithinkpad_softc *sc)
> {
> - aml_evalinteger(sc->sc_acpi, sc->sc_devnode,
> - "PBLG", 0, NULL, >sc_brightness);
> + int ret;
> +
> + ret = aml_evalinteger(sc->sc_acpi, sc->sc_devnode, "PBLG", 0, NULL,
> + >sc_brightness);
>
> DPRINTF(("%s: %s: 0x%llx\n", DEVNAME(sc), __func__, sc->sc_brightness));
> +
> + return ret;
> }
>
> -void
> +int
> thinkpad_set_brightness(void *arg0, int arg1)
> {
> struct acpithinkpad_softc *sc = arg0;
> struct aml_value arg;
> + int ret;
>
> DPRINTF(("%s: %s: 0x%llx\n", DEVNAME(sc), __func__, sc->sc_brightness));
>
> memset(, 0, sizeof(arg));
> arg.type = AML_OBJTYPE_INTEGER;
> arg.v_integer = sc->sc_brightness & 0xff;
> - aml_evalname(sc->sc_acpi, sc->sc_devnode,
> - "PBLS", 1, , NULL);
> + ret = aml_evalname(sc->sc_acpi, sc->sc_devnode, "PBLS", 1, , NULL);
> +
> + if (ret)
> + return ret;
>
> thinkpad_get_brightness(sc);
> +
> + return 0;
> }
>
> int
> @@ -765,7 +775,8 @@ thinkpad_set_param(struct wsdisplay_para
> dp->curval = maxval;
> sc->sc_brightness &= ~0xff;
> sc->sc_brightness |= dp->curval;
> - acpi_addtask(sc->sc_acpi, thinkpad_set_brightness, sc, 0);
> + acpi_addtask(sc->sc_acpi, (void *)thinkpad_set_brightness, sc,
> + 0);
> acpi_wakeup(sc->sc_acpi);
> return 0;
> default:
Re: Avoid system(3) in ikectl
On Thu, Mar 7, 2019 at 4:53 AM Stuart Henderson wrote: > > On 2019/03/06 22:20, Theo de Raadt wrote: > > I'm not sure why this matters. > > > > Fundamentally system is fork+exec via a shell. So you write it as > > minimal fork+exec. > > > > What is the particular benefit you see here, is it security -- and if > > so, what is the security benefit? Have you identified a quoting problem? > > Can you pinpoint the issue and explain it please? If an administrator is given doas access to ikectl, they can start a root shell with doas ikectl ca '; sh; : ' create. Additionally if a user wants to create a ca or certificate or key with a special character, it must be double quoted to survive system(3). At the very least this fact should be documented, but removing the use of system(3) is the safer and less surprising way. > > > I had sent a similar patch a while back. There seemed to me some > > > interest, but it was never comitted. Updated to apply to -current. > > At the time of the first version of this diff there was a quoting > problem (and a "passwords showing in ps(1)" problem) but it was fixed > differently in ikeca.c:1.46 The export password is still shown in ps(1) since it's put into the environment via env(1) (cf. ikeca.c 686).
Re: acpithinkpad: a fix for the x260
On 2019/03/06 20:55, joshua stein wrote:
> sthen found that the HKEY version metric failed on the x260 where it
> reports version 1 but requires the new ACPI method of changing
> backlight.
>
> This diff tries to do the ACPI method on all machines and falls back
> to the CMOS method if that fails.
>
> Can all those that tried the previous diff (which has been
> committed) try this one and make sure nothing broke?
>
> This also unmasks the microphone mute event which helps mixerctl
> stay in sync on the x260 (it has no effect on my x1c6, but probably
> can't hurt).
>
>
> Index: sys/dev/acpi/acpithinkpad.c
The patch in the mail doesn't apply cleanly for me due to some
whitespace issues, if anyone else is having problems with that here's
a regenerated version:
Index: acpithinkpad.c
===
RCS file: /cvs/src/sys/dev/acpi/acpithinkpad.c,v
retrieving revision 1.63
diff -u -p -r1.63 acpithinkpad.c
--- acpithinkpad.c 6 Mar 2019 15:36:30 - 1.63
+++ acpithinkpad.c 7 Mar 2019 13:29:46 -
@@ -124,6 +124,7 @@
#defineTHINKPAD_ADAPTIVE_MODE_HOME 1
#defineTHINKPAD_ADAPTIVE_MODE_FUNCTION 3
+#define THINKPAD_MASK_MIC_MUTE (1 << 14)
#define THINKPAD_MASK_BRIGHTNESS_UP(1 << 15)
#define THINKPAD_MASK_BRIGHTNESS_DOWN (1 << 16)
#define THINKPAD_MASK_KBD_BACKLIGHT(1 << 17)
@@ -171,8 +172,8 @@ int thinkpad_get_kbd_backlight(struct ws
intthinkpad_set_kbd_backlight(struct wskbd_backlight *);
extern int (*wskbd_get_backlight)(struct wskbd_backlight *);
extern int (*wskbd_set_backlight)(struct wskbd_backlight *);
-void thinkpad_get_brightness(struct acpithinkpad_softc *);
-void thinkpad_set_brightness(void *, int);
+intthinkpad_get_brightness(struct acpithinkpad_softc *);
+intthinkpad_set_brightness(void *, int);
intthinkpad_get_param(struct wsdisplay_param *);
intthinkpad_set_param(struct wsdisplay_param *);
extern int (*ws_get_param)(struct wsdisplay_param *);
@@ -345,7 +346,9 @@ thinkpad_enable_events(struct acpithinkp
}
/* Enable events we need to know about */
- mask |= (THINKPAD_MASK_BRIGHTNESS_UP | THINKPAD_MASK_BRIGHTNESS_DOWN |
+ mask |= (THINKPAD_MASK_MIC_MUTE |
+ THINKPAD_MASK_BRIGHTNESS_UP |
+ THINKPAD_MASK_BRIGHTNESS_DOWN |
THINKPAD_MASK_KBD_BACKLIGHT);
DPRINTF(("%s: setting event mask to 0x%llx\n", DEVNAME(sc), mask));
@@ -555,8 +558,7 @@ thinkpad_brightness_up(struct acpithinkp
{
int b;
- if (sc->sc_hkey_version == THINKPAD_HKEY_VERSION2) {
- thinkpad_get_brightness(sc);
+ if (thinkpad_get_brightness(sc) == 0) {
b = sc->sc_brightness & 0xff;
if (b < ((sc->sc_brightness >> 8) & 0xff)) {
sc->sc_brightness = b + 1;
@@ -573,8 +575,7 @@ thinkpad_brightness_down(struct acpithin
{
int b;
- if (sc->sc_hkey_version == THINKPAD_HKEY_VERSION2) {
- thinkpad_get_brightness(sc);
+ if (thinkpad_get_brightness(sc) == 0) {
b = sc->sc_brightness & 0xff;
if (b > 0) {
sc->sc_brightness = b - 1;
@@ -701,30 +702,39 @@ thinkpad_set_kbd_backlight(struct wskbd_
return 0;
}
-void
+int
thinkpad_get_brightness(struct acpithinkpad_softc *sc)
{
- aml_evalinteger(sc->sc_acpi, sc->sc_devnode,
- "PBLG", 0, NULL, >sc_brightness);
+ int ret;
+
+ ret = aml_evalinteger(sc->sc_acpi, sc->sc_devnode, "PBLG", 0, NULL,
+ >sc_brightness);
DPRINTF(("%s: %s: 0x%llx\n", DEVNAME(sc), __func__, sc->sc_brightness));
+
+ return ret;
}
-void
+int
thinkpad_set_brightness(void *arg0, int arg1)
{
struct acpithinkpad_softc *sc = arg0;
struct aml_value arg;
+ int ret;
DPRINTF(("%s: %s: 0x%llx\n", DEVNAME(sc), __func__, sc->sc_brightness));
memset(, 0, sizeof(arg));
arg.type = AML_OBJTYPE_INTEGER;
arg.v_integer = sc->sc_brightness & 0xff;
- aml_evalname(sc->sc_acpi, sc->sc_devnode,
- "PBLS", 1, , NULL);
+ ret = aml_evalname(sc->sc_acpi, sc->sc_devnode, "PBLS", 1, , NULL);
+
+ if (ret)
+ return ret;
thinkpad_get_brightness(sc);
+
+ return 0;
}
int
@@ -765,7 +775,8 @@ thinkpad_set_param(struct wsdisplay_para
dp->curval = maxval;
sc->sc_brightness &= ~0xff;
sc->sc_brightness |= dp->curval;
- acpi_addtask(sc->sc_acpi, thinkpad_set_brightness, sc, 0);
+ acpi_addtask(sc->sc_acpi, (void *)thinkpad_set_brightness, sc,
+ 0);
acpi_wakeup(sc->sc_acpi);
return 0;
default:
Re: Avoid system(3) in ikectl
On 2019/03/06 22:20, Theo de Raadt wrote: > I'm not sure why this matters. > > Fundamentally system is fork+exec via a shell. So you write it as > minimal fork+exec. > > What is the particular benefit you see here, is it security -- and if > so, what is the security benefit? Have you identified a quoting problem? > Can you pinpoint the issue and explain it please? > > I had sent a similar patch a while back. There seemed to me some > > interest, but it was never comitted. Updated to apply to -current. At the time of the first version of this diff there was a quoting problem (and a "passwords showing in ps(1)" problem) but it was fixed differently in ikeca.c:1.46
