Re: Using shift on external keyboards in softraid passphrases from efiboot
Hi,
I think the diff should be brought to arm64 as well. ok?
On Thu, 23 Aug 2018 11:21:57 +0900 (JST)
YASUOKA Masahiko wrote:
> On Mon, 20 Aug 2018 13:50:13 +0200
> Theo Buehler wrote:
>> On Thu, Aug 16, 2018 at 09:51:32PM +0200, Frank Groeneveld wrote:
>>> I haven't been able to type the passphrase of my softraid device on
>>> boot when using an external keyboard on my Thinkpad X260. Finally I
>>> had some time to debug this problem and this is what I discovered.
>>>
>>> On a different laptop with EFI, the ReadKeyStroke call will not return
>>> a packet when shift is pressed on the external keyboard. On the
>>> Thinkpad however, a packet is returned with UnicodeChar == 0, which
>>> results in a wrong passphrase being used.
>>>
>>> This seems like a bug in the firmware to me, because according to some
>>> EFI specifications I found online, this should not return a packet.
>>> I've attached a simple patch that fixes this, but I'm not sure whether
>>> this might break things on different systems.
>>
>> I can't comment on the technical side of this patch but I can confirm
>> that it allows me to enter the password from an external keyboard with
>> my x280.
>
> In the spec,
>
> | The UnicodeChar is the actual printable character or is zero if the
> | key does not represent a printable character (control key, function
> | key, etc.).
>
> It seems that UnicodeChar can be zero. So I think the diff is OK even
> on the spec.
>
> If there is no futher comment I'll commit it. Thanks.
Index: sys/arch/arm64/stand/efiboot/efiboot.c
===
RCS file: /cvs/src/sys/arch/arm64/stand/efiboot/efiboot.c,v
retrieving revision 1.20
diff -u -p -r1.20 efiboot.c
--- sys/arch/arm64/stand/efiboot/efiboot.c 23 Aug 2018 15:31:12 -
1.20
+++ sys/arch/arm64/stand/efiboot/efiboot.c 24 Aug 2018 02:44:30 -
@@ -129,7 +129,7 @@ efi_cons_getc(dev_t dev)
}
status = conin->ReadKeyStroke(conin, );
- while (status == EFI_NOT_READY) {
+ while (status == EFI_NOT_READY || key.UnicodeChar == 0) {
if (dev & 0x80)
return (0);
/*
Re: vmd/vmctl: allow to boot cdrom-only VMs
Penned by Theo de Raadt on 20180823 14:22.46, we have:
| Mike Larkin wrote:
|
| > On Thu, Aug 23, 2018 at 11:05:40AM -0500, Todd T. Fries wrote:
| > > This makes me wonder. Does it make sense to support booting a kernel
without
| > > disks? Some people have heard of the phrase 'diskless' ;-)
| > >
| > > Penned by Reyk Floeter on 20180822 13:35.23, we have:
| > > | Hi,
| > > |
| > > | vmctl doesn't allow to boot VMs with only a CDROM. I see no reason
| > > | for it and vmd already allows CDROM-only.
| > > |
| > > | OK?
| > > |
| > > | Via https://twitter.com/wizardishungry/status/1032327323125727232
| > > | "Jon Williams @wizardishungry
| > > | @reykfloeter Could you consider allowing booting ISO-only vms in 6.4?
| > > | This is helpful for running container hosts (e.g. boot2docker)."
| > > |
| > > | Reyk
| > > |
| > > | Index: usr.sbin/vmctl/vmctl.c
| > > | ===
| > > | RCS file: /cvs/src/usr.sbin/vmctl/vmctl.c,v
| > > | retrieving revision 1.54
| > > | diff -u -p -u -p -r1.54 vmctl.c
| > > | --- usr.sbin/vmctl/vmctl.c 12 Jul 2018 12:04:49 - 1.54
| > > | +++ usr.sbin/vmctl/vmctl.c 22 Aug 2018 18:29:44 -
| > > | @@ -98,8 +98,8 @@ vm_start(uint32_t start_id, const char *
| > > | errx(1, "too many disks");
| > > | else if (ndisks == 0)
| > > | warnx("starting without disks");
| > > | - if (kernel == NULL && ndisks == 0)
| > > | - errx(1, "no kernel or disk specified");
| > > | + if (kernel == NULL && ndisks == 0 && !iso)
| > > | + errx(1, "no kernel or disk/cdrom specified");
| > > | if (nnics == -1)
| > > | nnics = 0;
| > > | if (nnics > VMM_MAX_NICS_PER_VM)
| > >
| > > --
| > > Todd T. Fries . http://todd.fries.net/pgp.txt . @unix2mars .
github:toddfries
| > >
| >
| > I know personally of at least 3 people using vmm(4)/vmd(8) with gigantisch
| > ramdisks containing all sorts of goo. None of them use any disks.
|
| but no AFS in sight.
|
vmd(8) doesn't have a fdc(4) either, so no swapping on a floppy!
--
Todd T. Fries . http://todd.fries.net/pgp.txt . @unix2mars . github:toddfries
Re: Allocate PCI BAR for radeondrm(4)
On Thu, Aug 23, 2018 at 10:59:58PM +0200, Mark Kettenis wrote:
> > Date: Tue, 21 Aug 2018 23:34:38 +0200 (CEST)
> > From: Mark Kettenis
> >
> > If firmware doesn't initialize the device, the BAR will be zero and
> > mapping that address leads to "interesting" issues on many machines.
> > This BAR is a bit special since we don't want to immediately map it.
> > So we have to explicitly allocate the address space for the BAR. Bail
> > out if we fail. Don't do this on sparc64 because there the address
> > can actually be zero.
> >
> > ok?
>
> Ping?
>
> > Index: dev/pci/drm/radeon/radeon_kms.c
> > ===
> > RCS file: /cvs/src/sys/dev/pci/drm/radeon/radeon_kms.c,v
> > retrieving revision 1.56
> > diff -u -p -r1.56 radeon_kms.c
> > --- dev/pci/drm/radeon/radeon_kms.c 3 May 2018 10:09:51 - 1.56
> > +++ dev/pci/drm/radeon/radeon_kms.c 21 Aug 2018 21:30:45 -
> > @@ -485,6 +485,26 @@ radeondrm_attach_kms(struct device *pare
> > printf(": can't get frambuffer info\n");
> > return;
> > }
> > +#if !defined(__sparc64__)
> > + if (rdev->fb_aper_offset == 0) {
> > + bus_size_t start, end;
> > + bus_addr_t base;
> > +
> > + start = max(PCI_MEM_START, pa->pa_memex->ex_start);
> > + end = min(PCI_MEM_END, pa->pa_memex->ex_end);
> > + if (pa->pa_memex == NULL ||
> > + extent_alloc_subregion(pa->pa_memex, start, end,
> > + rdev->fb_aper_size, rdev->fb_aper_size, 0, 0, 0, )) {
> > + printf(": can't reserve framebuffer space\n");
> > + return;
> > + }
> > + pci_conf_write(pa->pa_pc, pa->pa_tag, RADEON_PCI_MEM, base);
> > + if (PCI_MAPREG_MEM_TYPE(type) == PCI_MAPREG_MEM_TYPE_64BIT)
> > + pci_conf_write(pa->pa_pc, pa->pa_tag,
> > + RADEON_PCI_MEM + 4, (uint64_t)base >> 32);
> > + rdev->fb_aper_offset = base;
> > + }
> > +#endif
> >
> > for (i = PCI_MAPREG_START; i < PCI_MAPREG_END ; i+= 4) {
> > type = pci_mapreg_type(pa->pa_pc, pa->pa_tag, i);
> >
> >
>
ok mlarkin
Re: Allocate PCI BAR for radeondrm(4)
> Date: Tue, 21 Aug 2018 23:34:38 +0200 (CEST)
> From: Mark Kettenis
>
> If firmware doesn't initialize the device, the BAR will be zero and
> mapping that address leads to "interesting" issues on many machines.
> This BAR is a bit special since we don't want to immediately map it.
> So we have to explicitly allocate the address space for the BAR. Bail
> out if we fail. Don't do this on sparc64 because there the address
> can actually be zero.
>
> ok?
Ping?
> Index: dev/pci/drm/radeon/radeon_kms.c
> ===
> RCS file: /cvs/src/sys/dev/pci/drm/radeon/radeon_kms.c,v
> retrieving revision 1.56
> diff -u -p -r1.56 radeon_kms.c
> --- dev/pci/drm/radeon/radeon_kms.c 3 May 2018 10:09:51 - 1.56
> +++ dev/pci/drm/radeon/radeon_kms.c 21 Aug 2018 21:30:45 -
> @@ -485,6 +485,26 @@ radeondrm_attach_kms(struct device *pare
> printf(": can't get frambuffer info\n");
> return;
> }
> +#if !defined(__sparc64__)
> + if (rdev->fb_aper_offset == 0) {
> + bus_size_t start, end;
> + bus_addr_t base;
> +
> + start = max(PCI_MEM_START, pa->pa_memex->ex_start);
> + end = min(PCI_MEM_END, pa->pa_memex->ex_end);
> + if (pa->pa_memex == NULL ||
> + extent_alloc_subregion(pa->pa_memex, start, end,
> + rdev->fb_aper_size, rdev->fb_aper_size, 0, 0, 0, )) {
> + printf(": can't reserve framebuffer space\n");
> + return;
> + }
> + pci_conf_write(pa->pa_pc, pa->pa_tag, RADEON_PCI_MEM, base);
> + if (PCI_MAPREG_MEM_TYPE(type) == PCI_MAPREG_MEM_TYPE_64BIT)
> + pci_conf_write(pa->pa_pc, pa->pa_tag,
> + RADEON_PCI_MEM + 4, (uint64_t)base >> 32);
> + rdev->fb_aper_offset = base;
> + }
> +#endif
>
> for (i = PCI_MAPREG_START; i < PCI_MAPREG_END ; i+= 4) {
> type = pci_mapreg_type(pa->pa_pc, pa->pa_tag, i);
>
>
Re: vmd/vmctl: allow to boot cdrom-only VMs
Mike Larkin wrote:
> On Thu, Aug 23, 2018 at 11:05:40AM -0500, Todd T. Fries wrote:
> > This makes me wonder. Does it make sense to support booting a kernel without
> > disks? Some people have heard of the phrase 'diskless' ;-)
> >
> > Penned by Reyk Floeter on 20180822 13:35.23, we have:
> > | Hi,
> > |
> > | vmctl doesn't allow to boot VMs with only a CDROM. I see no reason
> > | for it and vmd already allows CDROM-only.
> > |
> > | OK?
> > |
> > | Via https://twitter.com/wizardishungry/status/1032327323125727232
> > | "Jon Williams @wizardishungry
> > | @reykfloeter Could you consider allowing booting ISO-only vms in 6.4?
> > | This is helpful for running container hosts (e.g. boot2docker)."
> > |
> > | Reyk
> > |
> > | Index: usr.sbin/vmctl/vmctl.c
> > | ===
> > | RCS file: /cvs/src/usr.sbin/vmctl/vmctl.c,v
> > | retrieving revision 1.54
> > | diff -u -p -u -p -r1.54 vmctl.c
> > | --- usr.sbin/vmctl/vmctl.c12 Jul 2018 12:04:49 - 1.54
> > | +++ usr.sbin/vmctl/vmctl.c22 Aug 2018 18:29:44 -
> > | @@ -98,8 +98,8 @@ vm_start(uint32_t start_id, const char *
> > | errx(1, "too many disks");
> > | else if (ndisks == 0)
> > | warnx("starting without disks");
> > | - if (kernel == NULL && ndisks == 0)
> > | - errx(1, "no kernel or disk specified");
> > | + if (kernel == NULL && ndisks == 0 && !iso)
> > | + errx(1, "no kernel or disk/cdrom specified");
> > | if (nnics == -1)
> > | nnics = 0;
> > | if (nnics > VMM_MAX_NICS_PER_VM)
> >
> > --
> > Todd T. Fries . http://todd.fries.net/pgp.txt . @unix2mars .
> > github:toddfries
> >
>
> I know personally of at least 3 people using vmm(4)/vmd(8) with gigantisch
> ramdisks containing all sorts of goo. None of them use any disks.
but no AFS in sight.
Disable SMT/Hyperthreading in all Intel BIOSes
Two recently disclosed hardware bugs affected Intel cpus: - TLBleed - T1TF (the name "Foreshadow" refers to 1 of 3 aspects of this bug, more aspects are surely on the way) Solving these bugs requires new cpu microcode, a coding workaround, *AND* the disabling of SMT / Hyperthreading. SMT is fundamentally broken because it shares resources between the two cpu instances and those shared resources lack security differentiators. Some of these side channel attacks aren't trivial, but we can expect most of them to eventually work and leak kernel or cross-VM memory in common usage circumstances, even such as javascript directly in a browser. There will be more hardware bugs and artifacts disclosed. Due to the way SMT interacts with speculative execution on Intel cpus, I expect SMT to exacerbate most of the future problems. A few months back, I urged people to disable hyperthreading on all Intel cpus. I need to repeat that: DISABLE HYPERTHREADING ON ALL YOUR INTEL MACHINES IN THE BIOS. Also, update your BIOS firmware, if you can. OpenBSD -current (and therefore 6.4) will not use hyperthreading if it is enabled, and will update the cpu microcode if possible. But what about 6.2 and 6.3? The situation is very complex, continually evolving, and is taking too much manpower away from other tasks. Furthermore, Intel isn't telling us what is coming next, and are doing a terrible job by not publically documenting what operating systems must do to resolve the problems. We are having to do research by reading other operating systems. There is no time left to backport the changes -- we will not be issuing a complete set of errata and syspatches against 6.2 and 6.3 because it is turning into a distraction. Rather than working on every required patch for 6.2/6.3, we will re-focus manpower and make sure 6.4 contains the best solutions possible. So please try take responsibility for your own machines: Disable SMT in the BIOS menu, and upgrade your BIOS if you can. I'm going to spend my money at a more trustworthy vendor in the future.
Re: vmd/vmctl: allow to boot cdrom-only VMs
On Thu, Aug 23, 2018 at 11:05:40AM -0500, Todd T. Fries wrote:
> This makes me wonder. Does it make sense to support booting a kernel without
> disks? Some people have heard of the phrase 'diskless' ;-)
>
> Penned by Reyk Floeter on 20180822 13:35.23, we have:
> | Hi,
> |
> | vmctl doesn't allow to boot VMs with only a CDROM. I see no reason
> | for it and vmd already allows CDROM-only.
> |
> | OK?
> |
> | Via https://twitter.com/wizardishungry/status/1032327323125727232
> | "Jon Williams @wizardishungry
> | @reykfloeter Could you consider allowing booting ISO-only vms in 6.4?
> | This is helpful for running container hosts (e.g. boot2docker)."
> |
> | Reyk
> |
> | Index: usr.sbin/vmctl/vmctl.c
> | ===
> | RCS file: /cvs/src/usr.sbin/vmctl/vmctl.c,v
> | retrieving revision 1.54
> | diff -u -p -u -p -r1.54 vmctl.c
> | --- usr.sbin/vmctl/vmctl.c 12 Jul 2018 12:04:49 - 1.54
> | +++ usr.sbin/vmctl/vmctl.c 22 Aug 2018 18:29:44 -
> | @@ -98,8 +98,8 @@ vm_start(uint32_t start_id, const char *
> | errx(1, "too many disks");
> | else if (ndisks == 0)
> | warnx("starting without disks");
> | - if (kernel == NULL && ndisks == 0)
> | - errx(1, "no kernel or disk specified");
> | + if (kernel == NULL && ndisks == 0 && !iso)
> | + errx(1, "no kernel or disk/cdrom specified");
> | if (nnics == -1)
> | nnics = 0;
> | if (nnics > VMM_MAX_NICS_PER_VM)
>
> --
> Todd T. Fries . http://todd.fries.net/pgp.txt . @unix2mars . github:toddfries
>
I know personally of at least 3 people using vmm(4)/vmd(8) with gigantisch
ramdisks containing all sorts of goo. None of them use any disks.
-ml
Re: vmd/vmctl: allow to boot cdrom-only VMs
On Thu, Aug 23, 2018 at 11:05:40AM -0500, Todd T. Fries wrote:
> This makes me wonder. Does it make sense to support booting a kernel without
> disks? Some people have heard of the phrase 'diskless' ;-)
We already do:
$ head -n4 /etc/vm.conf
vm "rd" {
disable
memory 128M
boot /bsd.rd
}
$ vmctl start rd -c
That way I can quickly test things in the installer, screw with modified
images, etc.
Re: md5: convert from fgetln(3) to getline(3)
On Tue, Aug 14, 2018 at 03:11:47PM -0500, Scott Cheloha wrote:
> This patch is ok cheloha@ and I can commit if someone else
> is ok, too.
1 week bump. Anyone else ok?
> On Tue, Aug 14, 2018 at 10:47:07PM +0300, Lauri Tirkkonen wrote:
> > [...]
> >
> > diff --git a/bin/md5/md5.c b/bin/md5/md5.c
> > index 84adbcf0989..1abf28cfa16 100644
> > --- a/bin/md5/md5.c
> > +++ b/bin/md5/md5.c
> > @@ -549,11 +549,11 @@ digest_filelist(const char *file, struct
> > hash_function *defhash, int selcount,
> > int found, base64, error, cmp, i;
> > size_t algorithm_max, algorithm_min;
> > const char *algorithm;
> > - char *filename, *checksum, *buf, *p;
> > + char *filename, *checksum, *line, *p, *tmpline;
> > char digest[MAX_DIGEST_LEN + 1];
> > - char *lbuf = NULL;
> > + ssize_t linelen;
> > FILE *listfp, *fp;
> > - size_t len, nread;
> > + size_t len, linesize, nread;
> > int *sel_found = NULL;
> > u_char data[32 * 1024];
> > union ANY_CTX context;
> > @@ -580,20 +580,15 @@ digest_filelist(const char *file, struct
> > hash_function *defhash, int selcount,
> > }
> >
> > error = found = 0;
> > - while ((buf = fgetln(listfp, ))) {
> > + line = NULL;
> > + linesize = 0;
> > + while ((linelen = getline(, , listfp)) != -1) {
> > + tmpline = line;
> > base64 = 0;
> > - if (buf[len - 1] == '\n')
> > - buf[len - 1] = '\0';
> > - else {
> > - if ((lbuf = malloc(len + 1)) == NULL)
> > - err(1, NULL);
> > -
> > - (void)memcpy(lbuf, buf, len);
> > - lbuf[len] = '\0';
> > - buf = lbuf;
> > - }
> > - while (isspace((unsigned char)*buf))
> > - buf++;
> > + if (line[linelen - 1] == '\n')
> > + line[linelen - 1] = '\0';
> > + while (isspace((unsigned char)*tmpline))
> > + tmpline++;
> >
> > /*
> > * Crack the line into an algorithm, filename, and checksum.
> > @@ -603,11 +598,11 @@ digest_filelist(const char *file, struct
> > hash_function *defhash, int selcount,
> > * Fallback on GNU form:
> > * CHECKSUM FILENAME
> > */
> > - p = strchr(buf, ' ');
> > + p = strchr(tmpline, ' ');
> > if (p != NULL && *(p + 1) == '(') {
> > /* BSD form */
> > *p = '\0';
> > - algorithm = buf;
> > + algorithm = tmpline;
> > len = strlen(algorithm);
> > if (len > algorithm_max || len < algorithm_min)
> > continue;
> > @@ -658,7 +653,7 @@ digest_filelist(const char *file, struct hash_function
> > *defhash, int selcount,
> > if ((hf = defhash) == NULL)
> > continue;
> > algorithm = hf->name;
> > - checksum = buf;
> > + checksum = tmpline;
> > if ((p = strchr(checksum, ' ')) == NULL)
> > continue;
> > if (hf->style == STYLE_CKSUM) {
> > @@ -725,11 +720,15 @@ digest_filelist(const char *file, struct
> > hash_function *defhash, int selcount,
> > error = 1;
> > }
> > }
> > + free(line);
> > + if (ferror(listfp)) {
> > + warn("%s: getline", file);
> > + error = 1;
> > + }
> > if (listfp != stdin)
> > fclose(listfp);
> > if (!found)
> > warnx("%s: no properly formatted checksum lines found", file);
> > - free(lbuf);
> > if (sel_found != NULL) {
> > /*
> > * Mark found files by setting them to NULL so that we can
> >
> > --
> > Lauri Tirkkonen | lotheac @ IRCnet
Re: vmd/vmctl: allow to boot cdrom-only VMs
This makes me wonder. Does it make sense to support booting a kernel without
disks? Some people have heard of the phrase 'diskless' ;-)
Penned by Reyk Floeter on 20180822 13:35.23, we have:
| Hi,
|
| vmctl doesn't allow to boot VMs with only a CDROM. I see no reason
| for it and vmd already allows CDROM-only.
|
| OK?
|
| Via https://twitter.com/wizardishungry/status/1032327323125727232
| "Jon Williams @wizardishungry
| @reykfloeter Could you consider allowing booting ISO-only vms in 6.4?
| This is helpful for running container hosts (e.g. boot2docker)."
|
| Reyk
|
| Index: usr.sbin/vmctl/vmctl.c
| ===
| RCS file: /cvs/src/usr.sbin/vmctl/vmctl.c,v
| retrieving revision 1.54
| diff -u -p -u -p -r1.54 vmctl.c
| --- usr.sbin/vmctl/vmctl.c12 Jul 2018 12:04:49 - 1.54
| +++ usr.sbin/vmctl/vmctl.c22 Aug 2018 18:29:44 -
| @@ -98,8 +98,8 @@ vm_start(uint32_t start_id, const char *
| errx(1, "too many disks");
| else if (ndisks == 0)
| warnx("starting without disks");
| - if (kernel == NULL && ndisks == 0)
| - errx(1, "no kernel or disk specified");
| + if (kernel == NULL && ndisks == 0 && !iso)
| + errx(1, "no kernel or disk/cdrom specified");
| if (nnics == -1)
| nnics = 0;
| if (nnics > VMM_MAX_NICS_PER_VM)
--
Todd T. Fries . http://todd.fries.net/pgp.txt . @unix2mars . github:toddfries
Re: drop getpw promise from pwd_mkdb(8)
Perhaps...
I will warn you that the "running YP" case can be really surprising
for "getpw", since it opens up partial "inet" holes. By that I mean
YP support can get subtly broken completely accidentally.
I do intend to circle back one day and change the underlying YP
mechanism (in a similar way to sendsyslog and isatty) such that
swaths of "inet" support isn't exported in this circumstance, it
remains tricky and perhaps needs a new system call...
Ricardo Mestre wrote:
> Hi,
>
> After pledge(2) is called there are no getpw* associated functions that get
> called so we can drop the promise.
>
> Although we manipulate files associated with getpw all the remaining code is
> kept happy with "rpath wpath cpath fattr flock", this has been tested directly
> and also via vipw(8).
>
> OK?
>
> Index: pwd_mkdb.c
> ===
> RCS file: /cvs/src/usr.sbin/pwd_mkdb/pwd_mkdb.c,v
> retrieving revision 1.53
> diff -u -p -u -r1.53 pwd_mkdb.c
> --- pwd_mkdb.c5 Nov 2015 15:10:11 - 1.53
> +++ pwd_mkdb.c23 Aug 2018 09:29:24 -
> @@ -233,7 +233,7 @@ main(int argc, char **argv)
> warn("%s: unable to make group readable", _PATH_SMP_DB);
> clean |= FILE_SECURE;
>
> - if (pledge("stdio rpath wpath cpath getpw fattr flock", NULL) == -1)
> + if (pledge("stdio rpath wpath cpath fattr flock", NULL) == -1)
> err(1, "pledge");
>
> /* Open the temporary insecure password database. */
>
Re: drop getpw promise from pwd_mkdb(8)
On Thu, 23 Aug 2018 10:43:21 +0100, Ricardo Mestre wrote: > After pledge(2) is called there are no getpw* associated functions that > get called so we can drop the promise. > > Although we manipulate files associated with getpw all the remaining > code is kept happy with "rpath wpath cpath fattr flock", this has > been tested directly and also via vipw(8). OK millert@ - todd
drop getpw promise from pwd_mkdb(8)
Hi,
After pledge(2) is called there are no getpw* associated functions that get
called so we can drop the promise.
Although we manipulate files associated with getpw all the remaining code is
kept happy with "rpath wpath cpath fattr flock", this has been tested directly
and also via vipw(8).
OK?
Index: pwd_mkdb.c
===
RCS file: /cvs/src/usr.sbin/pwd_mkdb/pwd_mkdb.c,v
retrieving revision 1.53
diff -u -p -u -r1.53 pwd_mkdb.c
--- pwd_mkdb.c 5 Nov 2015 15:10:11 - 1.53
+++ pwd_mkdb.c 23 Aug 2018 09:29:24 -
@@ -233,7 +233,7 @@ main(int argc, char **argv)
warn("%s: unable to make group readable", _PATH_SMP_DB);
clean |= FILE_SECURE;
- if (pledge("stdio rpath wpath cpath getpw fattr flock", NULL) == -1)
+ if (pledge("stdio rpath wpath cpath fattr flock", NULL) == -1)
err(1, "pledge");
/* Open the temporary insecure password database. */
Re: re-enable -Wshadow for openssl(1)?
> The -Wshadow warning was briefly enabled, but it turned out to break vax > builds with gcc3. Among other things, this warning helps catching stupid > mistakes in refactoring early, so I was wondering whether we could > re-enable it. The commit that disabled it also mentions gcc3, so I'm > unsure whether luna88k would be affected by this. gcc3 will warn like this: In file included from /usr/src/usr.bin/openssl/apps.h:117, from /usr/src/usr.bin/openssl/apps.c:138: /usr/include/openssl/bio.h:342: warning: declaration of `write' shadows a global declaration /usr/include/unistd.h:374: warning: shadowed declaration is here This is caused by the BIO_meth_*() prototypes using formal parameters nawed `read', `puts', etc. Alternatively, you may choose to enable -Wshadow only if building with a non-gcc3 toolchain.
kcov: trace threads
Hi.
Currently kcov is enabled on a per process (pid) basis. A process with
multiple threads therefore share the same coverage buffer which leads to
non-deterministic results. Instead, kcov should be enabled on a per
thread basis; just like how kcov behaves on Linux and FreeBSD. The
decision to trace processes made development easier initially but I'd
like to get this right now.
visa@ gave some helpful pointers since he did similar work related to
witness and the p_sleeplocks member in struct proc. He also pointed out
that conditionally (#ifdef) adding a member to struct proc is a bad idea
since it's part of userland-visible ABI.
Another pleasant side-effect of this change is the removed linear search
for the corresponding kcov descriptor inside __sanitizer_cov_trace_pc().
Survived a make release on amd64.
Comments? OK?
Index: dev/kcov.c
===
RCS file: /cvs/src/sys/dev/kcov.c,v
retrieving revision 1.2
diff -u -p -r1.2 kcov.c
--- dev/kcov.c 21 Aug 2018 18:06:12 - 1.2
+++ dev/kcov.c 23 Aug 2018 06:05:58 -
@@ -41,7 +41,6 @@ struct kd {
KCOV_MODE_TRACE_PC,
}kd_mode;
int kd_unit; /* device minor */
- pid_tkd_pid;/* process being traced */
uintptr_t *kd_buf;/* traced coverage */
size_t kd_nmemb;
size_t kd_size;
@@ -54,7 +53,6 @@ void kcovattach(int);
int kd_alloc(struct kd *, unsigned long);
struct kd *kd_lookup(int);
-static inline struct kd *kd_lookup_pid(pid_t);
static inline int inintr(void);
TAILQ_HEAD(, kd) kd_list = TAILQ_HEAD_INITIALIZER(kd_list);
@@ -69,8 +67,8 @@ int kcov_debug = 1;
* each block instructions that maps to a single line in the original source
* code.
*
- * If kcov is enabled for the current process, the executed address will be
- * stored in the corresponding coverage buffer.
+ * If kcov is enabled for the current thread, the kernel program counter will
+ * be stored in its corresponding coverage buffer.
* The first element in the coverage buffer holds the index of next available
* element.
*/
@@ -89,8 +87,8 @@ __sanitizer_cov_trace_pc(void)
if (inintr())
return;
- kd = kd_lookup_pid(curproc->p_p->ps_pid);
- if (kd == NULL)
+ kd = curproc->p_kd;
+ if (kd == NULL || kd->kd_mode != KCOV_MODE_TRACE_PC)
return;
idx = kd->kd_buf[0];
@@ -132,6 +130,9 @@ kcovclose(dev_t dev, int flag, int mode,
DPRINTF("%s: unit=%d\n", __func__, minor(dev));
+ if (kd == p->p_kd)
+ p->p_kd = NULL;
+
TAILQ_REMOVE(_list, kd, kd_entry);
free(kd->kd_buf, M_SUBPROC, kd->kd_size);
free(kd, M_SUBPROC, sizeof(struct kd));
@@ -159,30 +160,30 @@ kcovioctl(dev_t dev, u_long cmd, caddr_t
kd->kd_mode = KCOV_MODE_INIT;
break;
case KIOENABLE:
- if (kd->kd_mode != KCOV_MODE_INIT) {
+ /* Only one kcov descriptor can be enabled per thread. */
+ if (p->p_kd != NULL || kd->kd_mode != KCOV_MODE_INIT) {
error = EBUSY;
break;
}
kd->kd_mode = KCOV_MODE_TRACE_PC;
- kd->kd_pid = p->p_p->ps_pid;
+ p->p_kd = kd;
break;
case KIODISABLE:
- /* Only the enabled process may disable itself. */
- if (kd->kd_pid != p->p_p->ps_pid ||
- kd->kd_mode != KCOV_MODE_TRACE_PC) {
+ /* Only the enabled thread may disable itself. */
+ if (p->p_kd != kd || kd->kd_mode != KCOV_MODE_TRACE_PC) {
error = EBUSY;
break;
}
kd->kd_mode = KCOV_MODE_INIT;
- kd->kd_pid = 0;
+ p->p_kd = NULL;
break;
default:
error = EINVAL;
DPRINTF("%s: %lu: unknown command\n", __func__, cmd);
}
- DPRINTF("%s: unit=%d, mode=%d, pid=%d, error=%d\n",
- __func__, kd->kd_unit, kd->kd_mode, kd->kd_pid, error);
+ DPRINTF("%s: unit=%d, mode=%d, error=%d\n",
+ __func__, kd->kd_unit, kd->kd_mode, error);
return (error);
}
@@ -212,12 +213,14 @@ kcov_exit(struct proc *p)
{
struct kd *kd;
- kd = kd_lookup_pid(p->p_p->ps_pid);
+ kd = p->p_kd;
if (kd == NULL)
return;
+ DPRINTF("%s: unit=%d\n", __func__, kd->kd_unit);
+
kd->kd_mode = KCOV_MODE_INIT;
- kd->kd_pid = 0;
+ p->p_kd = NULL;
}
struct kd *
@@ -248,18 +251,6 @@ kd_alloc(struct kd *kd, unsigned long nm
kd->kd_nmemb = nmemb - 1;
kd->kd_size = size;
return (0);
-}
-
-static inline struct kd *
-kd_lookup_pid(pid_t pid)
-{
- struct kd *kd;
-
-
