Re: [PATCH] allow notAfter after 2038 with 32-bit time_t

[Bob Beck]

On Thu, May 18, 2017 at 7:31 AM, Kyle J. McKay  wrote:

> RFC 5280 section 4.1.2.5 states:
>
> To indicate that a certificate has no well-defined expiration date,
> the notAfter SHOULD be assigned the GeneralizedTime value of
> 1231235959Z.
>
>
True enough.



> Unfortunately, if sizeof(time_t) == 4, -12-31T23:59:59Z cannot be
> represented as a time_t value causing valid certificates to be rejected
> just because the notAfter value is after 2038-01-19T03:14:07Z.
>

Correct

So, I'll ask - what is the platform you are using that needs this? OpenBSD
does not, nor do most modern unix systems - So what platform are we doing
this for?

I'm not asking it to be nasty, but to put it in a slightly different
context "OMG, windows 98 does not support this, but if you add this code I
can support windows 98". You and I both know what your answer would
probably be for the person with Windows 98, which is "Here's a nickel kid,
get a modern operating system".

We ripped out support for a lot of moribund platforms for a reason.  What
platform are you using with 32 bit time where this is an issue?  I'm not
saying no, I'm saying "convince me this matters for anything relevant
please", and I am open to being convinced.


Fix this problem by disabling the restriction in the X509_cmp_time function
> and "wrap" far in the future notAfter values to 2038-01-19T03:14:07Z in the
> tls_get_peer_cert_times function.
>
> With both of these changes certificates with "no well-defined expiration
> date" as specified by RFC 5280 are again accepted on platforms where the
> sizeof(time_t) == 4.
>
> In general, there's no reason that a notAfter value should not be wrapped
> to 2038-01-19T03:14:07Z on a system with a 32-bit time_t.  The system
> itself
> can never have a time after 2038-01-19T03:14:07Z because of the size of the
> time_t type and so wrapping a notAfter date that is after
> 2038-01-19T03:14:07Z
> to 2038-01-19T03:14:07Z can never result in any additional certificates
> being
> accepted on such a system.
>
> Signed-off-by: Kyle J. McKay 
> ---
>
> For those using the libressl-2.5.4.tar.gz distribution, an equivalent
> patch that updates the tarball files instead can be found here:
>
>   https://gist.github.com/7d4d59bbae9e4d18444b86aa79d6f350
>
> Without this patch (or an equivalent), libressl-portable is not a viable
> alternative to OpenSSL on systems with a 32-bit time_t.
>
> It rejects valid TLS connections to any site that contains a notAfter date
> after 2038-01-19T03:14:07Z in any certificate in the chain.
>
> Besides the special case date mentioned above, there are many root
> certificates
> already in use today that have notAfter dates beyond 2038-01-19.
>
> These patches have been tested with libressl-portable 2.5.4 on a system
> with
> a 32-bit time_t.  With both of these patches the "nc -c" command
> successfully
> connects to sites with certificates that include notAfter dates after
> 2038-01-19.
>
> If either patch is omitted, "nc -c" fails to connect.
>
>  src/lib/libcrypto/x509/x509_vfy.c | 3 ++-
>  src/lib/libtls/tls_conninfo.c | 8 ++--
>  2 files changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/src/lib/libcrypto/x509/x509_vfy.c
> b/src/lib/libcrypto/x509/x509_vfy.c
> index d8c09a12..c59bd258 100644
> --- a/src/lib/libcrypto/x509/x509_vfy.c
> +++ b/src/lib/libcrypto/x509/x509_vfy.c
> @@ -1882,7 +1882,8 @@ X509_cmp_time(const ASN1_TIME *ctm, time_t *cmp_time)
>  * a time_t. A time_t must be sane if you care about times after
>  * Jan 19 2038.
>  */
> -   if ((time1 = timegm()) == -1)
> +   if (((time1 = timegm()) == -1) &&
> +   ((sizeof(time_t) != 4) || tm1.tm_year < 138))
> goto out;
>
> if (gmtime_r(, ) == NULL)
> diff --git a/src/lib/libtls/tls_conninfo.c b/src/lib/libtls/tls_conninfo.c
> index 5cdd0f77..a59b4ba2 100644
> --- a/src/lib/libtls/tls_conninfo.c
> +++ b/src/lib/libtls/tls_conninfo.c
> @@ -142,8 +142,12 @@ tls_get_peer_cert_times(struct tls *ctx, time_t
> *notbefore,
> goto err;
> if ((*notbefore = timegm(_tm)) == -1)
> goto err;
> -   if ((*notafter = timegm(_tm)) == -1)
> -   goto err;
> +   if ((*notafter = timegm(_tm)) == -1) {
> +   if (sizeof(time_t) == 4 && after_tm.tm_year >= 138)
> +   *notafter = 2147483647;
> +   else
> +   goto err;
> +   }
>
> return (0);
>
> ---
>
>

Re: Standard conformance of strtol(3)

[Theo de Raadt]

> Olivier Antoine wrote:
> > Hi all,
> > 
> > Recently a bug has been identified in Tor:
> > 
> > https://trac.torproject.org/projects/tor/ticket/22789
> > 
> > As comments were made, questions were raised about the use of strtol(3),
> > the different interpretations of the standard and their implementation.
> > 
> > To summarize, the question revolves around the processing of strings in
> > base=16 and with the optional prefix '0x'.
> > 
> > l = strtol ("0xquux", & rest, 16);
> > 
> > Produce
> > l=0 rest=0xquux on OpenBSD
> > l=0 rest=xquux on Linux
> > 
> > Do specialists of the standard or developers have an opinion on this point
> > of detail?
> > Is there a defined behavior?
> 
> My opinion is that well written code would avoid feeding ambigious strings to
> strtol. Today's it's 0xquux and tomorrow it's 0xaquux and now you have a
> problem.
> 
> But, let's read 
> http://pubs.opengroup.org/onlinepubs/9699919799/functions/strtol.html
> 
> It's actually unclear IMO. But I don't see anything prohibiting interpreting
> the string as an optional prefix with an empty body.
> 
> I'm inclined to say that strtol parsing should involve minimal lookahead and
> backtracking. So if it sees 0x, it thinks hex prefix, and then parses the
> rest. It doesn't try parsing the rest, fail, and then backtrack and start over
> with a new parse strategy.

What does the original code from AT do?

Does the BSD 4.0 code do the same?

How about the BSD 4.2 code?

How about the BSD 4.4 code?

How about all the vendors who simply used that code unmodified?

Who is the outlier?

Is it glibc?

Is it possible the spec wasn't written in a proscriptive fashion?

How much code breaks as a result?

I expect some language layering will happen over this in the next year.

I wonder if some people are going to say "the original way of doing
this is so wrong, glibc does it so much better, and the written text
lets us get away with it no matter how much code it affects".

Always fun.

ifstated remove unused logging code

[Rob Pierce]

This code has been here since version 1.1/1.2, but never used.

Rob

Index: ifstated.c
===
RCS file: /cvs/src/usr.sbin/ifstated/ifstated.c,v
retrieving revision 1.50
diff -u -p -r1.50 ifstated.c
--- ifstated.c  4 Jul 2017 21:09:52 -   1.50
+++ ifstated.c  6 Jul 2017 01:04:40 -
@@ -656,9 +656,6 @@ remove_action(struct ifsd_action *action
return;
 
switch (action->type) {
-   case IFSD_ACTION_LOG:
-   free(action->act.logmessage);
-   break;
case IFSD_ACTION_COMMAND:
free(action->act.command);
break;

Index: ifstated.h
===
RCS file: /cvs/src/usr.sbin/ifstated/ifstated.h,v
retrieving revision 1.16
diff -u -p -r1.16 ifstated.h
--- ifstated.h  4 Jul 2017 21:04:14 -   1.16
+++ ifstated.h  6 Jul 2017 01:04:40 -
@@ -63,7 +63,6 @@ struct ifsd_action {
TAILQ_ENTRY(ifsd_action) entries;
struct ifsd_action  *parent;
union {
-   char*logmessage;
char*command;
struct ifsd_state   *nextstate;
char*statename;
@@ -73,7 +72,6 @@ struct ifsd_action {
} c;
} act;
u_int32_ttype;
-#define IFSD_ACTION_LOG0
 #define IFSD_ACTION_COMMAND1
 #define IFSD_ACTION_CHANGESTATE2
 #define IFSD_ACTION_CONDITION  3

Re: Standard conformance of strtol(3)

[Ted Unangst]

Olivier Antoine wrote:
> Hi all,
> 
> Recently a bug has been identified in Tor:
> 
> https://trac.torproject.org/projects/tor/ticket/22789
> 
> As comments were made, questions were raised about the use of strtol(3),
> the different interpretations of the standard and their implementation.
> 
> To summarize, the question revolves around the processing of strings in
> base=16 and with the optional prefix '0x'.
> 
> l = strtol ("0xquux", & rest, 16);
> 
> Produce
> l=0 rest=0xquux on OpenBSD
> l=0 rest=xquux on Linux
> 
> Do specialists of the standard or developers have an opinion on this point
> of detail?
> Is there a defined behavior?

My opinion is that well written code would avoid feeding ambigious strings to
strtol. Today's it's 0xquux and tomorrow it's 0xaquux and now you have a
problem.

But, let's read 
http://pubs.opengroup.org/onlinepubs/9699919799/functions/strtol.html

It's actually unclear IMO. But I don't see anything prohibiting interpreting
the string as an optional prefix with an empty body.

I'm inclined to say that strtol parsing should involve minimal lookahead and
backtracking. So if it sees 0x, it thinks hex prefix, and then parses the
rest. It doesn't try parsing the rest, fail, and then backtrack and start over
with a new parse strategy.

Re: Standard conformance of strtol(3)

[Todd C. Miller]

C99 states that the 0x or 0X prefix is optional so we should only
consume the prefix if the following character is a valid hex char.

This is equivalent to the fix in FreeBSD but I used isxdigit(3).

 - todd

Index: lib/libc/stdlib/strtoimax.c
===
RCS file: /cvs/src/lib/libc/stdlib/strtoimax.c,v
retrieving revision 1.3
diff -u -p -u -r1.3 strtoimax.c
--- lib/libc/stdlib/strtoimax.c 12 Sep 2015 16:23:14 -  1.3
+++ lib/libc/stdlib/strtoimax.c 5 Jul 2017 22:58:33 -
@@ -74,8 +74,8 @@ strtoimax(const char *nptr, char **endpt
if (c == '+')
c = *s++;
}
-   if ((base == 0 || base == 16) &&
-   c == '0' && (*s == 'x' || *s == 'X')) {
+   if ((base == 0 || base == 16) && c == '0' &&
+   (*s == 'x' || *s == 'X') && isxdigit((unsigned char)s[1])) {
c = s[1];
s += 2;
base = 16;
Index: lib/libc/stdlib/strtol.c
===
RCS file: /cvs/src/lib/libc/stdlib/strtol.c,v
retrieving revision 1.11
diff -u -p -u -r1.11 strtol.c
--- lib/libc/stdlib/strtol.c13 Sep 2015 08:31:48 -  1.11
+++ lib/libc/stdlib/strtol.c5 Jul 2017 22:59:13 -
@@ -75,8 +75,8 @@ strtol(const char *nptr, char **endptr, 
if (c == '+')
c = *s++;
}
-   if ((base == 0 || base == 16) &&
-   c == '0' && (*s == 'x' || *s == 'X')) {
+   if ((base == 0 || base == 16) && c == '0' &&
+   (*s == 'x' || *s == 'X') && isxdigit((unsigned char)s[1])) {
c = s[1];
s += 2;
base = 16;
Index: lib/libc/stdlib/strtoll.c
===
RCS file: /cvs/src/lib/libc/stdlib/strtoll.c,v
retrieving revision 1.9
diff -u -p -u -r1.9 strtoll.c
--- lib/libc/stdlib/strtoll.c   13 Sep 2015 08:31:48 -  1.9
+++ lib/libc/stdlib/strtoll.c   5 Jul 2017 22:59:13 -
@@ -77,8 +77,8 @@ strtoll(const char *nptr, char **endptr,
if (c == '+')
c = *s++;
}
-   if ((base == 0 || base == 16) &&
-   c == '0' && (*s == 'x' || *s == 'X')) {
+   if ((base == 0 || base == 16) && c == '0' &&
+   (*s == 'x' || *s == 'X') && isxdigit((unsigned char)s[1])) {
c = s[1];
s += 2;
base = 16;
Index: lib/libc/stdlib/strtoul.c
===
RCS file: /cvs/src/lib/libc/stdlib/strtoul.c,v
retrieving revision 1.10
diff -u -p -u -r1.10 strtoul.c
--- lib/libc/stdlib/strtoul.c   13 Sep 2015 08:31:48 -  1.10
+++ lib/libc/stdlib/strtoul.c   5 Jul 2017 22:59:13 -
@@ -69,8 +69,8 @@ strtoul(const char *nptr, char **endptr,
if (c == '+')
c = *s++;
}
-   if ((base == 0 || base == 16) &&
-   c == '0' && (*s == 'x' || *s == 'X')) {
+   if ((base == 0 || base == 16) && c == '0' &&
+   (*s == 'x' || *s == 'X') && isxdigit((unsigned char)s[1])) {
c = s[1];
s += 2;
base = 16;
Index: lib/libc/stdlib/strtoull.c
===
RCS file: /cvs/src/lib/libc/stdlib/strtoull.c,v
retrieving revision 1.8
diff -u -p -u -r1.8 strtoull.c
--- lib/libc/stdlib/strtoull.c  13 Sep 2015 08:31:48 -  1.8
+++ lib/libc/stdlib/strtoull.c  5 Jul 2017 22:59:13 -
@@ -71,8 +71,8 @@ strtoull(const char *nptr, char **endptr
if (c == '+')
c = *s++;
}
-   if ((base == 0 || base == 16) &&
-   c == '0' && (*s == 'x' || *s == 'X')) {
+   if ((base == 0 || base == 16) && c == '0' &&
+   (*s == 'x' || *s == 'X') && isxdigit((unsigned char)s[1])) {
c = s[1];
s += 2;
base = 16;
Index: lib/libc/stdlib/strtoumax.c
===
RCS file: /cvs/src/lib/libc/stdlib/strtoumax.c,v
retrieving revision 1.3
diff -u -p -u -r1.3 strtoumax.c
--- lib/libc/stdlib/strtoumax.c 12 Sep 2015 16:23:14 -  1.3
+++ lib/libc/stdlib/strtoumax.c 5 Jul 2017 22:59:13 -
@@ -68,8 +68,8 @@ strtoumax(const char *nptr, char **endpt
if (c == '+')
c = *s++;
}
-   if ((base == 0 || base == 16) &&
-   c == '0' && (*s == 'x' || *s == 'X')) {
+   if ((base == 0 || base == 16) && c == '0' &&
+   (*s == 'x' || *s == 'X') && isxdigit((unsigned char)s[1])) {
c = s[1];
s += 2;
base = 16;

lpt.4: make configuration lines match GENERIC files

[Frederic Cambus]

Hi tech@,

Make configuration lines match GENERIC files. This adds amd64 and splits
up alpha and i386.

Comments? OK?

Index: share/man/man4/lpt.4
===
RCS file: /cvs/src/share/man/man4/lpt.4,v
retrieving revision 1.7
diff -u -p -r1.7 lpt.4
--- share/man/man4/lpt.420 Jun 2007 17:44:07 -  1.7
+++ share/man/man4/lpt.45 Jul 2017 22:16:28 -
@@ -35,10 +35,18 @@
 .Nm lpt
 .Nd parallel port driver
 .Sh SYNOPSIS
-.Cd "# alpha/i386"
+.Cd "# alpha"
+.Cd "lpt* at isa? port 0x3bc irq 7"
+.Pp
+.Cd "# amd64"
+.Cd "lpt0 at isa? port 0x378 irq 7"
+.Cd "lpt* at puc?"
+.Pp
+.Cd "# i386"
 .Cd "lpt0 at isa? port 0x378 irq 7"
 .Cd "lpt1 at isa? port 0x278"
 .Cd "lpt2 at isa? port 0x3bc"
+.Cd "lpt* at puc?"
 .Pp
 .Cd "# hppa"
 .Cd "lpt0 at gsc? irq 7"

Re: patch: make lex rules parallel-safe

[Marc Espie]

On Wed, Jul 05, 2017 at 01:25:59PM -0700, Philip Guenther wrote:
> On Wed, 5 Jul 2017, Marc Espie wrote:
> > This is a very slight deviation from posix rules, but not in spirit. My 
> > interpretation is that posix rules describe the intent of the make rules 
> > (produce a file in such a way), but don't really care about intermediate 
> > names.
> ...
> >  .l.c:
> > -   ${LEX.l} ${.IMPSRC}
> > -   mv lex.yy.c ${.TARGET}
> > +   ${LEX.l} -o ${.TARGET} ${.IMPSRC}
> 
> Only concern I would have is whether an up-to-date-but-broken .c file 
> would be left behind if lex fails after creating the file, resulting in 
> the next build thinking it doesn't need to run lex (and then doing the 
> Wrong Thing with the broken file).
> 
> Will lex remove the bogus output file if it fails?  (That's a benefit to 
> using "-o $@" and _not_ ">$@", as the former gives lex the name of the 
> file so it can cleanup on failure.)
> 
> (The current rule of course doesn't have this issue because the broken 
> file left behind would be the intermediate and not the actual target.)

If necessary, || rm -f ${.TARGET}
will deal with this.

Standard conformance of strtol(3)

[Olivier Antoine]

Hi all,

Recently a bug has been identified in Tor:

https://trac.torproject.org/projects/tor/ticket/22789

As comments were made, questions were raised about the use of strtol(3),
the different interpretations of the standard and their implementation.

To summarize, the question revolves around the processing of strings in
base=16 and with the optional prefix '0x'.

l = strtol ("0xquux", & rest, 16);

Produce
l=0 rest=0xquux on OpenBSD
l=0 rest=xquux on Linux

Do specialists of the standard or developers have an opinion on this point
of detail?
Is there a defined behavior?

-- 
Olivier

Re: [patch] mg: fix overflow on vteeol() (resend/bump)

[Hiltjo Posthuma]

On Sat, Jun 24, 2017 at 02:45:44PM +0200, Hiltjo Posthuma wrote:
> On Sun, Jun 18, 2017 at 03:04:31PM +0200, Hiltjo Posthuma wrote:
> > Hey,
> > 
> > This is a resend/bump of a patch about a month ago, can it get applied?
> > 
> > Original message below:
> > 
> > 
> > mg crashes with certain (unicode) characters and moving the cursor to the
> > end of the line.
> > 
> > The characters are printed to the screen as \nnn in vtpute() and vtcol is
> > updated, however vteeol() will write beyond the buffer.
> > 
> > A test file contains the data:
> > 
> > ——
> > 
> > It is printed to the screen as: \342\200\224\342... etc.
> > 
> > It is safer to use vtpute() here because it checks if vtcol >= 0.
> > 
> > Below is a patch:
> > 
> > 
> > diff --git a/usr.bin/mg/display.c b/usr.bin/mg/display.c
> > index 7af723ce268..d7c22554753 100644
> > --- a/usr.bin/mg/display.c
> > +++ b/usr.bin/mg/display.c
> > @@ -381,11 +381,8 @@ vtpute(int c)
> >  void
> >  vteeol(void)
> >  {
> > -   struct video *vp;
> > -
> > -   vp = vscreen[vtrow];
> > while (vtcol < ncol)
> > -   vp->v_text[vtcol++] = ' ';
> > +   vtpute(' ');
> >  }
> >  
> >  /*
> > 
> 

Weekly *Bump!* Any feedback? :)

-- 
Kind regards,
Hiltjo

Re: patch: make lex rules parallel-safe

[Philip Guenther]

On Wed, 5 Jul 2017, Marc Espie wrote:
> This is a very slight deviation from posix rules, but not in spirit. My 
> interpretation is that posix rules describe the intent of the make rules 
> (produce a file in such a way), but don't really care about intermediate 
> names.
...
>  .l.c:
> - ${LEX.l} ${.IMPSRC}
> - mv lex.yy.c ${.TARGET}
> + ${LEX.l} -o ${.TARGET} ${.IMPSRC}

Only concern I would have is whether an up-to-date-but-broken .c file 
would be left behind if lex fails after creating the file, resulting in 
the next build thinking it doesn't need to run lex (and then doing the 
Wrong Thing with the broken file).

Will lex remove the bogus output file if it fails?  (That's a benefit to 
using "-o $@" and _not_ ">$@", as the former gives lex the name of the 
file so it can cleanup on failure.)

(The current rule of course doesn't have this issue because the broken 
file left behind would be the intermediate and not the actual target.)


Philip

Re: sed(1): missing NUL in pattern space

[Otto Moerbeek]

On Sat, Jul 01, 2017 at 01:20:19PM +, kshe wrote:

> On Tue, 27 Jun 2017 09:29:10 +, Otto Moerbeek wrote:
>  >
>  > at last a followup, for the original problem.
>  >
>  > This diff incorporates your later comment. It does not cause the newly
>  > added regress test to fail, though.
>  >
>  > So that poses the question if this is what you meant.
>  >
>  > -Otto
>  >
>  > Index: process.c
>  > ===
>  > RCS file: /cvs/src/usr.bin/sed/process.c,v
>  > retrieving revision 1.32
>  > diff -u -p -r1.32 process.c
>  > --- process.c22 Feb 2017 14:09:09 -1.32
>  > +++ process.c27 Jun 2017 09:16:33 -
>  > @@ -120,8 +120,10 @@ redirect:
>  >  cp = cp->u.c;
>  >  goto redirect;
>  >  case 'c':
>  > +if (pd)
>  > +break;
>  >  pd = 1;
>  > -psl = 0;
>  > +ps[psl = 0] = '\0';
>  >  if (cp->a2 == NULL || lastaddr || lastline())
>  >  (void)fprintf(outfile, "%s", cp->t);
>  >  break;
>  > @@ -138,6 +140,7 @@ redirect:
>  >  } else {
>  >  psl -= (p + 1) - ps;
>  >  memmove(ps, p + 1, psl);
>  > +ps[psl] = '\0';
>  >  goto top;
>  >  }
>  >  case 'g':
>  >
> 
> Indeed, there was a wording error in my second paragraph: I meant to
> align the behaviour of `l' (not `c') with that of `p', such that using
> `l' after `c' would not print anything (not even `$'), just like `p' in
> this case, which does not print anything either (not even a newline).

somebody take over this, please. I do not seem to be able to find the
time to validate this diff.

-Otto

> 
> Index: process.c
> ===
> RCS file: /cvs/src/usr.bin/sed/process.c,v
> retrieving revision 1.32
> diff -up -r1.32 process.c
> --- process.c 22 Feb 2017 14:09:09 -  1.32
> +++ process.c 1 Jul 2017 10:24:21 -
> @@ -121,7 +121,7 @@ redirect:
>   goto redirect;
>   case 'c':
>   pd = 1;
> - psl = 0;
> + ps[psl = 0] = '\0';
>   if (cp->a2 == NULL || lastaddr || lastline())
>   (void)fprintf(outfile, "%s", cp->t);
>   break;
> @@ -138,6 +138,7 @@ redirect:
>   } else {
>   psl -= (p + 1) - ps;
>   memmove(ps, p + 1, psl);
> + ps[psl] = '\0';
>   goto top;
>   }
>   case 'g':
> @@ -158,6 +159,8 @@ redirect:
>   (void)fprintf(outfile, "%s", cp->t);
>   break;
>   case 'l':
> + if (pd)
> + break;
>   lputs(ps);
>   break;
>   case 'n':
> 
> That being said, there is more to this issue than what such a simple
> patch could fix.  For example, in a way your diff also makes sense: if
> it is believed that one ought not to use `l' nor `p' to print an
> inexistant pattern space, then surely deleting it again with `c' should
> be disallowed too.  However, as partly demonstrated by my last message,
> there is absolutely no coherence anywhere about this, so in the end one
> might wonder which command is correctly behaved and which isn't.
> 
> For instance, it is a matter of course that the sequence `N;s/.*\n//;p'
> should be just the same as `n' when normal output has not been disabled;
> but, in OpenBSD's sed, because that `pd' flag is implemented in such an
> erratic fashion, this isn't always the case:
> 
>   $ jot 6 | sed 'c\
>   > text
>   > :loop
>   > N;s/.*\n//;p
>   > bloop'
>   text
>   2
>   4
>   6
> 
>   $ jot 6 | sed 'c\
>   > text
>   > :loop
>   > n
>   > bloop'
>   text
>   2
>   3
>   4
>   5
>   6
> 
> For the same reason, nothing is printed by the following script, even if
> all lines are read:
> 
>   $ jot 6 | sed 'c\
>   > text
>   > :loop
>   > $q;N
>   > bloop'
>   text
> 
> (As a side note, amusingly, replacing the `q' above with `s/$//p' would
> actually produce the expected output, when in contrast using something
> like `s/^//p' would not...  But this is an unrelated bug.)
> 
> Likewise, one would believe the command `y/a/b/' to be functionally
> equivalent to `s/a/b/g'; but, again, many examples of the opposite
> behaviour can be 

Re: armv7 small bootstrap improvement/simplification

[Artturi Alm]

On Wed, Jul 05, 2017 at 04:05:16PM +0300, Artturi Alm wrote:
> On Wed, Jul 05, 2017 at 11:27:06AM +0200, Mark Kettenis wrote:
> > > Date: Wed, 5 Jul 2017 09:34:59 +0300
> > > From: Artturi Alm 
> > > 
> > > On Wed, Jul 05, 2017 at 02:27:46AM +0300, Artturi Alm wrote:
> > > > Hi,
> > > > 
> > > > instead of messing w/bs_tags, use the fact pmap_kernel()->pm_refs is 
> > > > going
> > > > to be 0 until pmap_bootstrap() has ran. tmp_bs_tag was unused, and
> > > > bootstrap_bs_map doesn't need/use the void *t-arg when being ran 
> > > > indirectly
> > > > via armv7_bs_map().
> > > > 
> > > > the whole existence of bootstrap_bs_map is another story, and the 
> > > > comment in
> > > > /* Now, map the FDT area. */ is somewhat an stupid excuse, it's already 
> > > > mapped
> > > > before initarm() w/VA=PA, and could well be _init()&_get_size()'d & 
> > > > memcpy'ed
> > > > somewhere in reach within bootstrap KVA, guess diff might follow for 
> > > > that,
> > > > too, if anyone has time for these simplifications.
> > > > 
> > > 
> > > Ok, i was wrong ^there, and the bootstrap code before initarm() didn't 
> > > fill
> > > the L1 w/VA=PA anymore, for reasons i don't understand, so i 'fixed' it,
> > > with diff below. tested to boot and eeprom -p normally on cubie2 and 
> > > wandb.
> > > 
> > > i kept the diff minimal, to the point it does fdt_get_size() twice just 
> > > like
> > > before, which i don't like, nor the name of size-variable and what not, 
> > > but
> > > minimal it is. Would be the first step towards earlier physmem load :)
> > > 
> > > -Artturi
> > 
> > What are you trying to achieve heree?
> > 
> > The current code quite deliberately does not create a cachable 1:1
> > mapping for the entire address space.  Such a mapping is dangerous as
> > the CPU might speculatively load from any valid mapping and that is a
> > terrible idea for device mappings.
> > 
> 
> Point taken, and adapted the diff to map only 4mb at the expected fdt pa. So
> something like below, guess you read the one mail in this thread w/o diff
> in it, ofc. the aim is really higher, make arm/armv7 more consistent/
> readable/structured/cleaned/ all around, hoping it will make maintenance
> and future innovations easier or something, now stop worrying, i'm not
> NIH-patient about to design a new wheel or anything xD.
> 
> Diff below is still rather raw, tested to boot and build a new kernel
> while running the diff correctly on sxi, unfortunately the diff has a few
> unnecessary things in it, but the purpose of this is just to show the kind of
> things rather small reorganizing could bring.
> 
> been up +24hrs, and might have had a few too long streches hacking w/o
> turning on the windows vm for a game or anything, so any stupid mistakes
> are because of that, i usually take a break atleast every 90mins or so:)
> 
> And forgive the stupid ugly printf()s in _bs_valloc(), i forgot, and am
> already late from where i was supposed to be now, 'til later o/
> -Artturi
> 
> 

oh my, sorry to anyone if i wasted your time with the previous diff,
out of two branches, i chose the wrong one, and while mailing it,
i saw some extra cleanup, and the printfs, when i quickly visited
the end of the diff to make sure it was the 4mb one, for which i had
this diff on a separate non-cleanup branch, but thought this one had
just those that i saw and not much else, blind, even if my editor doesn't
color the diffs for me, was crazy to slip thru the renames.
won't happen again in the near future, i will try..

Correct diff below, the testing i said i had done, was with this
diff.
-Artturi


diff --git a/sys/arch/armv7/armv7/armv7_machdep.c 
b/sys/arch/armv7/armv7/armv7_machdep.c
index aa1c549b29b..105fbf1 100644
--- a/sys/arch/armv7/armv7/armv7_machdep.c
+++ b/sys/arch/armv7/armv7/armv7_machdep.c
@@ -356,6 +356,30 @@ copy_io_area_map(pd_entry_t *new_pd)
}
 }
 
+static inline paddr_t
+_bs_alloc(size_t sz)
+{
+   paddr_t addr, pa = 0;
+
+   for (sz = round_page(sz); sz > 0; sz -= PAGE_SIZE) {
+   if (uvm_page_physget() == FALSE)
+   panic("uvm_page_physget() failed");
+   memset((char *)addr, 0, PAGE_SIZE);
+   if (pa == 0)
+   pa = addr;
+   }
+   return pa;
+}
+
+/* RelativePA 2 KVA */
+#define_BS_RPA2KVA(x, y)   (KERNEL_BASE + (x) - (y))
+static inline void
+_bs_valloc(pv_addr_t *pv, vsize_t sz, paddr_t off)
+{
+   pv->pv_pa = _bs_alloc(sz);
+   pv->pv_va = _BS_RPA2KVA(pv->pv_pa, off);
+}
+
 /*
  * u_int initarm(...)
  *
@@ -379,7 +403,7 @@ initarm(void *arg0, void *arg1, void *arg2, paddr_t 
loadaddr)
paddr_t memstart;
psize_t memsize;
paddr_t memend;
-   void *config;
+   void *config = arg2;
size_t size;
void *node;
extern uint32_t esym; /* &_end if no symbols are loaded */
@@ -420,18 +444,8 @@ initarm(void *arg0, void *arg1, void *arg2, paddr_t 

Re: swab: Swap bytes directly, simplify

[Klemens Nanni]

On Wed, Jul 05, 2017 at 09:47:11AM -0600, Theo de Raadt wrote:
> > So I'd say for cases like src == dst we don't have to guarantee that
> > bytes are swapped.
> 
> and you've audited all the callers to this function?
> 
> > Agreed, I haven't checked for bad/dangerous usage in existing code for
> > reasons explained above.
> 
> No you haven't.
To be fair I glanced through sys and base for callers to swab() before
patching but didn't find any:

$ grep -slRF swab\( /usr/src
/usr/src/gnu/gcc/gcc/sys-protos.h
/usr/src/gnu/usr.bin/gcc/gcc/sys-protos.h
/usr/src/include/unistd.h
/usr/src/lib/libc/string/Makefile.inc
/usr/src/lib/libc/string/swab.c

> Completely irresponsible.
> 
> What a waste of time.

But you're right about irresponsibly not thinking this through.
Definitely noted, thanks.

Re: swab: Swap bytes directly, simplify

[Ingo Schwarze]

Hi Klemens,

Klemens Nanni wrote on Wed, Jul 05, 2017 at 05:44:42PM +0200:
> On Wed, Jul 05, 2017 at 05:27:18PM +0200, Ingo Schwarze wrote:

>> No need to fix it because the patch is not likely to go anywhere,
>> but once again you mangled the patch such that it won't even apply.

> Hm, the diff taken from my mail as is applies cleanly here.

Oops, i'd like to offer half an apology.

Your diff was fine, but your mail headers contain a specification
that prevents correct display, which mislead me.  I had never seen
such misformatting in mutt(1) before.

The problem is here:

> Content-Type: text/plain; charset=us-ascii; format=flowed

Please disable the "format=flowed".  Patches are not flowed content.
At least mutt(1) does weird stuff by default and screws up the
display when finding that header, and other mail clients might be
worse.

I found the problem by reading part of

  /usr/ports/pobj/mutt-1.8.3/mutt-1.8.3/pager.c

Now, how do i get the vomit out of my keyboard.
Apparently, even mutt(1) is bloatware nowadays.

Of course, i also fixed my .muttrc settings to not change the
formatting of the mails i'm trying to read.  Adding "unset reflow_text"
to .muttrc fixes the screwup.  What a bother that such insane
features are enabled by default and send you on a wild goose chase
to the docomentation (and even to the source code) before you can
use the mail client.  What the hell, who wants his mail client to
lie to them about the contents of mail they receive?

Unfortunately, mutt is the only usable mail client i have ever
seen after elm and pine died (and it was always better than pine
in the first place).  So no choice here...  :-(

Enough ranted, back to work now.  :)

Yours,
  Ingo

Re: dhcpd: don't reject DHCPINFORM from behind relay

[Kenneth R Westerback]

On Wed, Jul 05, 2017 at 04:37:39PM +0200, Reyk Floeter wrote:
> Hi,
> 
> landry@ sees many log messages 'DHCPINFORM from xx but ciaddr yy is
> not consistent with actual address' in a setup where dhcpd runs behind
> dhcrelay.
> 
> The code in dhcpd's dhcpinform() seems wrong - it assumes that ciaddr
> (the client IP) is identical to the packet source address and it
> doesn't consider a relay, indicated by giaddr (gateway).
> 
> I looked at isc-dhcpd code and, omg my eyes are bleeding, it seems to
> handle DHCPINFORM from relayed clients with giaddr set.
> 
> So it seems that dhcpd never accepted DHCPINFORM from clients behind a
> relay, and the diff below changes it and stops the log spam.  But it
> also changes behavior, so it needs some testing and maybe feedback
> from DHCP experts (prodding krw).
> 
> Comments?

Can' find anything that says this would be wrong. I have no way to
test.

 Ken

> 
> Reyk
> 
> Index: usr.sbin/dhcpd/dhcp.c
> ===
> RCS file: /cvs/src/usr.sbin/dhcpd/dhcp.c,v
> retrieving revision 1.56
> diff -u -p -u -1 -1 -p -r1.56 dhcp.c
> --- usr.sbin/dhcpd/dhcp.c 24 Apr 2017 14:58:36 -  1.56
> +++ usr.sbin/dhcpd/dhcp.c 5 Jul 2017 14:23:12 -
> @@ -519,23 +519,23 @@ void
>  dhcpinform(struct packet *packet)
>  {
>   struct lease lease;
>   struct iaddr cip;
>   struct subnet *subnet;
>  
>   /*
>* ciaddr should be set to client's IP address but
>* not all clients are standards compliant.
>*/
>   cip.len = 4;
> - if (packet->raw->ciaddr.s_addr) {
> + if (packet->raw->ciaddr.s_addr && !packet->raw->giaddr.s_addr) {
>   if (memcmp(>raw->ciaddr.s_addr,
>   packet->client_addr.iabuf, 4) != 0) {
>   log_info("DHCPINFORM from %s but ciaddr %s is not "
>   "consistent with actual address",
>   piaddr(packet->client_addr),
>   inet_ntoa(packet->raw->ciaddr));
>   return;
>   }
>   memcpy(cip.iabuf, >raw->ciaddr.s_addr, 4);
>   } else
>   memcpy(cip.iabuf, >client_addr.iabuf, 4);
> 

Re: relayd - multiple instances

[Reyk Floeter]

Yes

On Wed, Jul 05, 2017 at 06:17:21PM +0200, Maxim Bourmistrov wrote:
> 
> Hello,
> Are there plans for relayd to run multiple instances?
> Eg. dropping socket to a configurable location.
> 
> Regards
> 

relayd - multiple instances

[Maxim Bourmistrov]

Hello,
Are there plans for relayd to run multiple instances?
Eg. dropping socket to a configurable location.

Regards

Re: swab: Swap bytes directly, simplify

[Theo de Raadt]

> So I'd say for cases like src == dst we don't have to guarantee that
> bytes are swapped.

and you've audited all the callers to this function?

> Agreed, I haven't checked for bad/dangerous usage in existing code for
> reasons explained above.

No you haven't.

Completely irresponsible.

What a waste of time.

Re: swab: Swap bytes directly, simplify

[Klemens Nanni]

On Wed, Jul 05, 2017 at 05:27:18PM +0200, Ingo Schwarze wrote:

Hi Klemens,

Klemens Nanni wrote on Wed, Jul 05, 2017 at 05:02:05PM +0200:


No need for buffers t0, t1 here.


Your patch changes behaviour in some cases where the buffers do
overlap.  For example, if src == dst, right now, the code swaps
bytes.  With your patch, i'm not sure it is even deterministic
any longer, and whatever it does exactly, it definitely destroys
information.

But the restrict keyword tells the compiler to expect distinct addresses
and POSIX explicitly says that overlapping regions cause undefined
behaviour.

So I'd say for cases like src == dst we don't have to guarantee that
bytes are swapped.


Even if behaviour is explicitly unspecified, changing it still
requires a rationale and a careful assessment of potential
damage to existing software, even if software may only be
affected when carelessly written.

Agreed, I haven't checked for bad/dangerous usage in existing code for
reasons explained above.



POSIX leaves treatment of the last odd byte unspecified but we
don't touch it at all so why not documenting this behaviour?


I strongly oppose your patch to the manual page.

Documenting implementation details is not among the goals of
manual pages, in particular when they are not portable and should
not be relied upon.

Documenting what is specified, and what is not, is among the
goals of manual pages.

In that sense, your patch to the manual page introduces a bug and
turns the STANDARDS section into a lie.

Point taken, as I already mentioned I wasn't sure about the manual diff.


No need to fix it because the patch is not likely to go anywhere,
but once again you mangled the patch such that it won't even apply.

Hm, the diff taken from my mail as is applies cleanly here.

Re: swab: Swap bytes directly, simplify

[Ingo Schwarze]

Hi Klemens,

Klemens Nanni wrote on Wed, Jul 05, 2017 at 05:02:05PM +0200:

> No need for buffers t0, t1 here.

Your patch changes behaviour in some cases where the buffers do
overlap.  For example, if src == dst, right now, the code swaps
bytes.  With your patch, i'm not sure it is even deterministic
any longer, and whatever it does exactly, it definitely destroys
information.

Even if behaviour is explicitly unspecified, changing it still
requires a rationale and a careful assessment of potential
damage to existing software, even if software may only be
affected when carelessly written.

> POSIX leaves treatment of the last odd byte unspecified but we
> don't touch it at all so why not documenting this behaviour?

I strongly oppose your patch to the manual page.

Documenting implementation details is not among the goals of
manual pages, in particular when they are not portable and should
not be relied upon.

Documenting what is specified, and what is not, is among the
goals of manual pages.

In that sense, your patch to the manual page introduces a bug and
turns the STANDARDS section into a lie.

> Feedback? Comments?

No need to fix it because the patch is not likely to go anywhere,
but once again you mangled the patch such that it won't even apply.

Yours,
  Ingo


> Index: swab.c
> ===
> RCS file: /cvs/src/lib/libc/string/swab.c,v
> retrieving revision 1.9
> diff -u -p -r1.9 swab.c
> --- swab.c11 Dec 2014 23:05:38 -  1.9
> +++ swab.c5 Jul 2017 14:12:34 -
> @@ -21,15 +21,8 @@ swab(const void *__restrict from, void *
> {
>   const unsigned char *src = from;
>   unsigned char *dst = to;
> - unsigned char t0, t1;
> + ssize_t i;
> 
> - while (len > 1) {
> - t0 = src[0];
> - t1 = src[1];
> - dst[0] = t1;
> - dst[1] = t0;
> - src += 2;
> - dst += 2;
> - len -= 2;
> - }
> + for (i = 0; i < (len & ~1) - 1; i += 2)
> + dst[i] = src[i + 1], dst[i + 1] = src[i];
> }
> Index: swab.3
> ===
> RCS file: /cvs/src/lib/libc/string/swab.3,v
> retrieving revision 1.9
> diff -u -p -r1.9 swab.3
> --- swab.312 Dec 2014 20:06:13 -  1.9
> +++ swab.35 Jul 2017 14:12:34 -
> @@ -57,7 +57,9 @@ If
> is zero or less,
> .Nm
> does nothing.
> -If it is odd, what happens to the last byte is unspecified.
> +If it is odd,
> +.Fa len Ns \-1
> +bytes are swapped and the last byte is ignored.
> If
> .Fa src
> and

Re: swab: Swap bytes directly, simplify

[Theo de Raadt]

I don't understand what you are fixing here.

It looks like you have rewritten it entirely, without cause.

Even the manual page chunk: that is a warning to unprepared people
about an error they might make.

What are you fixing??

> No need for buffers t0, t1 here. This way we only have to step/move the
> current position instead of the total bytes left as well as both source
> and destination position.
> 
> Always swap an even number of bytes by clearing the length's last bit
> and never write past it.
> 
> The , operator can be used safely here as the order of execution is
> irrelevant (in case it's not guaranteed).
> 
> POSIX leaves treatment of the last odd byte unspecified but we don't
> touch it at all so why not documenting this behaviour?
> 
> Feedback? Comments?
> 
> Index: swab.c
> ===
> RCS file: /cvs/src/lib/libc/string/swab.c,v
> retrieving revision 1.9
> diff -u -p -r1.9 swab.c
> --- swab.c11 Dec 2014 23:05:38 -  1.9
> +++ swab.c5 Jul 2017 14:12:34 -
> @@ -21,15 +21,8 @@ swab(const void *__restrict from, void *
>  {
>   const unsigned char *src = from;
>   unsigned char *dst = to;
> - unsigned char t0, t1;
> + ssize_t i;
>  
> - while (len > 1) {
> - t0 = src[0];
> - t1 = src[1];
> - dst[0] = t1;
> - dst[1] = t0;
> - src += 2;
> - dst += 2;
> - len -= 2;
> - }
> + for (i = 0; i < (len & ~1) - 1; i += 2)
> + dst[i] = src[i + 1], dst[i + 1] = src[i];
>  }
> Index: swab.3
> ===
> RCS file: /cvs/src/lib/libc/string/swab.3,v
> retrieving revision 1.9
> diff -u -p -r1.9 swab.3
> --- swab.312 Dec 2014 20:06:13 -  1.9
> +++ swab.35 Jul 2017 14:12:34 -
> @@ -57,7 +57,9 @@ If
>  is zero or less,
>  .Nm
>  does nothing.
> -If it is odd, what happens to the last byte is unspecified.
> +If it is odd,
> +.Fa len Ns \-1
> +bytes are swapped and the last byte is ignored.
>  If
>  .Fa src
>  and
> 

`

swab: Swap bytes directly, simplify

[Klemens Nanni]

No need for buffers t0, t1 here. This way we only have to step/move the
current position instead of the total bytes left as well as both source
and destination position.

Always swap an even number of bytes by clearing the length's last bit
and never write past it.

The , operator can be used safely here as the order of execution is
irrelevant (in case it's not guaranteed).

POSIX leaves treatment of the last odd byte unspecified but we don't
touch it at all so why not documenting this behaviour?

Feedback? Comments?

Index: swab.c
===
RCS file: /cvs/src/lib/libc/string/swab.c,v
retrieving revision 1.9
diff -u -p -r1.9 swab.c
--- swab.c  11 Dec 2014 23:05:38 -  1.9
+++ swab.c  5 Jul 2017 14:12:34 -
@@ -21,15 +21,8 @@ swab(const void *__restrict from, void *
{
const unsigned char *src = from;
unsigned char *dst = to;
-   unsigned char t0, t1;
+   ssize_t i;

-   while (len > 1) {
-   t0 = src[0];
-   t1 = src[1];
-   dst[0] = t1;
-   dst[1] = t0;
-   src += 2;
-   dst += 2;
-   len -= 2;
-   }
+   for (i = 0; i < (len & ~1) - 1; i += 2)
+   dst[i] = src[i + 1], dst[i + 1] = src[i];
}
Index: swab.3
===
RCS file: /cvs/src/lib/libc/string/swab.3,v
retrieving revision 1.9
diff -u -p -r1.9 swab.3
--- swab.3  12 Dec 2014 20:06:13 -  1.9
+++ swab.3  5 Jul 2017 14:12:34 -
@@ -57,7 +57,9 @@ If
is zero or less,
.Nm
does nothing.
-If it is odd, what happens to the last byte is unspecified.
+If it is odd,
+.Fa len Ns \-1
+bytes are swapped and the last byte is ignored.
If
.Fa src
and

Re: relayd ipv6 ttl check_icmp / check_tcp

[Kapetanakis Giannis]

On 04/07/17 23:56, Sebastian Benoit wrote:
> Florian Obser(flor...@openbsd.org) on 2017.07.04 19:27:15 +:
>> On Fri, Jun 23, 2017 at 01:52:52PM +0300, Kapetanakis Giannis wrote:
>>> Hi,
>>>
>>> Using relayd's redirect/forward on ipv6 addresses I discovered problems 
>>> relating to setting TTL.
>>>
>>> There is no check for address family and setsockopt tries to apply IP_TTL 
>>> always.
>>>
>>> Without ip ttl on ipv6 table, check_icmp gives
>>> send_icmp: getsockopt: Invalid argument
>>>
>>> I've removed the IP_IPDEFTTL check. Was this ok?
>>
>> Nope, relayd reuses the raw socket between config reloads (I think),
>> if the ttl gets removed from the config we need to reset to the
>> default. Don't think there is a getsockopt for v6, you can take a look
> 
> i think jca@ once had a diff for somethin called IPV6_MINHOPLIMIT? Unsure if
> thats what we need here though.
> 
>> at the sysctl(3) song and dance in traceroute(8) how to do this
>> somewhat AF independet.
>>
>> Also please make sure to not exceed 80 cols

Thanks for the commit on check_tcp.

My tabstop was set to 3 and not 8. fixed that, but it looks ugly.

According to ip6(4):
IPV6_UNICAST_HOPS int *
 Get or set the default hop limit header field for outgoing
 unicast datagrams sent on this socket.  A value of -1 resets to
 the default value.

So I changed the diff and use this. Couldn't make it work with sysctl.

comments?

Giannis
ps. There is still a patch on @tech for alternative socket name.
Could you also have a look there when you have some time?
thanks

Index: check_icmp.c
===
RCS file: /cvs/src/usr.sbin/relayd/check_icmp.c,v
retrieving revision 1.45
diff -u -p -r1.45 check_icmp.c
--- check_icmp.c28 May 2017 10:39:15 -  1.45
+++ check_icmp.c5 Jul 2017 14:35:03 -
@@ -168,6 +168,7 @@ send_icmp(int s, short event, void *arg)
socklen_tslen, len;
int  i = 0, ttl;
u_int32_tid;
+   int  ip6_def_hlim = -1;
 
if (event == EV_TIMEOUT) {
icmp_checks_timeout(cie, HCE_ICMP_WRITE_TIMEOUT);
@@ -220,18 +221,46 @@ send_icmp(int s, short event, void *arg)
sizeof(packet));
}
 
-   if ((ttl = host->conf.ttl) > 0)
-   (void)setsockopt(s, IPPROTO_IP, IP_TTL,
-   >conf.ttl, sizeof(int));
-   else {
-   /* Revert to default TTL */
-   len = sizeof(ttl);
-   if (getsockopt(s, IPPROTO_IP, IP_IPDEFTTL,
-   , ) == 0)
-   (void)setsockopt(s, IPPROTO_IP, IP_TTL,
-   , len);
-   else
-   log_warn("%s: getsockopt",__func__);
+   switch(cie->af) {
+   case AF_INET:
+   if ((ttl = host->conf.ttl) > 0) {
+   if (setsockopt(s, IPPROTO_IP, IP_TTL,
+   >conf.ttl, sizeof(int)) == -1)
+   log_warn("%s: setsockopt",
+   __func__);
+   }
+   else {
+   /* Revert to default TTL */
+   len = sizeof(ttl);
+   if (getsockopt(s, IPPROTO_IP,
+   IP_IPDEFTTL, , ) == 0) {
+   if (setsockopt(s, IPPROTO_IP,
+   IP_TTL, , len) == -1)
+   log_warn(
+   "%s: setsockopt",
+   __func__);
+   }
+   else
+   log_warn("%s: getsockopt",__func__);
+   }
+   break;
+   case AF_INET6:
+   if ((ttl = host->conf.ttl) > 0) {
+   if (setsockopt(s, IPPROTO_IPV6,
+   IPV6_UNICAST_HOPS, >conf.ttl,
+   sizeof(int)) == -1)
+   log_warn("%s: setsockopt",
+   __func__);
+   }
+   else {
+   

dhcpd: don't reject DHCPINFORM from behind relay

[Reyk Floeter]

Hi,

landry@ sees many log messages 'DHCPINFORM from xx but ciaddr yy is
not consistent with actual address' in a setup where dhcpd runs behind
dhcrelay.

The code in dhcpd's dhcpinform() seems wrong - it assumes that ciaddr
(the client IP) is identical to the packet source address and it
doesn't consider a relay, indicated by giaddr (gateway).

I looked at isc-dhcpd code and, omg my eyes are bleeding, it seems to
handle DHCPINFORM from relayed clients with giaddr set.

So it seems that dhcpd never accepted DHCPINFORM from clients behind a
relay, and the diff below changes it and stops the log spam.  But it
also changes behavior, so it needs some testing and maybe feedback
from DHCP experts (prodding krw).

Comments?

Reyk

Index: usr.sbin/dhcpd/dhcp.c
===
RCS file: /cvs/src/usr.sbin/dhcpd/dhcp.c,v
retrieving revision 1.56
diff -u -p -u -1 -1 -p -r1.56 dhcp.c
--- usr.sbin/dhcpd/dhcp.c   24 Apr 2017 14:58:36 -  1.56
+++ usr.sbin/dhcpd/dhcp.c   5 Jul 2017 14:23:12 -
@@ -519,23 +519,23 @@ void
 dhcpinform(struct packet *packet)
 {
struct lease lease;
struct iaddr cip;
struct subnet *subnet;
 
/*
 * ciaddr should be set to client's IP address but
 * not all clients are standards compliant.
 */
cip.len = 4;
-   if (packet->raw->ciaddr.s_addr) {
+   if (packet->raw->ciaddr.s_addr && !packet->raw->giaddr.s_addr) {
if (memcmp(>raw->ciaddr.s_addr,
packet->client_addr.iabuf, 4) != 0) {
log_info("DHCPINFORM from %s but ciaddr %s is not "
"consistent with actual address",
piaddr(packet->client_addr),
inet_ntoa(packet->raw->ciaddr));
return;
}
memcpy(cip.iabuf, >raw->ciaddr.s_addr, 4);
} else
memcpy(cip.iabuf, >client_addr.iabuf, 4);

Re: armv7 small bootstrap improvement/simplification

[Artturi Alm]

On Wed, Jul 05, 2017 at 11:27:06AM +0200, Mark Kettenis wrote:
> > Date: Wed, 5 Jul 2017 09:34:59 +0300
> > From: Artturi Alm 
> > 
> > On Wed, Jul 05, 2017 at 02:27:46AM +0300, Artturi Alm wrote:
> > > Hi,
> > > 
> > > instead of messing w/bs_tags, use the fact pmap_kernel()->pm_refs is going
> > > to be 0 until pmap_bootstrap() has ran. tmp_bs_tag was unused, and
> > > bootstrap_bs_map doesn't need/use the void *t-arg when being ran 
> > > indirectly
> > > via armv7_bs_map().
> > > 
> > > the whole existence of bootstrap_bs_map is another story, and the comment 
> > > in
> > > /* Now, map the FDT area. */ is somewhat an stupid excuse, it's already 
> > > mapped
> > > before initarm() w/VA=PA, and could well be _init()&_get_size()'d & 
> > > memcpy'ed
> > > somewhere in reach within bootstrap KVA, guess diff might follow for that,
> > > too, if anyone has time for these simplifications.
> > > 
> > 
> > Ok, i was wrong ^there, and the bootstrap code before initarm() didn't fill
> > the L1 w/VA=PA anymore, for reasons i don't understand, so i 'fixed' it,
> > with diff below. tested to boot and eeprom -p normally on cubie2 and wandb.
> > 
> > i kept the diff minimal, to the point it does fdt_get_size() twice just like
> > before, which i don't like, nor the name of size-variable and what not, but
> > minimal it is. Would be the first step towards earlier physmem load :)
> > 
> > -Artturi
> 
> What are you trying to achieve heree?
> 
> The current code quite deliberately does not create a cachable 1:1
> mapping for the entire address space.  Such a mapping is dangerous as
> the CPU might speculatively load from any valid mapping and that is a
> terrible idea for device mappings.
> 

Point taken, and adapted the diff to map only 4mb at the expected fdt pa. So
something like below, guess you read the one mail in this thread w/o diff
in it, ofc. the aim is really higher, make arm/armv7 more consistent/
readable/structured/cleaned/ all around, hoping it will make maintenance
and future innovations easier or something, now stop worrying, i'm not
NIH-patient about to design a new wheel or anything xD.

Diff below is still rather raw, tested to boot and build a new kernel
while running the diff correctly on sxi, unfortunately the diff has a few
unnecessary things in it, but the purpose of this is just to show the kind of
things rather small reorganizing could bring.

been up +24hrs, and might have had a few too long streches hacking w/o
turning on the windows vm for a game or anything, so any stupid mistakes
are because of that, i usually take a break atleast every 90mins or so:)

And forgive the stupid ugly printf()s in _bs_valloc(), i forgot, and am
already late from where i was supposed to be now, 'til later o/
-Artturi



diff --git a/sys/arch/armv7/armv7/armv7_machdep.c 
b/sys/arch/armv7/armv7/armv7_machdep.c
index aa1c549b29b..12eac8acc00 100644
--- a/sys/arch/armv7/armv7/armv7_machdep.c
+++ b/sys/arch/armv7/armv7/armv7_machdep.c
@@ -151,11 +151,6 @@ char *boot_args = NULL;
 char *boot_file = "";
 u_int cpu_reset_address = 0;
 
-vaddr_t physical_start;
-vaddr_t physical_freestart;
-vaddr_t physical_freeend;
-vaddr_t physical_end;
-u_int free_pages;
 int physmem = 0;
 
 /*int debug_flags;*/
@@ -356,6 +351,31 @@ copy_io_area_map(pd_entry_t *new_pd)
}
 }
 
+static inline paddr_t
+_bs_alloc(size_t sz)
+{
+   paddr_t addr, pa = 0;
+
+   for (sz = round_page(sz); sz > 0; sz -= PAGE_SIZE) {
+   if (uvm_page_physget() == FALSE)
+   panic("uvm_page_physget() failed");
+   memset((char *)addr, 0, PAGE_SIZE);
+   if (pa == 0)
+   pa = addr;
+   }
+   return pa;
+}
+
+#define_BS_RPA2VA(x, y)(KERNEL_BASE + (x) - (y))
+static inline void
+_bs_valloc(pv_addr_t *pv, vsize_t sz, paddr_t off)
+{
+   printf("_bs_valloc: pv %p sz %#8lx off %#8lx\n", pv, sz, off);
+   pv->pv_pa = _bs_alloc(sz);
+   pv->pv_va = _BS_RPA2VA(pv->pv_pa, off);
+   printf("\tpv_pa %#8lx pv_va %#8lx\n", pv->pv_pa, pv->pv_va);
+}
+
 /*
  * u_int initarm(...)
  *
@@ -371,15 +391,14 @@ copy_io_area_map(pd_entry_t *new_pd)
 u_int
 initarm(void *arg0, void *arg1, void *arg2, paddr_t loadaddr)
 {
-   int loop, loop1, i, physsegs = VM_PHYSSEG_MAX;
-   u_int l1pagetable;
+   int loop, i, physsegs = VM_PHYSSEG_MAX;
pv_addr_t kernel_l1pt;
pv_addr_t fdt;
struct fdt_reg reg;
-   paddr_t memstart;
-   psize_t memsize;
-   paddr_t memend;
-   void *config;
+   vaddr_t physical_start, physical_freestart, physical_end;
+   paddr_t kl1pt, klxpt_areap;
+   u_int free_pages;
+   void *config = arg2;
size_t size;
void *node;
extern uint32_t esym; /* &_end if no symbols are loaded */
@@ -420,18 +439,8 @@ initarm(void *arg0, void *arg1, void *arg2, paddr_t 
loadaddr)
tmp_bs_tag.bs_map = bootstrap_bs_map;
 
/*
-

Re: patch: make lex rules parallel-safe

[Marc Espie]

On Wed, Jul 05, 2017 at 06:49:30AM -0600, Todd C. Miller wrote:
> I wonder if it would be better to use lex.${.PREFIX}.c instead of
> ${.PREFIX}.lex.c.  This would be more consistent with how lex's
> -Pprefix flag behaves.
> 
> It's not a big deal either way as the file is strictly temporary.
> 
>  - todd

Sure thing.

Index: bsd.sys.mk
===
RCS file: /cvs/src/share/mk/bsd.sys.mk,v
retrieving revision 1.11
diff -u -p -r1.11 bsd.sys.mk
--- bsd.sys.mk  1 Jul 2017 14:41:54 -   1.11
+++ bsd.sys.mk  5 Jul 2017 12:53:46 -
@@ -11,19 +11,6 @@ CXXFLAGS+= -idirafter ${DESTDIR}/usr/inc
 .endif
 
 .if defined(PARALLEL)
-# Lex
-.l:
-   ${LEX.l} -o${.TARGET:R}.yy.c ${.IMPSRC}
-   ${LINK.c} -o ${.TARGET} ${.TARGET:R}.yy.c ${LDLIBS} -ll
-   rm -f ${.TARGET:R}.yy.c
-.l.c:
-   ${LEX.l} -o${.TARGET} ${.IMPSRC}
-.l.o:
-   ${LEX.l} -o${.TARGET:R}.yy.c ${.IMPSRC}
-   ${COMPILE.c} -o ${.TARGET} ${.TARGET:R}.yy.c 
-   rm -f ${.TARGET:R}.yy.c
-   if test -f ${.TARGET:R}.d; then sed -i -e 
's,${.TARGET:R}.yy.c,${.IMPSRC},' ${.TARGET:R}.d; fi
-
 # Yacc
 .y:
${YACC.y} -b ${.TARGET:R} ${.IMPSRC}
Index: sys.mk
===
RCS file: /cvs/src/share/mk/sys.mk,v
retrieving revision 1.78
diff -u -p -r1.78 sys.mk
--- sys.mk  1 Jul 2017 14:41:54 -   1.78
+++ sys.mk  5 Jul 2017 12:53:46 -
@@ -185,17 +185,16 @@ CTAGS?=   /usr/bin/ctags
 
 # Lex
 .l:
-   ${LEX.l} ${.IMPSRC}
-   ${LINK.c} -o ${.TARGET} lex.yy.c ${LDLIBS} -ll
-   rm -f lex.yy.c
+   ${LEX.l} -o lex.${.PREFIX}.c ${.IMPSRC}
+   ${LINK.c} -o ${.TARGET} lex.${.PREFIX}.c ${LDLIBS} -ll
+   rm -f lex.${.PREFIX}.c
 .l.c:
-   ${LEX.l} ${.IMPSRC}
-   mv lex.yy.c ${.TARGET}
+   ${LEX.l} -o ${.TARGET} ${.IMPSRC}
 .l.o:
-   ${LEX.l} ${.IMPSRC}
-   ${COMPILE.c} -o ${.TARGET} lex.yy.c 
-   rm -f lex.yy.c
-   if test -f ${.TARGET:R}.d; then sed -i -e 's,lex.yy.c,${.IMPSRC},' 
${.TARGET:R}.d; fi
+   ${LEX.l} -o lex.${.PREFIX}.c ${.IMPSRC}
+   ${COMPILE.c} -c lex.${.PREFIX}.c
+   rm -f lex.${.PREFIX}.c
+   if test -f ${.TARGET:R}.d; then sed -i -e 
's,lex.${.PREFIX}.lex.c,${.IMPSRC},' ${.TARGET:R}.d; fi
 
 # Yacc
 .y:

apart from that, okay ?

Re: patch: make lex rules parallel-safe

[Todd C. Miller]

I wonder if it would be better to use lex.${.PREFIX}.c instead of
${.PREFIX}.lex.c.  This would be more consistent with how lex's
-Pprefix flag behaves.

It's not a big deal either way as the file is strictly temporary.

 - todd

Re: mpsafe malloc(9)

[Mark Kettenis]

> Date: Wed, 5 Jul 2017 09:44:19 +1000
> From: David Gwynne 
> 
> the following adds a mutex to malloc and free to protect their
> internal state. this should be enough to make the api mpsafe,
> assuming the way they interact with uvm is mpsafe.
> 
> this only uses a single mutex around the entire malloc subsystem,
> but shuffles the code slightly to avoid holding it around calls
> into uvm. the shuffling is mostly to account for an allocation (ie,
> bump memuse early) before releasing the mutex to try and allocate
> it from uvm. if uvm fails, it rolls this back.
> 
> this is modelled on how the item accounting and locking is done in
> pools.
> 
> can anyone comment on the uvm safety?

As far as I can tell the use of uvm_km_kmemalloc_pla(), uvm_km_free()
and uvm_map_checkprot() is all "mpsafe".  This removes the
releasing/reacquiring of the kernel lock when pool_debug is set.  I
think that's unavoidable.

> also, it is obvious that the malloc debug code has started to rot
> a bit. ive fixed it up a bit, but now there's also a possible mp
> race when items are moved from the used list to the free list, and
> when the item is unmapped between that. id like to remove the malloc
> debug code and improve the robustness of malloc itself. pools have
> shows they can be fast and strict at the same time.

I don't think I ever used the MALLOC_DEBUG code.

ok kettenis@

> Index: kern_malloc.c
> ===
> RCS file: /cvs/src/sys/kern/kern_malloc.c,v
> retrieving revision 1.129
> diff -u -p -r1.129 kern_malloc.c
> --- kern_malloc.c 7 Jun 2017 13:30:36 -   1.129
> +++ kern_malloc.c 3 Jul 2017 10:14:49 -
> @@ -39,6 +39,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  
>  #include 
> @@ -94,6 +95,7 @@ u_int   nkmempages_min = 0;
>  #endif
>  u_intnkmempages_max = 0;
>  
> +struct mutex malloc_mtx = MUTEX_INITIALIZER(IPL_VM);
>  struct kmembuckets bucket[MINBUCKET + 16];
>  #ifdef KMEMSTATS
>  struct kmemstats kmemstats[M_LAST];
> @@ -151,8 +153,8 @@ malloc(size_t size, int type, int flags)
>   struct kmemusage *kup;
>   struct kmem_freelist *freep;
>   long indx, npg, allocsize;
> - int s;
>   caddr_t va, cp;
> + int s;
>  #ifdef DIAGNOSTIC
>   int freshalloc;
>   char *savedtype;
> @@ -164,9 +166,6 @@ malloc(size_t size, int type, int flags)
>   panic("malloc: bogus type %d", type);
>  #endif
>  
> - if (!cold)
> - KERNEL_ASSERT_LOCKED();
> -
>   KASSERT(flags & (M_WAITOK | M_NOWAIT));
>  
>   if ((flags & M_NOWAIT) == 0) {
> @@ -176,10 +175,6 @@ malloc(size_t size, int type, int flags)
>   if (pool_debug == 2)
>   yield();
>  #endif
> - if (!cold && pool_debug) {
> - KERNEL_UNLOCK();
> - KERNEL_LOCK();
> - }
>   }
>  
>  #ifdef MALLOC_DEBUG
> @@ -193,6 +188,7 @@ malloc(size_t size, int type, int flags)
>   if (size > 65535 * PAGE_SIZE) {
>   if (flags & M_CANFAIL) {
>  #ifndef SMALL_KERNEL
> + /* XXX lock */
>   if (ratecheck(_lasterr, _errintvl))
>   printf("malloc(): allocation too large, "
>   "type = %d, size = %lu\n", type, size);
> @@ -204,33 +200,38 @@ malloc(size_t size, int type, int flags)
>   }
>  
>   indx = BUCKETINDX(size);
> + if (size > MAXALLOCSAVE)
> + allocsize = round_page(size);
> + else
> + allocsize = 1 << indx;
>   kbp = [indx];
> - s = splvm();
> + mtx_enter(_mtx);
>  #ifdef KMEMSTATS
>   while (ksp->ks_memuse >= ksp->ks_limit) {
>   if (flags & M_NOWAIT) {
> - splx(s);
> + mtx_leave(_mtx);
>   return (NULL);
>   }
>   if (ksp->ks_limblocks < 65535)
>   ksp->ks_limblocks++;
> - tsleep(ksp, PSWP+2, memname[type], 0);
> + msleep(ksp, _mtx, PSWP+2, memname[type], 0);
>   }
> + ksp->ks_memuse += allocsize; /* account for this early */
>   ksp->ks_size |= 1 << indx;
>  #endif
> - if (size > MAXALLOCSAVE)
> - allocsize = round_page(size);
> - else
> - allocsize = 1 << indx;
>   if (XSIMPLEQ_FIRST(>kb_freelist) == NULL) {
> + mtx_leave(_mtx);
>   npg = atop(round_page(allocsize));
> + s = splvm();
>   va = (caddr_t)uvm_km_kmemalloc_pla(kmem_map, NULL,
>   (vsize_t)ptoa(npg), 0,
>   ((flags & M_NOWAIT) ? UVM_KMF_NOWAIT : 0) |
>   ((flags & M_CANFAIL) ? UVM_KMF_CANFAIL : 0),
>   no_constraint.ucr_low, no_constraint.ucr_high,
>   0, 0, 0);
> + splx(s);
>   if (va == NULL) {
> + long 

Re: strmode.3: Remove return values section

[Theo Buehler]

On Wed, Jul 05, 2017 at 01:31:12PM +0200, Klemens Nanni wrote:
> 
> strmode(3) is void and thus never returns anything.

Committed, thanks.

> Feedback/OK?

I think you should wait until you have commit access before you ask for
OKs. It's a bit confusing otherwise.

strmode.3: Remove return values section

[Klemens Nanni]

strmode(3) is void and thus never returns anything.

Feedback/OK?

Index: strmode.3
===
RCS file: /cvs/src/lib/libc/string/strmode.3,v
retrieving revision 1.16
diff -u -p -r1.16 strmode.3
--- strmode.3   5 Jun 2013 03:39:23 -   1.16
+++ strmode.3   5 Jul 2017 11:28:41 -
@@ -140,10 +140,6 @@ The last character is a plus sign
if there are any alternate
or additional access control methods associated with the inode, otherwise
it will be a space.
-.Sh RETURN VALUES
-The
-.Fn strmode
-function always returns 0.
.Sh SEE ALSO
.Xr chmod 1 ,
.Xr find 1 ,

patch: make lex rules parallel-safe

[Marc Espie]

This is a very slight deviation from posix rules, but not in spirit.
My interpretation is that posix rules  describe the intent of the make
rules (produce a file in such a way), but don't really care about
intermediate names.

FreeBSD already has something like this in tree (though they use
lex >$@   instad of lex -o $@ ? )

They also have some non-working mechanism for full posix compatibility
(because you deduce you run in posix mode if your first comment says
.POSIX, but sys.mk is read before that, ahah.   That would actually be
fixable, it shouldn't be that complicated to delay reading sys.mk until
you either run into a .POSIX line or something else)

Something should also be done for yacc, but that's bound to be slightly
different.


My plans for yacc are as follows:

- don't touch the sys.mk rules.  They are mandated by posix and CAN'T be
changed.
- put up proper rules in bsd.sys.mk.  So that a yacc file produces a .c
and a .h  by default *with the same names*.  This requires adjusting
a few makefiles to cope
- use that on SRCS  so that we can finally  have "correct"
file.c file.h: file.y
dependency lines without needing to adjust every makefile.

Reading thru freebsd, again, they did something like this (read their
bsd.dep.mk). The main departure from their scheme is to put the actual
rule in bsd.sys.mk, and the dependency in bsd.dep.mk like they do.

So, for now, I need okays for this patch.



Index: bsd.sys.mk
===
RCS file: /cvs/src/share/mk/bsd.sys.mk,v
retrieving revision 1.11
diff -u -p -r1.11 bsd.sys.mk
--- bsd.sys.mk  1 Jul 2017 14:41:54 -   1.11
+++ bsd.sys.mk  5 Jul 2017 11:11:26 -
@@ -11,19 +11,6 @@ CXXFLAGS+= -idirafter ${DESTDIR}/usr/inc
 .endif
 
 .if defined(PARALLEL)
-# Lex
-.l:
-   ${LEX.l} -o${.TARGET:R}.yy.c ${.IMPSRC}
-   ${LINK.c} -o ${.TARGET} ${.TARGET:R}.yy.c ${LDLIBS} -ll
-   rm -f ${.TARGET:R}.yy.c
-.l.c:
-   ${LEX.l} -o${.TARGET} ${.IMPSRC}
-.l.o:
-   ${LEX.l} -o${.TARGET:R}.yy.c ${.IMPSRC}
-   ${COMPILE.c} -o ${.TARGET} ${.TARGET:R}.yy.c 
-   rm -f ${.TARGET:R}.yy.c
-   if test -f ${.TARGET:R}.d; then sed -i -e 
's,${.TARGET:R}.yy.c,${.IMPSRC},' ${.TARGET:R}.d; fi
-
 # Yacc
 .y:
${YACC.y} -b ${.TARGET:R} ${.IMPSRC}
Index: sys.mk
===
RCS file: /cvs/src/share/mk/sys.mk,v
retrieving revision 1.78
diff -u -p -r1.78 sys.mk
--- sys.mk  1 Jul 2017 14:41:54 -   1.78
+++ sys.mk  5 Jul 2017 11:11:26 -
@@ -185,17 +185,16 @@ CTAGS?=   /usr/bin/ctags
 
 # Lex
 .l:
-   ${LEX.l} ${.IMPSRC}
-   ${LINK.c} -o ${.TARGET} lex.yy.c ${LDLIBS} -ll
-   rm -f lex.yy.c
+   ${LEX.l} -o ${.PREFIX}.lex.c ${.IMPSRC}
+   ${LINK.c} -o ${.TARGET} ${.PREFIX}.lex.c ${LDLIBS} -ll
+   rm -f ${.PREFIX}.lex.c
 .l.c:
-   ${LEX.l} ${.IMPSRC}
-   mv lex.yy.c ${.TARGET}
+   ${LEX.l} -o ${.TARGET} ${.IMPSRC}
 .l.o:
-   ${LEX.l} ${.IMPSRC}
-   ${COMPILE.c} -o ${.TARGET} lex.yy.c 
-   rm -f lex.yy.c
-   if test -f ${.TARGET:R}.d; then sed -i -e 's,lex.yy.c,${.IMPSRC},' 
${.TARGET:R}.d; fi
+   ${LEX.l} -o ${.PREFIX}.lex.c ${.IMPSRC}
+   ${COMPILE.c} -c ${.PREFIX}.lex.c
+   rm -f ${.PREFIX}.lex.c
+   if test -f ${.TARGET:R}.d; then sed -i -e 
's,${.PREFIX}.lex.c,${.IMPSRC},' ${.TARGET:R}.d; fi
 
 # Yacc
 .y:

Re: elf.h

[Martin Pieuchot]

On 04/07/17(Tue) 22:12, Karel Gardas wrote:
> > I think that moving towards  is a good thing.  However are you
> > sure that  provides all the definitions required by
> > ?
> 
> Not yet. At least a lot of machine related definitions are missing, but
> they are not required if neither base nor ports need them.

They are required because third party software will see,"Oh OpenBSD
has " and assume it contains such define.

> > Are you sure it doesn't provide any other definition that
> > would make code written on OpenBSD non portable?
> 
> PT_OPENBSD_* and NT_OPENBSD_* are there. The first I moved to ifdef 
> ELF_OPENBSD_EXTENSION. The second is used only by kernel hence _KERNEL. 
> Otherwise a lot of _DEFINED_ from sys/types.h leak in, but the question 
> is if this hurts or not.

Defines with OPENBSD in them do not hurt, since they are specific to
OpenBSD.

The #ifdef _KERNEL makes sense.

Exporting  hurts, I don't think that Solaris includes it.
With it programs will compile on OpenBSD with  but might require
 on other OSes.

> > What would it take to convert base programs to ?
> 
> Base is quite clean except few exceptions. I used ELF_OPENBSD_EXTENSION for 
> those. Anyway, both toolchains (bintils+gcc/llvm+clang) seem to live their 
> own ELF lifes without even attempting to include system elf.h/elf_abi.h so 
> they are clean and not touched.

So we should maybe just replace  by , and change
programs including  to include .  That implies
updating elf(5).

> > If base starts to provide  you also need to make sure it doesn't
> > break any port.
> 
> Both elf.h/elf_abi.h so far points to the exactly same definitions, hence if 
> there is port which is broken, then it'll be broken at compile time. Quite 
> easily discoverable by building all ports. Not yet done on my side though.

That would be a nice thing to do or ask something to do it for you once you
have a final diff.

> > I would welcome such move since it would make easier to port programs
> > from OpenBSD to other platforms.  However it's not as easy as including
> > a header in another.  Are you ready to tackle the above mentioned
> > issues?
> 
> Don't know about complexity of this. Anyway, below is another step which 
> fixes machine definition (obvious issues), adds mentioned EM_PPC64 and 
> _KERNEL ifdefs NT_OPENBSD and also uses ELF_OPENBSD_EXTENSION to filter out 
> PT_OPENBSD. However later is questionable since PT_GNU is there without any 
> #ifdef. So I can see several ways how to improve on this. For example:
> 
> - move #define ELF_OPENBSD_EXTENSION to elf_abi.h and remove from base 
> progs/libs. This way elf.h will be portable and elf_abi.h OBSD specific with 
> an option to rename to elf_obsd.h or so in the future.
> - if you don't like ELF_OPENBSD_EXTENSION I'm free to rename to whatever is 
> preferred.

I'd leave it as it is for now, I don't think it makes sense to keep
.  And we generally don't use conditionals for headers, so
just drop ELF_OPENBSD_EXTENSION.

Martin

Reduce pool cache items when there is no contention

[Visa Hankala]

The current pool cache code increases the number of items that can be
cached locally in response to lock contention. This patch adds a tweak
that lowers the number when contention does not occur. The idea is to
let resources be returned to the common pool when pressure on the cache
has decreased.

This change alone does not move elements from the cache to the common
pool. The cache has to have some activity for the moving to happen.

It is likely that `pr_cache_items' does not settle on a single value
but oscillates within some range. In my testing, I have not seen any
wild swings that would indicate positive feedback in the process.
The adjustments are constant per step and happen relatively rarely.

OK?

Index: kern/subr_pool.c
===
RCS file: src/sys/kern/subr_pool.c,v
retrieving revision 1.217
diff -u -p -r1.217 subr_pool.c
--- kern/subr_pool.c23 Jun 2017 01:21:55 -  1.217
+++ kern/subr_pool.c5 Jul 2017 09:27:40 -
@@ -1957,6 +1957,9 @@ pool_cache_gc(struct pool *pp)
if ((contention - pp->pr_cache_contention_prev) > 8 /* magic */) {
if ((ncpusfound * 8 * 2) <= pp->pr_cache_nitems)
pp->pr_cache_items += 8;
+   } else if ((contention - pp->pr_cache_contention_prev) == 0) {
+   if (pp->pr_cache_items > 8)
+   pp->pr_cache_items--;
}
pp->pr_cache_contention_prev = contention;
 }

Re: mpsafe malloc(9)

[Martin Pieuchot]

On 05/07/17(Wed) 09:44, David Gwynne wrote:
> the following adds a mutex to malloc and free to protect their
> internal state. this should be enough to make the api mpsafe,
> assuming the way they interact with uvm is mpsafe.
> 
> this only uses a single mutex around the entire malloc subsystem,
> but shuffles the code slightly to avoid holding it around calls
> into uvm. the shuffling is mostly to account for an allocation (ie,
> bump memuse early) before releasing the mutex to try and allocate
> it from uvm. if uvm fails, it rolls this back.
> 
> this is modelled on how the item accounting and locking is done in
> pools.
> 
> can anyone comment on the uvm safety?
> 
> also, it is obvious that the malloc debug code has started to rot
> a bit. ive fixed it up a bit, but now there's also a possible mp
> race when items are moved from the used list to the free list, and
> when the item is unmapped between that. id like to remove the malloc
> debug code and improve the robustness of malloc itself. pools have
> shows they can be fast and strict at the same time.

I'd welcome a mpsafe malloc(9), but I'm not sure it is worth spending
too much time making it shine.  I though the plan was to base it on
pool(9) as soon as all the free(xx, 0) are converted.  Only ~700 left!

That said we're being slowed down by a non MP-safe malloc(9) and free(9).

> Index: kern_malloc.c
> ===
> RCS file: /cvs/src/sys/kern/kern_malloc.c,v
> retrieving revision 1.129
> diff -u -p -r1.129 kern_malloc.c
> --- kern_malloc.c 7 Jun 2017 13:30:36 -   1.129
> +++ kern_malloc.c 3 Jul 2017 10:14:49 -
> @@ -39,6 +39,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  
>  #include 
> @@ -94,6 +95,7 @@ u_int   nkmempages_min = 0;
>  #endif
>  u_intnkmempages_max = 0;
>  
> +struct mutex malloc_mtx = MUTEX_INITIALIZER(IPL_VM);
>  struct kmembuckets bucket[MINBUCKET + 16];
>  #ifdef KMEMSTATS
>  struct kmemstats kmemstats[M_LAST];
> @@ -151,8 +153,8 @@ malloc(size_t size, int type, int flags)
>   struct kmemusage *kup;
>   struct kmem_freelist *freep;
>   long indx, npg, allocsize;
> - int s;
>   caddr_t va, cp;
> + int s;
>  #ifdef DIAGNOSTIC
>   int freshalloc;
>   char *savedtype;
> @@ -164,9 +166,6 @@ malloc(size_t size, int type, int flags)
>   panic("malloc: bogus type %d", type);
>  #endif
>  
> - if (!cold)
> - KERNEL_ASSERT_LOCKED();
> -
>   KASSERT(flags & (M_WAITOK | M_NOWAIT));
>  
>   if ((flags & M_NOWAIT) == 0) {
> @@ -176,10 +175,6 @@ malloc(size_t size, int type, int flags)
>   if (pool_debug == 2)
>   yield();
>  #endif
> - if (!cold && pool_debug) {
> - KERNEL_UNLOCK();
> - KERNEL_LOCK();
> - }
>   }
>  
>  #ifdef MALLOC_DEBUG
> @@ -193,6 +188,7 @@ malloc(size_t size, int type, int flags)
>   if (size > 65535 * PAGE_SIZE) {
>   if (flags & M_CANFAIL) {
>  #ifndef SMALL_KERNEL
> + /* XXX lock */
>   if (ratecheck(_lasterr, _errintvl))
>   printf("malloc(): allocation too large, "
>   "type = %d, size = %lu\n", type, size);
> @@ -204,33 +200,38 @@ malloc(size_t size, int type, int flags)
>   }
>  
>   indx = BUCKETINDX(size);
> + if (size > MAXALLOCSAVE)
> + allocsize = round_page(size);
> + else
> + allocsize = 1 << indx;
>   kbp = [indx];
> - s = splvm();
> + mtx_enter(_mtx);
>  #ifdef KMEMSTATS
>   while (ksp->ks_memuse >= ksp->ks_limit) {
>   if (flags & M_NOWAIT) {
> - splx(s);
> + mtx_leave(_mtx);
>   return (NULL);
>   }
>   if (ksp->ks_limblocks < 65535)
>   ksp->ks_limblocks++;
> - tsleep(ksp, PSWP+2, memname[type], 0);
> + msleep(ksp, _mtx, PSWP+2, memname[type], 0);
>   }
> + ksp->ks_memuse += allocsize; /* account for this early */
>   ksp->ks_size |= 1 << indx;
>  #endif
> - if (size > MAXALLOCSAVE)
> - allocsize = round_page(size);
> - else
> - allocsize = 1 << indx;
>   if (XSIMPLEQ_FIRST(>kb_freelist) == NULL) {
> + mtx_leave(_mtx);
>   npg = atop(round_page(allocsize));
> + s = splvm();
>   va = (caddr_t)uvm_km_kmemalloc_pla(kmem_map, NULL,
>   (vsize_t)ptoa(npg), 0,
>   ((flags & M_NOWAIT) ? UVM_KMF_NOWAIT : 0) |
>   ((flags & M_CANFAIL) ? UVM_KMF_CANFAIL : 0),
>   no_constraint.ucr_low, no_constraint.ucr_high,
>   0, 0, 0);
> + splx(s);
>   if (va == NULL) {
> + long wake;
>   /*
>  

telnet(1): remove unnecessary #ifdefs

[Frederic Cambus]

Hi tech@,

Remove unnecessary #ifdefs in telnet. No binary change.

Comments? OK?

Index: usr.bin/telnet/externs.h
===
RCS file: /cvs/src/usr.bin/telnet/externs.h,v
retrieving revision 1.30
diff -u -p -r1.30 externs.h
--- usr.bin/telnet/externs.h24 Nov 2015 05:06:24 -  1.30
+++ usr.bin/telnet/externs.h5 Jul 2017 09:18:09 -
@@ -277,55 +277,15 @@ extern struct termios new_tc;
 # define termKillChar  new_tc.c_cc[VKILL]
 # define termQuitChar  new_tc.c_cc[VQUIT]
 # define termSuspChar  new_tc.c_cc[VSUSP]
-
-# if   defined(VFLUSHO) && !defined(VDISCARD)
-#  define VDISCARD VFLUSHO
-# endif
-# ifndef   VDISCARD
-extern cc_t termFlushChar;
-# else
-#  define termFlushCharnew_tc.c_cc[VDISCARD]
-# endif
-# ifndef VWERASE
-extern cc_t termWerasChar;
-# else
-#  define termWerasCharnew_tc.c_cc[VWERASE]
-# endif
-# ifndef   VREPRINT
-extern cc_t termRprntChar;
-# else
-#  define termRprntCharnew_tc.c_cc[VREPRINT]
-# endif
-# ifndef   VLNEXT
-extern cc_t termLiteralNextChar;
-# else
-#  define termLiteralNextChar  new_tc.c_cc[VLNEXT]
-# endif
-# ifndef   VSTART
-extern cc_t termStartChar;
-# else
-#  define termStartCharnew_tc.c_cc[VSTART]
-# endif
-# ifndef   VSTOP
-extern cc_t termStopChar;
-# else
-#  define termStopChar new_tc.c_cc[VSTOP]
-# endif
-# ifndef   VEOL
-extern cc_t termForw1Char;
-# else
-#  define termForw1Charnew_tc.c_cc[VEOL]
-# endif
-# ifndef   VEOL2
-extern cc_t termForw2Char;
-# else
-#  define termForw2Charnew_tc.c_cc[VEOL]
-# endif
-# ifndef   VSTATUS
-extern cc_t termAytChar;
-#else
-#  define termAytChar  new_tc.c_cc[VSTATUS]
-#endif
+# define termFlushChar new_tc.c_cc[VDISCARD]
+# define termWerasChar new_tc.c_cc[VWERASE]
+# define termRprntChar new_tc.c_cc[VREPRINT]
+# define termLiteralNextChar   new_tc.c_cc[VLNEXT]
+# define termStartChar new_tc.c_cc[VSTART]
+# define termStopChar  new_tc.c_cc[VSTOP]
+# define termForw1Char new_tc.c_cc[VEOL]
+# define termForw2Char new_tc.c_cc[VEOL]
+# define termAytChar   new_tc.c_cc[VSTATUS]
 
 /* Ring buffer structures which are shared */
 
Index: usr.bin/telnet/sys_bsd.c
===
RCS file: /cvs/src/usr.bin/telnet/sys_bsd.c,v
retrieving revision 1.32
diff -u -p -r1.32 sys_bsd.c
--- usr.bin/telnet/sys_bsd.c16 Mar 2016 15:41:11 -  1.32
+++ usr.bin/telnet/sys_bsd.c5 Jul 2017 09:18:09 -
@@ -123,28 +123,6 @@ TerminalSaveState(void)
 tcgetattr(0, _tc);
 
 new_tc = old_tc;
-
-#ifndefVDISCARD
-termFlushChar = CONTROL('O');
-#endif
-#ifndefVWERASE
-termWerasChar = CONTROL('W');
-#endif
-#ifndefVREPRINT
-termRprntChar = CONTROL('R');
-#endif
-#ifndefVLNEXT
-termLiteralNextChar = CONTROL('V');
-#endif
-#ifndefVSTART
-termStartChar = CONTROL('Q');
-#endif
-#ifndefVSTOP
-termStopChar = CONTROL('S');
-#endif
-#ifndefVSTATUS
-termAytChar = CONTROL('T');
-#endif
 }
 
 cc_t *
@@ -161,22 +139,11 @@ tcval(int func)
 case SLC_FORW1:return();
 case SLC_FORW2:return();
 case SLC_SUSP: return();
-# ifdefVDISCARD
 case SLC_AO:   return();
-# endif
-# ifdefVWERASE
 case SLC_EW:   return();
-# endif
-# ifdefVREPRINT
 case SLC_RP:   return();
-# endif
-# ifdefVLNEXT
 case SLC_LNEXT:return();
-# endif
-# ifdefVSTATUS
 case SLC_AYT:  return();
-# endif
-
 case SLC_SYNCH:
 case SLC_BRK:
 case SLC_EOR:
@@ -189,27 +156,6 @@ void
 TerminalDefaultChars(void)
 {
 memcpy(new_tc.c_cc, old_tc.c_cc, sizeof(old_tc.c_cc));
-# ifndef   VDISCARD
-termFlushChar = CONTROL('O');
-# endif
-# ifndef   VWERASE
-termWerasChar = CONTROL('W');
-# endif
-# ifndef   VREPRINT
-termRprntChar = CONTROL('R');
-# endif
-# ifndef   VLNEXT
-termLiteralNextChar = CONTROL('V');
-# endif
-# ifndef   VSTART
-termStartChar = CONTROL('Q');
-# endif
-# ifndef   VSTOP
-termStopChar = CONTROL('S');
-# endif
-# ifndef   VSTATUS
-termAytChar = CONTROL('T');
-# endif
 }
 
 /*
Index: usr.bin/telnet/terminal.c
===
RCS file: /cvs/src/usr.bin/telnet/terminal.c,v
retrieving revision 1.13
diff -u -p -r1.13 terminal.c
--- usr.bin/telnet/terminal.c   22 Jul 2014 07:30:24 -  1.13
+++ usr.bin/telnet/terminal.c   5 Jul 2017 09:18:09 -
@@ -41,34 +41,6 @@ unsigned charttyobuf[2*BUFSIZ], ttyibuf
 
 int termdata;  /* Debugging flag */
 
-# ifndef VDISCARD
-cc_t termFlushChar;
-# endif
-# ifndef VLNEXT
-cc_t termLiteralNextChar;
-# endif
-# ifndef VWERASE
-cc_t termWerasChar;
-# endif

Re: armv7 small bootstrap improvement/simplification

[Mark Kettenis]

> Date: Wed, 5 Jul 2017 09:34:59 +0300
> From: Artturi Alm 
> 
> On Wed, Jul 05, 2017 at 02:27:46AM +0300, Artturi Alm wrote:
> > Hi,
> > 
> > instead of messing w/bs_tags, use the fact pmap_kernel()->pm_refs is going
> > to be 0 until pmap_bootstrap() has ran. tmp_bs_tag was unused, and
> > bootstrap_bs_map doesn't need/use the void *t-arg when being ran indirectly
> > via armv7_bs_map().
> > 
> > the whole existence of bootstrap_bs_map is another story, and the comment in
> > /* Now, map the FDT area. */ is somewhat an stupid excuse, it's already 
> > mapped
> > before initarm() w/VA=PA, and could well be _init()&_get_size()'d & 
> > memcpy'ed
> > somewhere in reach within bootstrap KVA, guess diff might follow for that,
> > too, if anyone has time for these simplifications.
> > 
> 
> Ok, i was wrong ^there, and the bootstrap code before initarm() didn't fill
> the L1 w/VA=PA anymore, for reasons i don't understand, so i 'fixed' it,
> with diff below. tested to boot and eeprom -p normally on cubie2 and wandb.
> 
> i kept the diff minimal, to the point it does fdt_get_size() twice just like
> before, which i don't like, nor the name of size-variable and what not, but
> minimal it is. Would be the first step towards earlier physmem load :)
> 
> -Artturi

What are you trying to achieve heree?

The current code quite deliberately does not create a cachable 1:1
mapping for the entire address space.  Such a mapping is dangerous as
the CPU might speculatively load from any valid mapping and that is a
terrible idea for device mappings.

> diff --git a/sys/arch/armv7/armv7/armv7_machdep.c 
> b/sys/arch/armv7/armv7/armv7_machdep.c
> index aa1c549b29b..3b860cc662f 100644
> --- a/sys/arch/armv7/armv7/armv7_machdep.c
> +++ b/sys/arch/armv7/armv7/armv7_machdep.c
> @@ -379,7 +379,7 @@ initarm(void *arg0, void *arg1, void *arg2, paddr_t 
> loadaddr)
>   paddr_t memstart;
>   psize_t memsize;
>   paddr_t memend;
> - void *config;
> + void *config = arg2;
>   size_t size;
>   void *node;
>   extern uint32_t esym; /* &_end if no symbols are loaded */
> @@ -420,18 +420,8 @@ initarm(void *arg0, void *arg1, void *arg2, paddr_t 
> loadaddr)
>   tmp_bs_tag.bs_map = bootstrap_bs_map;
>  
>   /*
> -  * Now, map the FDT area.
> -  *
> -  * As we don't know the size of a possible FDT, map the size of a
> -  * typical bootstrap bs map.  The FDT might not be aligned, so this
> -  * might take up to two L1_S_SIZEd mappings.
> -  *
> -  * XXX: There's (currently) no way to unmap a bootstrap mapping, so
> -  * we might lose a bit of the bootstrap address space.
> +  * Now, init the FDT @ PA, reloc and reinit to KVA later.
>*/
> - bootstrap_bs_map(NULL, (bus_addr_t)arg2, L1_S_SIZE, 0,
> - (bus_space_handle_t *));
> -
>   if (!fdt_init(config) || fdt_get_size(config) == 0)
>   panic("initarm: no FDT");
>  
> @@ -572,11 +562,15 @@ initarm(void *arg0, void *arg1, void *arg2, paddr_t 
> loadaddr)
>  #endif
>  
>   /*
> -  * Allocate pages for an FDT copy.
> +  * Allocate pages for FDT, copy it there, and zero the original.
>*/
>   size = fdt_get_size(config);
>   valloc_pages(fdt, round_page(size) / PAGE_SIZE);
>   memcpy((void *)fdt.pv_pa, config, size);
> + memset(config, 0, size);
> +
> + /* Now we must reinit the FDT, using the virtual address. */
> + fdt_init((void *)fdt.pv_va);
>  
>   /*
>* XXX Defer this to later so that we can reclaim the memory
> @@ -726,9 +720,6 @@ initarm(void *arg0, void *arg1, void *arg2, paddr_t 
> loadaddr)
>   prefetch_abort_handler_address = (u_int)prefetch_abort_handler;
>   undefined_handler_address = (u_int)undefinedinstruction_bounce;
>  
> - /* Now we can reinit the FDT, using the virtual address. */
> - fdt_init((void *)fdt.pv_va);
> -
>   /* Initialise the undefined instruction handlers */
>  #ifdef VERBOSE_INIT_ARM
>   printf("undefined ");
> diff --git a/sys/arch/armv7/armv7/locore0.S b/sys/arch/armv7/armv7/locore0.S
> index 2a4e98cbe8c..5fc7d250cf5 100644
> --- a/sys/arch/armv7/armv7/locore0.S
> +++ b/sys/arch/armv7/armv7/locore0.S
> @@ -127,19 +127,9 @@ _C_LABEL(bootstrap_start):
>   bne 2b
>  
>   adr r4, mmu_init_table
> -
> - mov r2, r9, lsr #18
> - ldr r3, [r4, #8]
> - bic r3, r3, #0xf000
> - orr r3, r3, r9
> - str r2, [r4, #4]
> - str r3, [r4, #8]
> - str r3, [r4, #0x14] // ram address for 0xc000
> -
> - /*
> -  * the first entry has two fields that need to be updated for
> -  * specific ram configuration of this board.
> -  */
> + ldr r3, [r4, #(12+8)]   /* r3 = pte attributes */
> + orr r3, r3, r9  /* r3 |= pa */
> + str r3, [r4, #(12+8)]   /* ram address(PA) for 0xc000 */
>   b   4f
>  
>  3:
> @@ -186,7 +176,7 @@ Lstart:
>  
> 

Re: Missed ifconfig [[-]txpower dBm] option for 802.11

[Stefan Sperling]

On Tue, Jul 04, 2017 at 01:32:41PM -0400, Ted Unangst wrote:
> Denis wrote:
> > Looking for ifconfig '[[-]txpower dBm]' option which was present in
> > OpenBSD 5.4 amd64. Try to find 'txpower' on 6.0 amd64 but seems it
> > missed out.
> > 
> > Actively using it to match power for 802.11 card and it's RF recipient
> > (post amp). What mechanism of output power matching is provided
> > currently since 5.4 amd64?
> 
> txpower was removed because only the wi driver supported it and the relevance
> of the wi driver has faded.
> 

FWIW, the drivers generally use default values from EEPROM.

Ideally, drivers would automatically adjust to Tx power in accordance
to the current regulatory domain. But at present there is no support
for regulatory domains, either.

You could hack the driver you're using to write a different tx power
threshold to hardware.
If you manage to do that, please consider helping us with adding support
for regulatory domains, which would then be configurable from userspace.

Re: ping: Style fixes/cleanups

[Florian Obser]

On Tue, Jul 04, 2017 at 09:43:59PM +0200, Klemens Nanni wrote:
> On Tue, Jul 04, 2017 at 04:00:43PM +, Florian Obser wrote:
> >yeah, this is arse backwards, I'm willing to commit the oposite though,
> >i.e. get rid of the void casts for printf
> 
> Casts removed, cosecutive calls merged where suitable.
> 
> Feedback/OK?
> 

not quite, you mix mechanical changes (remove of (void)) with the merges.
That made review much more difficult. Also it shouldn't be one commit.

I disentangled it and removed a few more (void)s.

I'm not sure I agree with the printf merges, it seems to make it more
difficult to read, also it no longer applies. If you feel strongly
about it, can you re-submit those changes on top of what's in cvs now
and explain why it's a good thing. It is possible that I missed
something because of all the (void) noise. Thanks!

I commited this:

diff --git sbin/ping/ping.c sbin/ping/ping.c
index 8dce3d33647..4533372ab0c 100644
--- sbin/ping/ping.c
+++ sbin/ping/ping.c
@@ -806,7 +806,7 @@ main(int argc, char *argv[])
nmissedmax = ntransmitted - nreceived - 1;
if (!(options & F_FLOOD) &&
(options & F_AUD_MISS))
-   (void)fputc('\a', stderr);
+   fputc('\a', stderr);
}
continue;
}
@@ -914,10 +914,10 @@ fill(char *bp, char *patp)
for (jj = 0; jj < ii; ++jj)
bp[jj + kk] = pat[jj];
if (!(options & F_QUIET)) {
-   (void)printf("PATTERN: 0x");
+   printf("PATTERN: 0x");
for (jj = 0; jj < ii; ++jj)
-   (void)printf("%02x", bp[jj] & 0xFF);
-   (void)printf("\n");
+   printf("%02x", bp[jj] & 0xFF);
+   printf("\n");
}
 }
 
@@ -1093,7 +1093,7 @@ pinger(int s)
printf("ping: wrote %s %d chars, ret=%d\n", hostname, cc, i);
}
if (!(options & F_QUIET) && options & F_FLOOD)
-   (void)write(STDOUT_FILENO, , 1);
+   write(STDOUT_FILENO, , 1);
 
return (0);
 }
@@ -1209,7 +1209,7 @@ pr_pack(u_char *buf, int cc, struct msghdr *mhdr)
 
if (timingsafe_memcmp(mac, ,
sizeof(mac)) != 0) {
-   (void)printf("signature mismatch!\n");
+   printf("signature mismatch!\n");
return;
}
timinginfo=1;
@@ -1243,21 +1243,21 @@ pr_pack(u_char *buf, int cc, struct msghdr *mhdr)
return;
 
if (options & F_FLOOD)
-   (void)write(STDOUT_FILENO, , 1);
+   write(STDOUT_FILENO, , 1);
else {
-   (void)printf("%d bytes from %s: icmp_seq=%u", cc,
+   printf("%d bytes from %s: icmp_seq=%u", cc,
pr_addr(from, fromlen), ntohs(seq));
if (v6flag)
-   (void)printf(" hlim=%d", hoplim);
+   printf(" hlim=%d", hoplim);
else
-   (void)printf(" ttl=%d", ip->ip_ttl);
+   printf(" ttl=%d", ip->ip_ttl);
if (cc >= ECHOLEN + ECHOTMLEN)
-   (void)printf(" time=%.3f ms", triptime);
+   printf(" time=%.3f ms", triptime);
if (dupflag)
-   (void)printf(" (DUP!)");
+   printf(" (DUP!)");
/* check the data */
if (cc - ECHOLEN < datalen)
-   (void)printf(" (TRUNC!)");
+   printf(" (TRUNC!)");
if (v6flag)
cp = buf + ECHOLEN + ECHOTMLEN;
else
@@ -1267,7 +1267,7 @@ pr_pack(u_char *buf, int cc, struct msghdr *mhdr)
i < cc && i < datalen;
++i, ++cp, ++dp) {
if (*cp != *dp) {
-   (void)printf("\nwrong data byte #%d "
+   printf("\nwrong data byte #%d "
"should be 0x%x but was 0x%x",
i - ECHOLEN, *dp, *cp);
if (v6flag)
@@ -1278,8 +1278,8 @@ pr_pack(u_char *buf, int cc, struct msghdr *mhdr)
for (i = ECHOLEN; i < cc && i < datalen;
++i, ++cp) {
if ((i % 32) == 

Re: armv7 small bootstrap improvement/simplification

[Artturi Alm]

On Wed, Jul 05, 2017 at 02:27:46AM +0300, Artturi Alm wrote:
> Hi,
> 
> instead of messing w/bs_tags, use the fact pmap_kernel()->pm_refs is going
> to be 0 until pmap_bootstrap() has ran. tmp_bs_tag was unused, and
> bootstrap_bs_map doesn't need/use the void *t-arg when being ran indirectly
> via armv7_bs_map().
> 
> the whole existence of bootstrap_bs_map is another story, and the comment in
> /* Now, map the FDT area. */ is somewhat an stupid excuse, it's already mapped
> before initarm() w/VA=PA, and could well be _init()&_get_size()'d & memcpy'ed
> somewhere in reach within bootstrap KVA, guess diff might follow for that,
> too, if anyone has time for these simplifications.
> 

Ok, i was wrong ^there, and the bootstrap code before initarm() didn't fill
the L1 w/VA=PA anymore, for reasons i don't understand, so i 'fixed' it,
with diff below. tested to boot and eeprom -p normally on cubie2 and wandb.

i kept the diff minimal, to the point it does fdt_get_size() twice just like
before, which i don't like, nor the name of size-variable and what not, but
minimal it is. Would be the first step towards earlier physmem load :)

-Artturi


diff --git a/sys/arch/armv7/armv7/armv7_machdep.c 
b/sys/arch/armv7/armv7/armv7_machdep.c
index aa1c549b29b..3b860cc662f 100644
--- a/sys/arch/armv7/armv7/armv7_machdep.c
+++ b/sys/arch/armv7/armv7/armv7_machdep.c
@@ -379,7 +379,7 @@ initarm(void *arg0, void *arg1, void *arg2, paddr_t 
loadaddr)
paddr_t memstart;
psize_t memsize;
paddr_t memend;
-   void *config;
+   void *config = arg2;
size_t size;
void *node;
extern uint32_t esym; /* &_end if no symbols are loaded */
@@ -420,18 +420,8 @@ initarm(void *arg0, void *arg1, void *arg2, paddr_t 
loadaddr)
tmp_bs_tag.bs_map = bootstrap_bs_map;
 
/*
-* Now, map the FDT area.
-*
-* As we don't know the size of a possible FDT, map the size of a
-* typical bootstrap bs map.  The FDT might not be aligned, so this
-* might take up to two L1_S_SIZEd mappings.
-*
-* XXX: There's (currently) no way to unmap a bootstrap mapping, so
-* we might lose a bit of the bootstrap address space.
+* Now, init the FDT @ PA, reloc and reinit to KVA later.
 */
-   bootstrap_bs_map(NULL, (bus_addr_t)arg2, L1_S_SIZE, 0,
-   (bus_space_handle_t *));
-
if (!fdt_init(config) || fdt_get_size(config) == 0)
panic("initarm: no FDT");
 
@@ -572,11 +562,15 @@ initarm(void *arg0, void *arg1, void *arg2, paddr_t 
loadaddr)
 #endif
 
/*
-* Allocate pages for an FDT copy.
+* Allocate pages for FDT, copy it there, and zero the original.
 */
size = fdt_get_size(config);
valloc_pages(fdt, round_page(size) / PAGE_SIZE);
memcpy((void *)fdt.pv_pa, config, size);
+   memset(config, 0, size);
+
+   /* Now we must reinit the FDT, using the virtual address. */
+   fdt_init((void *)fdt.pv_va);
 
/*
 * XXX Defer this to later so that we can reclaim the memory
@@ -726,9 +720,6 @@ initarm(void *arg0, void *arg1, void *arg2, paddr_t 
loadaddr)
prefetch_abort_handler_address = (u_int)prefetch_abort_handler;
undefined_handler_address = (u_int)undefinedinstruction_bounce;
 
-   /* Now we can reinit the FDT, using the virtual address. */
-   fdt_init((void *)fdt.pv_va);
-
/* Initialise the undefined instruction handlers */
 #ifdef VERBOSE_INIT_ARM
printf("undefined ");
diff --git a/sys/arch/armv7/armv7/locore0.S b/sys/arch/armv7/armv7/locore0.S
index 2a4e98cbe8c..5fc7d250cf5 100644
--- a/sys/arch/armv7/armv7/locore0.S
+++ b/sys/arch/armv7/armv7/locore0.S
@@ -127,19 +127,9 @@ _C_LABEL(bootstrap_start):
bne 2b
 
adr r4, mmu_init_table
-
-   mov r2, r9, lsr #18
-   ldr r3, [r4, #8]
-   bic r3, r3, #0xf000
-   orr r3, r3, r9
-   str r2, [r4, #4]
-   str r3, [r4, #8]
-   str r3, [r4, #0x14] // ram address for 0xc000
-
-   /*
-* the first entry has two fields that need to be updated for
-* specific ram configuration of this board.
-*/
+   ldr r3, [r4, #(12+8)]   /* r3 = pte attributes */
+   orr r3, r3, r9  /* r3 |= pa */
+   str r3, [r4, #(12+8)]   /* ram address(PA) for 0xc000 */
b   4f
 
 3:
@@ -186,7 +176,7 @@ Lstart:
 
 mmu_init_table:
/* map SDRAM VA==PA, WT cacheable */
-   MMU_INIT(0x, 0x, 64,
+   MMU_INIT(0x, 0x, 4096,
 L1_TYPE_S|L1_S_C|L1_S_V7_AP(AP_KRW)|L1_S_V7_AF)
/* map VA 0xc000..0xc3ff to PA */
MMU_INIT(0xc000, 0x, 64,