Re: Basic SHA3 support

[bytevolcano]

On Wed, 10 Jan 2018 20:32:30 -0700
"Theo de Raadt"  wrote:

> > I also use the sha512 command on a regular basis, and notice a
> > performance improvement (compared to sha256) with amd64 systems.  
> 
> But is /bin the place to put a performance improving command?
> 
> No.  That isn't the place for putting strange performance commands.
> It is a place for putting *interoperating* commands.

It isn't really a "performance improving" command, it's a different
hash. I just mentioned the performance improvement in passing.

> 
> I don't think Todd's original question was answered, except along
> the lines of "i love features".
> 

His question was "who uses the sha512 link", so I answered. I don't see
where the issue is here.

Though if the link were to go, I would be able to adapt to "cksum -a
sha512 ..."

Re: Basic SHA3 support

[bytevolcano]

I also use the sha512 command on a regular basis, and notice a
performance improvement (compared to sha256) with amd64 systems.

On Wed, 10 Jan 2018 17:10:40 + (GMT)
Tom Cosgrove  wrote:

> >>> "Todd C. Miller" 10-Jan-18 16:28 >>>  
> >
> > On Wed, 10 Jan 2018 15:59:30 +0100, Jeremie Courreges-Anglas wrote:
> >  
> > > Do we really want these?  For SHA-2, sha256 and sha512 are enough IMHO.  
> >
> > Does anyone actually use the sha512 command?  I'd be fine with
> > retiring the sha512 link and adding a sha3-256 one.
> >
> >  - todd  
> 
> I'd like to keep the sha512 link - I do use it.
> 
> Tom
> 

Re: patch: mv(1): Add -p flag to preserve time stamps for moved directories

[bytevolcano]

On Tue, 11 Apr 2017 16:41:16 -0600
"Theo de Raadt"  wrote:

> > Note that I have noatime on this FS.  
> 
> Maybe we should remove this broken feature.
> 

I would love to know what is broken about it, so I can investigate
this and disable it on my systems if it's too risky for me to keep.

I almost never need the atime stamps, and none of the applications I
have run are broken by noatime, so I disable it where I do not need (or
want) atime updated every time a file is read.

My only reasons for having noatime there in the first place is to save
write cycles on the SSDs I use, and to improve performance on an older
system I run OpenBSD on. That being said, I do make backups regularly,
so I am happy to sacrifice some performance if need be.

Re: patch: mv(1): Add -p flag to preserve time stamps for moved directories

[bytevolcano]

On Tue, 11 Apr 2017 19:39:28 -0700
Philip Guenther  wrote:

> The 'issue' is that he's changing the parent of a directory and that
> means it has to update the ".." link inside the directory, so it
> alters the directory contents and the mtime is thus updated.

That explains a lot. I thought the ".." link was a dynamic OS-created
entity, rather than something that's actually in the FS.

Though this is inconsistent with preserving the mtime when moving the
directory across filesystems, which also changes the parent.

> 
> 
> If I hit this, I would have to reconsider why I'm using the mtime of
> the directory for whatever alternate purpose makes me want to preserve
> the timestamp here.  If changing that use case is too much effort,
> then I would just script the preservation and use "touch -r" to copy
> the timestamps to a placeholder file, do the move, then "touch -r"
> again to restore them.  Introducing a non-standard option for this
> particular case pushes the cost the wrong direction, IMO.

I agree with that logic to a large extent.

I was initially thinking a similar thing, but then I got the idea that,
since cp(1) has such an option to preserve the time stamps, it seems
logical for mv(1) to have a similar option as well, and it is such a
trivial thing to add.

Re: patch: mv(1): Add -p flag to preserve time stamps for moved directories

[bytevolcano]

Sorry I was copying this out from a VM, so I ended up typing it up
wrong here:

$ mkdir Foo
$ touch -t 2101 Foo
$ ls -dlT Foo
drwxr-xr-x  2 bob  bob  512 Jan  1 00:00:00 2000 Foo
$ ls -dluT Foo
drwxr-xr-x  2 bob  bob  512 Jan  1 00:00:00 2000 Foo
$ mkdir Bar
$ mv Foo Bar/
$ ls -dlT Bar/Foo
drwxr-xr-x  2 bob  bob  512 Apr 12 07:55:30 2017 Bar/Foo
$ ls -dluT Bar/Foo
drwxr-xr-x  2 bob  bob  512 Jan  1 00:00:00 2000 Bar/Foo

Running openbsd -current (Feb 26)

On Wed, 12 Apr 2017 08:37:37 +1000
 wrote:

