vmm: Call for testing: Expose TSC to guest

[Pratik Vyas]

Hello tech@,

The following diff should expose TSC to guest vm and OpenBSD guests
should be able to choose tsc as a preferred timecounter if the host
machine is >= skylake. 


This should improve the guest clock drift situation significantly.

I am aware that this breaks received vms and am working on handling
that.

Thanks,
Pratik

Index: sys/arch/amd64/amd64/vmm.c
===
RCS file: /home/pdvyas/cvs/src/sys/arch/amd64/amd64/vmm.c,v
retrieving revision 1.157
diff -u -p -a -u -r1.157 vmm.c
--- sys/arch/amd64/amd64/vmm.c  2 Jul 2017 19:49:31 -   1.157
+++ sys/arch/amd64/amd64/vmm.c  23 Jul 2017 03:49:30 -
@@ -5200,7 +5200,7 @@ vmm_handle_cpuid(struct vcpu *vcpu)

switch (*rax) {
case 0x00:  /* Max level and vendor ID */
-   *rax = 0x0d; /* cpuid_level */
+   *rax = cpuid_level;
*rbx = *((uint32_t *)_vendor);
*rdx = *((uint32_t *)_vendor + 1);
*rcx = *((uint32_t *)_vendor + 2);
@@ -5226,7 +5226,6 @@ vmm_handle_cpuid(struct vcpu *vcpu)
 *  direct cache access (CPUIDECX_DCA)
 *  x2APIC (CPUIDECX_X2APIC)
 *  apic deadline (CPUIDECX_DEADLINE)
-*  timestamp (CPUID_TSC)
 *  apic (CPUID_APIC)
 *  psn (CPUID_PSN)
 *  self snoop (CPUID_SS)
@@ -5243,10 +5242,9 @@ vmm_handle_cpuid(struct vcpu *vcpu)
CPUIDECX_SDBG | CPUIDECX_XTPR | CPUIDECX_PCID |
CPUIDECX_DCA | CPUIDECX_X2APIC | CPUIDECX_DEADLINE);
*rdx = curcpu()->ci_feature_flags &
-   ~(CPUID_ACPI | CPUID_TM | CPUID_TSC |
- CPUID_HTT | CPUID_DS | CPUID_APIC |
- CPUID_PSN | CPUID_SS | CPUID_PBE |
- CPUID_MTRR);
+   ~(CPUID_ACPI | CPUID_TM | CPUID_HTT |
+ CPUID_DS | CPUID_APIC | CPUID_PSN |
+ CPUID_SS | CPUID_PBE | CPUID_MTRR);
break;
case 0x02:  /* Cache and TLB information */
*rax = eax;
@@ -5399,13 +5397,8 @@ vmm_handle_cpuid(struct vcpu *vcpu)
*rcx = 0;
*rdx = 0;
break;
-   case 0x15:  /* TSC / Core Crystal Clock info (not supported) */
-   DPRINTF("%s: function 0x15 (TSC / CCC info) not supported\n",
-   __func__);
-   *rax = 0;
-   *rbx = 0;
-   *rcx = 0;
-   *rdx = 0;
+   case 0x15:
+   CPUID(0x15, *rax, *rbx, *rcx, *rdx);
break;
case 0x16:  /* Processor frequency info (not supported) */
DPRINTF("%s: function 0x16 (frequency info) not supported\n",
@@ -5432,7 +5425,6 @@ vmm_handle_cpuid(struct vcpu *vcpu)
*rbx = 0;   /* Reserved */
*rcx = curcpu()->ci_efeature_ecx;
*rdx = curcpu()->ci_feature_eflags;
-   *rdx &= ~CPUID_RDTSCP;
break;
case 0x8002:/* Brand string */
*rax = curcpu()->ci_brand[0];
@@ -5465,10 +5457,7 @@ vmm_handle_cpuid(struct vcpu *vcpu)
*rdx = curcpu()->ci_extcacheinfo[3];
break;
case 0x8007:/* apmi */
-   *rax = 0;   /* Reserved */
-   *rbx = 0;   /* Reserved */
-   *rcx = 0;   /* Reserved */
-   *rdx = 0;   /* unsupported ITSC */
+   CPUID(0x8007, *rax, *rbx, *rcx, *rdx);
break;
case 0x8008:/* Phys bits info and topology (AMD) */
DPRINTF("%s: function 0x8008 (phys bits info) not "

Re: getdelim(3): perror -> err in example

[Jeremie Courreges-Anglas]

On Sun, Jul 23 2017, Ingo Schwarze  wrote:
> Hi Jeremie,
>
> Jeremie Courreges-Anglas wrote on Sun, Jul 23, 2017 at 12:43:14AM +0200:
>
>> getline(3) and perror(3) are in posix, but fgetln(3) and err(3) come
>> from 4.4BSD (according to their manpage).  We should probably keep the
>> example code posix-compliant.
>
> Paul Janzen asked me the same question in private mail,
> and i answered as follows:

[snip]

>  (6) We already use err(3) in getopt(3), malloc(3), strdup(3),
>  wcsdup(3), which are all POSIX.

Well I guess my objection doesn't stand, then. :)

-- 
jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF  DDCC 0DFA 74AE 1524 E7EE

Re: getdelim(3): perror -> err in example

[Ingo Schwarze]

Hi Jeremie,

Jeremie Courreges-Anglas wrote on Sun, Jul 23, 2017 at 12:43:14AM +0200:

> getline(3) and perror(3) are in posix, but fgetln(3) and err(3) come
> from 4.4BSD (according to their manpage).  We should probably keep the
> example code posix-compliant.

Paul Janzen asked me the same question in private mail,
and i answered as follows:

Here is my six cents:

 (1) We should promote  functions even for portable
 applications because they are better than the alternatives
 and providing fallback implementations in a portable
 application is trivial (see mandoc).

 (2) EXAMPLES should promote best practices, not portability purism,
 even in pages for standardized functions.  We write OpenBSD
 manuals, not POSIX manuals.  (Of course, that doesn't imply
 we should be imprecise about which part of the functionality
 of the function *itself* that is documented in the page is
 standard and which parts are extensions.)

 (3) The focus of this part of the example is how to check for
 errors, not what to do in that case.
 if (ferror(fp)) err(...) is quite comprehensible generically
 even if you don't know what exactly err(...) is.

 (4) If you want to know details about err(...), refer to err(3).

 (5) EXAMPLES are code snippets, almost always incomplete.
 An additional line "#include " would seem more
 like a distraction than like a helpful addition.

 (6) We already use err(3) in getopt(3), malloc(3), strdup(3),
 wcsdup(3), which are all POSIX.

Yours,
  Ingo


>> Index: getdelim.3
>> ===
>> RCS file: /cvs/src/lib/libc/stdio/getdelim.3,v
>> retrieving revision 1.4
>> diff -u -p -r1.4 getdelim.3
>> --- getdelim.3   4 Apr 2016 19:23:52 -   1.4
>> +++ getdelim.3   22 Jul 2017 08:51:16 -
>> @@ -124,7 +124,7 @@ while ((linelen = getline(\*[Am]line, \*
>>  
>>  free(line);
>>  if (ferror(fp))
>> -perror("getline");
>> +err(1, "getline");
>>  .Ed
>>  .Sh ERRORS
>>  .Bl -tag -width [EOVERFLOW]

Re: simple ifstated pledge

[Jeremie Courreges-Anglas]

On Sat, Jul 22 2017, Rob Pierce  wrote:
> On Sun, Jul 23, 2017 at 12:26:53AM +0200, Jeremie Courreges-Anglas wrote:
>> On Sat, Jul 22 2017, Rob Pierce  wrote:
>> > With the most recent commit ifstated can now be pledged in a straight 
>> > forward
>> > manner. A better pledge is possible with more work.
>> >
>> > Does it make sense to get this one in now?
>> 
>> Regress tests pass.  I think this is the way to go.  ok jca@
>
> I just realized that we can do a stricter pledge with route instead of inet.

This sounds looks even better.

> Rob
>
> Index: ifstated.c
> ===
> RCS file: /cvs/src/usr.sbin/ifstated/ifstated.c,v
> retrieving revision 1.53
> diff -u -p -r1.53 ifstated.c
> --- ifstated.c22 Jul 2017 19:52:01 -  1.53
> +++ ifstated.c22 Jul 2017 23:36:15 -
> @@ -159,6 +159,9 @@ main(int argc, char *argv[])
>   , sizeof(rtfilter)) == -1) /* not fatal */
>   log_warn("%s: setsockopt tablefilter", __func__);
>  
> + if (pledge("stdio rpath route proc exec", NULL) == -1)
> + fatal("pledge");
> +
>   signal_set(_ev, SIGCHLD, sigchld_handler, NULL);
>   signal_add(_ev, NULL);
>  
>

-- 
jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF  DDCC 0DFA 74AE 1524 E7EE

Re: getdelim(3): perror -> err in example

[Jeremie Courreges-Anglas]

On Sat, Jul 22 2017, Anton Lindqvist  wrote:
> Hi,
> This changes the behavior of the example but makes it consistent with
> the example in fgetln(3) and many other manuals.
>
> Comments? OK?

getline(3) and perror(3) are in posix, but fgetln(3) and err(3) come
from 4.4BSD (according to their manpage).  We should probably keep the
example code posix-compliant.

> Index: getdelim.3
> ===
> RCS file: /cvs/src/lib/libc/stdio/getdelim.3,v
> retrieving revision 1.4
> diff -u -p -r1.4 getdelim.3
> --- getdelim.34 Apr 2016 19:23:52 -   1.4
> +++ getdelim.322 Jul 2017 08:51:16 -
> @@ -124,7 +124,7 @@ while ((linelen = getline(\*[Am]line, \*
>  
>  free(line);
>  if (ferror(fp))
> - perror("getline");
> + err(1, "getline");
>  .Ed
>  .Sh ERRORS
>  .Bl -tag -width [EOVERFLOW]
>

-- 
jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF  DDCC 0DFA 74AE 1524 E7EE

Re: simple ifstated pledge

[Jeremie Courreges-Anglas]

On Sat, Jul 22 2017, Rob Pierce  wrote:
> With the most recent commit ifstated can now be pledged in a straight forward
> manner. A better pledge is possible with more work.
>
> Does it make sense to get this one in now?

Regress tests pass.  I think this is the way to go.  ok jca@

> Rob
>
> Index: ifstated.c
> ===
> RCS file: /cvs/src/usr.sbin/ifstated/ifstated.c,v
> retrieving revision 1.52
> diff -u -p -r1.52 ifstated.c
> --- ifstated.c21 Jul 2017 16:32:18 -  1.52
> +++ ifstated.c22 Jul 2017 03:58:23 -
> @@ -160,6 +160,9 @@ main(int argc, char *argv[])
>   , sizeof(rtfilter)) == -1) /* not fatal */
>   log_warn("%s: setsockopt tablefilter", __func__);
>  
> + if (pledge("stdio rpath inet proc exec", NULL) == -1)
> + fatal("pledge");
> +
>   signal_set(_ev, SIGCHLD, sigchld_handler, NULL);
>   signal_add(_ev, NULL);
>  
>

-- 
jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF  DDCC 0DFA 74AE 1524 E7EE

pflogd: cope with interface departure

[Jeremie Courreges-Anglas]

If you destroy the interface pflogd(8) listens on, you get killed
because socket(2) is denied by the current pledge(2) restrictions:

  pflogd(15868): syscall 97 "inet"

The ioctl(SIOCGIFDATA) call would be fatal too.

The diff below just uses if_nametoindex(3), which is always allowed.
The if_exists() function is then so simple that it could be deleted.

# ./obj/pflogd -s 160 -D -i pflog1
[priv]: msg PRIV_OPEN_LOG received
interface pflog1 went away
Exiting

Opinions / ok?


Index: pflogd.c
===
RCS file: /d/cvs/src/sbin/pflogd/pflogd.c,v
retrieving revision 1.53
diff -u -p -p -u -r1.53 pflogd.c
--- pflogd.c16 Jan 2016 03:17:48 -  1.53
+++ pflogd.c22 Jul 2017 19:28:21 -
@@ -194,23 +194,7 @@ set_pcap_filter(void)
 int
 if_exists(char *ifname)
 {
-   int s, ret = 1;
-   struct ifreq ifr;
-   struct if_data ifrdat;
-
-   if ((s = socket(AF_INET, SOCK_DGRAM, 0)) == -1)
-   err(1, "socket");
-   bzero(, sizeof(ifr));
-   if (strlcpy(ifr.ifr_name, ifname, sizeof(ifr.ifr_name)) >=
-   sizeof(ifr.ifr_name))
-   errx(1, "main ifr_name: strlcpy");
-   ifr.ifr_data = (caddr_t)
-   if (ioctl(s, SIOCGIFDATA, (caddr_t)) == -1)
-   ret = 0;
-   if (close(s))
-   err(1, "close");
-
-   return (ret);
+   return (if_nametoindex(ifname) != 0);
 }
 
 int

-- 
jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF  DDCC 0DFA 74AE 1524 E7EE

newsyslog: simplify mail sending

[Jeremie Courreges-Anglas]

I have nothing against asprintf(3), but the openmail function looks
needlessly complicated.

ok?


Index: newsyslog.c
===
RCS file: /d/cvs/src/usr.bin/newsyslog/newsyslog.c,v
retrieving revision 1.107
diff -u -p -p -u -r1.107 newsyslog.c
--- newsyslog.c 22 Jul 2017 17:06:40 -  1.107
+++ newsyslog.c 22 Jul 2017 19:21:23 -
@@ -147,7 +147,6 @@ charhostname[HOST_NAME_MAX+1]; /* Hostn
 char   daytime[33];/* timenow in human readable form */
 char   *arcdir;/* Dir to put archives in (if it exists) */
 
-FILE   *openmail(void);
 char   *lstat_log(char *, size_t, int);
 char   *missing_field(char *, char *, int);
 char   *sob(char *);
@@ -1072,13 +1071,12 @@ domonitor(struct conf_entry *ent)
fclose(fp);
goto cleanup;
}
-
-   /* Send message. */
fclose(fp);
 
-   fp = openmail();
+   /* Send message. */
+   fp = popen(SENDMAIL " -t", "w");
if (fp == NULL) {
-   warn("openmail");
+   warn("popen");
goto cleanup;
}
fprintf(fp, "Auto-Submitted: auto-generated\n");
@@ -1103,20 +1101,6 @@ cleanup:
free(flog);
free(rb);
return (1);
-}
-
-FILE *
-openmail(void)
-{
-   char *cmdbuf = NULL;
-   FILE *ret;
-
-   if (asprintf(, "%s -t", SENDMAIL) != -1) {
-   ret = popen(cmdbuf, "w");
-   free(cmdbuf);
-   return (ret);
-   }
-   return (NULL);
 }
 
 /* ARGSUSED */

-- 
jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF  DDCC 0DFA 74AE 1524 E7EE

Re: acpibat: add _BIX support

[joshua stein]

On Sat, 22 Jul 2017 at 18:07:56 +0200, Mark Kettenis wrote:
> > Date: Fri, 21 Jul 2017 23:31:28 -0500
> > From: joshua stein 
> > 
> > ACPI 4.0 deprecated _BIF for battery status, so some newer machines
> > have _BIX instead which provides the same info plus some extra
> > fields.
> > 
> > I used some macro magic to make the diff less painful, and added a
> > sensor for the new cycle count exported by _BIX which can be useful
> > to see.
> 
> Why not just switch to acpibat_bix and have acpibat_getbif() populate
> that struct?  That would make the diff much simpler.
> 
> I had some code that did it the other way around (have
> acpibat_getbix() populate the fields in struct acpibat_bif).  That
> would be acceptable too and result in an even simplet diff, but you'd
> lost the extra sensor that you're adding.  Don't know where I left
> that diff though.  It didn't work because the battery was behind smbus
> or something.


Index: dev/acpi/acpi.c
===
RCS file: /cvs/src/sys/dev/acpi/acpi.c,v
retrieving revision 1.329
diff -u -p -u -p -r1.329 acpi.c
--- dev/acpi/acpi.c 20 Jul 2017 18:34:24 -  1.329
+++ dev/acpi/acpi.c 22 Jul 2017 18:01:34 -
@@ -3096,12 +3096,12 @@ acpiioctl(dev_t dev, u_long cmd, caddr_t
if (bat->aba_softc->sc_bat_present == 0)
continue;
 
-   if (bat->aba_softc->sc_bif.bif_last_capacity == 0)
+   if (bat->aba_softc->sc_bix.bix_last_capacity == 0)
continue;
 
bats++;
rem = (bat->aba_softc->sc_bst.bst_capacity * 100) /
-   bat->aba_softc->sc_bif.bif_last_capacity;
+   bat->aba_softc->sc_bix.bix_last_capacity;
if (rem > 100)
rem = 100;
remaining += rem;
Index: dev/acpi/acpibat.c
===
RCS file: /cvs/src/sys/dev/acpi/acpibat.c,v
retrieving revision 1.63
diff -u -p -u -p -r1.63 acpibat.c
--- dev/acpi/acpibat.c  12 Mar 2017 21:30:44 -  1.63
+++ dev/acpi/acpibat.c  22 Jul 2017 18:01:34 -
@@ -44,7 +44,7 @@ const char *acpibat_hids[] = { ACPI_DEV_
 
 void   acpibat_monitor(struct acpibat_softc *);
 void   acpibat_refresh(void *);
-intacpibat_getbif(struct acpibat_softc *);
+intacpibat_getbix(struct acpibat_softc *);
 intacpibat_getbst(struct acpibat_softc *);
 intacpibat_notify(struct aml_node *, int, void *);
 
@@ -78,18 +78,22 @@ acpibat_attach(struct device *parent, st
 
if ((sta & STA_BATTERY) != 0) {
sc->sc_bat_present = 1;
-   acpibat_getbif(sc);
+   acpibat_getbix(sc);
+
+   printf(": %s", sc->sc_devnode->name);
+
acpibat_getbst(sc);
 
printf(": %s", sc->sc_devnode->name);
-   if (sc->sc_bif.bif_model[0])
-   printf(" model \"%s\"", sc->sc_bif.bif_model);
-   if (sc->sc_bif.bif_serial[0])
-   printf(" serial %s", sc->sc_bif.bif_serial);
-   if (sc->sc_bif.bif_type[0])
-   printf(" type %s", sc->sc_bif.bif_type);
-   if (sc->sc_bif.bif_oem[0])
-   printf(" oem \"%s\"", sc->sc_bif.bif_oem);
+   if (sc->sc_bix.bix_model[0])
+   printf(" model \"%s\"", sc->sc_bix.bix_model);
+   if (sc->sc_bix.bix_serial[0])
+   printf(" serial %s", sc->sc_bix.bix_serial);
+   if (sc->sc_bix.bix_type[0])
+   printf(" type %s", sc->sc_bix.bix_type);
+   if (sc->sc_bix.bix_oem[0])
+   printf(" oem \"%s\"", sc->sc_bix.bix_oem);
+
printf("\n");
} else {
sc->sc_bat_present = 0;
@@ -111,34 +115,34 @@ acpibat_monitor(struct acpibat_softc *sc
 {
int type;
 
-   /* assume _BIF and _BST have been called */
+   /* assume _BIF/_BIX and _BST have been called */
strlcpy(sc->sc_sensdev.xname, DEVNAME(sc),
sizeof(sc->sc_sensdev.xname));
 
-   type = sc->sc_bif.bif_power_unit ? SENSOR_AMPHOUR : SENSOR_WATTHOUR;
+   type = sc->sc_bix.bix_power_unit ? SENSOR_AMPHOUR : SENSOR_WATTHOUR;
 
strlcpy(sc->sc_sens[0].desc, "last full capacity",
sizeof(sc->sc_sens[0].desc));
sc->sc_sens[0].type = type;
sensor_attach(>sc_sensdev, >sc_sens[0]);
-   sc->sc_sens[0].value = sc->sc_bif.bif_last_capacity * 1000;
+   sc->sc_sens[0].value = sc->sc_bix.bix_last_capacity * 1000;
 
strlcpy(sc->sc_sens[1].desc, "warning capacity",
sizeof(sc->sc_sens[1].desc));
sc->sc_sens[1].type = type;
sensor_attach(>sc_sensdev, >sc_sens[1]);
-   

Re: time(1): perror(3) -> err(3) and friends

[Ingo Schwarze]

Hi,

Scott Cheloha wrote on Thu, Jul 13, 2017 at 07:30:26PM -0500:

> We currently use a mix of perror(3) and err(3).
> 
> In one case you can merge perror + exit into err, which is nice.
> 
> The warns, though, are not equivalent (you get a "time: " prefix),
> so maybe this is too risky.
> 
> Putting it out here anyway.

We can do even better.

 * In main(), prefer "return" over exit(3).

 * This is ugly and confusing:

  $ sh -c 'ulimit -p 29; /usr/bin/time ls' 
 time: Resource temporarily unavailable

   This is easier to understand:

  $ sh -c 'ulimit -p 29; /usr/bin/time ls' 
 time: fork: Resource temporarily unavailable

 * In complex situations, this can be confusing:

  $ /usr/bin/time foo/bar
 foo/bar: No such file or directory

   This is easier to understand:

  $ /usr/bin/time foo/bar 
 time: foo/bar: No such file or directory

 * This is plain nonsense:

  $ ( /usr/bin/time sleep 100 ; echo $? ) & pkill -KILL sleep 
 Command terminated abnormally.
0.00 real 0.00 user 0.00 sys
 signal: Invalid argument
 1

   signal(3) can only fail for SIGKILL or SIGSTOP (or for an invalid
   argument, which cannot happen here).  Printing "Invalid argument"
   is completely pointless, and confusing.

   However, we do want an EXIT STATUS of 128 + 9 in this case:

 $ ( /usr/bin/time sleep 100 ; echo $? ) & pkill -KILL sleep 
Command terminated abnormally.
0.00 real 0.00 user 0.00 sys
137

To wit, look at the builtin:

 $ ( time sleep 100 ; echo $? ) & pkill -KILL sleep 
Killed 
0m00.00s real 0m00.00s user 0m00.00s system
137

OK?
  Ingo

   
Index: time.c
===
RCS file: /cvs/src/usr.bin/time/time.c,v
retrieving revision 1.24
diff -u -p -r1.24 time.c
--- time.c  22 Jul 2017 17:01:09 -  1.24
+++ time.c  22 Jul 2017 17:43:57 -
@@ -67,7 +67,6 @@ main(int argc, char *argv[])
break;
default:
usage();
-   /* NOTREACHED */
}
}
 
@@ -80,14 +79,12 @@ main(int argc, char *argv[])
clock_gettime(CLOCK_MONOTONIC, );
switch(pid = vfork()) {
case -1:/* error */
-   perror("time");
-   exit(1);
-   /* NOTREACHED */
+   warn("fork");
+   return 1;
case 0: /* child */
execvp(*argv, argv);
-   perror(*argv);
+   warn("%s", *argv);
_exit((errno == ENOENT) ? 127 : 126);
-   /* NOTREACHED */
}
 
/* parent */
@@ -168,11 +165,11 @@ main(int argc, char *argv[])
 
if (exitonsig) {
if (signal(exitonsig, SIG_DFL) == SIG_ERR)
-   perror("signal");
+   return 128 + exitonsig;
else
kill(getpid(), exitonsig);
}
-   exit(WIFEXITED(status) ? WEXITSTATUS(status) : EXIT_FAILURE);
+   return WIFEXITED(status) ? WEXITSTATUS(status) : EXIT_FAILURE;
 }
 
 __dead void

Re: time(1): make global flags local

[Ingo Schwarze]

Hi,

Scott Cheloha wrote on Thu, Jul 13, 2017 at 07:14:08PM -0500:

> The flags don't need to be global, and there are more obvious
> ways to zero a variable.
> 
> While here, order the stack structures and variables by size.

Committed with tweaks.
  Ingo


> Index: usr.bin/time/time.c
> ===
> RCS file: /cvs/src/usr.bin/time/time.c,v
> retrieving revision 1.22
> diff -u -p -r1.22 time.c
> --- usr.bin/time/time.c   13 Jul 2017 06:39:54 -  1.22
> +++ usr.bin/time/time.c   14 Jul 2017 00:10:09 -
> @@ -42,19 +42,17 @@
>  #include 
>  #include 
>  
> -int lflag;
> -int portableflag;
> -
>  __dead void usage(void);
>  
>  int
>  main(int argc, char *argv[])
>  {
> - pid_t pid;
> - int ch, status;
> - struct timespec before, after, during;
>   struct rusage ru;
> - int exitonsig = 0;
> + struct timespec before, after, during;
> + pid_t pid;
> + int ch, exitonsig, lflag, portableflag, status;
> +
> + exitonsig = lflag = portableflag = 0;
>  
>   if (pledge("stdio proc exec", NULL) == -1)
>   err(1, "pledge");

Re: style.9: discourage (void)ing unused return values

[Ingo Schwarze]

Hi Scott,

Scott Cheloha wrote on Sat, Jul 22, 2017 at 11:27:17AM -0500:

> Okay, going back and rereading, it appears he meant specifically to not
> (void) stuff like printf or fprintf, e.g.
> 
>   (void)fprintf(stderr, "usage: prog [args]\n");  /* don't do this */

Even on printf(3), (void) may occasionally make sense, for example
when using %ls or %lc with a user-supplied string, in which case
you normally have to check that the return value and be wary of
EILSEQ, which almost everybody always forgets.

But not in the case you quote above, of course.

> My (maybe unfounded) concern is that the (void) cast doesn't
> necessarily signal the same thing to everyone.

There are more than two levels of obviousness.

When it's obvious that ignoring the return value makes sense,
don't put anything.

When an auditor might think that ignoring the return value might
possibly be a bug and is in danger of wasting time to investigate,
and you *have* audited the call and are sure it is OK, and it is
obvious what type of potential bug is involved, just put (void).

If it is not even obvious what the call needs to be audited for
and a mere (void) would be ambiguous, write an explicit comment
in addition to the (void) cast.  That is rarely needed, though.

> The preponderance of (void) casts on stuff like memcpy
> and fprintf seems to confirm this.

I guess most (void)close() out there is just cargo cult
or compiler bootlicking.

Yours,
  Ingo

Re: style.9: discourage (void)ing unused return values

[Scott Cheloha]

> Date: Sat, 22 Jul 2017 15:29:17 +0200
> From: Ingo Schwarze 
> 
> Hi Scott,

> 
> Scott Cheloha wrote on Fri, Jul 21, 2017 at 05:03:11PM -0500:
> 
>> Per encouragement from deraadt@,
> 
> Not sure what exactly he said, but i'm quite sure you misunderstood him.

Okay, going back and rereading, it appears he meant specifically to not
(void) stuff like printf or fprintf, e.g.

(void)fprintf(stderr, "usage: prog [args]\n");  /* don't do this */

> [...]
> 
> Added to functions like strlcpy(3) where ignoring the return value
> is often a serious bug.  In such a case, (void) is not intended for
> some compiler, but for human consumption.  Its meaning is: This
> call has been carefully audited.  Contrary to the usual situation,
> we can safely ignore the return value here, either because the
> buffer is so large that it can never become full at this point, or
> because truncation is not a problem at this point.
> 
> This cannot be formalized.
> 
> [...]


My (maybe unfounded) concern is that the (void) cast doesn't necessarily
signal the same thing to everyone.  The preponderance of (void) casts
on stuff like memcpy and fprintf seems to confirm this.

But given how context-dependent the casts are, I suppose that trying to
formalize when and when not to employ them would probably be futile.

Re: time(1): kill some lint-era voids, switch to getprogname(3)

[Ingo Schwarze]

Hi Scott,

Scott Cheloha wrote on Thu, Jul 13, 2017 at 07:37:46PM -0500:

> The (void) casts are going out of style.  While here,
> switch from __progname to getprogname(3)

Committed.
  Ingo


> Index: usr.bin/time/time.c
> ===
> RCS file: /cvs/src/usr.bin/time/time.c,v
> retrieving revision 1.22
> diff -u -p -r1.22 time.c
> --- usr.bin/time/time.c   13 Jul 2017 06:39:54 -  1.22
> +++ usr.bin/time/time.c   14 Jul 2017 00:34:33 -
> @@ -93,8 +93,8 @@ main(int argc, char *argv[])
>   }
>  
>   /* parent */
> - (void)signal(SIGINT, SIG_IGN);
> - (void)signal(SIGQUIT, SIG_IGN);
> + signal(SIGINT, SIG_IGN);
> + signal(SIGQUIT, SIG_IGN);
>   while (wait3(, 0, ) != pid)
>   ;
>   clock_gettime(CLOCK_MONOTONIC, );
> @@ -180,9 +180,7 @@ main(int argc, char *argv[])
>  __dead void
>  usage(void)
>  {
> - extern char *__progname;
> -
> - (void)fprintf(stderr, "usage: %s [-lp] utility [argument ...]\n",
> - __progname);
> + fprintf(stderr, "usage: %s [-lp] utility [argument ...]\n",
> + getprogname());
>   exit(1);
>  }

Tolerate imprecise fault address in siginfo-fault regress

[Mark Kettenis]

This makes the tests pass on sparc64 where the hardware doesn't give
us the low bits of the faulting address.

ok?


Index: regress/sys/kern/siginfo-fault/siginfo-fault.c
===
RCS file: /cvs/src/regress/sys/kern/siginfo-fault/siginfo-fault.c,v
retrieving revision 1.5
diff -u -p -r1.5 siginfo-fault.c
--- regress/sys/kern/siginfo-fault/siginfo-fault.c  20 Jul 2017 18:22:25 
-  1.5
+++ regress/sys/kern/siginfo-fault/siginfo-fault.c  22 Jul 2017 15:25:56 
-
@@ -24,10 +24,20 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
 
+/*
+ * Some architectures deliver an imprecise fault address.
+ */
+#ifdef __sparc64__
+#define EXPADDR_MASK   ~(3UL)
+#else
+#define EXPADDR_MASK   ~(0UL)
+#endif
+
 #define CHECK_EQ(a, b) assert((a) == (b))
 #define CHECK_NE(a, b) assert((a) != (b))
 #define CHECK_LE(a, b) assert((a) <= (b))
@@ -76,6 +86,9 @@ checksig(const char *name, int expsigno,
 {
int fail = 0;
char str1[NL_TEXTMAX], str2[NL_TEXTMAX];
+
+   expaddr = (char *)((uintptr_t)expaddr & EXPADDR_MASK);
+
if (expsigno != gotsigno) {
strlcpy(str1, strsignal(expsigno), sizeof(str1));
strlcpy(str2, strsignal(gotsigno), sizeof(str2));

Re: llvm - xor return pointers

[Theo de Raadt]

> Cool stuff.  The downside is that this probably kills doing
> backtraces, making debugging stuff hard.  Unless this also changes the
> DWARF debugging information to reflect the xor operation.  But I'm not
> sure that's possible.

I think it is possible, but some help is probably needed in gdb.

> Having a "constant" (per process) cookie would make things easier for
> debuggers, but also weaken the mechanism.  It might be possible to
> have the kernel initialize such a cookie in the TIB (thread
> information block) that is used for fast access to thread-local
> variables.

Pointless effort I think.  We already have something very similar to
that, which is -fstack-protector-all.  It isn't a SP modification but a
cookie placement, but it doesn't gain any perturbative advantage from
the SP ASLR.

Re: llvm - xor return pointers

[Mark Kettenis]

> Date: Sat, 22 Jul 2017 02:25:29 -0400
> From: Todd Mortimer 
> 
> Hello tech,
> 
> I've been working on llvm/clang some lately, and am experimenting with
> the llvm Pass infrastructure. Passes essentially let you perform
> arbitrary transforms on the program at various points in the compilation
> process.
> 
> I've attached a patch that defines a machine function pass that adds:
> 
> xor [rsp], rsp
> 
> at the start of each function, and before every RET. This means that the
> return pointer on the stack is xored with the stack address where it
> happens to be. When the function returns, the same transform is applied
> again to reverse the process. The consequences of doing this are (a) The
> return address is obfuscated on the stack, which mitigates against some
> info leaks and (b) All the RETs in the program become hard to use in ROP
> gadgets, because the return address is permuted before being popped.
> ASLR on the stack makes predicting stack addresses difficult, so if an
> attacker is dumping a ROP chain onto the stack that uses these gadgets,
> they will need to know the addresses they are writing to on the stack in
> order for them to work, which adds an extra hurdle. 
> 
> The performance cost here is 2 extra instructions per function call. I
> picked rsp to xor with the return address because it is cheap to use,
> non-constant, and makes the transform simple and easy to reason about. I
> am happy to bikeshed about better choices if people are interested.
> 
> I've done some light testing on this, and it seems to work on my test
> programs and when I did it to my libc. I am not really sure if this is
> interesting enough to warrant more work, but I figured there isn't any
> harm in posting it up. So I'm not looking for okays, but figured it
> might be interesting to some others on the list.

Cool stuff.  The downside is that this probably kills doing
backtraces, making debugging stuff hard.  Unless this also changes the
DWARF debugging information to reflect the xor operation.  But I'm not
sure that's possible.

Having a "constant" (per process) cookie would make things easier for
debuggers, but also weaken the mechanism.  It might be possible to
have the kernel initialize such a cookie in the TIB (thread
information block) that is used for fast access to thread-local
variables.

Re: style.9: discourage (void)ing unused return values

[Mark Kettenis]

> Date: Sat, 22 Jul 2017 15:29:17 +0200
> From: Ingo Schwarze 
> 
> Hi Scott,
> 
> Scott Cheloha wrote on Fri, Jul 21, 2017 at 05:03:11PM -0500:
> 
> > Per encouragement from deraadt@,
> 
> Not sure what exactly he said, but i'm quite sure you misunderstood him.
> 
> I have both removed and added (void) casts in the past.
> 
> Removed from functions like close(3) where they are usually pointless
> and only a distraction to the reader.
> 
> Added to functions like strlcpy(3) where ignoring the return value
> is often a serious bug.  In such a case, (void) is not intended for
> some compiler, but for human consumption.  Its meaning is: This
> call has been carefully audited.  Contrary to the usual situation,
> we can safely ignore the return value here, either because the
> buffer is so large that it can never become full at this point, or
> because truncation is not a problem at this point.
> 
> This cannot be formalized.
> 
> There may be cases where (void) makes sense even on a function like
> close(3) - if for some specific reason, an auditor might think that
> failure is exceptionally dangerous in that particular situation,
> but actually, it is not.  And there may be situations where strlcpy(3)
> without (void) is not a style issue, for example if a whole file
> uses it a lot with some consistent idiom that doesn't require
> overflow checking.

Note that GCC hase a "warn_unused_result" attribute that can be used
to annotate functions for which ignoring the return value is
dangerous:

--- unused.c ---

int foo(void)  __attribute__((warn_unused_result));

int
foo(void)
{
return -1;
}

void
bar(void)
{
(void)foo();
}

--- unused.c ---

$ gcc -c unused.c 
unused.c: In function 'bar':
unused.c:12: warning: ignoring return value of 'foo', declared with attribute 
warn_unused_result

I think it would be good if you started using that in our system
headers for some functions.  clang allows one to suppress the warning
by using the (void) cast, gcc doesn't though.

macppc: initialize brightness

[Anton Lindqvist]

Hi,
The brightness on my PowerBook6,4 prior pressing either the brightness
down/up key looks odd:

  $ wsconsctl display.brightness
  display.brightness=4294967271.4294967235%

Since cons_brightness is initialized to 0 and MIN_BRIGHTNESS is greater
than 0, the percentage calculation performed by wsconsctl blows up.

This diff makes sure to initialize cons_brightness if a backlight is
found. I chose to initialize it to MAX_BRIGHTNESS since the brightness
on my machine is not restored on boot and therefore initially always set
to MAX_BRIGHTNESS. This also fixes another problem: pressing either
brightness down/up will set the brightness to MIN_BRIGHTNESS since
cons_brightness is below MIN_BRIGHTNESS initially.

Comments? OK?

Index: ofw_machdep.c
===
RCS file: /cvs/src/sys/arch/macppc/macppc/ofw_machdep.c,v
retrieving revision 1.55
diff -u -p -r1.55 ofw_machdep.c
--- ofw_machdep.c   21 Jul 2015 05:58:34 -  1.55
+++ ofw_machdep.c   22 Jul 2017 09:17:01 -
@@ -469,8 +469,10 @@ of_display_console(void)
}
}
 
-   if (OF_getnodebyname(0, "backlight") != 0)
+   if (OF_getnodebyname(0, "backlight") != 0) {
cons_backlight_available = 1;
+   cons_brightness = MAX_BRIGHTNESS;
+   }
 
 #if 1
printf(": memaddr %x, size %x ", addr[0].phys_lo, addr[0].size_lo);

Re: style.9: discourage (void)ing unused return values

[Ingo Schwarze]

Hi Scott,

Scott Cheloha wrote on Fri, Jul 21, 2017 at 05:03:11PM -0500:

> Per encouragement from deraadt@,

Not sure what exactly he said, but i'm quite sure you misunderstood him.

I have both removed and added (void) casts in the past.

Removed from functions like close(3) where they are usually pointless
and only a distraction to the reader.

Added to functions like strlcpy(3) where ignoring the return value
is often a serious bug.  In such a case, (void) is not intended for
some compiler, but for human consumption.  Its meaning is: This
call has been carefully audited.  Contrary to the usual situation,
we can safely ignore the return value here, either because the
buffer is so large that it can never become full at this point, or
because truncation is not a problem at this point.

This cannot be formalized.

There may be cases where (void) makes sense even on a function like
close(3) - if for some specific reason, an auditor might think that
failure is exceptionally dangerous in that particular situation,
but actually, it is not.  And there may be situations where strlcpy(3)
without (void) is not a style issue, for example if a whole file
uses it a lot with some consistent idiom that doesn't require
overflow checking.

It is really a case-by-case decision:  Does it help or distract
a human auditor?


> here's a diff that explicitly
> discourages casting unused return values to void.

Not OK.


> Index: share/man/man9/style.9
> ===
> RCS file: /cvs/src/share/man/man9/style.9,v
> retrieving revision 1.71
> diff -u -p -r1.71 style.9
> --- share/man/man9/style.910 Jul 2017 21:39:38 -  1.71
> +++ share/man/man9/style.921 Jul 2017 21:49:14 -
> @@ -514,6 +514,9 @@ Routines returning
>  .Li void *
>  should not have their return values cast to any pointer type.
>  .Pp
> +Do not cast unused return values to
> +.Li void .
> +.Pp
>  Use the
>  .Xr err 3
>  and

Re: crypt_checkpass.3: mention additional failure case for crypt_newhash

[Scott Cheloha]

> On Jul 21, 2017, at 10:24 PM, Ted Unangst  wrote:
> 
> Scott Cheloha wrote:
>> crypt_newhash(3) will return -1 and set errno to EINVAL if hashsize is
>> too small to accommodate bcrypt's hash space.  I imagine this would
>> also be the case if anything other than bcrypt were supported.
> 
> i went ahead and reworked the page a bit. i think it clarifies a few other
> edge conditions better now.

It does.  I didn't even know you could omit ",a" to equal effect.

pfkeyv2 rename struct keycb pointer

[Claudio Jeker]

Suggested by bluhm@, switch from struct keycb *pk to struct keycb *kp.

OK?
-- 
:wq Claudio

Index: net/pfkeyv2.c
===
RCS file: /cvs/src/sys/net/pfkeyv2.c,v
retrieving revision 1.163
diff -u -p -r1.163 pfkeyv2.c
--- net/pfkeyv2.c   3 Jul 2017 19:23:47 -   1.163
+++ net/pfkeyv2.c   10 Jul 2017 06:28:58 -
@@ -214,19 +214,19 @@ int
 pfkeyv2_attach(struct socket *so, int proto)
 {
struct rawcb *rp;
-   struct keycb *pk;
+   struct keycb *kp;
int error;
 
if ((so->so_state & SS_PRIV) == 0)
return EACCES;
 
-   pk = malloc(sizeof(struct keycb), M_PCB, M_WAITOK | M_ZERO);
-   rp = >rcb;
+   kp = malloc(sizeof(struct keycb), M_PCB, M_WAITOK | M_ZERO);
+   rp = >rcb;
so->so_pcb = rp;
 
error = raw_attach(so, proto);
if (error) {
-   free(pk, M_PCB, sizeof(struct keycb));
+   free(kp, M_PCB, sizeof(struct keycb));
return (error);
}
 
@@ -234,15 +234,15 @@ pfkeyv2_attach(struct socket *so, int pr
soisconnected(so);
 
rp->rcb_faddr = _addr;
-   pk->pid = curproc->p_p->ps_pid;
+   kp->pid = curproc->p_p->ps_pid;
 
/*
 * XXX we should get this from the socket instead but
 * XXX rawcb doesn't store the rdomain like inpcb does.
 */
-   pk->rdomain = rtable_l2(curproc->p_p->ps_rtableid);
+   kp->rdomain = rtable_l2(curproc->p_p->ps_rtableid);
 
-   LIST_INSERT_HEAD(_sockets, pk, kcb_list);
+   LIST_INSERT_HEAD(_sockets, kp, kcb_list);
 
return (0);
 }
@@ -253,21 +253,21 @@ pfkeyv2_attach(struct socket *so, int pr
 int
 pfkeyv2_detach(struct socket *so, struct proc *p)
 {
-   struct keycb *pk;
+   struct keycb *kp;
 
-   pk = sotokeycb(so);
-   if (pk == NULL)
+   kp = sotokeycb(so);
+   if (kp == NULL)
return ENOTCONN;
 
-   LIST_REMOVE(pk, kcb_list);
+   LIST_REMOVE(kp, kcb_list);
 
-   if (pk->flags & PFKEYV2_SOCKETFLAGS_REGISTERED)
+   if (kp->flags & PFKEYV2_SOCKETFLAGS_REGISTERED)
nregistered--;
 
-   if (pk->flags & PFKEYV2_SOCKETFLAGS_PROMISC)
+   if (kp->flags & PFKEYV2_SOCKETFLAGS_PROMISC)
npromisc--;
 
-   raw_detach(>rcb);
+   raw_detach(>rcb);
return (0);
 }
 
@@ -942,7 +942,7 @@ pfkeyv2_send(struct socket *so, void *me
struct radix_node_head *rnh;
struct radix_node *rn = NULL;
 
-   struct keycb *pk, *bpk = NULL;
+   struct keycb *kp, *bkp = NULL;
 
void *freeme = NULL, *bckptr = NULL;
void *headers[SADB_EXT_MAX + 1];
@@ -964,14 +964,13 @@ pfkeyv2_send(struct socket *so, void *me
/* Verify that we received this over a legitimate pfkeyv2 socket */
bzero(headers, sizeof(headers));
 
-   pk = sotokeycb(so);
-
-   if (!pk) {
+   kp = sotokeycb(so);
+   if (!kp) {
rval = EINVAL;
goto ret;
}
 
-   rdomain = pk->rdomain;
+   rdomain = kp->rdomain;
 
/* If we have any promiscuous listeners, send them a copy of the 
message */
if (npromisc) {
@@ -1000,10 +999,10 @@ pfkeyv2_send(struct socket *so, void *me
goto ret;
 
/* Send to all promiscuous listeners */
-   LIST_FOREACH(bpk, _sockets, kcb_list) {
-   if ((bpk->flags & PFKEYV2_SOCKETFLAGS_PROMISC) &&
-   (bpk->rdomain == rdomain))
-   pfkey_sendup(bpk, packet, 1);
+   LIST_FOREACH(bkp, _sockets, kcb_list) {
+   if ((bkp->flags & PFKEYV2_SOCKETFLAGS_PROMISC) &&
+   (bkp->rdomain == rdomain))
+   pfkey_sendup(bkp, packet, 1);
}
 
m_freem(packet);
@@ -1392,8 +1391,8 @@ pfkeyv2_send(struct socket *so, void *me
break;
 
case SADB_REGISTER:
-   if (!(pk->flags & PFKEYV2_SOCKETFLAGS_REGISTERED)) {
-   pk->flags |= PFKEYV2_SOCKETFLAGS_REGISTERED;
+   if (!(kp->flags & PFKEYV2_SOCKETFLAGS_REGISTERED)) {
+   kp->flags |= PFKEYV2_SOCKETFLAGS_REGISTERED;
nregistered++;
}
 
@@ -1423,7 +1422,7 @@ pfkeyv2_send(struct socket *so, void *me
}
 
/* Keep track what this socket has registered for */
-   pk->registration |= (1 << ((struct sadb_msg 
*)message)->sadb_msg_satype);
+   kp->registration |= (1 << ((struct sadb_msg 
*)message)->sadb_msg_satype);
 
ssup = (struct sadb_supported *) freeme;
ssup->sadb_supported_len = i / sizeof(uint64_t);
@@ -1768,12 +1767,12 @@ pfkeyv2_send(struct socket *so, void *me
if ((rval = pfdatatopacket(message, len, )) != 0)
  

Re: time(1): use monotonic clock for computing elapsed time

[Anton Lindqvist]

On Sat, Jul 22, 2017 at 10:19:22AM +0200, Anton Lindqvist wrote:
> On Fri, Jul 21, 2017 at 04:34:53PM -0500, Scott Cheloha wrote:
> > ~1 week bump.  Changes to time(1) were committed by tedu@.
> > 
> > Any feedback on the ksh/csh portions of the patch?
> 
> Looks good. I'm willing to commit this diff if I can get another ok.

Committed, thanks!

getdelim(3): perror -> err in example

[Anton Lindqvist]

Hi,
This changes the behavior of the example but makes it consistent with
the example in fgetln(3) and many other manuals.

Comments? OK?

Index: getdelim.3
===
RCS file: /cvs/src/lib/libc/stdio/getdelim.3,v
retrieving revision 1.4
diff -u -p -r1.4 getdelim.3
--- getdelim.3  4 Apr 2016 19:23:52 -   1.4
+++ getdelim.3  22 Jul 2017 08:51:16 -
@@ -124,7 +124,7 @@ while ((linelen = getline(\*[Am]line, \*
 
 free(line);
 if (ferror(fp))
-   perror("getline");
+   err(1, "getline");
 .Ed
 .Sh ERRORS
 .Bl -tag -width [EOVERFLOW]

Re: time(1): use monotonic clock for computing elapsed time

[Anton Lindqvist]

On Fri, Jul 21, 2017 at 04:34:53PM -0500, Scott Cheloha wrote:
> ~1 week bump.  Changes to time(1) were committed by tedu@.
> 
> Any feedback on the ksh/csh portions of the patch?

Looks good. I'm willing to commit this diff if I can get another ok.

llvm - xor return pointers

[Todd Mortimer]

Hello tech,

I've been working on llvm/clang some lately, and am experimenting with
the llvm Pass infrastructure. Passes essentially let you perform
arbitrary transforms on the program at various points in the compilation
process.

I've attached a patch that defines a machine function pass that adds:

xor [rsp], rsp

at the start of each function, and before every RET. This means that the
return pointer on the stack is xored with the stack address where it
happens to be. When the function returns, the same transform is applied
again to reverse the process. The consequences of doing this are (a) The
return address is obfuscated on the stack, which mitigates against some
info leaks and (b) All the RETs in the program become hard to use in ROP
gadgets, because the return address is permuted before being popped.
ASLR on the stack makes predicting stack addresses difficult, so if an
attacker is dumping a ROP chain onto the stack that uses these gadgets,
they will need to know the addresses they are writing to on the stack in
order for them to work, which adds an extra hurdle. 

The performance cost here is 2 extra instructions per function call. I
picked rsp to xor with the return address because it is cheap to use,
non-constant, and makes the transform simple and easy to reason about. I
am happy to bikeshed about better choices if people are interested.

I've done some light testing on this, and it seems to work on my test
programs and when I did it to my libc. I am not really sure if this is
interesting enough to warrant more work, but I figured there isn't any
harm in posting it up. So I'm not looking for okays, but figured it
might be interesting to some others on the list.

Todd


diff --git llvm/lib/Target/X86/CMakeLists.txt llvm/lib/Target/X86/CMakeLists.txt
index 9dfd09022bd..dc2d58073fd 100644
--- llvm/lib/Target/X86/CMakeLists.txt
+++ llvm/lib/Target/X86/CMakeLists.txt
@@ -45,6 +45,7 @@ set(sources
   X86MachineFunctionInfo.cpp
   X86OptimizeLEAs.cpp
   X86PadShortFunction.cpp
+  X86GuardStackRet.cpp
   X86RegisterInfo.cpp
   X86SelectionDAGInfo.cpp
   X86ShuffleDecodeConstantPool.cpp
diff --git llvm/lib/Target/X86/X86.h llvm/lib/Target/X86/X86.h
index 2cb80a482d0..4b8a132f648 100644
--- llvm/lib/Target/X86/X86.h
+++ llvm/lib/Target/X86/X86.h
@@ -50,6 +50,10 @@ FunctionPass *createX86IssueVZeroUpperPass();
 /// This will prevent a stall when returning on the Atom.
 FunctionPass *createX86PadShortFunctions();
 
+/// Return a pass that adds xor instructions for return pointers
+/// on the stack
+FunctionPass *createX86GuardStackRet();
+
 /// Return a pass that selectively replaces certain instructions (like add,
 /// sub, inc, dec, some shifts, and some multiplies) by equivalent LEA
 /// instructions, in order to eliminate execution delays in some processors.
diff --git llvm/lib/Target/X86/X86GuardStackRet.cpp 
llvm/lib/Target/X86/X86GuardStackRet.cpp
new file mode 100644
index 000..e7261a85aee
--- /dev/null
+++ llvm/lib/Target/X86/X86GuardStackRet.cpp
@@ -0,0 +1,98 @@
+//=== X86GuardStackRet.cpp - xor return pointers ---===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+//
+// This file defines a pass that will add an xor (rsp), rsp instruction to
+// each function preamble, and before any ret.
+//
+//===--===//
+
+#include 
+
+#include "X86.h"
+#include "X86InstrInfo.h"
+#include "X86Subtarget.h"
+#include "X86InstrBuilder.h"
+#include "llvm/ADT/Statistic.h"
+#include "llvm/CodeGen/MachineFunctionPass.h"
+#include "llvm/CodeGen/MachineInstrBuilder.h"
+#include "llvm/CodeGen/MachineRegisterInfo.h"
+#include "llvm/CodeGen/Passes.h"
+#include "llvm/IR/Function.h"
+#include "llvm/Support/Debug.h"
+#include "llvm/Support/raw_ostream.h"
+#include "llvm/Target/TargetInstrInfo.h"
+
+using namespace llvm;
+
+#define DEBUG_TYPE "x86-guard-ret"
+
+namespace {
+  struct GuardStackRet : public MachineFunctionPass {
+static char ID;
+GuardStackRet() : MachineFunctionPass(ID)
+, STI(nullptr), TII(nullptr) {}
+
+bool runOnMachineFunction(MachineFunction ) override;
+
+MachineFunctionProperties getRequiredProperties() const override {
+  return MachineFunctionProperties().set(
+  MachineFunctionProperties::Property::NoVRegs);
+}
+
+StringRef getPassName() const override {
+  return "X86 Guard RET Instructions";
+}
+
+  private:
+void addXORInst(MachineBasicBlock , MachineInstr );
+
+const X86Subtarget *STI;
+const TargetInstrInfo *TII;
+   bool is64bit;
+  };
+
+  char GuardStackRet::ID = 0;
+}
+
+FunctionPass *llvm::createX86GuardStackRet() {
+  return new GuardStackRet();
+}
+
+/// runOnMachineFunction - Loop over all of the