Re: fuse: vfs create does not map 1:1 to fuse create

[Helg Bredow]

On Fri, 10 Nov 2017 09:09:32 +0100
Martin Pieuchot  wrote:

> On 09/11/17(Thu) 01:20, Helg Bredow wrote:
> > > On 08/11/17(Wed) 14:12, Helg Bredow wrote:
> > > > There is a bug when creating a file in fuse-exfat and then deleting it
> > > > again without first unmounting the file system. The reason for this is
> > > > that fuse-exfat maintains strict reference counts and fuse currently
> > > > calls the file system create and open functions when it should only
> > > > call create. 
> > > > [...]
> > > 
[...]
> 
> In the end that's your call.  Nobody understand the problem, what you
> say is "we are not linux".  Which we already know.  And you're pushing
> for a diff which works saying that the best solution.  It believe it
> fixes the problem so go for it.

I now this caused some confusion but I believe the patch below is the
correct solution. Here's the mapping of file system calls so it's clear.

vfs | fuse
+
atomic_open | create
create  | mknod
mknod   | mknod
open| open

encfs and fuse-exfat now work.
curlftsfs now returns an error on file append since it doesn't support
it.

I've contacted the developer of fuse-zip and he's adding mknod()
support.

ok?


Index: sys/miscfs/fuse/fuse_vnops.c
===
RCS file: /cvs/src/sys/miscfs/fuse/fuse_vnops.c,v
retrieving revision 1.33
diff -u -p -p -u -r1.33 fuse_vnops.c
--- sys/miscfs/fuse/fuse_vnops.c7 Sep 2016 17:53:35 -   1.33
+++ sys/miscfs/fuse/fuse_vnops.c18 Nov 2017 01:58:34 -
@@ -211,7 +211,7 @@ fusefs_open(void *v)
struct fusefs_node *ip;
struct fusefs_mnt *fmp;
enum fufh_type fufh_type = FUFH_RDONLY;
-   int flags = O_RDONLY;
+   int flags;
int error;
int isdir;
 
@@ -226,24 +226,27 @@ fusefs_open(void *v)
if (ap->a_vp->v_type == VDIR)
isdir = 1;
else {
-   if ((ap->a_mode & FREAD) && (ap->a_mode & FWRITE)) {
+   if ((ap->a_mode & FREAD) && (ap->a_mode & FWRITE))
fufh_type = FUFH_RDWR;
-   flags = O_RDWR;
-   } else if (ap->a_mode  & (FWRITE)) {
+   else if (ap->a_mode  & (FWRITE))
fufh_type = FUFH_WRONLY;
-   flags = O_WRONLY;
-   }
}
 
/* already open i think all is ok */
if (ip->fufh[fufh_type].fh_type != FUFH_INVALID)
return (0);
 
+   /* no creation and truncation flags are passed to open */
+   flags = OFLAGS(ap->a_mode);
+   flags &= ~O_CREAT;
+   flags &= ~O_EXCL;
+   flags &= ~O_TRUNC;
+
error = fusefs_file_open(fmp, ip, fufh_type, flags, isdir, ap->a_p);
if (error)
return (error);
 
-   return (error);
+   return (0);
 }
 
 int
@@ -891,16 +894,15 @@ fusefs_create(void *v)
goto out;
}
 
-   if (fmp->undef_op & UNDEF_CREATE) {
+   if (fmp->undef_op & UNDEF_MKNOD) {
error = ENOSYS;
goto out;
}
 
fbuf = fb_setup(cnp->cn_namelen + 1, ip->ufs_ino.i_number,
-   FBT_CREATE, p);
+   FBT_MKNOD, p);
 
fbuf->fb_io_mode = mode;
-   fbuf->fb_io_flags = O_CREAT | O_RDWR;
 
memcpy(fbuf->fb_dat, cnp->cn_nameptr, cnp->cn_namelen);
fbuf->fb_dat[cnp->cn_namelen] = '\0';
@@ -908,7 +910,7 @@ fusefs_create(void *v)
error = fb_queue(fmp->dev, fbuf);
if (error) {
if (error == ENOSYS)
-   fmp->undef_op |= UNDEF_CREATE;
+   fmp->undef_op |= UNDEF_MKNOD;
 
fb_delete(fbuf);
goto out;
Index: lib/libfuse/fuse_ops.c
===
RCS file: /cvs/src/lib/libfuse/fuse_ops.c,v
retrieving revision 1.27
diff -u -p -p -u -r1.27 fuse_ops.c
--- lib/libfuse/fuse_ops.c  17 Nov 2017 15:45:17 -  1.27
+++ lib/libfuse/fuse_ops.c  18 Nov 2017 01:58:34 -
@@ -579,6 +579,8 @@ ifuse_ops_write(struct fuse *f, struct f
return (0);
 }
 
+#if 0
+/* fuse create requires the kernel to support atomic_open() */
 static int
 ifuse_ops_create(struct fuse *f, struct fusebuf *fbuf)
 {
@@ -611,8 +613,6 @@ ifuse_ops_create(struct fuse *f, struct 
 
if (f->op.create)
fbuf->fb_err = f->op.create(realname, mode,  );
-   else if (f->op.mknod)
-   fbuf->fb_err = f->op.mknod(realname, S_IFREG | mode, 0);
else
fbuf->fb_err = -ENOSYS;
 
@@ -625,6 +625,7 @@ ifuse_ops_create(struct fuse *f, struct 
 
return (0);
 }
+#endif
 
 static int
 ifuse_ops_mkdir(struct fuse *f, struct fusebuf *fbuf)
@@ -1125,9 +1126,11 @@ ifuse_exec_opcode(struct fuse *f, struct
case FBT_ACCESS:
ret = ifuse_ops_access(f, fbuf);
break;
+#if 0
case 

Re: __warn_references: drop redundant "warning: " prefix

[Jeremie Courreges-Anglas]

On Sun, Nov 12 2017, Scott Cheloha  wrote:
> Hi,
>
> GNU ld has prefixed the contents of .gnu.warning.SYMBOL sections
> with "warning: " since 2003, so the messages themselves need not
> contain the prefix anymore.
>
> If LLVM ld ever acknowledges .gnu.warning sections I imagine it
> would emulate this behavior.

It would be good if lld gained support for .gnu.warning sections, those
warnings put the light on code that should be improved; not only in the
ports tree, as shown by the libcrypto warnings in base.

After a quick glance it seems that recent binutils versions have the
same behavior, so it's reasonable to assume that lld would follow
GNU ld(1) by prepending "warning:".  Especially if they don't want to
get the same bug report as the gcc folks got:

  https://sourceware.org/ml/binutils/2003-08/msg8.html

> Thoughts?

The double "warning: " message looks just weird, removing it makes sense
IMO.  ok jca@

> --
> Scott Cheloha
>
> Index: lib/libc/arch/i386/string/strcat.S
> ===
> RCS file: /cvs/src/lib/libc/arch/i386/string/strcat.S,v
> retrieving revision 1.9
> diff -u -p -r1.9 strcat.S
> --- lib/libc/arch/i386/string/strcat.S31 Aug 2015 02:53:56 -  
> 1.9
> +++ lib/libc/arch/i386/string/strcat.S12 Nov 2017 04:21:37 -
> @@ -9,7 +9,7 @@
>  #if defined(APIWARN)
>  #APP
>   .section .gnu.warning.strcat
> - .ascii "warning: strcat() is almost always misused, please use 
> strlcat()"
> + .ascii "strcat() is almost always misused, please use strlcat()"
>  #NO_APP
>  #endif
>  
> Index: lib/libc/arch/i386/string/strcpy.S
> ===
> RCS file: /cvs/src/lib/libc/arch/i386/string/strcpy.S,v
> retrieving revision 1.9
> diff -u -p -r1.9 strcpy.S
> --- lib/libc/arch/i386/string/strcpy.S31 Aug 2015 02:53:56 -  
> 1.9
> +++ lib/libc/arch/i386/string/strcpy.S12 Nov 2017 04:21:37 -
> @@ -9,7 +9,7 @@
>  #if defined(APIWARN)
>  #APP
>   .section .gnu.warning.strcpy
> - .ascii "warning: strcpy() is almost always misused, please use 
> strlcpy()"
> + .ascii "strcpy() is almost always misused, please use strlcpy()"
>  #NO_APP
>  #endif
>  
> Index: lib/libc/compat-43/getwd.c
> ===
> RCS file: /cvs/src/lib/libc/compat-43/getwd.c,v
> retrieving revision 1.11
> diff -u -p -r1.11 getwd.c
> --- lib/libc/compat-43/getwd.c30 Sep 2013 12:02:30 -  1.11
> +++ lib/libc/compat-43/getwd.c12 Nov 2017 04:21:37 -
> @@ -46,4 +46,4 @@ getwd(char *buf)
>  }
>  
>  __warn_references(getwd,
> -"warning: getwd() possibly used unsafely; consider using getcwd()");
> +"getwd() possibly used unsafely; consider using getcwd()");
> Index: lib/libc/stdio/mktemp.c
> ===
> RCS file: /cvs/src/lib/libc/stdio/mktemp.c,v
> retrieving revision 1.38
> diff -u -p -r1.38 mktemp.c
> --- lib/libc/stdio/mktemp.c   13 Sep 2015 08:31:47 -  1.38
> +++ lib/libc/stdio/mktemp.c   12 Nov 2017 04:21:37 -
> @@ -119,7 +119,7 @@ _mktemp(char *path)
>  }
>  
>  __warn_references(mktemp,
> -"warning: mktemp() possibly used unsafely; consider using mkstemp()");
> +"mktemp() possibly used unsafely; consider using mkstemp()");
>  
>  char *
>  mktemp(char *path)
> Index: lib/libc/stdio/sprintf.c
> ===
> RCS file: /cvs/src/lib/libc/stdio/sprintf.c,v
> retrieving revision 1.18
> diff -u -p -r1.18 sprintf.c
> --- lib/libc/stdio/sprintf.c  1 Oct 2015 02:32:07 -   1.18
> +++ lib/libc/stdio/sprintf.c  12 Nov 2017 04:21:37 -
> @@ -39,7 +39,7 @@
>  
>  #if defined(APIWARN)
>  __warn_references(sprintf,
> -"warning: sprintf() is often misused, please use snprintf()");
> +"sprintf() is often misused, please use snprintf()");
>  #endif
>  
>  int
> Index: lib/libc/stdio/tempnam.c
> ===
> RCS file: /cvs/src/lib/libc/stdio/tempnam.c,v
> retrieving revision 1.19
> diff -u -p -r1.19 tempnam.c
> --- lib/libc/stdio/tempnam.c  31 Aug 2015 02:53:57 -  1.19
> +++ lib/libc/stdio/tempnam.c  12 Nov 2017 04:21:37 -
> @@ -37,7 +37,7 @@
>  #include 
>  
>  __warn_references(tempnam,
> -"warning: tempnam() possibly used unsafely; consider using mkstemp()");
> +"tempnam() possibly used unsafely; consider using mkstemp()");
>  
>  char *
>  tempnam(const char *dir, const char *pfx)
> Index: lib/libc/stdio/tmpnam.c
> ===
> RCS file: /cvs/src/lib/libc/stdio/tmpnam.c,v
> retrieving revision 1.11
> diff -u -p -r1.11 tmpnam.c
> --- lib/libc/stdio/tmpnam.c   31 Aug 2015 02:53:57 -  1.11
> +++ lib/libc/stdio/tmpnam.c   12 Nov 2017 04:21:37 -
> @@ -37,7 +37,7 @@
>  