> On Tue, 11 Apr 2017 16:54:29 +0200
> Ingo Schwarze  wrote:
> 
> > Hi,
> > 
> > bytevolc...@safe-mail.net wrote on Fri, Apr 07, 2017 at 12:25:10PM
> > +1000:
> >   
> > > Although mv(1) will preserve atime and mtime for moved directories
> > > if moved to a different file system, it doesn't preserve them when
> > > moved in the same file system.
> > 
> > Cannot reproduce.
> > 
> >$ mkdir Foo
> >$ touch -t 2101 Foo 
> >$ ls -dl Foo
> >   drwxr-xr-x  2 schwarze  schwarze  512 Jan  1  2000 Foo
> >$ ls -dlu Foo
> >   drwxr-xr-x  2 schwarze  schwarze  512 Jan  1  2000 Foo
> >$ mv Foo Bar
> >$ ls -dl Bar  
> >   drwxr-xr-x  2 schwarze  schwarze  512 Jan  1  2000 Bar
> >$ ls -dlu Bar
> >   drwxr-xr-x  2 schwarze  schwarze  512 Jan  1  2000 Bar  
> 
> I can:
> 
> $ mkdir Foo
> $ touch -t 2101 Foo
> $ ls -dlT Foo
> drwxr-xr-x  2 bob  bob  512 Jan  1 00:00:00 2000 Foo
> $ ls -dluT Foo
> drwxr-xr-x  2 bob  bob  512 Jan  1 00:00:00 2000 Foo
> $ mkdir Bar
> $ mv Foo Bar/
> $ ls -dlT Bar/Foo
> drwxr-xr-x  2 bob  bob  512 Apr 12 07:55:30 2017 Foo/Bar
> $ ls -dluT Bar/Foo
> drwxr-xr-x  2 bob  bob  512 Jan  1 00:00:00 2000 Foo/Bar
> 
> Note that I have noatime on this FS.
> 
> > Sounds like a bad idea.  Do not add non-standard options to standard
> > utilities unless they are useful to a very unusual degree.  Even if
> > there is some corner case that you failed to describe, this seems
> > by far not important enough for adding an option.  
> 
> The problem I am solving here is that, when moving folders into other
> folders (often to clean up its parent a little and organise the FS),
> mv(1) does not preserve the time stamps of moved directories.
> 
> If I move the directory into another FS, this information is
> preserved.
> 
> > Besides, neither FreeBSD nor NetBSD have -p, so it would be an
> > egregious example of introducing gratuitious incompatibility.  
> 
> I don't see any gratuitious incompatibility here. The utility behaves
> exactly as before without the -p option, and will therefore be
> compatible with existing scripts.
> 
> It may help to modify the behaviour of rename(2), since there is
> nothing I see in the POSIX standard
> (http://pubs.opengroup.org/onlinepubs/9699919799/functions/rename.html)
> about requiring the change in time stamps. The advantage is consistent
> behaviour across all applications rather than just mv(1).
> 

Re: patch: mv(1): Add -p flag to preserve time stamps for moved directories

[bytevolcano]

On Tue, 11 Apr 2017 16:54:29 +0200
Ingo Schwarze  wrote:

> Hi,
> 
> bytevolc...@safe-mail.net wrote on Fri, Apr 07, 2017 at 12:25:10PM
> +1000:
> 
> > Although mv(1) will preserve atime and mtime for moved directories
> > if moved to a different file system, it doesn't preserve them when
> > moved in the same file system.  
> 
> Cannot reproduce.
> 
>$ mkdir Foo
>$ touch -t 2101 Foo 
>$ ls -dl Foo
>   drwxr-xr-x  2 schwarze  schwarze  512 Jan  1  2000 Foo
>$ ls -dlu Foo
>   drwxr-xr-x  2 schwarze  schwarze  512 Jan  1  2000 Foo
>$ mv Foo Bar
>$ ls -dl Bar  
>   drwxr-xr-x  2 schwarze  schwarze  512 Jan  1  2000 Bar
>$ ls -dlu Bar
>   drwxr-xr-x  2 schwarze  schwarze  512 Jan  1  2000 Bar

I can:

$ mkdir Foo
$ touch -t 2101 Foo
$ ls -dlT Foo
drwxr-xr-x  2 bob  bob  512 Jan  1 00:00:00 2000 Foo
$ ls -dluT Foo
drwxr-xr-x  2 bob  bob  512 Jan  1 00:00:00 2000 Foo
$ mkdir Bar
$ mv Foo Bar/
$ ls -dlT Bar/Foo
drwxr-xr-x  2 bob  bob  512 Apr 12 07:55:30 2017 Foo/Bar
$ ls -dluT Bar/Foo
drwxr-xr-x  2 bob  bob  512 Jan  1 00:00:00 2000 Foo/Bar

Note that I have noatime on this FS.

> Sounds like a bad idea.  Do not add non-standard options to standard
> utilities unless they are useful to a very unusual degree.  Even if
> there is some corner case that you failed to describe, this seems
> by far not important enough for adding an option.

The problem I am solving here is that, when moving folders into other
folders (often to clean up its parent a little and organise the FS),
mv(1) does not preserve the time stamps of moved directories.

If I move the directory into another FS, this information is preserved.

> Besides, neither FreeBSD nor NetBSD have -p, so it would be an
> egregious example of introducing gratuitious incompatibility.

I don't see any gratuitious incompatibility here. The utility behaves
exactly as before without the -p option, and will therefore be
compatible with existing scripts.

It may help to modify the behaviour of rename(2), since there is
nothing I see in the POSIX standard
(http://pubs.opengroup.org/onlinepubs/9699919799/functions/rename.html)
about requiring the change in time stamps. The advantage is consistent
behaviour across all applications rather than just mv(1).

patch: mv(1): Add -p flag to preserve time stamps for moved directories

[bytevolcano]

Although mv(1) will preserve atime and mtime for moved directories if
moved to a different file system, it doesn't preserve them when moved
in the same file system. Upon looking around, it appears to be standard
POSIX behavior, as the utility is meant to perform a rename() and exit.

This patch adds a "-p" flag as an extension to allow preserving the
time stamps, whilst allowing mv(1) to retain POSIX behaviour without
the flag.

I am not too sure about my man page wording though.

Index: bin/mv/mv.1
===
RCS file: /cvs/src/bin/mv/mv.1,v
retrieving revision 1.31
diff -u -p -r1.31 mv.1
--- bin/mv/mv.1 18 Jun 2012 23:41:13 -  1.31
+++ bin/mv/mv.1 7 Apr 2017 02:16:54 -
@@ -41,10 +41,10 @@
 .Nd move files
 .Sh SYNOPSIS
 .Nm mv
-.Op Fl fi
+.Op Fl fip
 .Ar source target
 .Nm mv
-.Op Fl fi
+.Op Fl fip
 .Ar source ... directory
 .Sh DESCRIPTION
 In its first form, the
@@ -103,6 +103,9 @@ The
 option overrides any previous
 .Fl f
 options.
+.It Fl p
+Preserve the modification and access times for all objects. These time stamps
+are not always preserved for some objects (such as directories) by default.
 .El
 .Pp
 The
@@ -165,6 +168,10 @@ The
 utility is compliant with the
 .St -p1003.1-2008
 specification.
+.Pp
+The
+.Fl p
+flag is an extension to that specification.
 .Sh HISTORY
 A
 .Nm
Index: bin/mv/mv.c
===
RCS file: /cvs/src/bin/mv/mv.c,v
retrieving revision 1.44
diff -u -p -r1.44 mv.c
--- bin/mv/mv.c 11 Oct 2016 16:16:44 -  1.44
+++ bin/mv/mv.c 7 Apr 2017 02:16:54 -
@@ -52,6 +52,7 @@
 extern char *__progname;
 
 int fflg, iflg;
+int pflg = 0;
 int stdin_ok;
 
 extern int cpmain(int argc, char **argv);
@@ -71,7 +72,7 @@ main(int argc, char *argv[])
int ch;
char path[PATH_MAX];
 
-   while ((ch = getopt(argc, argv, "if")) != -1)
+   while ((ch = getopt(argc, argv, "ifp")) != -1)
switch (ch) {
case 'i':
fflg = 0;
@@ -81,6 +82,9 @@ main(int argc, char *argv[])
iflg = 0;
fflg = 1;
break;
+   case 'p':
+   pflg = 1;
+   break;
default:
usage();
}
@@ -208,8 +212,22 @@ do_move(char *from, char *to)
 *  message to standard error, and do nothing more with the
 *  current source file...
 */
-   if (!rename(from, to))
+   if (!rename(from, to)) {
+   /*
+* Extension to (2): If -p is specified, copy the atime and 
mtime
+* as this is not always done with rename() for certain objects.
+*/
+   if(pflg) {
+   struct timespec ts[2];
+   ts[0] = fsb.st_atim;
+   ts[1] = fsb.st_mtim;
+   if (utimensat(AT_FDCWD, to, ts, AT_SYMLINK_NOFOLLOW)) {
+   warn("update times: %s", to);
+   return (1);
+   }
+   }
return (0);
+   }
 
if (errno != EXDEV) {
warn("rename %s to %s", from, to);
@@ -368,8 +386,8 @@ mvcopy(char *from, char *to)
 void
 usage(void)
 {
-   (void)fprintf(stderr, "usage: %s [-fi] source target\n", __progname);
-   (void)fprintf(stderr, "   %s [-fi] source ... directory\n",
+   (void)fprintf(stderr, "usage: %s [-fip] source target\n", __progname);
+   (void)fprintf(stderr, "   %s [-fip] source ... directory\n",
__progname);
exit(1);
 }

Re: [patch] doas(1): use a #define for the environment variable name length limit

[bytevolcano]

On Wed, 05 Apr 2017 15:07:40 -0400
"Ted Unangst"  wrote:

> bytevolc...@safe-mail.net wrote:
> > No functional or user-visible changes here.
> > On that note, where did the "1024" (1023-char) come from? Is there
> > anyone who has environment variables whose name goes near 1023
> > chars?  
> 
> Probably not, but there's not much benefit to be artifically limiting
> here.

I guess in this case it's better than strdup'ing a super short-lived
variable like this. I was thinking in the lines of having such a large
variable on the stack where the space is unused.

> 
> > @@ -95,7 +97,7 @@ createenv(struct rule *rule)
> > if ((eq = strchr(e, '=')) == NULL || eq ==
> > e) continue;
> > len = eq - e;
> > -   if (len > sizeof(name) - 1)
> > +   if (len > VARNAME_MAX)
> > continue;
> > memcpy(name, e, len);
> > name[len] = '\0';  
> 
> I don't like changes like this because if the size ever changes
> again, it's possible for the check to become decoupled.
> 

I see where you are coming from here. Would it be worth just keeping
the "#define" change? The 1024 limit is in 2 places and I can see a
situation where they end up out of sync.

Also I was thinking about this:

if (len > (sizeof(name) - 1))

I guess the C order of operations will not require the inner
parentheses, but the intent seems clearer this way without having to
get caught up with the specifics of C.

[patch] doas(1): use a #define for the environment variable name length limit

[bytevolcano]

No functional or user-visible changes here.
On that note, where did the "1024" (1023-char) come from? Is there
anyone who has environment variables whose name goes near 1023 chars?

Index: usr.bin/doas/env.c
===
RCS file: /cvs/src/usr.bin/doas/env.c,v
retrieving revision 1.5
diff -u -p -u -r1.5 env.c
--- usr.bin/doas/env.c  15 Sep 2016 00:58:23 -  1.5
+++ usr.bin/doas/env.c  5 Apr 2017 05:36:39 -
@@ -27,6 +27,8 @@
 
 #include "doas.h"
 
+#define VARNAME_MAX 1023
+
 struct envnode {
RB_ENTRY(envnode) node;
const char *key;
@@ -87,7 +89,7 @@ createenv(struct rule *rule)
struct envnode *node;
const char *e, *eq;
size_t len;
-   char name[1024];
+   char name[VARNAME_MAX + 1];
 
e = environ[i];
 
@@ -95,7 +97,7 @@ createenv(struct rule *rule)
if ((eq = strchr(e, '=')) == NULL || eq == e)
continue;
len = eq - e;
-   if (len > sizeof(name) - 1)
+   if (len > VARNAME_MAX)
continue;
memcpy(name, e, len);
name[len] = '\0';
@@ -139,7 +141,7 @@ fillenv(struct env *env, const char **en
struct envnode *node, key;
const char *e, *eq;
const char *val;
-   char name[1024];
+   char name[VARNAME_MAX + 1];
u_int i;
size_t len;
 
@@ -151,7 +153,7 @@ fillenv(struct env *env, const char **en
len = strlen(e);
else
len = eq - e;
-   if (len > sizeof(name) - 1)
+   if (len > VARNAME_MAX)
continue;
memcpy(name, e, len);
name[len] = '\0';

Re: [patch 1/2] doas(1): Moved some parsing from env.c into parse.y

[bytevolcano]

Hello Philippe,

On Thu, 16 Mar 2017 10:19:12 -0400
Philippe Meunier  wrote:

> Ted Unangst wrote:
> >Did I get it backwards? If you have setenv { HERE= there }, your diff
> >changes behavior.  
> 
> Speaking from the peanut gallery here, but I find this syntax rather
> confusing and error-prone, especially for a security-related file
> such as doas.conf.  How about making the list of variables
> comma-separated instead of space-separated, so that the intent is
> clearer from the syntax?

Whilst I agree with this, I presume this would depend on the popular use
case. If Ted and co found the major use case to be just to pass
variables along like this:

 setenv { FOO BAR }

then a space-delimited list is fine, and the parser is simpler. As I
see it, this is also acceptable:

 setenv { "FOO=" "BAR" }

Adding a comma-separated list will be similar to what I did to move the
'=' processing into the lexer. It's possible, but it does add some code
to the lexer and doesn't allow for much refactoring, unless you also
have command line args like this:

 ... cmd hello args foo, bar

It also means that command-line arguments with commas in them will need
to be enclosed in quote marks.

I have had a lot of time to think about this. Since equal sign
processing is needed anyway in env.c to process the strings in the
"environ" variable (filled in by the OS), might as well keep it all in
one place.

Re: regarding OpenSSL License change

[bytevolcano]

On Thu, 23 Mar 2017 20:51:06 -0600
"Theo de Raadt"  wrote:

> Dude, you are being melodramatic
> 
> it is great that someone found a way to convert between licenses.
> 
> AGPL -> GPL -> ISC -> PD
> 
> thumbs up to the people who found a shortcut
> 

Now this is genius.

Re: regarding OpenSSL License change

[bytevolcano]

...

> If we do not hear from you, we will assume that you have no objection.

So, they will claim that, by not responding, the recipient agreed.

Some jurisdictions I am aware of accept verbal contracts or this kind
of written contracts, since civil proceedings will not be held up to a
high standard of proof. Even then, there must have been evidence of a
contractual agreement, ie. no response = no agreement.

I say the lawyers are now working to prove that no response means the
potential recipient agreed.

If this email has been caught by enough spam filters, they will claim
the majority agreed.

Re: [patch 1/2] doas(1): Moved some parsing from env.c into parse.y

[bytevolcano]

On Wed, 15 Mar 2017 20:15:26 -0400
"Ted Unangst"  wrote:

> Did I get it backwards? If you have setenv { HERE= there }, your diff
> changes behavior.
> 

Now I see where you are coming from. It's meant to empty $HERE, and copy
$there. Thanks, I'll look into this.

Re: [patch 1/2] doas(1): Moved some parsing from env.c into parse.y

[bytevolcano]

On Wed, 15 Mar 2017 13:22:53 -0400
"Ted Unangst"  wrote:
...
> > 1. Loops - Use C99 style initialisation?
> > for(int i = 0; i < monsters; i < 666)  
> 
> No.
> 
> > 3. Anything of major concern.  
> 
> Adding a new character to the lexer potentially breaks existing
> config files.

I was considering that, considering that maybe someone will want
"foo=bar" parameters in their command lines.
Which is why I am thinking it is best to strike here while the iron is
hot rather than, say, 5 years later, when people have come up with
complex and tuned-over-the-years configurations. Not something difficult to
fix either, just enclose in quotes or escape, as necessary with curly
braces.

There is a way around this (such as not moving to "eow" on special
characters, when the lexer is in a 'command-line argument-parsing
mode') but that may require code added to the lexer. How much
extra code is needed, I don't know.

This function of parsing is the lexer's domain, and seems neater than
having what is essentially two separate lexers; all I am doing here is
merging the "mini-lexer" into the main lexer.

> 
> Also, I'm not sure how you tested this, because it doesn't work like
> you say it does. setenv { HERE=there } copies both $HERE and $there
> from the current environment. It does not set HERE=there. That's very
> wrong.
> 

How are you getting that result? Here is what I am getting:

$ export HERE=hello
$ export there=world
$ echo $HERE $there
hello world
$ doas ~/test.sh
HERE is there
there is

The shell script:

#!/bin/sh
echo "HERE" is $HERE
echo "there" is $there

And /etc/doas.conf:

permit persist setenv { HERE=there } :wheel
permit nopass keepenv root

This test behaves correctly on my end.

[patch 1/2] doas(1): Moved some parsing from env.c into parse.y

[bytevolcano]

Instead of having to use another 1KB buffer just to figure out where
the '=' sign is, use the parser infrastructure already present. Also
allows for variable timeout option. Another (negligible) advantage: if
one day someone wants to change the maximum line length, they only need
to do so in one spot.

I haven't touched the code in env.c that handles the '-' and '$' since
these are extremely simple anyway, and aren't needed for the next patch.

This patch doesn't need changes to doas(1) or doas.conf(5) man pages, but there
is no mention of characters that are required to be escaped when specifying 
args.
Tested with all kinds of variable names and values, works fine:

... keepenv { FIRE SWITCH=1 NO_PLACE_LIKE=$HOME WATER}

Things I would like feedback on, in particular:

1. Loops - Use C99 style initialisation?
for(int i = 0; i < monsters; i < 666)
...

2. Initialisation of arrays of two-member structs, where one member
   is initially set to NULL:

struct envar safeset[] = {{"DISPLAY"}, {"HOME"}, ...};
   or
struct envar safeset[] = {{"DISPLAY",NULL}, {"HOME",NULL}, ...};

3. Anything of major concern.

 - - - - - - - - - - - - - - - - - 

Index: usr.bin/doas/doas.h
===
RCS file: /cvs/src/usr.bin/doas/doas.h,v
retrieving revision 1.12
diff -u -p -r1.12 doas.h
--- usr.bin/doas/doas.h 5 Oct 2016 17:40:25 -   1.12
+++ usr.bin/doas/doas.h 15 Mar 2017 15:11:31 -
@@ -15,6 +15,11 @@
  * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
  */
 
+struct envar {
+   const char *name;
+   const char *value;
+};
+
 struct rule {
int action;
int options;
@@ -22,7 +27,7 @@ struct rule {
const char *target;
const char *cmd;
const char **cmdargs;
-   const char **envlist;
+   struct envar *envlist;
 };
 
 extern struct rule **rules;
Index: usr.bin/doas/env.c
===
RCS file: /cvs/src/usr.bin/doas/env.c,v
retrieving revision 1.5
diff -u -p -r1.5 env.c
--- usr.bin/doas/env.c  15 Sep 2016 00:58:23 -  1.5
+++ usr.bin/doas/env.c  15 Mar 2017 15:11:31 -
@@ -134,51 +134,39 @@ flattenenv(struct env *env)
 }
 
 static void
-fillenv(struct env *env, const char **envlist)
+fillenv(struct env *env, const struct envar *envlist)
 {
struct envnode *node, key;
-   const char *e, *eq;
+   struct envar ev;
const char *val;
-   char name[1024];
u_int i;
-   size_t len;
 
-   for (i = 0; envlist[i]; i++) {
-   e = envlist[i];
-
-   /* parse out env name */
-   if ((eq = strchr(e, '=')) == NULL)
-   len = strlen(e);
-   else
-   len = eq - e;
-   if (len > sizeof(name) - 1)
-   continue;
-   memcpy(name, e, len);
-   name[len] = '\0';
+   for (i = 0; envlist[i].name; i++) {
+   ev = envlist[i];
 
/* delete previous copies */
-   key.key = name;
-   if (*name == '-')
-   key.key = name + 1;
+   key.key = ev.name;
+   if (*ev.name == '-')
+   key.key = ev.name + 1;
if ((node = RB_FIND(envtree, >root, ))) {
RB_REMOVE(envtree, >root, node);
freenode(node);
env->count--;
}
-   if (*name == '-')
+   if (*ev.name == '-')
continue;
 
/* assign value or inherit from environ */
-   if (eq) {
-   val = eq + 1;
+   if (ev.value) {
+   val = ev.value;
if (*val == '$')
val = getenv(val + 1);
} else {
-   val = getenv(name);
+   val = getenv(ev.name);
}
/* at last, we have something to insert */
if (val) {
-   node = createnode(name, val);
+   node = createnode(ev.name, val);
RB_INSERT(envtree, >root, node);
env->count++;
}
@@ -188,10 +176,10 @@ fillenv(struct env *env, const char **en
 char **
 prepenv(struct rule *rule)
 {
-   static const char *safeset[] = {
-   "DISPLAY", "HOME", "LOGNAME", "MAIL",
-   "PATH", "TERM", "USER", "USERNAME",
-   NULL
+   static const struct envar safeset[] = {
+   {"DISPLAY"}, {"HOME"}, {"LOGNAME"}, {"MAIL"},
+   {"PATH"}, {"TERM"}, {"USER"}, {"USERNAME"},
+   {NULL}
};
struct env *env;
 
Index: usr.bin/doas/parse.y

Re: newsyslog timestamp

[bytevolcano]

Is it worth making newsyslog use the syslog API instead of directly
writing to the top of the file?

Perhaps even integrating this functionality into syslogd so it can use
the existing infrastructure, and LOG_SYSLOG facility, fork+privsep
style:

[syslogd]->|->[syslogd:logger]
   |->[syslogd:rotate]


On Mon, 13 Mar 2017 16:31:35 -0600
"Todd C. Miller"  wrote:

> How about we just eliminate the "logfile turned over" message
> entirely?  It's kind of bogus for newsyslog to be writing to the
> log files directly.  I don't think that message provides any useful
> info.
> 
>  - todd
> 

Re: syspatch(8): How to install patches?

[bytevolcano]

On Wed, 9 Nov 2016 09:58:51 +0100
Antoine Jacoutot  wrote:

> On Wed, Nov 09, 2016 at 07:12:38PM +1100, bytevolc...@safe-mail.net
> wrote:
> > Hello,
> > 
> > In the manual page for syspatch(8), it provides a list of options,
> > yet does not specify how to invoke it for fetching and installing
> > the latest patches.
> > 
> > To avoid this confusion, it is worth adding a line like this:
> > 
> > "When invoked without options, syspatch(8) will fetch and install
> > the latest batch of patches."  
> 
> Yes. I have a few uncommitted things.
> But it's still a moving target and cannot be used right now by most
> people anyway. Thanks.
> 

Thank you for the response Antoine.

I understand it is a moving target, but I suppose where I was coming
from is, if you are going to document it in its current state in the
manual, why not document the bits that are fixed? Or is the "running
without arguments" thing likely to change?

syspatch(8): How to install patches?

[bytevolcano]

Hello,

In the manual page for syspatch(8), it provides a list of options, yet
does not specify how to invoke it for fetching and installing the
latest patches.

To avoid this confusion, it is worth adding a line like this:

"When invoked without options, syspatch(8) will fetch and install the
latest batch of patches."

Re: OFW/FDT regulator API

[bytevolcano]

I have to admit, it's a bit shocking that VOLTAGE REGULATORS have to be 
exposed to the software in the first place.
Just imagine a bug in some OS or firmware causing the voltages to spike 
up and fry the hell out of a device.


I guess that's modern-day hardware design for you.

Mark Kettenis wrote:

The diff below adds a simple "regulator" API, Regulators are devices
that apply voltage and/or current to subsystems to power them on.
Examples are applying voltage to an SD card, powering a USB bus,
turning on an Ethernet PHY, scaling the CPU voltage.  Regulators come
in many flavours.  The simples ones are simple gpio pins on which you
drive a voltage.  More complex ones are typically special power
control chips.

This diff only adds support for the "regulator-fixed" type.  This is a
simple gpio-based regulator that can only apply a fixed voltage.  It
doesn't need a driver of its own as it interacts with the hardware
through the gpio and pinctrl APIs.

In the future, when we need to add support for more complex types that
need their own driver, those drivers will register themselves in a
similar way to gpio controllers and pinctrl devices.  Ten we'll also
have to implement functions to set specific voltages and such.

ok?


Index: dev/ofw/ofw_regulator.c
===
RCS file: dev/ofw/ofw_regulator.c
diff -N dev/ofw/ofw_regulator.c
--- /dev/null   1 Jan 1970 00:00:00 -
+++ dev/ofw/ofw_regulator.c 12 Aug 2016 20:50:19 -
@@ -0,0 +1,75 @@
+/* $OpenBSD$   */
+/*
+ * Copyright (c) 2016 Mark Kettenis
+ *
+ * Permission to use, copy, modify, and distribute this software for any
+ * purpose with or without fee is hereby granted, provided that the above
+ * copyright notice and this permission notice appear in all copies.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
+ * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
+ * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
+ * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
+ * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
+ * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
+ * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
+ */
+
+#include 
+#include 
+
+#include 
+#include 
+#include 
+#include 
+
+int
+regulator_set(uint32_t phandle, int enable)
+{
+   uint32_t gpio[3];
+   uint32_t startup_delay;
+   int active;
+   int node;
+
+   node = OF_getnodebyphandle(phandle);
+   if (node == 0)
+   return -1;
+
+   if (!OF_is_compatible(node, "regulator-fixed"))
+   return -1;
+
+   pinctrl_byname(node, "default");
+
+   if (OF_getproplen(node, "enable-active-high") == 0)
+   active = 1;
+   else
+   active = 0;
+
+   if (OF_getpropintarray(node, "gpio", gpio,
+   sizeof(gpio)) != sizeof(gpio))
+   return -1;
+
+   gpio_controller_config_pin(gpio, GPIO_CONFIG_OUTPUT);
+   if (enable)
+   gpio_controller_set_pin(gpio, active);
+   else
+   gpio_controller_set_pin(gpio, !active);
+
+   startup_delay = OF_getpropint(node, "startup-delay-us", 0);
+   if (enable && startup_delay > 0)
+   delay(startup_delay);
+
+   return 0;
+}
+
+int
+regulator_enable(uint32_t phandle)
+{
+   return regulator_set(phandle, 1);
+}
+
+int
+regulator_disable(uint32_t phandle)
+{
+   return regulator_set(phandle, 0);
+}
Index: dev/ofw/ofw_regulator.h
===
RCS file: dev/ofw/ofw_regulator.h
diff -N dev/ofw/ofw_regulator.h
--- /dev/null   1 Jan 1970 00:00:00 -
+++ dev/ofw/ofw_regulator.h 12 Aug 2016 20:50:19 -
@@ -0,0 +1,24 @@
+/* $OpenBSD$   */
+/*
+ * Copyright (c) 2016 Mark Kettenis
+ *
+ * Permission to use, copy, modify, and distribute this software for any
+ * purpose with or without fee is hereby granted, provided that the above
+ * copyright notice and this permission notice appear in all copies.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
+ * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
+ * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
+ * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
+ * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
+ * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
+ * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
+ */
+
+#ifndef _DEV_OFW_REGULATOR_H_
+#define _DEV_OFW_REGULATOR_H_
+
+intregulator_enable(uint32_t);
+intregulator_disable(uint32_t);
+
+#endif /* _DEV_OFW_GPIO_H_ */

Re: ssl(8) kill "generating dsa server certificates"

[bytevolcano]

I see. I guess people who really need DSA keys can figure
this out from the general format of the other sections.

Thanks for putting this into perspective.

On Mon, 6 Jun 2016 14:57:54 +0100
Stuart Henderson  wrote:

> On 2016/06/06 23:52, bytevolc...@safe-mail.net wrote:
> > Whilst not a developer per se, I don't see any reason why omitting
> > the documentation for a feature that is present is a good idea,
> > unless the feature is/will be removed.  
> 
> The documentation is still in openssl(8).
> 
> ssl(8) is a higher-level "how to setup TLS for your web/etc server"
> and including non-useful things there just serves to confuse people.
> 

Re: ssl(8) kill "generating dsa server certificates"

[bytevolcano]

Whilst not a developer per se, I don't see any reason why omitting the
documentation for a feature that is present is a good idea, unless the
feature is/will be removed.

Perhaps just something like this:

Index: ssl.8
===
RCS file: /cvs/src/share/man/man8/ssl.8,v
retrieving revision 1.63
diff -u -p -r1.63 ssl.8
--- ssl.8   8 Feb 2016 19:29:58 -   1.63
+++ ssl.8   6 Jun 2016 12:38:26 -
@@ -112,38 +112,6 @@ you can switch to using the new certific
 with the certificate signed by your Certificate Authority, and then
 restarting
 .Xr httpd 8 .
-.Sh GENERATING DSA SERVER CERTIFICATES
+.Sh GENERATING DSA SERVER CERTIFICATES (INSECURE; NOT RECOMMENDED!)

On Mon, 6 Jun 2016 13:40:00 +0100
Stuart Henderson  wrote:

> I don't think we should be encouraging anyone to do this...ok?
> 
> Index: ssl.8
> ===
> RCS file: /cvs/src/share/man/man8/ssl.8,v
> retrieving revision 1.63
> diff -u -p -r1.63 ssl.8
> --- ssl.8 8 Feb 2016 19:29:58 -   1.63
> +++ ssl.8 6 Jun 2016 12:38:26 -
> @@ -112,38 +112,6 @@ you can switch to using the new certific
>  with the certificate signed by your Certificate Authority, and then
>  restarting
>  .Xr httpd 8 .
> -.Sh GENERATING DSA SERVER CERTIFICATES
> -Generating a DSA certificate involves several steps.
> -First, generate parameters for DSA keys.
> -The following command will generate 1024-bit keys:
> -.Bd -literal -offset indent
> -# openssl dsaparam 1024 -out dsa1024.pem
> -.Ed
> -.Pp
> -Once you have the DSA parameters generated, you can generate a
> -CSR and unencrypted private key using the command:
> -.Bd -literal -offset indent
> -# openssl req -nodes -newkey dsa:dsa1024.pem \e
> -  -out /etc/ssl/dsacert.csr -keyout /etc/ssl/private/dsakey.pem
> -.Ed
> -.Pp
> -To generate an encrypted private key, you would use:
> -.Bd -literal -offset indent
> -# openssl req -newkey dsa:dsa1024.pem \e
> -  -out /etc/ssl/dsacert.csr -keyout /etc/ssl/private/dsakey.pem
> -.Ed
> -.Pp
> -This
> -.Pa server.csr
> -file can then be given to a CA who will sign the key.
> -.Pp
> -You can also sign the key yourself, using the command:
> -.Bd -literal -offset indent
> -# openssl x509 -sha256 -req -days 365 \e
> -  -in /etc/ssl/private/dsacert.csr \e
> -  -signkey /etc/ssl/private/dsacert.key \e
> -  -out /etc/ssl/dsacert.crt
> -.Ed
>  .Sh GENERATING ECDSA SERVER CERTIFICATES
>  First, generate parameters for ECDSA keys.
>  The following command will use a NIST/SECG curve over a 384-bit
> 

Re: [PATCH] Allow softraid crypto to work with write-protected keys

[bytevolcano]

ping

On Wed, 18 May 2016 12:14:50 +0100
bytevolc...@safe-mail.net wrote:

> My apologies for the noise; the previous one was the wrong revision
> (r1.126 instead of 1.127) because both patches look similar; here is
> the most recent catch:
> 
> Index: sys/dev/softraid_crypto.c
> ===
> RCS file: /cvs/src/sys/dev/softraid_crypto.c,v
> retrieving revision 1.127
> diff -u -p -r1.127 softraid_crypto.c
> --- sys/dev/softraid_crypto.c 17 May 2016 19:28:59
> - 1.127 +++ sys/dev/softraid_crypto.c 18 May 2016
> 10:26:32 - @@ -795,7 +795,7 @@ sr_crypto_read_key_disk(struct
> sr_discip sr_error(sc, "cannot open key disk %s", devname);
>   goto done;
>   }
> - if (VOP_OPEN(vn, FREAD | FWRITE, NOCRED, curproc)) {
> + if (VOP_OPEN(vn, FREAD, NOCRED, curproc)) {
>   DNPRINTF(SR_D_META,"%s: sr_crypto_read_key_disk
> cannot " "open %s\n", DEVNAME(sc), devname);
>   vput(vn);
> @@ -809,8 +809,6 @@ sr_crypto_read_key_disk(struct sr_discip
>   NOCRED, curproc)) {
>   DNPRINTF(SR_D_META, "%s: sr_crypto_read_key_disk
> ioctl " "failed\n", DEVNAME(sc));
> - VOP_CLOSE(vn, FREAD | FWRITE, NOCRED, curproc);
> - vput(vn);
>   goto done;
>   }
>   if (label.d_partitions[part].p_fstype != FS_RAID) {
> 

Re: [PATCH] Allow softraid crypto to work with write-protected keys

[bytevolcano]

My apologies for the noise; the previous one was the wrong revision (r1.126 
instead of 1.127) because both patches look similar; here is the most recent 
catch:

Index: sys/dev/softraid_crypto.c
===
RCS file: /cvs/src/sys/dev/softraid_crypto.c,v
retrieving revision 1.127
diff -u -p -r1.127 softraid_crypto.c
--- sys/dev/softraid_crypto.c   17 May 2016 19:28:59 -  1.127
+++ sys/dev/softraid_crypto.c   18 May 2016 10:26:32 -
@@ -795,7 +795,7 @@ sr_crypto_read_key_disk(struct sr_discip
sr_error(sc, "cannot open key disk %s", devname);
goto done;
}
-   if (VOP_OPEN(vn, FREAD | FWRITE, NOCRED, curproc)) {
+   if (VOP_OPEN(vn, FREAD, NOCRED, curproc)) {
DNPRINTF(SR_D_META,"%s: sr_crypto_read_key_disk cannot "
"open %s\n", DEVNAME(sc), devname);
vput(vn);
@@ -809,8 +809,6 @@ sr_crypto_read_key_disk(struct sr_discip
NOCRED, curproc)) {
DNPRINTF(SR_D_META, "%s: sr_crypto_read_key_disk ioctl "
"failed\n", DEVNAME(sc));
-   VOP_CLOSE(vn, FREAD | FWRITE, NOCRED, curproc);
-   vput(vn);
goto done;
}
if (label.d_partitions[part].p_fstype != FS_RAID) {

Re: [PATCH] Allow softraid crypto to work with write-protected keys

[bytevolcano]

Ted Unangst wrote:
> i removed these last two lines, since they were incorrect. thanks for spotting
> that. however the vop_close at the end still needs updating.

Thanks Ted,

Also I found another stray VOP_CLOSE() setup.

I have also put in the VOP_OPEN(vn, FREAD, ...) as I am not sure about 100% 
clear on what you mean by "the vop_close at the end still needs updating."

Do you mean it needs to actually be called with VOP_CLOSE(vn, FREAD | FWRITE, 
...)?

I am also confused by the opening of the key disk in read-write mode, when 
there doesn't seem to be any obvious need to write to it; what bit of metadata 
needs to be updated? All I see it doing is preventing the possibility of using 
write-protected key disks.


Index: sys/dev/softraid_crypto.c
===
RCS file: /cvs/src/sys/dev/softraid_crypto.c,v
retrieving revision 1.126
diff -u -p -r1.126 softraid_crypto.c
--- sys/dev/softraid_crypto.c   12 Apr 2016 16:26:54 -  1.126
+++ sys/dev/softraid_crypto.c   17 May 2016 10:13:02 -
@@ -797,7 +797,7 @@ sr_crypto_read_key_disk(struct sr_discip
sr_error(sc, "cannot open key disk %s", devname);
goto done;
}
-   if (VOP_OPEN(vn, FREAD | FWRITE, NOCRED, curproc)) {
+   if (VOP_OPEN(vn, FREAD, NOCRED, curproc)) {
DNPRINTF(SR_D_META,"%s: sr_crypto_read_key_disk cannot "
"open %s\n", DEVNAME(sc), devname);
vput(vn);
@@ -811,8 +811,6 @@ sr_crypto_read_key_disk(struct sr_discip
NOCRED, curproc)) {
DNPRINTF(SR_D_META, "%s: sr_crypto_read_key_disk ioctl "
"failed\n", DEVNAME(sc));
-   VOP_CLOSE(vn, FREAD | FWRITE, NOCRED, curproc);
-   vput(vn);
goto done;
}
if (label.d_partitions[part].p_fstype != FS_RAID) {

Re: [PATCH] Allow softraid crypto to work with write-protected keys

[bytevolcano]

I have also noticed that bioctl reports the key disk as
incorrect. I initially thought it was my patch, but it seems to be wrong
whether or not my patch is applied. In fact the contents of the disk do
not change at all:

# bioctl softraid0
Volume  Status  Size Device
softraid0 0 Online   42197783040 sd3 CRYPTO
  0 Online   42197783040 0:0.0   noencl 
softraid0 1 Online   33082493440 sd4 CRYPTO
  0 Online   33082493440 1:0.0   noencl 
  1 Online  key disk 1:1.0   noencl 
softraid0 2 Online   42199204864 sd5 CRYPTO
  0 Online   42199204864 2:0.0   noencl 
  1 Online  key disk 2:1.0   noencl 

The key disk I use is at sd1 for all volumes mounted (though the chunks
used [sdXa/sdXe/sdXd] are reported correctly).

Tested with the unpatched GENERIC and GENERIC.MP kernel (15/5/16), and
with the write-protect switch off.

I don't really understand why the "sdX" device name is embedded in the
key anyway, since this is likely to change as USB devices are plugged
in, etc. I would greatly appreciate advise/feedback from a dev involved
with softraid on that one.


Improved version of the patch, removes the VOP_CLOSE() that appears to
now be redundant:

Index: sys/dev/softraid_crypto.c
===
RCS file: /cvs/src/sys/dev/softraid_crypto.c,v
retrieving revision 1.126
diff -u -p -r1.126 softraid_crypto.c
--- sys/dev/softraid_crypto.c   12 Apr 2016 16:26:54 -
1.126 +++ sys/dev/softraid_crypto.c 17 May 2016 10:13:02 -
@@ -797,7 +797,7 @@ sr_crypto_read_key_disk(struct sr_discip
sr_error(sc, "cannot open key disk %s", devname);
goto done;
}
-   if (VOP_OPEN(vn, FREAD | FWRITE, NOCRED, curproc)) {
+   if (VOP_OPEN(vn, FREAD, NOCRED, curproc)) {
DNPRINTF(SR_D_META,"%s: sr_crypto_read_key_disk cannot
" "open %s\n", DEVNAME(sc), devname);
vput(vn);
@@ -811,8 +811,6 @@ sr_crypto_read_key_disk(struct sr_discip
NOCRED, curproc)) {
DNPRINTF(SR_D_META, "%s: sr_crypto_read_key_disk ioctl
" "failed\n", DEVNAME(sc));
-   VOP_CLOSE(vn, FREAD | FWRITE, NOCRED, curproc);
-   vput(vn);
goto done;
}
if (label.d_partitions[part].p_fstype != FS_RAID) {

[PATCH] Allow softraid crypto to work with write-protected keys

[bytevolcano]

Softraid currently opens the key disk as read + write. This isn't
necessary when just *reading* from the key disk.

This patch allows for softraid crypto to mount volumes with
write-protected keydisks (eg. Kanguru Flash Blu:
https://www.kanguru.com/storage-accessories/flash-blu30.shtml).

Also tested volume creation with the Flash Blu write-protect switch off;
still works as usual.


Index: sys/dev/softraid_crypto.c
===
RCS file: /cvs/src/sys/dev/softraid_crypto.c,v
retrieving revision 1.126
diff -u -p -r1.126 softraid_crypto.c
--- sys/dev/softraid_crypto.c   12 Apr 2016 16:26:54 -
1.126 +++ sys/dev/softraid_crypto.c 17 May 2016 04:18:52 -
@@ -797,7 +797,7 @@ sr_crypto_read_key_disk(struct sr_discip
sr_error(sc, "cannot open key disk %s", devname);
goto done;
}
-   if (VOP_OPEN(vn, FREAD | FWRITE, NOCRED, curproc)) {
+   if (VOP_OPEN(vn, FREAD, NOCRED, curproc)) {
DNPRINTF(SR_D_META,"%s: sr_crypto_read_key_disk cannot
" "open %s\n", DEVNAME(sc), devname);
vput(vn);
@@ -811,7 +811,7 @@ sr_crypto_read_key_disk(struct sr_discip
NOCRED, curproc)) {
DNPRINTF(SR_D_META, "%s: sr_crypto_read_key_disk ioctl
" "failed\n", DEVNAME(sc));
-   VOP_CLOSE(vn, FREAD | FWRITE, NOCRED, curproc);
+   VOP_CLOSE(vn, FREAD, NOCRED, curproc);
vput(vn);
goto done;
}

Re: www.openbsd.org/cgi-bin/man.cgi

[bytevolcano]

I was thinking more of having a Javascript implementation of OpenBSD so
that you could type the 'man' command to get the page you are after.

It will only increase the size of the page by about 400MB, and hard
disk and CPU space is cheap.

And everyone has infinite bandwidth internet.

On Fri, 25 Mar 2016 09:24:29 -0600
Bob Beck  wrote:

> Now now, we can be more hipster than that.. a docker image that runs
> the man command for you after downloading
> all the openbsd man pages as a dependency - you can just deploy it
> automatically with vagrant and run it in AWS.. etc.
> 
> After all, isn't there no simple command that can't be made better by
> installing an os image someone else built to run?
> 
> 
> 
> 
> On Fri, Mar 25, 2016 at 8:45 AM, Ingo Schwarze 
> wrote:
> > Hi Gilles,
> >
> > Gilles Chehade wrote on Fri, Mar 25, 2016 at 03:34:02PM +0100:
> >  
> >> maybe we could provide MaaS (man as a service, copyright eric@)
> >>
> >> if user issues `man` and the man page is not found locally, man
> >> would transparently ssh to gu...@man.openbsd.org ?  
> >
> > Hilarious on so many levels...  :-D
> >   Ingo
> >  
> 

Re: [PATCH] mount_tmpfs -P option for populating after mounting (simplified)

[bytevolcano]

ping.

On Sun, 13 Sep 2015 14:50:19 +0100
bytevolc...@safe-mail.net wrote:

> Hello,
> 
> Although a patch like this was submitted before, this one is more
> "focused". It may be useful for those moving from MFS who use this
> option to pre-populate an MFS mount with a collection of files. This
> will refuse to work if the path specified for -P is not a directory.
> If the copy is unsuccessful, unmount and inform the user.
> 
> A few questions on this though:
> 
> 1. Is there a better way than executing pax, to copy all the files in
> a directory?
> 
> 2. The do_exec() function was sort of copied from sbin/newfs/newfs.c.
> It doesn't feel like the best way of handling this, but some research
> suggests it is better than calling system() due to security issues,
> one that springs to mind is a malformed input directory to the -P
> option. Is there a simple way to sanitise the input for use with
> system()?
> 
> 3. I noticed a pattern of functions that have about 2-3 lines of code
> in them. Is it best to keep things that way, or is it best to merge
> them into a single function (eg. copy_dir() merged into the
> mount_tmpfs() function)?
> 
> 4. Are variable declarations to be at the beginning of a function
> block, or the beginning of the immediate block? I feel limiting scope
> is a good idea, but would like some feedback on this as I cannot see
> anything in style(9).
> 
> 5. Is mount_tmpfs.h really necessary, given nothing in the tree seems
> to use anything related to the mount_tmpfs() function which is
> internal to the mount_tmpfs binary?
> 
> 
>  -
> 
> Index: sbin/mount_tmpfs/mount_tmpfs.8
> ===
> RCS file: /cvs/src/sbin/mount_tmpfs/mount_tmpfs.8,v
> retrieving revision 1.4
> diff -u -p -r1.4 mount_tmpfs.8
> --- sbin/mount_tmpfs/mount_tmpfs.816 Nov 2014 02:22:10
> - 1.4 +++ sbin/mount_tmpfs/mount_tmpfs.8  13 Sep
> 2015 13:03:34 - @@ -41,6 +41,7 @@
>  .Op Fl m Ar mode
>  .Op Fl n Ar nodes
>  .Op Fl o Ar options
> +.Op Fl P Ar directory
>  .Op Fl s Ar size
>  .Op Fl u Ar user
>  .Ar tmpfs
> @@ -80,6 +81,8 @@ flag followed by a comma-separated strin
>  See the
>  .Xr mount 8
>  man page for possible options and their meanings.
> +.It Fl P Ar directory
> +Populate the created tmpfs file system with the contents of the
> directory. .It Fl s Ar size
>  Specifies the total file system size in bytes.
>  If zero is given (the default), the available amount of memory
> (including @@ -136,6 +139,10 @@ and
>  .Ox 5.5 .
>  .Sh CAVEATS
>  The update of mount options (through mount -u) is currently not
> supported. +The
> +.Fl P
> +option will produce an error if the mount is read-only, or if the
> files +cannot be copied from the specified template directory.
>  .Sh BUGS
>  File system meta-data is not pageable.
>  If there is not enough main memory to hold this information, the
> system may Index: sbin/mount_tmpfs/mount_tmpfs.c
> ===
> RCS file: /cvs/src/sbin/mount_tmpfs/mount_tmpfs.c,v
> retrieving revision 1.5
> diff -u -p -r1.5 mount_tmpfs.c
> --- sbin/mount_tmpfs/mount_tmpfs.c16 Jan 2015 06:39:59
> - 1.5 +++ sbin/mount_tmpfs/mount_tmpfs.c  13 Sep
> 2015 13:03:34 - @@ -34,6 +34,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  
>  #include 
>  #include 
> @@ -41,6 +42,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -68,12 +70,15 @@ static inta_num(const char *, const cha
>  static mode_ta_mask(const char *);
>  static void  pathadj(const char *, char *);
>  
> +static int do_exec(const char *, const char *, char *const[]);
> +static int copy_dir(char *, char *);
> +
>  /*
> -
> */ void
>  mount_tmpfs_parseargs(int argc, char *argv[],
>   struct tmpfs_args *args, int *mntflags,
> - char *canon_dev, char *canon_dir)
> + char *canon_dev, char *canon_dir, char *pop_dir)
>  {
>   int gidset, modeset, uidset; /* Ought to be 'bool'. */
>   int ch;
> @@ -95,7 +100,7 @@ mount_tmpfs_parseargs(int argc, char *ar
>   modeset = 0; mode = 0;
>  
>   optind = optreset = 1;
> - while ((ch = getopt(argc, argv, "g:m:n:o:s:u:")) != -1 ) {
> + while ((ch = getopt(argc, argv, "P:g:m:n:o:s:u:")) != -1 ) {
>   switch (ch) {
>   case 'g':
>   gid = a_gid(optarg);
> @@ -131,6 +136,10 @@ mount_tmpfs_parseargs(int argc, char *ar
>   uidset = 1;
>   break;
>  
> + case 'P':
> + strlcpy(pop_dir, optarg, PATH_MAX);
> + break;
> +
>   case '?':
>   default:
>   usage();
> @@ -161,7 +170,8 @@ usage(void)
>   extern char *__progname;
>   (void)fprintf(stderr,
>   "usage: %s [-g group] [-m mode] [-n nodes] [-o 

[PATCH] mount_tmpfs -P option for populating after mounting (simplified)

[bytevolcano]

Hello,

Although a patch like this was submitted before, this one is more "focused". It 
may be useful for those moving from MFS who use this option to pre-populate an 
MFS mount with a collection of files.
This will refuse to work if the path specified for -P is not a directory.
If the copy is unsuccessful, unmount and inform the user.

A few questions on this though:

1. Is there a better way than executing pax, to copy all the files in a 
directory?

2. The do_exec() function was sort of copied from sbin/newfs/newfs.c. It 
doesn't feel like the best way of handling this, but some research suggests it 
is better than calling system() due to security issues, one that springs to 
mind is a malformed input directory to the -P option. Is there a simple way to 
sanitise the input for use with system()?

3. I noticed a pattern of functions that have about 2-3 lines of code in them. 
Is it best to keep things that way, or is it best to merge them into a single 
function (eg. copy_dir() merged into the mount_tmpfs() function)?

4. Are variable declarations to be at the beginning of a function block, or the 
beginning of the immediate block? I feel limiting scope is a good idea, but 
would like some feedback on this as I cannot see anything in style(9).

5. Is mount_tmpfs.h really necessary, given nothing in the tree seems to use 
anything related to the mount_tmpfs() function which is internal to the 
mount_tmpfs binary?


 -

Index: sbin/mount_tmpfs/mount_tmpfs.8
===
RCS file: /cvs/src/sbin/mount_tmpfs/mount_tmpfs.8,v
retrieving revision 1.4
diff -u -p -r1.4 mount_tmpfs.8
--- sbin/mount_tmpfs/mount_tmpfs.8  16 Nov 2014 02:22:10 -  1.4
+++ sbin/mount_tmpfs/mount_tmpfs.8  13 Sep 2015 13:03:34 -
@@ -41,6 +41,7 @@
 .Op Fl m Ar mode
 .Op Fl n Ar nodes
 .Op Fl o Ar options
+.Op Fl P Ar directory
 .Op Fl s Ar size
 .Op Fl u Ar user
 .Ar tmpfs
@@ -80,6 +81,8 @@ flag followed by a comma-separated strin
 See the
 .Xr mount 8
 man page for possible options and their meanings.
+.It Fl P Ar directory
+Populate the created tmpfs file system with the contents of the directory.
 .It Fl s Ar size
 Specifies the total file system size in bytes.
 If zero is given (the default), the available amount of memory (including
@@ -136,6 +139,10 @@ and
 .Ox 5.5 .
 .Sh CAVEATS
 The update of mount options (through mount -u) is currently not supported.
+The
+.Fl P
+option will produce an error if the mount is read-only, or if the files
+cannot be copied from the specified template directory.
 .Sh BUGS
 File system meta-data is not pageable.
 If there is not enough main memory to hold this information, the system may
Index: sbin/mount_tmpfs/mount_tmpfs.c
===
RCS file: /cvs/src/sbin/mount_tmpfs/mount_tmpfs.c,v
retrieving revision 1.5
diff -u -p -r1.5 mount_tmpfs.c
--- sbin/mount_tmpfs/mount_tmpfs.c  16 Jan 2015 06:39:59 -  1.5
+++ sbin/mount_tmpfs/mount_tmpfs.c  13 Sep 2015 13:03:34 -
@@ -34,6 +34,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include 
 #include 
@@ -41,6 +42,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -68,12 +70,15 @@ static int  a_num(const char *, const cha
 static mode_t  a_mask(const char *);
 static voidpathadj(const char *, char *);
 
+static int do_exec(const char *, const char *, char *const[]);
+static int copy_dir(char *, char *);
+
 /* - */
 
 void
 mount_tmpfs_parseargs(int argc, char *argv[],
struct tmpfs_args *args, int *mntflags,
-   char *canon_dev, char *canon_dir)
+   char *canon_dev, char *canon_dir, char *pop_dir)
 {
int gidset, modeset, uidset; /* Ought to be 'bool'. */
int ch;
@@ -95,7 +100,7 @@ mount_tmpfs_parseargs(int argc, char *ar
modeset = 0; mode = 0;
 
optind = optreset = 1;
-   while ((ch = getopt(argc, argv, "g:m:n:o:s:u:")) != -1 ) {
+   while ((ch = getopt(argc, argv, "P:g:m:n:o:s:u:")) != -1 ) {
switch (ch) {
case 'g':
gid = a_gid(optarg);
@@ -131,6 +136,10 @@ mount_tmpfs_parseargs(int argc, char *ar
uidset = 1;
break;
 
+   case 'P':
+   strlcpy(pop_dir, optarg, PATH_MAX);
+   break;
+
case '?':
default:
usage();
@@ -161,7 +170,8 @@ usage(void)
extern char *__progname;
(void)fprintf(stderr,
"usage: %s [-g group] [-m mode] [-n nodes] [-o options] [-s size]\n"
-   "   [-u user] tmpfs mount_point\n", __progname);
+   "   [-P directory] [-u user] tmpfs mount_point\n",
+   __progname);
exit(1);
 }
 
@@ -172,14 +182,33 @@ mount_tmpfs(int argc, char *argv[])
 {

Re: doas with a timeout

[bytevolcano]

An easier and more reliable compromise would be running 'doas sh' and
executing multiple commands in the shell that is root.

Having said that, I am unsure if doas(1) uses the $HOME of the current
user, or the user that the command is executed as.
If $HOME is that of the current user, the advantage of using doas(1) in
this way, compared to plain old 'su', is that you get a shell running
as a particular user, while keeping the current environment.

I will miss that timeout feature too, but not for long.

On Mon, 27 Jul 2015 10:54:02 +0300
Gregory Edigarov ediga...@qarea.com wrote:

 Hi,
 
 sudo was having a nice feature of not overwhelming the user with 
 password prompts (cookies :-) ).
 
 This diff is adding this back to doas(1).
 
[snip]

Re: bioctl -d: accept DUIDs

[bytevolcano]

Whilst I mostly like this patch, and it appears to be much cleaner than
the bodgy solution I came up with a few years back, perhaps there is
something here worth mentioning:

 + snprintf(devname, sizeof(devname), %s%d, blkname,
 + DISKUNIT(bd-bd_dev));
   TAILQ_FOREACH(sd, sc-sc_dis_list, sd_link) {
 - if (!strncmp(sd-sd_meta-ssd_devname, bd-bd_dev,
 + if (!strncmp(sd-sd_meta-ssd_devname, devname,
   sizeof(sd-sd_meta-ssd_devname)))
   break;
   }

The output of snprintf() isn't checked for overflowing. Though I think
the possibility of anything greater than sd9 is low,
probably even zero, I'm still of the view that it is not a bad idea to
code defensively.

If the above case was indeed satisfied, you would get the wrong
device. sd1, missing a zero, for example.

Might be worth something like this:

if(snprintf(devname, sizeof(devname), %s%d, blkname,
DISKUNIT(bd-bd_dev)) = sizeof(devname))
/* error */

Something like that perhaps.

(disclaimer: I'm not a dev; I'm not sure if they would consider it
overkill or precaution).

mount_tmpfs(8) MINOR man page correction

[bytevolcano]

The manual page for mount_tmpfs(8) states that it was included in
OpenBSD 5.4, when the first -release with tmpfs was 5.5.

Corrected in the following diff:



Index: sbin/mount_tmpfs/mount_tmpfs.8
===
RCS file: /cvs/src/sbin/mount_tmpfs/mount_tmpfs.8,v
retrieving revision 1.3
diff -u -p -r1.3 mount_tmpfs.8
--- sbin/mount_tmpfs/mount_tmpfs.8  5 Feb 2014 15:32:26
-   1.3 +++ sbin/mount_tmpfs/mount_tmpfs.8  16 Nov 2014
01:44:22 - @@ -133,7 +133,7 @@ The
 utility first appeared in
 .Nx 4.0
 and
-.Ox 5.4 .
+.Ox 5.5 .
 .Sh CAVEATS
 The update of mount options (through mount -u) is currently not
supported. .Sh BUGS

[PATCH] Option for mount_tmpfs to populate the volume after creation.

[bytevolcano]

This patch adds an option -t template to mount_tmpfs, which
populates the new tmpfs volume with a directory
immediately after creation.

Man page update included for explanation.

Much of the code was grafted from newfs
which implements this for mount_mfs (-P option).

Suggestions, fixes, criticism, etc. welcome.

Index: mount_tmpfs.8
===
RCS file: /cvs/src/sbin/mount_tmpfs/mount_tmpfs.8,v
retrieving revision 1.3
diff -u -p -u -r1.3 mount_tmpfs.8
--- mount_tmpfs.8   5 Feb 2014 15:32:26 -   1.3
+++ mount_tmpfs.8   17 Sep 2014 21:37:43 -
@@ -42,6 +42,7 @@
 .Op Fl n Ar nodes
 .Op Fl o Ar options
 .Op Fl s Ar size
+.Op Fl t Ar template
 .Op Fl u Ar user
 .Ar tmpfs
 .Ar mount_point
@@ -86,10 +87,28 @@ If zero is given (the default), the avai
 main memory and swap space) will be used.
 Note that four megabytes are always reserved for the system and cannot
 be assigned to the file system.
+.It Fl t Ar template
+If
+.Ar template
+is a directory, populate the created mfs file system with the
+contents of the directory.
+If
+.Ar template
+is a block device, populate the created mfs file system with the
+contents of the FFS file system contained on the device.
 .It Fl u Ar user
 Specifies the user name or UID of the root inode of the file system.
 Defaults to the mount point's UID.
 .El
+.Pp
+When the
+.Fl t Ar template
+option is used, permissions are always copied from the template.
+The
+.Fl u , Fl g , 
+and
+.Fl m
+options only affect the root inode of the file system.
 .Pp
 Every option that accepts a numerical value as its argument can take a
 trailing
Index: mount_tmpfs.c
===
RCS file: /cvs/src/sbin/mount_tmpfs/mount_tmpfs.c,v
retrieving revision 1.4
diff -u -p -u -r1.4 mount_tmpfs.c
--- mount_tmpfs.c   21 Jan 2014 21:58:27 -  1.4
+++ mount_tmpfs.c   17 Sep 2014 21:37:43 -
@@ -39,6 +39,7 @@ __RCSID($NetBSD: mount_tmpfs.c,v 1.24 2
 #include sys/param.h
 #include sys/mount.h
 #include sys/stat.h
+#include sys/wait.h
 
 #include ctype.h
 #include err.h
@@ -51,8 +52,9 @@ __RCSID($NetBSD: mount_tmpfs.c,v 1.24 2
 #include string.h
 #include unistd.h
 #include util.h
+#include paths.h
 
-#include mount_tmpfs.h
+#include pathnames.h
 
 /* - */
 
@@ -74,11 +76,34 @@ static void pathadj(const char *, char *
 
 /* - */
 
-void
-mount_tmpfs_parseargs(int argc, char *argv[],
-   struct tmpfs_args *args, int *mntflags,
-   char *canon_dev, char *canon_dir)
+static int do_exec(const char *, const char *, char *const[]);
+static int isdir(const char *);
+static void tcopy(char *, char *, struct tmpfs_args *);
+static int gettmpmnt(char *, size_t);
+
+/* - */
+
+static void
+usage(void)
+{
+   extern char *__progname;
+   (void)fprintf(stderr,
+   usage: %s [-g group] [-m mode] [-n nodes] [-o options] [-s size]\n
+  [-u user] tmpfs mount_point\n, __progname);
+   exit(1);
+}
+
+/* - */
+
+int
+mount_tmpfs(int argc, char *argv[])
 {
+   struct tmpfs_args args = {0};
+   char canon_dev[MAXPATHLEN], canon_dir[MAXPATHLEN];
+   int mntflags = 0;
+   
+   char *template = NULL;
+
int gidset, modeset, uidset; /* Ought to be 'bool'. */
int ch;
gid_t gid;
@@ -88,18 +113,16 @@ mount_tmpfs_parseargs(int argc, char *ar
struct stat sb;
 
/* Set default values for mount point arguments. */
-   memset(args, 0, sizeof(*args));
-   args-ta_version = TMPFS_ARGS_VERSION;
-   args-ta_size_max = 0;
-   args-ta_nodes_max = 0;
-   *mntflags = 0;
+   args.ta_version = TMPFS_ARGS_VERSION;
+   args.ta_size_max = 0;
+   args.ta_nodes_max = 0;
 
gidset = 0; gid = 0;
uidset = 0; uid = 0;
modeset = 0; mode = 0;
 
optind = optreset = 1;
-   while ((ch = getopt(argc, argv, g:m:n:o:s:u:)) != -1 ) {
+   while ((ch = getopt(argc, argv, g:m:n:o:s:t:u:)) != -1 ) {
switch (ch) {
case 'g':
gid = a_gid(optarg);
@@ -116,18 +139,22 @@ mount_tmpfs_parseargs(int argc, char *ar
if (scan_scaled(optarg, tmpnumber) == -1)
err(EXIT_FAILURE, failed to parse nodes `%s',
optarg);
-   args-ta_nodes_max = tmpnumber;
+   args.ta_nodes_max = tmpnumber;
break;
 
case 'o':
-   getmntopts(optarg, mopts, mntflags);
+   getmntopts(optarg, mopts, mntflags);
break;
 
case 's':
 

Re: [PATCH] Option for mount_tmpfs to populate the volume after creation.

[bytevolcano]

In the mount_tmpfs.8 diff, you talk about mfs... :)

Landry

Serves me right for coding while I'm sick.

Revised diffbelow:




Index: mount_tmpfs.8
===
RCS file: /cvs/src/sbin/mount_tmpfs/mount_tmpfs.8,v
retrieving revision 1.3
diff -u -p -r1.3 mount_tmpfs.8
--- mount_tmpfs.8   5 Feb 2014 15:32:26 -   1.3
+++ mount_tmpfs.8   18 Sep 2014 08:22:42 -
@@ -42,6 +42,7 @@
 .Op Fl n Ar nodes
 .Op Fl o Ar options
 .Op Fl s Ar size
+.Op Fl t Ar template
 .Op Fl u Ar user
 .Ar tmpfs
 .Ar mount_point
@@ -86,10 +87,28 @@ If zero is given (the default), the avai
 main memory and swap space) will be used.
 Note that four megabytes are always reserved for the system and cannot
 be assigned to the file system.
+.It Fl t Ar template
+If
+.Ar template
+is a directory, populate the created tmpfs file system with the
+contents of the directory.
+If
+.Ar template
+is a block device, populate the created tmpfs file system with the
+contents of the FFS file system contained on the device.
 .It Fl u Ar user
 Specifies the user name or UID of the root inode of the file system.
 Defaults to the mount point's UID.
 .El
+.Pp
+When the
+.Fl t Ar template
+option is used, permissions are always copied from the template.
+The
+.Fl u , Fl g , 
+and
+.Fl m
+options only affect the root inode of the file system.
 .Pp
 Every option that accepts a numerical value as its argument can take a
 trailing
Index: mount_tmpfs.c
===
RCS file: /cvs/src/sbin/mount_tmpfs/mount_tmpfs.c,v
retrieving revision 1.4
diff -u -p -r1.4 mount_tmpfs.c
--- mount_tmpfs.c   21 Jan 2014 21:58:27 -  1.4
+++ mount_tmpfs.c   18 Sep 2014 08:22:42 -
@@ -39,6 +39,7 @@ __RCSID($NetBSD: mount_tmpfs.c,v 1.24 2
 #include sys/param.h
 #include sys/mount.h
 #include sys/stat.h
+#include sys/wait.h
 
 #include ctype.h
 #include err.h
@@ -51,8 +52,9 @@ __RCSID($NetBSD: mount_tmpfs.c,v 1.24 2
 #include string.h
 #include unistd.h
 #include util.h
+#include paths.h
 
-#include mount_tmpfs.h
+#include pathnames.h
 
 /* - */
 
@@ -74,11 +76,34 @@ static void pathadj(const char *, char *
 
 /* - */
 
-void
-mount_tmpfs_parseargs(int argc, char *argv[],
-   struct tmpfs_args *args, int *mntflags,
-   char *canon_dev, char *canon_dir)
+static int do_exec(const char *, const char *, char *const[]);
+static int isdir(const char *);
+static void tcopy(char *, char *, struct tmpfs_args *);
+static int gettmpmnt(char *, size_t);
+
+/* - */
+
+static void
+usage(void)
+{
+   extern char *__progname;
+   (void)fprintf(stderr,
+   usage: %s [-g group] [-m mode] [-n nodes] [-o options] [-s size]\n
+  [-u user] tmpfs mount_point\n, __progname);
+   exit(1);
+}
+
+/* - */
+
+int
+mount_tmpfs(int argc, char *argv[])
 {
+   struct tmpfs_args args = {0};
+   char canon_dev[MAXPATHLEN], canon_dir[MAXPATHLEN];
+   int mntflags = 0;
+   
+   char *template = NULL;
+
int gidset, modeset, uidset; /* Ought to be 'bool'. */
int ch;
gid_t gid;
@@ -88,18 +113,16 @@ mount_tmpfs_parseargs(int argc, char *ar
struct stat sb;
 
/* Set default values for mount point arguments. */
-   memset(args, 0, sizeof(*args));
-   args-ta_version = TMPFS_ARGS_VERSION;
-   args-ta_size_max = 0;
-   args-ta_nodes_max = 0;
-   *mntflags = 0;
+   args.ta_version = TMPFS_ARGS_VERSION;
+   args.ta_size_max = 0;
+   args.ta_nodes_max = 0;
 
gidset = 0; gid = 0;
uidset = 0; uid = 0;
modeset = 0; mode = 0;
 
optind = optreset = 1;
-   while ((ch = getopt(argc, argv, g:m:n:o:s:u:)) != -1 ) {
+   while ((ch = getopt(argc, argv, g:m:n:o:s:t:u:)) != -1 ) {
switch (ch) {
case 'g':
gid = a_gid(optarg);
@@ -116,18 +139,22 @@ mount_tmpfs_parseargs(int argc, char *ar
if (scan_scaled(optarg, tmpnumber) == -1)
err(EXIT_FAILURE, failed to parse nodes `%s',
optarg);
-   args-ta_nodes_max = tmpnumber;
+   args.ta_nodes_max = tmpnumber;
break;
 
case 'o':
-   getmntopts(optarg, mopts, mntflags);
+   getmntopts(optarg, mopts, mntflags);
break;
 
case 's':
if (scan_scaled(optarg, tmpnumber) == -1)
err(EXIT_FAILURE, failed to parse size `%s',
optarg);
-  

Re: [PATCH] Option for mount_tmpfs to populate the volume aftercreation.

[bytevolcano]

 Original Message 
From: Otto Moerbeek o...@drijf.net
To: bytevolc...@safe-mail.net
Cc: tech@openbsd.org
Subject: Re: [PATCH] Option for mount_tmpfs to populate the volume 
aftercreation.
Date: Thu, 18 Sep 2014 10:54:34 +0200

 On Thu, Sep 18, 2014 at 08:40:44AM +0100, bytevolc...@safe-mail.net wrote:
 
  This patch adds an option -t template to mount_tmpfs, which
  populates the new tmpfs volume with a directory
  immediately after creation.
  
  Man page update included for explanation.
  
  Much of the code was grafted from newfs
  which implements this for mount_mfs (-P option).
  
  Suggestions, fixes, criticism, etc. welcome.
 
 - Please do not mix the argument parsing changes with the new
   functionality changes. Post separate diffs for that,

Thanks for the info. Will fix set of diffs.

   although I do not
   see the point of removing mount_tmpfs_parseargs().

My rationale for this is as follows: in order for the new code to
work, I need to store the user-provided template path somewhere.

I could add an extra parameter to mount_tmpfs_parseargs() as one of the 
following:

 1. char ** - risk optarg may change after the function has
returned, assuming wanted to call getopt() after calling
mount_tmpfs_parseargs() later on when they edit the code further

 2. char* - point to yet another buffer allocated just to store the template 
path. Much better than above.

Rather than going through one of the above, I thought a neater solution would 
be how newfs handles this, so I integrated mount_tmpfs_parseargs() into its 
caller. The code for tmpfs_parseargs() itself isn't removed; just relocated.

That being said, if having this as a separate function will make the overall 
code easier to maintain, I'll work around it. It does not seem too difficult.

 - Why use a different flag compared to mount_mfs?

Mostly a semantics thing IMO. I just thought it made more sense to use -t as 
in template, since most file attributes are copied over, as opposed to just 
let's populate the directory with a bunch of files.

As I was updating the man page I was thinking about whether to use -t or 
-P, but then I figured let's see what the devs think first.

The next diff will end up using -P instead then based on your input.

 - What's pathnames.h doing here?

I thought it would be a good idea to include it in the diff in the same 
directory as I wasn't sure if this would compile on another person's machine; 
it compiles and works fine on mine though.

I wasn't sure how include directories (#include ...) worked in the source 
tree so this was a just-in-case measure. If not needed, I am somewhat relieved.

   -Otto

 - Adam.

[no subject]

[bytevolcano]

Yes. As it would seem, that was unintentional, possibly caused by a bad merge. 
I didn't pick this up as it worked on my system. It was thanks to Otto's messae 
re parsing changes that I managed to pick that up.

I am working on a new diff which will be released some time within the next few 
hours.

 - Adam.

On Thu, 18 Sep 2014 17:58:14 +0200
Alexander Hall alexan...@beard.se wrote:

 Aren't you missing something relevant here...? :-)
 
 /Alexander

[PATCH] Allow bioctl(8) to recognise DUIDs (and such) for many operations

[bytevolcano]

This patch allows bioctl(8) to perform operations on volumes using either the 
fully-qualified device path (eg. /dev/sd0a) or a valid DUID pointing to the 
RAID volume or device.

Basically, anything that opendev(3) can open.

Index: sbin/bioctl/bioctl.c
===
RCS file: /cvs/src/sbin/bioctl/bioctl.c,v
retrieving revision 1.119
diff -u -p -r1.119 bioctl.c
--- sbin/bioctl/bioctl.c18 Jan 2014 09:11:12 -  1.119
+++ sbin/bioctl/bioctl.c17 Mar 2014 11:38:10 -
@@ -62,6 +62,7 @@ struct locator {
 
 void   usage(void);
 const char *str2locator(const char *, struct locator *);
+intopen_device(const char *, char *, size_t);
 void   bio_status(struct bio_status *);
 intbio_parse_devlist(char *, dev_t *);
 void   bio_kdf_derive(struct sr_crypto_kdfinfo *,
@@ -101,7 +102,8 @@ main(int argc, char *argv[])
extern char *optarg;
u_int64_t   func = 0;
char*devicename = NULL;
-   char*realname = NULL, *al_arg = NULL;
+   charrealname[MAXPATHLEN] = {0};
+   char*al_arg = NULL;
char*bl_arg = NULL, *dev_list = NULL;
char*key_disk = NULL;
const char  *errstr;
@@ -216,7 +218,9 @@ main(int argc, char *argv[])
if (devicename == NULL)
errx(1, need device);
 
-   devh = opendev(devicename, O_RDWR, OPENDEV_PART, realname);
+   devh = open_device(devicename, realname, MAXPATHLEN);
+   devicename = realname;
+   
if (devh == -1) {
devh = open(/dev/bio, O_RDWR);
if (devh == -1)
@@ -312,6 +316,30 @@ str2locator(const char *string, struct l
if (errstr)
return (errstr);
return (NULL);
+}
+
+int
+open_device(const char *devname, char *realname, size_t rnmax)
+{
+   int devh;
+   char *temp = NULL;
+   size_t slen = 0;
+   
+   devh = opendev(devname, O_RDWR, OPENDEV_PART, temp);
+   if(devh != -1) {
+   if(temp)
+   temp = basename(temp);
+   if(temp  (temp[0] != '\0'))
+   slen = strlcpy(realname, temp + 1, rnmax);
+
+   if(slen = rnmax)
+   slen = rnmax - 1;
+
+   if(slen  2)
+   realname[slen - 1] = '\0';
+   }
+
+   return devh;
 }
 
 void

[PATCH] (amd64) VIA CPU feature set detection

[bytevolcano]

Patch below allows the kernel (amd64) to detect CPU features (VIA PadLock, 
temperature sensors, etc.) on modern VIA Eden CPUs.

My apologies for the previous substandard overly-verbose message.



Index: sys/arch/amd64/amd64/identcpu.c
===
RCS file: /cvs/src/sys/arch/amd64/amd64/identcpu.c,v
retrieving revision 1.50
diff -u -p -r1.50 identcpu.c
--- sys/arch/amd64/amd64/identcpu.c 24 Aug 2013 23:45:31 -  1.50
+++ sys/arch/amd64/amd64/identcpu.c 26 Sep 2013 08:45:46 -
@@ -568,7 +568,7 @@ identifycpu(struct cpu_info *ci)
if (!strcmp(cpu_vendor, AuthenticAMD))
amd64_errata(ci);
 
-   if (strncmp(mycpu_model, VIA Nano processor, 18) == 0) {
+   if (!strcmp(cpu_vendor, CentaurHauls)) {
ci-cpu_setup = via_nano_setup;
 #ifndef SMALL_KERNEL
strlcpy(ci-ci_sensordev.xname, ci-ci_dev-dv_xname,

[PATCH] VIA CPU feature set recognition (amd64)

[bytevolcano]

Hi,

I noticed the following when trying to do some benchmarking for the padlock 
engine on a VIA Eden CPU using the openssl speed command on -current (as of 6 
Sept) amd64:

 - machdep.xcrypt=0 (should be 15; like i386 port)
 - no hw.sensors.cpu?.temp (shows up on i386 port)
 - openssl speed ... -evp aes-256-cbc ~ 35MB/s max.
   (over 415MB/s on i386 port)

I have seen an external link 
(http://www.capnfreedom.com/home/viac7padlockaesbenchmarks) which suggests 
OpenBSD supports PadLock, so after a rather exhaustive search for relevant CVS 
commits which suggest changes for VIA CPUs. Last one I could find was this:

==
CVSROOT:/cvs
Module name:src
Changes by: deraadt@XXX 2009/07/22 14:33:13

Modified files:
sys/arch/amd64/amd64: identcpu.c machdep.c 
sys/arch/amd64/include: cpu.h 

Log message:
via nano cpus are amd64, and so we need machdep.xcrypt 
==

Searched the relevant files, and found a single line (Line 571 of 
sys/arch/amd64/amd64/identcpu.c) to be the culprit.

 BEGIN DIFF 
--- sys/arch/amd64/amd64/identcpu.c 24 Aug 2013 23:45:31 -  1.50
+++ sys/arch/amd64/amd64/identcpu.c 6 Sep 2013 05:42:56 -
@@ -568,7 +568,7 @@ identifycpu(struct cpu_info *ci)
if (!strcmp(cpu_vendor, AuthenticAMD))
amd64_errata(ci);

-   if (strncmp(mycpu_model, VIA Nano processor, 18) == 0) {
+   if (!strcmp(cpu_vendor, CentaurHauls)) {
ci-cpu_setup = via_nano_setup;
 #ifndef SMALL_KERNEL
strlcpy(ci-ci_sensordev.xname, ci-ci_dev-dv_xname,
== END DIFF ===

This fix will correct feature recognition and support for VIA Eden and like 
CPUs.

The result after applying this fix:

$ openssl speed -engine cryptodev -evp aes-256-cbc
engine cryptodev set.
Doing aes-256-cbc for 3s on 16 size blocks: 4825080 aes-256-cbc's in 3.01s
Doing aes-256-cbc for 3s on 64 size blocks: 4360645 aes-256-cbc's in 3.01s
Doing aes-256-cbc for 3s on 256 size blocks: 2704061 aes-256-cbc's in 3.01s
Doing aes-256-cbc for 3s on 1024 size blocks: 1066366 aes-256-cbc's in 3.01s
Doing aes-256-cbc for 3s on 8192 size blocks: 161747 aes-256-cbc's in 3.01s
OpenSSL 1.0.1c 10 May 2012
built on: date not available
options:bn(64,64) rc4(8x,int) des(idx,cisc,16,int) aes(partial) idea(int) 
blowfish(idx)
compiler: information not available
The 'numbers' are in 1000s of bytes per second processed.
type 16 bytes 64 bytes256 bytes   1024 bytes   8192 bytes
aes-256-cbc  25648.27k92718.03k   229979.94k   362777.00k   440209.78k

$ sysctl machdep
machdep.console_device=ttyC0
machdep.bios.diskinfo.128=bootdev = 0xa200, cylinders = 1024, heads = 255, 
sectors = 63
machdep.bios.cksumlen=1
machdep.allowaperture=2
machdep.cpuvendor=CentaurHauls
machdep.cpuid=1788
machdep.cpufeature=-1076233217
machdep.kbdreset=0
machdep.xcrypt=15
machdep.lidsuspend=0

$ sysctl hw.sensors
hw.sensors.cpu0.temp0=44.00 degC
hw.sensors.acpiac0.indicator0=On (power supply)

$ dmesg
OpenBSD 5.4-current (GENERIC) #1: Fri Sep  6 06:20:50 UTC 2013
r...@m2.my.domain:/usr/src/sys/arch/amd64/compile/GENERIC
real mem = 4009426944 (3823MB)
avail mem = 3894644736 (3714MB)
mainbus0 at root
bios0 at mainbus0: SMBIOS rev. 2.6 @ 0x9f400 (45 entries)
bios0: vendor VIA technology Inc. version 410 AMI R102 date 04/03/2012
bios0: VIA technology Inc. VT3410 H3100MB
acpi0 at bios0: rev 2
acpi0: sleep states S0 S1 S3 S4 S5
acpi0: tables DSDT FACP APIC MCFG WDRT OEMB HPET
acpi0: wakeup devices NBP0(S4) NP0S(S4) NBP1(S4) NP1S(S4) NBP2(S4) NP2S(S4) 
NBP3(S4) NP3S(S4) USB0(S3) USB1(S3) USB2(S3) USB3(S3) EHCI(S3) SBRG(S5) 
PS2K(S4) PS2M(S4) [...]
acpitimer0 at acpi0: 3579545 Hz, 24 bits
acpimadt0 at acpi0 addr 0xfee0: PC-AT compat
cpu0 at mainbus0: apid 0 (boot processor)
cpu0: VIA Eden X2 U4200 @ 1.0+ GHz, 1000.17 MHz
cpu0: 
FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,CFLUSH,ACPI,MMX,FXSR,SSE,SSE2,SS,HTT,TM,PBE,SSE3,MWAIT,VMX,EST,TM2,SSSE3,CX16,xTPR,SSE4.1,POPCNT,NXE,LONG,LAHF,PERF
cpu0: 64KB 64b/line 16-way I-cache, 64KB 64b/line 16-way D-cache, 1MB 64b/line 
16-way L2 cache
cpu0: ITLB 128 4KB entries 8-way
cpu0: DTLB 128 4KB entries 8-way
cpu0: RNG AES AES-CTR SHA1 SHA256 RSA
cpu0: apic clock running at 200MHz
cpu at mainbus0: not configured
ioapic0 at mainbus0: apid 3 pa 0xfec0, version 3, 24 pins
ioapic0: misconfigured as apic 1, remapped to apid 3
ioapic1 at mainbus0: apid 4 pa 0xfecc, version 3, 24 pins
ioapic1: misconfigured as apic 3, remapped to apid 4
acpimcfg0 at acpi0 addr 0xe000, bus 0-255
acpihpet0 at acpi0: 14318179 Hz
acpiprt0 at acpi0: bus 0 (PCI0)
acpiprt1 at acpi0: bus 1 (NBP0)
acpiprt2 at acpi0: bus 2 (NBP1)
acpiprt3 at acpi0: bus 3 (NBP2)
acpiprt4 at acpi0: bus 5 (NBP3)
acpiprt5 at acpi0: bus 6 (P0P4)
acpicpu0 at acpi0
acpiac0 at acpi0: AC unit online
acpibtn0 at acpi0: SLPB
acpibtn1 at acpi0: PWRB
cpu0: unknown Enhanced