Re: hide wpakey from root by default

[Ted Unangst]

Stefan Sperling wrote:
> Or is modifying ifconfig sufficient?
> We are more concerned about textual display rather than the
> kernel/userland ioctl boundary, correct?
> 
> The option list for ifconfig is [-AaC]. Plenty of letters available.
> We could add:
> 
>-P  Show authentication details such as passwords (not displayed by 
> default))

I think putting this logic in ifconfig is much better than the kernel. That
didn't make much sense to me, I'm afraid. 

permit IPV6_V6ONLY in sockopt()

[Aaron Bieber]

Hi,

I ran into a pledge'ing weirdness with Go apps and 'inet'. Go tries to
probe available communication options:

https://github.com/golang/go/blob/master/src/net/ipsock_posix.go#L44-L56

The result of which ends up being 'inet' pledged go apps fail with:
'pledge "inet", syscall 105'

Removing the "#ifdef notyet"'d IPV6_V6ONLY fixes this issue for me.

Discussed with tb, jca and deraadt. OK?

Cheers,
Aaron

Index: kern/kern_pledge.c
===
RCS file: /cvs/src/sys/kern/kern_pledge.c,v
retrieving revision 1.223
diff -u -p -r1.223 kern_pledge.c
--- kern/kern_pledge.c  12 Oct 2017 15:04:33 -  1.223
+++ kern/kern_pledge.c  17 Nov 2017 22:29:54 -
@@ -1280,9 +1280,7 @@ pledge_sockopt(struct proc *p, int set,
case IPV6_PORTRANGE:
case IPV6_RECVPKTINFO:
case IPV6_RECVDSTPORT:
-#ifdef notyet
case IPV6_V6ONLY:
-#endif
return (0);
case IPV6_MULTICAST_IF:
case IPV6_MULTICAST_HOPS:

--
PGP: 0x1F81112D62A9ADCE / 3586 3350 BFEA C101 DB1A  4AF0 1F81 112D 62A9 ADCE

Re: relayd status code handling

[Rivo Nurges]

Hi!

I have 4 digit number of relayd instances and I'v seen plenty of such
products. ownCloud, Nextcloud and some Atlassian products to name few.
relayd seems to be the only one to enforce the RFC so strictly. I'd be
glad if this gets relaxed a bit.

Rivo

On Fri, 2017-11-17 at 18:52 +0100, Sebastian Benoit wrote:
> Hi,
> 
> relayd enforces a rule in rfc section 3.3.2:
> 
> rfc 7230 3.3 Message Body
> 
>All 1xx (Informational), 204 (No Content), and 304 (Not Modified)
>responses do not include a message body.  All other responses do
>include a message body, although the body might be of zero length.
> 
> rfc 7230 3.3.2.  Content-Length
> 
>A server MUST NOT send a Content-Length header field in any
> response
>with a status code of 1xx (Informational) or 204 (No Content).  A
>server MUST NOT send a Content-Length header field in any 2xx
>(Successful) response to a CONNECT request (Section 4.3.6 of
>[RFC7231]).
> 
> 
> There is also this paragraph in Section 3.3.3 thats relevant:
> 
> 
>4.  If a message is received without Transfer-Encoding and with
>either multiple Content-Length header fields having differing
>field-values or a single Content-Length header field having an
>invalid value, then the message framing is invalid and the
>recipient MUST treat it as an unrecoverable error.  If this is
> a
>request message, the server MUST respond with a 400 (Bad
> Request)
>status code and then close the connection.  If this is a
> response
>message received by a proxy, the proxy MUST close the
> connection
>to the server, discard the received response, and send a 502
> (Bad
>Gateway) response to the client.  If this is a response
> message
>received by a user agent, the user agent MUST close the
>connection to the server and discard the received response.
> 
> 
> There are two problems with this:
> 
> * I have found a Tomcat Server that sends "204 No Content" with a
>   "Content-Length: 0" Header.
> 
> * Relayd sends the wrong HTTP Status Code, it should be 502 Bad
> Gateway
> 
> Here is a diff that relaxes the logic: send a 502 Bad Gateway, but
> only if Content-Length != 0.
> 
> ok?
> 
> diff --git usr.sbin/relayd/relay_http.c usr.sbin/relayd/relay_http.c
> index 3b6f8e84e37..ad45cf2d5d6 100644
> --- usr.sbin/relayd/relay_http.c
> +++ usr.sbin/relayd/relay_http.c
> @@ -324,19 +324,6 @@ relay_read_http(struct bufferevent *bev, void
> *arg)
>   relay_abort_http(con, 400,
> "malformed", 0);
>   goto abort;
>   }
> - /*
> -  * response with a status code of 1xx
> -  * (Informational) or 204 (No Content) MUST
> -  * not have a Content-Length (rfc 7230
> 3.3.3)
> -  */
> - if (desc->http_method ==
> HTTP_METHOD_RESPONSE && (
> - ((desc->http_status >= 100 &&
> - desc->http_status < 200) ||
> - desc->http_status == 204))) {
> - relay_abort_http(con, 500,
> - "Internal Server Error", 0);
> - goto abort;
> - }
>   /*
>* Need to read data from the client after
> the
>* HTTP header.
> @@ -349,6 +336,23 @@ relay_read_http(struct bufferevent *bev, void
> *arg)
>   relay_abort_http(con, 500, errstr,
> 0);
>   goto abort;
>   }
> + /*
> +  * response with a status code of 1xx
> +  * (Informational) or 204 (No Content) MUST
> +  * not have a Content-Length (rfc 7230
> 3.3.3)
> +  * Instead we check for value != 0 because
> there are
> +  * servers that do not follow the rfc and
> send
> +  * Content-Length: 0.
> +  */
> + if (desc->http_method ==
> HTTP_METHOD_RESPONSE && (
> + ((desc->http_status >= 100 &&
> + desc->http_status < 200) ||
> + desc->http_status == 204)) &&
> + cre->toread != 0) {
> + relay_abort_http(con, 502,
> + "Bad Gateway", 0);
> + goto abort;
> + }
>   }
>   lookup:
>   if (strcasecmp("Transfer-Encoding", key) == 0 &&
> 

Re: pppd: explicit_bzero sensitive buffers

[Scott Cheloha]

> On Nov 17, 2017, at 3:07 PM, Stuart Henderson  wrote:
> 
> On 2017/11/17 21:55, Jeremie Courreges-Anglas wrote:
>> On Sat, Nov 11 2017, Scott Cheloha  wrote:
>>> Hi,
>>> 
>>> You want explicit_bzero(3) for these buffers.
>>> 
>>> Zeroing a buffer is compiler- and system-dependent, so I added a
>>> new macro.
>> 
>> I have committed the fixed version, with the macro.

Thanks!

Sorry about the quoted-printable garbage, working on setting up a real
mail client for patches asap.

>>> I'll send a pull request upstream if this goes in.
>> 
>> Well, pppd from base seems a bit away from "upstream", or am I missing
>> something?
> 
> Sadly, upstream removed a lot of their OS support some years ago.

Although base pppd has drifted and upstream's OS support has dwindled,
my impression from reading the source code is that the intent was to
clear the sensitive buffers, so perhaps Mackerras will accept the
patch and then individual downstreams can, at their leisure, substitute
a preferred compiler-proof buffer-clearing function in for the
EXPLICIT_BZERO macro.

Worth a shot, was all I was thinking.

--
Scott Cheloha

Re: pppd: explicit_bzero sensitive buffers

[Jeremie Courreges-Anglas]

On Sat, Nov 11 2017, Scott Cheloha  wrote:
> Hi,
>
> You want explicit_bzero(3) for these buffers.
>
> Zeroing a buffer is compiler- and system-dependent, so I added a
> new macro.

I have committed the fixed version, with the macro.

> I'll send a pull request upstream if this goes in.

Well, pppd from base seems a bit away from "upstream", or am I missing
something?

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

Re: faster printf

[Theo de Raadt]

> So C99 explicitly requires failure *for encoding errors* and
> explicitly requires multibyte encoding for the format string.
> So it appears that *everybody* (except us) is in blatant violation
> of C99.
> 
> To hell with multibyte characters!  How on earth do so many dragons
> fit into such a small rabbit hole!
> 
> Sigh,

Humans make mistakes.  Those can be fixed.

Doubling down on dangeorus weasel words of some document isn't
a mistake, it is foolish endangerment.

Can you please step back from your interpretation of the sloppy words
in the documents, and think HOW SHOULD THIS WORK AND WHAT WOULD BE THE
BEST DIRECTION.


THAT IS ENGINEERING RATHER THAN DOGMA.

Re: faster printf

[Theo de Raadt]

> Todd's research revealed that jtc@ got the information from the
> C standard in 1995, so i just checked what C89 (sic!) says:
> 
>   4.9.6.1 The fprintf function
>   [...]
>   The format shall be a multibyte character sequence, beginning and
>   ending in its initial shift state.  The format is composed of
>   zero or more directives: ordinary multibyte characters (not % ),
>   which are copied unchanged to the output stream; and conversion
>   specifications, each of which results in fetching zero or more
>   subsequent arguments.

I still don't understand Ingo

This means something to me:

which are copied unchanged to the output stream

Perhaps it should more clearly say "unchecked".

> In 7.19.6.1.14, C99 then goes on to say:
> 
>   The fprintf function returns the number of characters transmitted,
>   or a negative value if an output or encoding error occurred.

If the format characters are "copied unchanged to the output stream"
without checks, then there are no errors to worry about from them
and that point is irrelevant.

>   If an output error was encountered, these functions shall return a
>   negative value and set errno to indicate the error.
> 
> So C99 explicitly requires failure *for encoding errors* and
> explicitly requires multibyte encoding for the format string.
> So it appears that *everybody* (except us) is in blatant violation
> of C99.

I still see no words saying it must check the bytes in the
format string.

> To hell with multibyte characters!  How on earth do so many dragons
> fit into such a small rabbit hole!

Breaking old software is unacceptable.

Re: faster printf

[Ingo Schwarze]

Hi Theo,

Theo de Raadt wrote on Fri, Nov 17, 2017 at 10:43:10AM -0700:
> Ingo Schwarze wrote:

>> I don't think, though, that the commit message should advertise
>> this as a performance improvement.  It should be called an intentional
>> change of behaviour, now using the format string as a byte string
>> like everyone else, no matter whether POSIX explicitly specifies
>> it as a character string instead.

> I do not agree with your position that POSIX calls this a character
> string.

  http://pubs.opengroup.org/onlinepubs/9699919799/functions/fprintf.html

  "The format is a character string, ..."

Well, i does, and...

> Pure and simply, it is a legacy char *, and any attempt to re-standardize
> it from that would crash cars and other heavy equipment.

 ... given that POSIX-2008 is explicitly based on C99 and both C89
and C99 are even more explicit (see below), it follows that people
thirty years ago probably intended to crash cars and other heavy
equipment, even though you may quite possibly be right that it was
likely a bad idea back then.

> I'd like to mention in a previous email you used the term fail-closed
> incorrectly.
> 
> Returning an error number and changing behaviour (truncating)
> when there there potentially isn't a caller-checks isn't fail-closed.
> 
> Fail-closed implies the software crashes or terminates, so that the
> bug can be inspected and fixed.
> 
> Returning an error in a newer version of a standard, when a past
> standard passed data straight though?
> 
> With no way for authors to know the fallout, and requiring them to
> audit all code for the broad practice of missing caller-checks?
> Not realistic.

Point taken.

> So it would be responsible to assume a check can be here.  I think
> you have misread POSIX, or someone did 's/char/character/' too broadly.

Todd's research revealed that jtc@ got the information from the
C standard in 1995, so i just checked what C89 (sic!) says:

  4.9.6.1 The fprintf function
  [...]
  The format shall be a multibyte character sequence, beginning and
  ending in its initial shift state.  The format is composed of
  zero or more directives: ordinary multibyte characters (not % ),
  which are copied unchanged to the output stream; and conversion
  specifications, each of which results in fetching zero or more
  subsequent arguments.

C99 says exactly the same in 7.19.6.1.3.

That teaches me that i should stop trusting POSIX to accurately and
clearly paraphrase the C standard.

In 7.19.6.1.14, C99 then goes on to say:

  The fprintf function returns the number of characters transmitted,
  or a negative value if an output or encoding error occurred.

That is even more explicit than POSIX, which only says:

  If an output error was encountered, these functions shall return a
  negative value and set errno to indicate the error.

So C99 explicitly requires failure *for encoding errors* and
explicitly requires multibyte encoding for the format string.
So it appears that *everybody* (except us) is in blatant violation
of C99.

To hell with multibyte characters!  How on earth do so many dragons
fit into such a small rabbit hole!

Sigh,
  Ingo

relayd status code handling

[Sebastian Benoit]

Hi,

relayd enforces a rule in rfc section 3.3.2:

rfc 7230 3.3 Message Body

   All 1xx (Informational), 204 (No Content), and 304 (Not Modified)
   responses do not include a message body.  All other responses do
   include a message body, although the body might be of zero length.

rfc 7230 3.3.2.  Content-Length

   A server MUST NOT send a Content-Length header field in any response
   with a status code of 1xx (Informational) or 204 (No Content).  A
   server MUST NOT send a Content-Length header field in any 2xx
   (Successful) response to a CONNECT request (Section 4.3.6 of
   [RFC7231]).


There is also this paragraph in Section 3.3.3 thats relevant:


   4.  If a message is received without Transfer-Encoding and with
   either multiple Content-Length header fields having differing
   field-values or a single Content-Length header field having an
   invalid value, then the message framing is invalid and the
   recipient MUST treat it as an unrecoverable error.  If this is a
   request message, the server MUST respond with a 400 (Bad Request)
   status code and then close the connection.  If this is a response
   message received by a proxy, the proxy MUST close the connection
   to the server, discard the received response, and send a 502 (Bad
   Gateway) response to the client.  If this is a response message
   received by a user agent, the user agent MUST close the
   connection to the server and discard the received response.


There are two problems with this:

* I have found a Tomcat Server that sends "204 No Content" with a
  "Content-Length: 0" Header.

* Relayd sends the wrong HTTP Status Code, it should be 502 Bad Gateway

Here is a diff that relaxes the logic: send a 502 Bad Gateway, but
only if Content-Length != 0.

ok?

diff --git usr.sbin/relayd/relay_http.c usr.sbin/relayd/relay_http.c
index 3b6f8e84e37..ad45cf2d5d6 100644
--- usr.sbin/relayd/relay_http.c
+++ usr.sbin/relayd/relay_http.c
@@ -324,19 +324,6 @@ relay_read_http(struct bufferevent *bev, void *arg)
relay_abort_http(con, 400, "malformed", 0);
goto abort;
}
-   /*
-* response with a status code of 1xx
-* (Informational) or 204 (No Content) MUST
-* not have a Content-Length (rfc 7230 3.3.3)
-*/
-   if (desc->http_method == HTTP_METHOD_RESPONSE && (
-   ((desc->http_status >= 100 &&
-   desc->http_status < 200) ||
-   desc->http_status == 204))) {
-   relay_abort_http(con, 500,
-   "Internal Server Error", 0);
-   goto abort;
-   }
/*
 * Need to read data from the client after the
 * HTTP header.
@@ -349,6 +336,23 @@ relay_read_http(struct bufferevent *bev, void *arg)
relay_abort_http(con, 500, errstr, 0);
goto abort;
}
+   /*
+* response with a status code of 1xx
+* (Informational) or 204 (No Content) MUST
+* not have a Content-Length (rfc 7230 3.3.3)
+* Instead we check for value != 0 because there are
+* servers that do not follow the rfc and send
+* Content-Length: 0.
+*/
+   if (desc->http_method == HTTP_METHOD_RESPONSE && (
+   ((desc->http_status >= 100 &&
+   desc->http_status < 200) ||
+   desc->http_status == 204)) &&
+   cre->toread != 0) {
+   relay_abort_http(con, 502,
+   "Bad Gateway", 0);
+   goto abort;
+   }
}
  lookup:
if (strcasecmp("Transfer-Encoding", key) == 0 &&

Re: faster printf

[Theo de Raadt]

> I don't think, though, that the commit message should advertise
> this as a performance improvement.  It should be called an intentional
> change of behaviour, now using the format string as a byte string
> like everyone else, no matter whether POSIX explicitly specifies
> it as a character string instead.

I do not agree with your position that POSIX calls this a character
string.

Pure and simply, it is a legacy char *, and any attempt to re-standardize
it from that would crash cars and other heavy equipment.

I'd like to mention in a previous email you used the term fail-closed
incorrectly.

Returning an error number and changing behaviour (truncating)
when there there potentially isn't a caller-checks isn't fail-closed.

Fail-closed implies the software crashes or terminates, so that the
bug can be inspected and fixed.

Returning an error in a newer version of a standard, when a past
standard passed data straight though?

With no way for authors to know the fallout, and requiring them to
audit all code for the broad practice of missing caller-checks?
Not realistic.

So it would be responsible to assume a check can be here.  I think
you have misread POSIX, or someone did 's/char/character/' too broadly.

Re: faster printf

[Todd C. Miller]

On Fri, 17 Nov 2017 10:20:49 -0700, "Todd C. Miller" wrote:

> I've done a brief survey using the test program at the end of
> this message.  Here are the results:

Here's the missing test program.  It compares how mbrtowc() and
snprintf() treat an invalid UTF-8 sequence.  I chose a simple one.

 - todd

#include 
#include 
#include 
#include 
#include 

int main(int argc, char *argv[])
{
const char *fmt = "\xc0 ";
char buf[1024];
wchar_t wc;
mbstate_t ps;
size_t len;
int ret;

setlocale(LC_ALL, "en_US.UTF-8");

memset(, 0, sizeof(ps));
len = mbrtowc(, fmt, MB_CUR_MAX, );
if (len == (size_t)-1 || len == (size_t)-2)
printf("mbrtowc fail (expected)\n");
else
printf("mbrtowc OK, len %zu (unexpected)\n", len);

ret = snprintf(buf, sizeof(buf), "\xc0");
if ((size_t)ret >= sizeof(buf))
printf("printf fail (unexpected)\n");
else
printf("printf OK, ret %d (expected)\n", ret);
}

Re: faster printf

[Ingo Schwarze]

Ingo Schwarze wrote on Fri, Nov 17, 2017 at 03:07:48PM +0100:

[ regarding cases where this may matter in practice ]
>  (2) Programs legitimately calling *printf() with a variable format
>  string in any non-POSIX locale, even if it's just UTF-8.

Whoa.  I just realized there is a very widespread subclass of this:
Programs using gettext(3) for printf(3) format strings.
They are easy to grep for with /printf\(_/.

Here are a few examples:

  aspell-0.60.6.1/prog/aspell.cpp: setlocale (LC_ALL, "");
  aspell-0.60.6.1/prog/aspell.cpp: printf(_("\n  %s filter: %s\n"), ...
and many other instances, but i don't see any return value checks

  avahi-0.7/avahi-utils/avahi-browse.c: setlocale(LC_ALL, "");
  avahi-0.7/avahi-utils/avahi-browse.c: printf(_(": Cache exhausted\n"));
and some others, again no return value checks

  e2fsprogs-1.42.12/resize/main.c: setlocale(LC_CTYPE, "");
  e2fsprogs-1.42.12/resize/online.c: printf(_("Filesystem at %s is mounted...
and hundreds of others, no return value checks

  geeqie-1.3/src/main.c: setlocale(LC_ALL, "");
  geeqie-1.3/src/main.c: log_printf(_("Creating %s dir:%s\n"), ...
dozens of them, no return value checks in sight

  git-2.14.1/gettext.c: setlocale(LC_CTYPE, "");
  git-2.14.1/builtin/merge.c: printf(_("Wonderful.\n"));
hundreds of them, hard to say whether there are any return value
checks, quite possibly not

  gnupg-2.1.23/common/i18n.c: setlocale (LC_ALL, "" );
  gnupg-2.1.23/g10/keygen.c: tty_printf(_("Invalid selection.\n"));
dozens of them, no return value checks in sight

There are also many ports using g_strdup_printf(3) in this way, no
idea what that does, but is seems likely to call *printf(3) internally
in some way.

The show goes on (without checking for setlocale(3) to save time):
gnutls, libiconv, libv4l, mutt, openjp2, openldap, postgresql, vlc,
xz, ...

These are just some ports that i happened to build from source
recently.  So basically, almost *everybody* is using this, but
hardly anybody ever checks for success or failure.

When the return value is not checked, the change still makes the
following difference: Without the change, an invalidly encoded
format string prints nothing.  With the change, an invalidly encoded
format string prints invalidly encoded output.   The former may
sometimes be safer, but the missing information might sometimes
lead to trouble.


That said, i just checked what glibc and commercial Solaris 11 do,
and lo and behold:

  schwarze@donnerwolke:~/Test/printf$ uname -a
  Linux donnerwolke.asta-kit.de 4.9.0-0.bpo.3-686 #1 SMP \
Debian 4.9.30-2+deb9u2~bpo8+1 (2017-06-27) i686 GNU/Linux

  schwarze@donnerwolke:~/Test/printf$ cat printf.c
  #include 
  #include 
  #include 
  
  int
  main(void)
  {
int irc;

if (setlocale(LC_CTYPE, "en_US.UTF-8") == NULL)
errx(1, "setlocale");
irc = printf("start\xc3\xa9middle\xc3%s\n", "end");
printf("%d\n", irc);
return 0;
  }

  schwarze@donnerwolke:~/Test/printf$ make printf
  cc printf.c   -o printf

  schwarze@donnerwolke:~/Test/printf$ ./printf > tmp.txt ; echo $?
  0

  schwarze@donnerwolke:~/Test/printf$ hexdump -C tmp.txt
    73 74 61 72 74 c3 a9 6d  69 64 64 6c 65 c3 65 6e |start..middle.en|
  0010  64 0a 31 38 0a   |d.18.|
  0015

Same thing on Solaris.

Judging from a superficial look at the FreeBSD and NetBSD sources,
they don't appear to validate the format either.

So even if my reading of the standard should be correct (which some
here have challenged), given that everybody else here intuitively
expected the function to behave differently, that the stuff is
actually used a lot in practice, and given that most (if not all)
other implementations appear to behave the way that people intuitively
expect, i think i should stand down and no longer object to the
change.  Maybe i should consider a small clarification in the manual
page afterwards, not yet sure whether that is needed.

I don't think, though, that the commit message should advertise
this as a performance improvement.  It should be called an intentional
change of behaviour, now using the format string as a byte string
like everyone else, no matter whether POSIX explicitly specifies
it as a character string instead.

Yours,
  Ingo

Re: faster printf

[Theo de Raadt]

> On Thu, 16 Nov 2017 11:27:45 -0700, "Theo de Raadt" wrote:
> 
> > Yes, I already proposed that someone made a mistake a while ago.
> 
> This was added in NetBSD in 1995:
> 
> 
> revision 1.17
> date: 1995/05/02 19:52:41;  author: jtc;  state: Exp;  lines: +15 -8;
> The C Standard says that printf's format string is a multi-byte
> character string.  NA1 says that the 99 characters required by the
> Standard have representations in the initial state which are one byte
> long and do not alter the state.
> 
> Thus we can safely break apart the format string with mbtowc() until
> we reach a '%' character, and the process format directive characters
> one by one.
> 
> We really shouldn't be using mbtowc(), rather mbrtowc() (which takes a
> mbstate-t argument) but we don't have the NA1 functions implemented
> yet.  This is safe, because even when we do we're not likely to
> support multi-byte character encodings that use shift states.
> 
> 
> The change was never adopted by FreeBSD and modern NetBSD doesn't
> include do it either.  There is nothing in C99 that I can find to
> indicate that the format string is multi-byte.  Either this part
> of NA1 was not adopted as part of C99 or jtc misread the standard.
> 
> I've done a brief survey using the test program at the end of
> this message.  Here are the results:
> 
> OpenBSD:
> mbrtowc fail (expected)
> printf fail (unexpected)
> Linux:
> mbrtowc fail (expected)
> printf OK, ret 1 (expected)
> macOS:
> mbrtowc fail (expected)
> printf OK, ret 1 (expected)
> Solaris 11:
> mbrtowc fail (expected)
> printf OK, ret 1 (expected)
> 
> OpenBSD is the outlier here.  If everyone else interprets the
> standard differently that we do, I think it is reasonable to say
> that our interpretation in incorrect.  Furthermore, the source of
> that change (NetBSD) no longer includes it.

I agree completely with that assessment.

The POSIX and ANSI commitees are often insafe and careless.  But
if they had suddenly decided that the format string needed to be
PARSED, they would have risked all sorts of crazy embedded shit
using format strings using \0NNN breaking in crazy ways.

Re: faster printf

[Todd C. Miller]

On Thu, 16 Nov 2017 11:27:45 -0700, "Theo de Raadt" wrote:

> Yes, I already proposed that someone made a mistake a while ago.

This was added in NetBSD in 1995:


revision 1.17
date: 1995/05/02 19:52:41;  author: jtc;  state: Exp;  lines: +15 -8;
The C Standard says that printf's format string is a multi-byte
character string.  NA1 says that the 99 characters required by the
Standard have representations in the initial state which are one byte
long and do not alter the state.

Thus we can safely break apart the format string with mbtowc() until
we reach a '%' character, and the process format directive characters
one by one.

We really shouldn't be using mbtowc(), rather mbrtowc() (which takes a
mbstate-t argument) but we don't have the NA1 functions implemented
yet.  This is safe, because even when we do we're not likely to
support multi-byte character encodings that use shift states.


The change was never adopted by FreeBSD and modern NetBSD doesn't
include do it either.  There is nothing in C99 that I can find to
indicate that the format string is multi-byte.  Either this part
of NA1 was not adopted as part of C99 or jtc misread the standard.

I've done a brief survey using the test program at the end of
this message.  Here are the results:

OpenBSD:
mbrtowc fail (expected)
printf fail (unexpected)
Linux:
mbrtowc fail (expected)
printf OK, ret 1 (expected)
macOS:
mbrtowc fail (expected)
printf OK, ret 1 (expected)
Solaris 11:
mbrtowc fail (expected)
printf OK, ret 1 (expected)

OpenBSD is the outlier here.  If everyone else interprets the
standard differently that we do, I think it is reasonable to say
that our interpretation in incorrect.  Furthermore, the source of
that change (NetBSD) no longer includes it.

 - todd

Re: ifconfig gif0 deletetunnel is different

[Alexander Bluhm]

On Fri, Nov 17, 2017 at 04:51:55PM +0100, Sebastian Benoit wrote:
> ok?

OK bluhm@ with a nit

> +.It Cm -tunnel Ar src_address dest_address
> +Remove the source and destination tunnel addresses.

-tunnel does not take arguments

ifconfig gif0 deletetunnel is different

[Sebastian Benoit]

ifconfig  deletetunnel is different from other ifconfig  commands,
as others have options "something" and "-something".

Here i add "-tunnel" and keep "deletetunnel", but undocumented to be removed
2 releases hence. I would update current.html with this.

ok?

(benno_ifconfig_tunnel2.diff)

diff --git sbin/ifconfig/ifconfig.8 sbin/ifconfig/ifconfig.8
index 0e9da423f0c..fdbea82b652 100644
--- sbin/ifconfig/ifconfig.8
+++ sbin/ifconfig/ifconfig.8
@@ -1531,9 +1531,8 @@ for a complete list of the available protocols.
 .Bk -words
 .Nm ifconfig
 .Ar tunnel-interface
-.Op Cm deletetunnel Ar src_address dest_address
 .Op Oo Fl Oc Ns Cm keepalive Ar period count
-.Op Cm tunnel Ar src_address dest_address
+.Op Oo Fl Oc Ns Cm tunnel Ar src_address dest_address
 .Op Cm tunneldomain Ar tableid
 .Op Oo Fl Oc Ns Cm vnetid Ar network-id
 .Ek
@@ -1547,8 +1546,6 @@ and
 are all tunnel interfaces.
 The following options are available:
 .Bl -tag -width Ds
-.It Cm deletetunnel Ar src_address dest_address
-Remove the source and destination tunnel addresses.
 .It Cm keepalive Ar period count
 Enable
 .Xr gre 4
@@ -1576,6 +1573,8 @@ Both addresses must be of the same family.
 The optional destination port can be specified for interfaces such as
 .Xr vxlan 4 ,
 which further encapsulate the packets in UDP datagrams.
+.It Cm -tunnel Ar src_address dest_address
+Remove the source and destination tunnel addresses.
 .It Cm tunneldomain Ar tableid
 Use routing table
 .Ar tableid
diff --git sbin/ifconfig/ifconfig.c sbin/ifconfig/ifconfig.c
index 86cf4f505b0..6e371bb8b1d 100644
--- sbin/ifconfig/ifconfig.c
+++ sbin/ifconfig/ifconfig.c
@@ -424,10 +424,12 @@ const struct  cmd {
{ "maxupd", NEXTARG,0,  setpfsync_maxupd },
{ "defer",  1,  0,  setpfsync_defer },
{ "-defer", 0,  0,  setpfsync_defer },
-   { "tunnel", NEXTARG2,   0,  NULL, settunnel } ,
-   { "deletetunnel",  0,   0,  deletetunnel } ,
-   { "tunneldomain", NEXTARG,  0,  settunnelinst } ,
-   { "tunnelttl",  NEXTARG,0,  settunnelttl } ,
+   { "tunnel", NEXTARG2,   0,  NULL, settunnel },
+   { "-tunnel",0,  0,  deletetunnel },
+   /* deletetunnel is for backward compat, remove during 6.4-current */
+   { "deletetunnel",  0,   0,  deletetunnel },
+   { "tunneldomain", NEXTARG,  0,  settunnelinst },
+   { "tunnelttl",  NEXTARG,0,  settunnelttl },
{ "pppoedev",   NEXTARG,0,  setpppoe_dev },
{ "pppoesvc",   NEXTARG,0,  setpppoe_svc },
{ "-pppoesvc",  1,  0,  setpppoe_svc },

Re: hide wpakey from root by default

[Stefan Sperling]

On Fri, Nov 17, 2017 at 01:20:53PM -, Christian Weisgerber wrote:
> On 2017-11-17, Stefan Sperling  wrote:
> 
> > This diff makes the WPA key available only if the interface is in
> > debug mode (suggestion by phessler). If this is acceptable then I
> > can also try to squeeze a hint into the ifconfig man page so that
> > this mechanism can be discovered by those who don't read kernel code.
> 
> I don't care one way or the other except that I would like this to
> be CONSISTENT across all interface types with authentication details:
> wlan, ppp, etc.
> 
> "But ppp isn't a radio..." You don't know the details.  They're in
> the process of changing it, but for instance Deutsche Telekom's DSL
> used authentication information for PPPoE that would also be the
> default access to the online customer account settings.
> 

I understand the desire for consistency.
It's a bit more work than I have time for today, but I agree
it would be worthwhile to put more effort into this.

But do we now modify every such ioctl handler in the kernel?
Or is modifying ifconfig sufficient?
We are more concerned about textual display rather than the
kernel/userland ioctl boundary, correct?

The option list for ifconfig is [-AaC]. Plenty of letters available.
We could add:

   -P  Show authentication details such as passwords (not displayed by default))

Re: hide wpakey from root by default

[Christian Weisgerber]

On 2017-11-17, Stefan Sperling  wrote:

> This diff makes the WPA key available only if the interface is in
> debug mode (suggestion by phessler). If this is acceptable then I
> can also try to squeeze a hint into the ifconfig man page so that
> this mechanism can be discovered by those who don't read kernel code.

I don't care one way or the other except that I would like this to
be CONSISTENT across all interface types with authentication details:
wlan, ppp, etc.

"But ppp isn't a radio..." You don't know the details.  They're in
the process of changing it, but for instance Deutsche Telekom's DSL
used authentication information for PPPoE that would also be the
default access to the online customer account settings.

-- 
Christian "naddy" Weisgerber  na...@mips.inka.de

Re: add an iqdrops view to systat to show interface queue drops

[Christian Weisgerber]

On 2017-11-17, David Gwynne  wrote:

> can we have modified displays within a view?

We already have this for the ifstat view.

 The character B changes the counter view between bytes and
 bits.  Pressing b displays statistics as calculated from boot
 time.  r changes the counters to show their totals as
 calculated between display refreshes.  t changes the counters
 to show the average per second over the display refresh
 interval; this is the default.

-- 
Christian "naddy" Weisgerber  na...@mips.inka.de

Re: hide wpakey from root by default

[Jeremie Courreges-Anglas]

On Fri, Nov 17 2017, Stefan Sperling  wrote:
> There have been several instances of people mailing out WPA keys as
> part of ifconfig output, e.g. in bug reports. This happens when you
> run ifconfig as root and copy/paste without thinking.
>
> I see no real need to ever show the key except in circumstances where
> the key needs to be legitimately passed on to someone else ("do you
> happen to know the wifi key?" in a bar). Though for devices which
> want the plaintext passphrase instead of the hashed key our ifconfig
> output is already useless for this purpose anyway.
>
> This diff makes the WPA key available only if the interface is in
> debug mode (suggestion by phessler). If this is acceptable then I
> can also try to squeeze a hint into the ifconfig man page so that
> this mechanism can be discovered by those who don't read kernel code.
>
> OK?

ok jca@

> Index: ieee80211_ioctl.c
> ===
> RCS file: /cvs/src/sys/net80211/ieee80211_ioctl.c,v
> retrieving revision 1.55
> diff -u -p -r1.55 ieee80211_ioctl.c
> --- ieee80211_ioctl.c 27 Oct 2017 12:22:40 -  1.55
> +++ ieee80211_ioctl.c 17 Nov 2017 10:13:06 -
> @@ -491,8 +491,8 @@ ieee80211_ioctl(struct ifnet *ifp, u_lon
>   psk = (struct ieee80211_wpapsk *)data;
>   if (ic->ic_flags & IEEE80211_F_PSK) {
>   psk->i_enabled = 1;
> - /* do not show any keys to non-root user */
> - if (suser(curproc, 0) != 0) {
> + if (suser(curproc, 0) != 0 ||
> + (ifp->if_flags & IFF_DEBUG) == 0) {
>   psk->i_enabled = 2;
>   memset(psk->i_psk, 0, sizeof(psk->i_psk));
>   break;  /* return ok but w/o key */
>

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

Re: hide wpakey from root by default

[Sebastian Benoit]

Stefan Sperling(s...@stsp.name) on 2017.11.17 11:41:57 +0100:
> There have been several instances of people mailing out WPA keys as
> part of ifconfig output, e.g. in bug reports. This happens when you
> run ifconfig as root and copy/paste without thinking.
> 
> I see no real need to ever show the key except in circumstances where
> the key needs to be legitimately passed on to someone else ("do you
> happen to know the wifi key?" in a bar). Though for devices which
> want the plaintext passphrase instead of the hashed key our ifconfig
> output is already useless for this purpose anyway.
> 
> This diff makes the WPA key available only if the interface is in
> debug mode (suggestion by phessler). If this is acceptable then I
> can also try to squeeze a hint into the ifconfig man page so that
> this mechanism can be discovered by those who don't read kernel code.
> 
> OK?

ok

> Index: ieee80211_ioctl.c
> ===
> RCS file: /cvs/src/sys/net80211/ieee80211_ioctl.c,v
> retrieving revision 1.55
> diff -u -p -r1.55 ieee80211_ioctl.c
> --- ieee80211_ioctl.c 27 Oct 2017 12:22:40 -  1.55
> +++ ieee80211_ioctl.c 17 Nov 2017 10:13:06 -
> @@ -491,8 +491,8 @@ ieee80211_ioctl(struct ifnet *ifp, u_lon
>   psk = (struct ieee80211_wpapsk *)data;
>   if (ic->ic_flags & IEEE80211_F_PSK) {
>   psk->i_enabled = 1;
> - /* do not show any keys to non-root user */
> - if (suser(curproc, 0) != 0) {
> + if (suser(curproc, 0) != 0 ||
> + (ifp->if_flags & IFF_DEBUG) == 0) {
>   psk->i_enabled = 2;
>   memset(psk->i_psk, 0, sizeof(psk->i_psk));
>   break;  /* return ok but w/o key */
> 

Re: hide wpakey from root by default

[Paul Irofti]

On Fri, Nov 17, 2017 at 11:41:57AM +0100, Stefan Sperling wrote:
> There have been several instances of people mailing out WPA keys as
> part of ifconfig output, e.g. in bug reports. This happens when you
> run ifconfig as root and copy/paste without thinking.
> 
> I see no real need to ever show the key except in circumstances where
> the key needs to be legitimately passed on to someone else ("do you
> happen to know the wifi key?" in a bar). Though for devices which
> want the plaintext passphrase instead of the hashed key our ifconfig
> output is already useless for this purpose anyway.
> 
> This diff makes the WPA key available only if the interface is in
> debug mode (suggestion by phessler). If this is acceptable then I
> can also try to squeeze a hint into the ifconfig man page so that
> this mechanism can be discovered by those who don't read kernel code.
> 
> OK?

I wanted this for so long, but never bugged me enough to fix it.
Thanks for that and OK!

> 
> Index: ieee80211_ioctl.c
> ===
> RCS file: /cvs/src/sys/net80211/ieee80211_ioctl.c,v
> retrieving revision 1.55
> diff -u -p -r1.55 ieee80211_ioctl.c
> --- ieee80211_ioctl.c 27 Oct 2017 12:22:40 -  1.55
> +++ ieee80211_ioctl.c 17 Nov 2017 10:13:06 -
> @@ -491,8 +491,8 @@ ieee80211_ioctl(struct ifnet *ifp, u_lon
>   psk = (struct ieee80211_wpapsk *)data;
>   if (ic->ic_flags & IEEE80211_F_PSK) {
>   psk->i_enabled = 1;
> - /* do not show any keys to non-root user */
> - if (suser(curproc, 0) != 0) {
> + if (suser(curproc, 0) != 0 ||
> + (ifp->if_flags & IFF_DEBUG) == 0) {
>   psk->i_enabled = 2;
>   memset(psk->i_psk, 0, sizeof(psk->i_psk));
>   break;  /* return ok but w/o key */

hide wpakey from root by default

[Stefan Sperling]

There have been several instances of people mailing out WPA keys as
part of ifconfig output, e.g. in bug reports. This happens when you
run ifconfig as root and copy/paste without thinking.

I see no real need to ever show the key except in circumstances where
the key needs to be legitimately passed on to someone else ("do you
happen to know the wifi key?" in a bar). Though for devices which
want the plaintext passphrase instead of the hashed key our ifconfig
output is already useless for this purpose anyway.

This diff makes the WPA key available only if the interface is in
debug mode (suggestion by phessler). If this is acceptable then I
can also try to squeeze a hint into the ifconfig man page so that
this mechanism can be discovered by those who don't read kernel code.

OK?

Index: ieee80211_ioctl.c
===
RCS file: /cvs/src/sys/net80211/ieee80211_ioctl.c,v
retrieving revision 1.55
diff -u -p -r1.55 ieee80211_ioctl.c
--- ieee80211_ioctl.c   27 Oct 2017 12:22:40 -  1.55
+++ ieee80211_ioctl.c   17 Nov 2017 10:13:06 -
@@ -491,8 +491,8 @@ ieee80211_ioctl(struct ifnet *ifp, u_lon
psk = (struct ieee80211_wpapsk *)data;
if (ic->ic_flags & IEEE80211_F_PSK) {
psk->i_enabled = 1;
-   /* do not show any keys to non-root user */
-   if (suser(curproc, 0) != 0) {
+   if (suser(curproc, 0) != 0 ||
+   (ifp->if_flags & IFF_DEBUG) == 0) {
psk->i_enabled = 2;
memset(psk->i_psk, 0, sizeof(psk->i_psk));
break;  /* return ok but w/o key */

armv7/man4: sxitimer.4 small update

[Artturi Alm]

Hi,

variations of this timer "IP" can be found from all sunxis i've seen,
but let's add only the few where this is actually used now, instead of
keeping A20 listed.
Hopefully the addition of agtimer timer to the description makes sense too :)

-Artturi


diff --git a/share/man/man4/man4.armv7/sxitimer.4 
b/share/man/man4/man4.armv7/sxitimer.4
index 4b91d370a9f..2f9706b1203 100644
--- a/share/man/man4/man4.armv7/sxitimer.4
+++ b/share/man/man4/man4.armv7/sxitimer.4
@@ -19,14 +19,16 @@
 .Os
 .Sh NAME
 .Nm sxitimer
-.Nd A1X/A20 timer device
+.Nd A10/A10s/A13 timer device
 .Sh SYNOPSIS
 .Cd "sxitimer* at sunxi?"
 .Sh DESCRIPTION
 The
 .Nm
-driver provides support for the timer devices integrated in Allwinner 
Technology
-A1X/A20 SoCs.
+driver provides support for the timers integrated in Allwinner Technology
+SoCs, without ARM generic timer.
+.Sh SEE ALSO
+.Xr agtimer 4
 .Sh HISTORY
 The
 .Nm

Re: clang: Avoid EBX/RBX

[Antoine Jacoutot]

On Fri, Nov 17, 2017 at 08:46:28AM +, Stuart Henderson wrote:
> On 2017/11/17 09:36, Antoine Jacoutot wrote:
> > > here are.  Personally I'd like to see devel/libexecinfo to be removed
> > > from ports if possible.
> > 
> > +1
> > I even volunteer to do that part.
> > AFAIK it's not a *hard* requirement for anything.
> 
> It would actually be easier not to have it, ports-wise. Things tend
> to pick it up as a hidden dependency. (Often they don't even pick it up
> properly, just notice the header and try to pull that in, but not actually
> link against the library - weird set of functions these, Linux has them
> in libc).

Good, I'll tedu it and send a patch tomorrow :-)

-- 
Antoine

Re: clang: Avoid EBX/RBX

[Stuart Henderson]

On 2017/11/17 09:36, Antoine Jacoutot wrote:
> > here are.  Personally I'd like to see devel/libexecinfo to be removed
> > from ports if possible.
> 
> +1
> I even volunteer to do that part.
> AFAIK it's not a *hard* requirement for anything.

It would actually be easier not to have it, ports-wise. Things tend
to pick it up as a hidden dependency. (Often they don't even pick it up
properly, just notice the header and try to pull that in, but not actually
link against the library - weird set of functions these, Linux has them
in libc).

Re: clang: Avoid EBX/RBX

[Antoine Jacoutot]

> here are.  Personally I'd like to see devel/libexecinfo to be removed
> from ports if possible.

+1
I even volunteer to do that part.
AFAIK it's not a *hard* requirement for anything.

-- 
Antoine

man4/openprom.4: striking ioctl

[Artturi Alm]

Hi,

highly visible outside the default output,
ie. here: https://man.openbsd.org/openprom

-Artturi


diff --git a/share/man/man4/man4.macppc/openprom.4 
b/share/man/man4/man4.macppc/openprom.4
index 1f5145c578f..580caaea517 100644
--- a/share/man/man4/man4.macppc/openprom.4
+++ b/share/man/man4/man4.macppc/openprom.4
@@ -88,7 +88,7 @@ The following ioctls are supported:
 .Bl -tag -width OPIOCGETOPTNODE
 .It Dv OPIOCGETOPTNODE
 Takes nothing, and fills in the options node number.
-.It OPIOCGETNEXT
+.It Dv OPIOCGETNEXT
 Takes a node number and returns the number of the following node.
 The node following the last node is number 0;
 the node following number 0 is the first node.
diff --git a/share/man/man4/man4.sparc64/openprom.4 
b/share/man/man4/man4.sparc64/openprom.4
index 78e6b036735..1c6efca94d8 100644
--- a/share/man/man4/man4.sparc64/openprom.4
+++ b/share/man/man4/man4.sparc64/openprom.4
@@ -88,7 +88,7 @@ The following ioctls are supported:
 .Bl -tag -width OPIOCGETOPTNODE
 .It Dv OPIOCGETOPTNODE
 Takes nothing, and fills in the options node number.
-.It OPIOCGETNEXT
+.It Dv OPIOCGETNEXT
 Takes a node number and returns the number of the following node.
 The node following the last node is number 0;
 the node following number 0 is the first node.

Re: Tweak OF_getnodebyname()

[Artturi Alm]

On Thu, Nov 16, 2017 at 09:39:20PM +0100, Mark Kettenis wrote:
> The current FDT implementation is fairly useless since it doesn't
> actually look at the child nodes.  The macppc implementation walks the
> entire tree.  But all current use cases of this function only look at
> children of the passed node.  Diff below makes OF_getnodebyname()
> behave like that.
> 
> ok?
> 

Makes sense, but i'd be tempted to rename to OF_getchildbyname().
tested on top of 6.2, and /dev/openprom did work with eeprom -p as always,
with bbb+cubie2+panda+wandboard.
slightly related, as the options thing in openprom was the only user on arm,
should it be "chosen" there instead of "options" ?

while there, maybe this too:

diff --git a/sys/dev/ofw/fdt.c b/sys/dev/ofw/fdt.c
index 54e1e01edbb..e9a717fae8f 100644
--- a/sys/dev/ofw/fdt.c
+++ b/sys/dev/ofw/fdt.c
@@ -336,7 +336,7 @@ fdt_node_property_int(void *node, char *name, int *out)
 }
 
 /*
- * Retrieves next node, skipping all the children nodes of the pointed node
+ * Retrieves the first child of an node.
  */
 void *
 fdt_child_node(void *node)



-Artturi

> 
> Index: ofw/fdt.c
> ===
> RCS file: /cvs/src/sys/dev/ofw/fdt.c,v
> retrieving revision 1.20
> diff -u -p -r1.20 fdt.c
> --- ofw/fdt.c 12 Mar 2017 11:44:42 -  1.20
> +++ ofw/fdt.c 16 Nov 2017 20:28:27 -
> @@ -773,11 +773,9 @@ OF_getnodebyname(int handle, const char 
>   if (handle == 0)
>   node = fdt_find_node("/");
>  
> - while (node) {
> + for (node = fdt_child_node(node); node; node = fdt_next_node(node)) {
>   if (strcmp(name, fdt_node_name(node)) == 0)
>   break;
> -
> - node = fdt_next_node(node);
>   }
>  
>   return node ? ((char *)node - (char *)tree.header) : 0;
> 

Re: athn(4) USB open firmware support

[Kevin Lo]

On Fri, Nov 17, 2017 at 12:28:35AM +0100, Stefan Sperling wrote:
> 
> This diff switches athn(4) USB devices to open source firmware.
> 
> I only have an AR9271 device which I can test with:
> athn0 at uhub1 port 2 configuration 1 interface 0 "ATHEROS USB2.0 WLAN" rev 
> 2.00/1.08 addr 3
> athn0: AR9271 rev 1 (1T1R), ROM rev 13, address xx:xx:xx:xx:xx:xx
> 
> The diff switches AR7010 devices over as well because this new code
> will *not* support the old binary-only firmware anyway.
> But it is possible that AR7010 devices don't work yet with this diff.
> Can anybody help and test such a device? And if anyone would like to
> donate an AR7010 device for me to develop with, that would be appreciated.
> 
> The new firmware package 'athn-firmware-1.1p3', which contains the open
> firmware files, is required. Specifically, the diff needs these files
> from the new firmware package:
> 
>   /etc/firmware/athn-open-ar7010
>   /etc/firmware/athn-open-ar9271
> 
> The firmware mirrors currently still ship version 1.1p2 which lacks the
> open firmware files. But the pre-built firmware images are already part
> of package mirrors, so a new firmware package can be built without
> having to first cross-compile the firmware, like this:
> 
>   pkg_delete athn-firmware  # else conflict during install, no idea why
>   cd /usr/ports/sysutils/firmware/athn
>   make FETCH_PACKAGES=Yes install
> 
> Thanks again to bentley@ for porting both the required cross toolchain
> and the open ath9k firmware during p2k17!

Thanks for stepping up and getting this done.  Tested on TP-LINK TL-WN722N,
and it works for me :)

$ dmesg | grep athn0
athn0 at uhub0 port 4 configuration 1 interface 0 "ATHEROS USB2.0 WLAN" rev 
2.00/1.08 addr 3
athn0: AR9271 rev 1 (1T1R), ROM rev 13, address d8:5d:4c:98:60:f9

$ pkg_info -L athn-firmware-1.1p3
Information for inst:athn-firmware-1.1p3

Files:
/etc/firmware/athn-ar7010
/etc/firmware/athn-open-ar7010
/etc/firmware/athn-ar7010-11
/etc/firmware/athn-ar9271
/etc/firmware/athn-open-ar9271
/etc/firmware/athn-license

$ ifconfig athn0
athn0: flags=8843 mtu 1500
lladdr d8:5d:4c:98:60:f9
index 6 priority 4 llprio 3
groups: wlan egress
media: IEEE802.11 autoselect (HT-MCS0 mode 11n)
status: active
ieee80211: nwid bf-2.4g chan 3 bssid f4:28:53:6c:04:1a -20dBm wpakey 
0xd5d0c0f79932dd4619e08d995981d9aa9acc622d6f3a40f0dd4fb2e8bdbc8c98 wpaprotos 
wpa2 wpaakms psk wpaciphers ccmp wpagroupcipher ccmp
inet 172.29.30.123 netmask 0xff00 broadcast 172.29.30.255