libc: wrap for internal calls

[Philip Guenther]

alpha uses sysarch() to implement the floating-point control routines and 
mips64 uses sysarch() to implement cacheflush().  Let's provide 
lib/libc/hidden/machine/sysarch.h with the obvious contents so that those 
internal calls go directly to the syscall stub without possiblity of 
override or unnecessary relocations.

ok?

Philip Guenther

-
/*  $OpenBSD$   */
/*
 * Copyright (c) 2016 Philip Guenther 
 *
 * 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 _LIBC_MACHINE_SYSARCH_H_
#define _LIBC_MACHINE_SYSARCH_H_

#include_next 

PROTO_NORMAL(sysarch);

#endif /* !_LIBC_MACHINE_SYSARCH_H_ */

libc/amd64: stop using sigblock/sigsetmask in setjmp/longjmp

[Philip Guenther]

This converts setjmp/longjmp and their sig* versions to use the 
sigprocmask syscall directly instead of the obsolete sigblock/sigsetmask 
APIs.

No change in the results of the regress/lib/libc/*setjmp* tests.

Ok?

Philip Guenther


Index: setjmp.S
===
RCS file: /data/src/openbsd/src/lib/libc/arch/amd64/gen/setjmp.S,v
retrieving revision 1.6
diff -u -p -r1.6 setjmp.S
--- setjmp.S12 May 2016 15:46:03 -  1.6
+++ setjmp.S20 May 2016 08:38:12 -
@@ -37,7 +37,7 @@
  */
 
 
-#include 
+#include "SYS.h"
 #include 
 
.section.openbsd.randomdata,"aw",@progbits
@@ -59,10 +59,12 @@ __jmpxor:
  */
 
 ENTRY(setjmp)
-   pushq   %rdi
-   xorq%rdi,%rdi
-   call_C_LABEL(_libc_sigblock)
-   popq%rdi
+   movq%rdi,%r8/* save jmpbuf addr */
+   movl$1,%edi /* how = SIG_BLOCK */
+   xorl%esi,%esi   /* set = empty */
+   movl$SYS_sigprocmask,%eax
+   syscall
+   movq%r8,%rdi/* restore jmpbuf addr */
movq%rax,(_JB_SIGMASK * 8)(%rdi)
 
movq(%rsp),%r11
@@ -91,10 +93,11 @@ ENTRY(longjmp)
movq%rdi,%r12
movl%esi,%r8d
 
-   movq(_JB_SIGMASK * 8)(%rdi),%rdi
-   pushq   %r8
-   call_C_LABEL(_libc_sigsetmask)
-   popq%r8
+   movq(_JB_SIGMASK * 8)(%rdi),%rsi/* get set from sc_mask */
+   movl$3,%edi /* how = SIG_SETMASK */
+   movl$SYS_sigprocmask,%eax
+   syscall
+   movl%r8d,%eax
 
leaq __jmpxor(%rip),%rcx
movq(_JB_RBX * 8)(%r12),%rbx
@@ -112,7 +115,6 @@ ENTRY(longjmp)
movq(_JB_R12 * 8)(%r12),%r12
xorq%rcx,%rcx
 
-   movl%r8d,%eax
testl   %eax,%eax
jnz 1f
incl%eax
Index: sigsetjmp.S
===
RCS file: /data/src/openbsd/src/lib/libc/arch/amd64/gen/sigsetjmp.S,v
retrieving revision 1.6
diff -u -p -r1.6 sigsetjmp.S
--- sigsetjmp.S 12 May 2016 15:46:03 -  1.6
+++ sigsetjmp.S 20 May 2016 08:38:12 -
@@ -37,7 +37,7 @@
  */
 
 
-#include 
+#include "SYS.h"
 #include 
 
.hidden __jmpxor
@@ -57,10 +57,12 @@ ENTRY(sigsetjmp)
testl   %esi,%esi
jz  2f
 
-   pushq   %rdi
-   xorq%rdi,%rdi
-   call_C_LABEL(_libc_sigblock)
-   popq%rdi
+   movq%rdi,%r8/* save jmpbuf addr */
+   movl$1,%edi /* how = SIG_BLOCK */
+   xorl%esi,%esi   /* set = empty */
+   movl$SYS_sigprocmask,%eax
+   syscall
+   movq%r8,%rdi/* restore jmpbuf addr */
movq%rax,(_JB_SIGMASK * 8)(%rdi)
 
 2: movq(%rsp),%r11
@@ -87,13 +89,15 @@ END(sigsetjmp)
 
 ENTRY(siglongjmp)
movq%rdi,%r12
-   pushq   %rsi
+   movl%esi,%r8d
cmpl$0, (_JB_SIGFLAG * 8)(%rdi)
jz  2f
 
-   movq(_JB_SIGMASK * 8)(%rdi),%rdi
-   call_C_LABEL(_libc_sigsetmask)
-2: popq%rax
+   movq(_JB_SIGMASK * 8)(%rdi),%rsi/* get set from sc_mask */
+   movl$3,%edi /* how = SIG_SETMASK */
+   movl$SYS_sigprocmask,%eax
+   syscall
+2: movl%r8d,%eax
 
leaq __jmpxor(%rip),%rcx
movq(_JB_RBX * 8)(%r12),%rbx

prefer AF_* over PF_*

[Philip Guenther]

About the only place userland code should use PF_* socket constants is 
with sysctl(3)'s CTL_NET hierarchy.  All the standardized functions are 
defined as taking AF_* values.  Let's use the preferred names in the 
getaddrinfo(3) and socketpair(2) manpages.

ok?

Philip Guenther

Index: net/getaddrinfo.3
===
RCS file: /data/src/openbsd/src/lib/libc/net/getaddrinfo.3,v
retrieving revision 1.57
diff -u -p -r1.57 getaddrinfo.3
--- net/getaddrinfo.3   16 Feb 2015 18:26:56 -  1.57
+++ net/getaddrinfo.3   20 May 2016 00:06:21 -
@@ -94,7 +94,7 @@ The protocol family that should be used.
 When
 .Fa ai_family
 is set to
-.Dv PF_UNSPEC ,
+.Dv AF_UNSPEC ,
 it means the caller will accept any protocol family supported by the
 operating system.
 .It Fa ai_socktype
@@ -229,7 +229,7 @@ behaves as if the caller provided a
 with
 .Fa ai_family
 set to
-.Dv PF_UNSPEC ,
+.Dv AF_UNSPEC ,
 .Fa ai_flags
 set to
 .Dv AI_ADDRCONFIG ,
@@ -351,7 +351,7 @@ int s;
 const char *cause = NULL;
 
 memset(, 0, sizeof(hints));
-hints.ai_family = PF_UNSPEC;
+hints.ai_family = AF_UNSPEC;
 hints.ai_socktype = SOCK_STREAM;
 error = getaddrinfo("www.kame.net", "www", , );
 if (error)
@@ -393,7 +393,7 @@ int nsock;
 const char *cause = NULL;
 
 memset(, 0, sizeof(hints));
-hints.ai_family = PF_UNSPEC;
+hints.ai_family = AF_UNSPEC;
 hints.ai_socktype = SOCK_STREAM;
 hints.ai_flags = AI_PASSIVE;
 error = getaddrinfo(NULL, "www", , );
Index: sys/socketpair.2
===
RCS file: /data/src/openbsd/src/lib/libc/sys/socketpair.2,v
retrieving revision 1.19
diff -u -p -r1.19 socketpair.2
--- sys/socketpair.219 Mar 2016 22:10:49 -  1.19
+++ sys/socketpair.220 May 2016 00:05:48 -
@@ -124,7 +124,7 @@ This call is currently implemented only 
 Many operating systems only accept a
 .Fa protocol
 of
-.Dv PF_UNSPEC ,
+.Dv AF_UNSPEC ,
 so that should be used instead of
-.Dv PF_LOCAL
+.Dv AF_LOCAL
 for maximal portability.

libc: delete unused hash algorithms

[Philip Guenther]

Overriding the hash algorithm used by the Berkeley DB bits isn't support 
(it would break getpw* if nothing else) and hasn't been possible since the 
symbol hiding effort last fall.  So eliminate the redirection through a 
variable and declare it as a hidden function to eliminate the relocations 
for it.

Ok?


Philip Guenther


Index: hidden/db.h
===
RCS file: /data/src/openbsd/src/lib/libc/hidden/db.h,v
retrieving revision 1.3
diff -u -p -r1.3 db.h
--- hidden/db.h 17 Oct 2015 21:48:42 -  1.3
+++ hidden/db.h 20 May 2016 09:03:13 -
@@ -73,6 +73,9 @@ DB*__bt_open(const char *, int, int, co
 DB *__hash_open(const char *, int, int, const HASHINFO *, int);
 DB *__rec_open(const char *, int, int, const RECNOINFO *, int);
 void   __dbpanic(DB *dbp);
+
+/* Default hash function, from db/hash/hash_func.c */
+u_int32_t  __default_hash(const void *, size_t);
 __END_HIDDEN_DECLS
 
 PROTO_NORMAL(dbopen);
Index: stdlib/hcreate.c
===
RCS file: /data/src/openbsd/src/lib/libc/stdlib/hcreate.c,v
retrieving revision 1.6
diff -u -p -r1.6 hcreate.c
--- stdlib/hcreate.c10 Sep 2015 18:13:46 -  1.6
+++ stdlib/hcreate.c20 May 2016 09:03:55 -
@@ -55,6 +55,8 @@
 #include 
 #include 
 
+#include /* for __default_hash */
+
 #ifndef _DIAGASSERT
 #define _DIAGASSERT(x)
 #endif
@@ -79,9 +81,6 @@ SLIST_HEAD(internal_head, internal_entry
 #defineMAX_BUCKETS_LG2 (sizeof (size_t) * 8 - 1 - 5)
 #defineMAX_BUCKETS ((size_t)1 << MAX_BUCKETS_LG2)
 
-/* Default hash function, from db/hash/hash_func.c */
-extern u_int32_t (*__default_hash)(const void *, size_t);
-
 static struct internal_head *htable;
 static size_t htablesize;
 
@@ -164,7 +163,7 @@ hsearch(ENTRY item, ACTION action)
_DIAGASSERT(action == ENTER || action == FIND);
 
len = strlen(item.key);
-   hashval = (*__default_hash)(item.key, len);
+   hashval = __default_hash(item.key, len);
 
head = [hashval & (htablesize - 1)];
ie = SLIST_FIRST(head);
Index: db/hash/extern.h
===
RCS file: /data/src/openbsd/src/lib/libc/db/hash/extern.h,v
retrieving revision 1.8
diff -u -p -r1.8 extern.h
--- db/hash/extern.h27 Aug 2015 04:37:09 -  1.8
+++ db/hash/extern.h20 May 2016 09:04:27 -
@@ -56,9 +56,6 @@ int__put_page(HTAB *, char *, u_int32_
 void__reclaim_buf(HTAB *, BUFHEAD *);
 int __split_page(HTAB *, u_int32_t, u_int32_t);
 
-/* Default hash routine. */
-extern u_int32_t (*__default_hash)(const void *, size_t);
-
 #ifdef HASH_STATISTICS
 extern int hash_accesses, hash_collisions, hash_expansions, hash_overflows;
 #endif
Index: db/hash/hash_func.c
===
RCS file: /data/src/openbsd/src/lib/libc/db/hash/hash_func.c,v
retrieving revision 1.10
diff -u -p -r1.10 hash_func.c
--- db/hash/hash_func.c 5 Aug 2005 13:03:00 -   1.10
+++ db/hash/hash_func.c 29 May 2016 02:41:12 -
@@ -35,118 +35,10 @@
 #include 
 
 #include 
-#include "hash.h"
-#include "page.h"
-#include "extern.h"
-
-#ifdef notdef
-static u_int32_t hash1(const void *, size_t);
-static u_int32_t hash2(const void *, size_t);
-static u_int32_t hash3(const void *, size_t);
-#endif
-static u_int32_t hash4(const void *, size_t);
-
-/* Default hash function. */
-u_int32_t (*__default_hash)(const void *, size_t) = hash4;
-
-#ifdef notdef
-/*
- * Assume that we've already split the bucket to which this key hashes,
- * calculate that bucket, and check that in fact we did already split it.
- *
- * EJB's original hsearch hash.
- */
-#define PRIME1 37
-#define PRIME2 1048583
-
-u_int32_t
-hash1(const void *key, size_t len)
-{
-   u_int32_t h;
-   u_int8_t *k;
-
-   h = 0;
-   k = (u_int8_t *)key;
-   /* Convert string to integer */
-   while (len--)
-   h = h * PRIME1 ^ (*k++ - ' ');
-   h %= PRIME2;
-   return (h);
-}
-
-/*
- * Phong Vo's linear congruential hash
- */
-#define dcharhash(h, c)((h) = 0x63c63cd9*(h) + 0x9c39c33d + (c))
-
-u_int32_t
-hash2(const void *key, size_t len)
-{
-   u_int32_t h;
-   u_int8_t *e, c, *k;
-
-   k = (u_int8_t *)key;
-   e = k + len;
-   for (h = 0; k != e;) {
-   c = *k++;
-   if (!c && k > e)
-   break;
-   dcharhash(h, c);
-   }
-   return (h);
-}
-
-/*
- * This is INCREDIBLY ugly, but fast.  We break the string up into 8 byte
- * units.  On the first time through the loop we get the "leftover bytes"
- * (strlen % 8).  On every other iteration, we perform 8 HASHC's so we handle
- * all 8 bytes.  Essentially, this saves us 7 cmp & branch instructions.  If
- * this routine is heavily used enough, it's worth the ugly coding.
- *
- * Ozan Yigit's 

libc: declare hidden YP stuff as hidden

[Philip Guenther]

The _yp_bind() function and the _ypbindlist, _yp_domain, and 
_yplib_timeout variables are no longer exported in Symbols.list, so just 
declare them as hidden so they don't require GOT relocations.

ok?

Philip Guenther


Index: yp/yp_bind.c
===
RCS file: /data/src/openbsd/src/lib/libc/yp/yp_bind.c,v
retrieving revision 1.26
diff -u -p -r1.26 yp_bind.c
--- yp/yp_bind.c26 Nov 2015 07:26:50 -  1.26
+++ yp/yp_bind.c20 May 2016 09:12:19 -
@@ -257,7 +257,6 @@ _yp_unbind(struct dom_binding *ypb)
ypb->dom_client = NULL;
ypb->dom_socket = -1;
 }
-DEF_WEAK(_yp_unbind);
 
 int
 yp_bind(const char *dom)
Index: yp/ypinternal.h
===
RCS file: /data/src/openbsd/src/lib/libc/yp/ypinternal.h,v
retrieving revision 1.10
diff -u -p -r1.10 ypinternal.h
--- yp/ypinternal.h 14 Sep 2015 12:09:35 -  1.10
+++ yp/ypinternal.h 20 May 2016 09:11:50 -
@@ -44,12 +44,13 @@ struct dom_binding {
 #define BINDINGDIR "/var/yp/binding"
 #define YPBINDLOCK "/var/run/ypbind.lock"
 
+__BEGIN_HIDDEN_DECLS
 extern struct dom_binding *_ypbindlist;
 extern char _yp_domain[HOST_NAME_MAX+1];
 extern int _yplib_timeout;
 
 void   _yp_unbind(struct dom_binding *);
+__END_HIDDEN_DECLS
+
 int_yp_check(char **);
-PROTO_NORMAL(_yp_unbind);
 PROTO_NORMAL(_yp_check);
-

tunefs: remove pledge before opendev

[Theo Buehler]

In both code paths, openpartition() will call opendev after pledge was
called. Just leave the "stdio" promise afterwards.

Index: tunefs.c
===
RCS file: /var/cvs/src/sbin/tunefs/tunefs.c,v
retrieving revision 1.40
diff -u -p -r1.40 tunefs.c
--- tunefs.c9 Dec 2015 01:08:31 -   1.40
+++ tunefs.c28 May 2016 22:58:40 -
@@ -147,14 +147,6 @@ main(int argc, char *argv[])
if (argc != 1)
usage();
 
-   if (Nflag) {
-   if (pledge("stdio rpath disklabel", NULL) == -1)
-   err(1, "pledge");
-   } else {
-   if (pledge("stdio rpath wpath disklabel", NULL) == -1)
-   err(1, "pledge");
-   }
-
special = argv[0];
openflags = Nflag ? O_RDONLY : O_RDWR;
if (Fflag)

ncheck_ffs and pledge disklabel

[Theo Buehler]

more of the same:

$ ktrace ncheck_ffs /dev/tty
Abort trap (core dumped)
$ kdump | tail
 65350 ncheck_ffs CALL  open(0xd76b761d460,0)
 65350 ncheck_ffs NAMI  "/dev/tty"
 65350 ncheck_ffs RET   open 3
 65350 ncheck_ffs CALL  fstat(3,0x7f7ef100)
 65350 ncheck_ffs STRU  struct stat { dev=1040, ino=1280, mode=crw-rw-rw- , 
nlink=1, uid=0<"root">, gid=0<"wheel">, rdev=256, atime=1464476868<"May 29 
01:07:48 2016">.691119578, mtime=1464466717<"May 28 22:18:37 2016">.835647187, 
ctime=1464466717<"May 28 22:18:37 2016">.835647187, size=0, blocks=0, 
blksize=65536, flags=0x0, gen=0x0 }
 65350 ncheck_ffs RET   fstat 0
 65350 ncheck_ffs CALL  ioctl(3,DIOCGDINFO,0xd76b7623100)
 65350 ncheck_ffs PLDG  ioctl, "ioctl", errno 1 Operation not permitted
 65350 ncheck_ffs PSIG  SIGABRT SIG_DFL
 65350 ncheck_ffs NAMI  "ncheck_ffs.core"

remove the early pledge and leave the pledge "stdio" right after
DIOCGDINFO.

Index: ncheck_ffs.c
===
RCS file: /var/cvs/src/sbin/ncheck_ffs/ncheck_ffs.c,v
retrieving revision 1.52
diff -u -p -r1.52 ncheck_ffs.c
--- ncheck_ffs.c23 Nov 2015 19:19:30 -  1.52
+++ ncheck_ffs.c28 May 2016 23:16:37 -
@@ -510,9 +510,6 @@ main(int argc, char *argv[])
char *ep;
int c, i;
 
-   if (pledge("stdio rpath disklabel", NULL) == -1)
-   err(1, "pledge");
-
while ((c = getopt(argc, argv, "af:i:ms")) != -1)
switch (c) {
case 'a':

Re: growfs: hoist ioctl DIOCGDINFO over pledge disklabel

[Bob Beck]

Yeah, go for it!

On Sat, May 28, 2016 at 2:14 PM, Theo Buehler  wrote:
> A slightly different beast:
>
> $ touch abcdabcdabcdabcd.a
> $ ktrace growfs abcdabcdabcdabcd.a
> Abort trap (core dumped)
> $ kdump | tail
>  63324 growfs   CALL  mprotect(0xe2b84265000,0x1000,0x1)
>  63324 growfs   RET   mprotect 0
>  63324 growfs   CALL  
> mmap(0,0x1000,0x3,0x1002,-1,0)
>  63324 growfs   RET   mmap 15591131877376/0xe2e17e09000
>  63324 growfs   CALL  
> mmap(0,0x1000,0x3,0x1002,-1,0)
>  63324 growfs   RET   mmap 15592351502336/0xe2e60929000
>  63324 growfs   CALL  ioctl(3,DIOCGDINFO,0xe2e17e09a00)
>  63324 growfs   PLDG  ioctl, "ioctl", errno 1 Operation not permitted
>  63324 growfs   PSIG  SIGABRT SIG_DFL
>  63324 growfs   NAMI  "growfs.core"
>
> Moving the pledge call a bit down will allow growfs to error out with
> ENOTTY instead of crashing. As there are other "disklabel" operations,
> further down (e.g. in return_disklabel()), keep the "disklabel" pledge.
>
> Index: growfs.c
> ===
> RCS file: /var/cvs/src/sbin/growfs/growfs.c,v
> retrieving revision 1.50
> diff -u -p -r1.50 growfs.c
> --- growfs.c17 Mar 2016 05:27:10 -  1.50
> +++ growfs.c28 May 2016 20:02:36 -
> @@ -1767,9 +1767,6 @@ main(int argc, char **argv)
> err(1, "%s", device);
> }
>
> -   if (pledge("stdio disklabel", NULL) == -1)
> -   err(1, "pledge");
> -
> /*
>  * Now we have a file descriptor for our device, fstat() it to
>  * figure out the partition number.
> @@ -1788,6 +1785,9 @@ main(int argc, char **argv)
> else
> errx(1, "%s: invalid partition number %u",
> device, DISKPART(st.st_rdev));
> +
> +   if (pledge("stdio disklabel", NULL) == -1)
> +   err(1, "pledge");
>
> /*
>  * Check if that partition is suitable for growing a file system.
>

Re: mklocale(1): ignore blanks after VARIABLE

[Ingo Schwarze]

Hi Jeremie,

Jeremie Courreges-Anglas wrote on Sat, May 28, 2016 at 04:56:52PM +0200:
> "Todd C. Miller"  writes:

>> Currently, mklocale only ignores the first blank after the VARIABLE
>> definition.  This means that we store the variable definition along
>> with the leading blanks.
>>
>> The lexer should eat the blanks after VARIABLE before storing the
>> variable definition.

> ok
> 
> The manpage documents the existing constraint, thus it should probably
> be amended.

This matters even less.  On OpenBSD, you could put almost arbitrary
garbage in between without breaking anything, but i doubt we want
to document that.  Besides, running this program on OpenBSD outside
the build makes very little sense in the first place.

Maybe OpenBSD manuals are renowned for their quality and people
using other systems will look at them?  But then, will those other
systems support the less strict syntax?  I think researching that
would be a waste of time.

That said, feel free if you think it makes anything better.

Yours,
  Ingo


> Index: mklocale.1
> ===
> RCS file: /cvs/src/usr.bin/mklocale/mklocale.1,v
> retrieving revision 1.5
> diff -u -p -p -u -r1.5 mklocale.1
> --- mklocale.18 May 2016 15:25:44 -   1.5
> +++ mklocale.128 May 2016 14:54:02 -
> @@ -154,7 +154,7 @@ vendors of
>  systems.
>  .El
>  .It Dv VARIABLE
> -This keyword must be followed by a single tab or space character,
> +This keyword must be followed by whitespace,
>  after which encoding specific data is placed.
>  Currently only the
>  .Dv "EUC"

Re: mklocale(1): ignore blanks after VARIABLE

[Ingo Schwarze]

Hi Todd,

Todd C. Miller wrote on Fri, May 27, 2016 at 04:37:18PM -0600:

> Currently, mklocale only ignores the first blank after the VARIABLE
> definition.  This means that we store the variable definition along
> with the leading blanks.
> 
> The lexer should eat the blanks after VARIABLE before storing the
> variable definition.

Not quite sure what the benefit is - reducing the size of
/usr/share/locale/UTF-8/LC_CTYPE from 401093 to 401086 bytes?
Functionally, it doesn't matter, the only place in the code using
it, find_codeset() in libc/locale/rune.c, ignores leading garbage
anyway by using strstr(3).  Maybe you just consider it more
aestethically pleasing in the generated binary file?

But i don't see any downside either.  Given the above, it doesn't
break compatibility with other systems, and besides, even if it
would, that would hardly matter at this point, given that we are
using the program on exactly one file, during the build, and running
mklocale(1) doesn't really make sense on OpenBSD except during the
build for that one file.

So feel free if you want to.

Yours,
  Ingo


> Index: usr.bin/mklocale/lex.l
> ===
> RCS file: /cvs/src/usr.bin/mklocale/lex.l,v
> retrieving revision 1.3
> diff -u -p -u -r1.3 lex.l
> --- usr.bin/mklocale/lex.l5 Dec 2012 23:20:25 -   1.3
> +++ usr.bin/mklocale/lex.l27 May 2016 12:20:01 -
> @@ -114,7 +114,7 @@ SWIDTH1   { yylval.i = 
> _RUNETYPE_SW1; r
>  SWIDTH2  { yylval.i = _RUNETYPE_SW2; 
> return(LIST); }
>  SWIDTH3  { yylval.i = _RUNETYPE_SW3; 
> return(LIST); }
>  
> -VARIABLE[\t ]{ static char vbuf[1024];
> +VARIABLE[\t ]+   { static char vbuf[1024];
> char *v = vbuf;
> while ((*v = input()) && *v != '\n')
>   ++v;

Re: sed/regcomp bug?

[Martijn van Duren]

Hello tech@,

Here's part two of the sed fix. Here's part two of the sed fix. It
applies the just added REG_NOTBOL|REG_STARTEND change to sed, so that
begin of word matches directly after a previous match, ending in not a
word, can match.

It passes regress and an earlier version of this patch (based on an
earlier attempt of the libregex patch) passed a full ports build thanks
to aja@ for testing.

before:
$ echo x,x,x,x,x,x, | sed 's/\type == AT_RE ? regexec_e((a)->u.r, ps, 0, 1, psl) :   \
+   (a)->type == AT_RE ? regexec_e((a)->u.r, ps, 0, 1, 0, psl) :\
(a)->type == AT_LINE ? linenum == (a)->u.l : lastline()
 
 /*
@@ -335,6 +336,7 @@ substitute(struct s_command *cp)
regex_t *re;
regoff_t slen;
int n, lastempty;
+   size_t le = 0;
char *s;
 
s = ps;
@@ -346,7 +348,7 @@ substitute(struct s_command *cp)
cp->u.s->maxbref);
}
}
-   if (!regexec_e(re, s, 0, 0, psl))
+   if (!regexec_e(re, s, 0, 0, 0, psl))
return (0);
 
SS.len = 0; /* Clean substitute space. */
@@ -356,28 +358,30 @@ substitute(struct s_command *cp)
 
do {
/* Copy the leading retained string. */
-   if (n <= 1 && match[0].rm_so)
-   cspace(, s, match[0].rm_so, APPEND);
+   if (n <= 1 && (match[0].rm_so - le))
+   cspace(, s, match[0].rm_so - le, APPEND);
 
/* Skip zero-length matches right after other matches. */
-   if (lastempty || match[0].rm_so ||
+   if (lastempty || (match[0].rm_so - le) ||
match[0].rm_so != match[0].rm_eo) {
if (n <= 1) {
/* Want this match: append replacement. */
-   regsub(, s, cp->u.s->new);
+   regsub(, ps, cp->u.s->new);
if (n == 1)
n = -1;
} else {
/* Want a later match: append original. */
-   if (match[0].rm_eo)
-   cspace(, s, match[0].rm_eo, APPEND);
+   if (match[0].rm_eo - le)
+   cspace(, s, match[0].rm_eo - le,
+   APPEND);
n--;
}
}
 
/* Move past this match. */
-   s += match[0].rm_eo;
-   slen -= match[0].rm_eo;
+   s += (match[0].rm_eo - le);
+   slen -= (match[0].rm_eo - le);
+   le = match[0].rm_eo;
 
/*
 * After a zero-length match, advance one byte,
@@ -388,13 +392,16 @@ substitute(struct s_command *cp)
slen = -1;
else
slen--;
-   if (*s != '\0')
+   if (*s != '\0') {
cspace(, s++, 1, APPEND);
+   le++;
+   }
lastempty = 1;
} else
lastempty = 0;
 
-   } while (n >= 0 && slen >= 0 && regexec_e(re, s, REG_NOTBOL, 0, slen));
+   } while (n >= 0 && slen >= 0 &&
+   regexec_e(re, ps, REG_NOTBOL, 0, le, psl));
 
/* Did not find the requested number of matches. */
if (n > 1)
@@ -509,7 +516,7 @@ lputs(char *s)
 
 static inline int
 regexec_e(regex_t *preg, const char *string, int eflags,
-int nomatch, size_t slen)
+int nomatch, size_t start, size_t stop)
 {
int eval;
 
@@ -520,8 +527,8 @@ regexec_e(regex_t *preg, const char *str
defpreg = 

ksh(1): Make PPID read-only

[Anthony Coulter]

The ksh(1) man page says that PPID should be a read-only variable.
This diff makes that happen. It does not work to combine the two
lines into "typeset -ir PPID" as the following session demonstrates:
  $ NUM=42
  $ typeset -ir NUM
  ksh: NUM: is read only

Curiously, I did this right the first time and tested it, but before
submitting it yesterday I changed it to the incorrect typeset -ir and
apparently failed to retest. I also submitted it from the command line
with the title "ksh ${PPID} should be read-only" and my shell helpfully
replaced ${PPID} with 641.

This change works correctly and hopefully has a correct subject line.

Index: main.c
===
RCS file: /cvs/src/bin/ksh/main.c,v
retrieving revision 1.79
diff -u -p -r1.79 main.c
--- main.c  4 Mar 2016 15:11:06 -   1.79
+++ main.c  28 May 2016 20:37:25 -
@@ -85,6 +85,7 @@ static const char *initcoms [] = {
"typeset", "-r", "KSH_VERSION", NULL,
"typeset", "-x", "SHELL", "PATH", "HOME", NULL,
"typeset", "-i", "PPID", NULL,
+   "typeset", "-r", "PPID", NULL,
"typeset", "-i", "OPTIND=1", NULL,
"eval", "typeset -i RANDOM MAILCHECK=\"${MAILCHECK-600}\" 
SECONDS=\"${SECONDS-0}\" TMOUT=\"${TMOUT-0}\"", NULL,
"alias",

Re: growfs: hoist ioctl DIOCGDINFO over pledge disklabel

[Theo de Raadt]

Same story.

Looks good.


> A slightly different beast:
> 
> $ touch abcdabcdabcdabcd.a
> $ ktrace growfs abcdabcdabcdabcd.a
> Abort trap (core dumped)
> $ kdump | tail
>  63324 growfs   CALL  mprotect(0xe2b84265000,0x1000,0x1)
>  63324 growfs   RET   mprotect 0
>  63324 growfs   CALL  
> mmap(0,0x1000,0x3,0x1002,-1,0)
>  63324 growfs   RET   mmap 15591131877376/0xe2e17e09000
>  63324 growfs   CALL  
> mmap(0,0x1000,0x3,0x1002,-1,0)
>  63324 growfs   RET   mmap 15592351502336/0xe2e60929000
>  63324 growfs   CALL  ioctl(3,DIOCGDINFO,0xe2e17e09a00)
>  63324 growfs   PLDG  ioctl, "ioctl", errno 1 Operation not permitted
>  63324 growfs   PSIG  SIGABRT SIG_DFL
>  63324 growfs   NAMI  "growfs.core"
> 
> Moving the pledge call a bit down will allow growfs to error out with
> ENOTTY instead of crashing. As there are other "disklabel" operations,
> further down (e.g. in return_disklabel()), keep the "disklabel" pledge.
> 
> Index: growfs.c
> ===
> RCS file: /var/cvs/src/sbin/growfs/growfs.c,v
> retrieving revision 1.50
> diff -u -p -r1.50 growfs.c
> --- growfs.c  17 Mar 2016 05:27:10 -  1.50
> +++ growfs.c  28 May 2016 20:02:36 -
> @@ -1767,9 +1767,6 @@ main(int argc, char **argv)
>   err(1, "%s", device);
>   }
>  
> - if (pledge("stdio disklabel", NULL) == -1)
> - err(1, "pledge");
> -
>   /*
>* Now we have a file descriptor for our device, fstat() it to
>* figure out the partition number.
> @@ -1788,6 +1785,9 @@ main(int argc, char **argv)
>   else
>   errx(1, "%s: invalid partition number %u",
>   device, DISKPART(st.st_rdev));
> +
> + if (pledge("stdio disklabel", NULL) == -1)
> + err(1, "pledge");
>  
>   /*
>* Check if that partition is suitable for growing a file system.
> 

growfs: hoist ioctl DIOCGDINFO over pledge disklabel

[Theo Buehler]

A slightly different beast:

$ touch abcdabcdabcdabcd.a
$ ktrace growfs abcdabcdabcdabcd.a
Abort trap (core dumped)
$ kdump | tail
 63324 growfs   CALL  mprotect(0xe2b84265000,0x1000,0x1)
 63324 growfs   RET   mprotect 0
 63324 growfs   CALL  
mmap(0,0x1000,0x3,0x1002,-1,0)
 63324 growfs   RET   mmap 15591131877376/0xe2e17e09000
 63324 growfs   CALL  
mmap(0,0x1000,0x3,0x1002,-1,0)
 63324 growfs   RET   mmap 15592351502336/0xe2e60929000
 63324 growfs   CALL  ioctl(3,DIOCGDINFO,0xe2e17e09a00)
 63324 growfs   PLDG  ioctl, "ioctl", errno 1 Operation not permitted
 63324 growfs   PSIG  SIGABRT SIG_DFL
 63324 growfs   NAMI  "growfs.core"

Moving the pledge call a bit down will allow growfs to error out with
ENOTTY instead of crashing. As there are other "disklabel" operations,
further down (e.g. in return_disklabel()), keep the "disklabel" pledge.

Index: growfs.c
===
RCS file: /var/cvs/src/sbin/growfs/growfs.c,v
retrieving revision 1.50
diff -u -p -r1.50 growfs.c
--- growfs.c17 Mar 2016 05:27:10 -  1.50
+++ growfs.c28 May 2016 20:02:36 -
@@ -1767,9 +1767,6 @@ main(int argc, char **argv)
err(1, "%s", device);
}
 
-   if (pledge("stdio disklabel", NULL) == -1)
-   err(1, "pledge");
-
/*
 * Now we have a file descriptor for our device, fstat() it to
 * figure out the partition number.
@@ -1788,6 +1785,9 @@ main(int argc, char **argv)
else
errx(1, "%s: invalid partition number %u",
device, DISKPART(st.st_rdev));
+
+   if (pledge("stdio disklabel", NULL) == -1)
+   err(1, "pledge");
 
/*
 * Check if that partition is suitable for growing a file system.

Re: Pledge failure in nc(1)

[Bob Beck]

committed.. Thanks Anthony!


On Sat, May 28, 2016 at 09:58:55PM +0200, Theo Buehler wrote:
> On Sat, May 28, 2016 at 01:47:22PM -0600, Bob Beck wrote:
> > Nice catch, and the right analysis.. Thanks Anthony. I think that can
> > be committed
> > 
> 
> ok tb@
> 
> > I don't think we will get away from a bit of a maze there without
> > deprecating nc options, so
> > IMO this is fine.
> > 
> > 
> > On Sat, May 28, 2016 at 12:36 PM, Anthony Coulter
> >  wrote:
> > > When nc(1) tries to connect through an HTTP proxy that requires
> > > authentication, nc calls readpassphrase(3) and aborts. Pledging "tty"
> > > fixes this problem, but you'll notice that the diff has a lot of nasty
> > > branches. My failure to check Pflag when connecting over unix sockets
> > > is not an oversight; nc does not support that configuration.
> > >
> > > To reproduce the failure without setting up a real HTTP proxy, open
> > > two terminals and run nc as a coprocess in the first. The following
> > > session causes a core dump:
> > >   (tty1)$ nc -lk 8080 |&
> > >   (tty2)$ nc -Xconnect -xlocalhost:8080 -Puser localhost 8081
> > >   (tty1)$ print -np "HTTP/1.0 407 Authentication Required\r\n\r\n"
> > >   (tty2) Abort trap (core dumped)
> > >
> > >
> > > Index: netcat.c
> > > ===
> > > RCS file: /cvs/src/usr.bin/nc/netcat.c,v
> > > retrieving revision 1.150
> > > diff -u -p -r1.150 netcat.c
> > > --- netcat.c4 Jan 2016 02:18:31 -   1.150
> > > +++ netcat.c28 May 2016 18:33:30 -
> > > @@ -323,7 +323,13 @@ main(int argc, char *argv[])
> > > if (pledge("stdio rpath wpath cpath tmppath unix", NULL) 
> > > == -1)
> > > err(1, "pledge");
> > > } else if (Fflag) {
> > > -   if (pledge("stdio inet dns sendfd", NULL) == -1)
> > > +   if (Pflag) {
> > > +   if (pledge("stdio inet dns sendfd tty", NULL) == 
> > > -1)
> > > +   err(1, "pledge");
> > > +   } else if (pledge("stdio inet dns sendfd", NULL) == -1)
> > > +   err(1, "pledge");
> > > +   } else if (Pflag) {
> > > +   if (pledge("stdio inet dns tty", NULL) == -1)
> > > err(1, "pledge");
> > > } else if (usetls) {
> > > if (pledge("stdio rpath inet dns", NULL) == -1)
> > > @@ -434,7 +440,10 @@ main(int argc, char *argv[])
> > > if (Kflag && (privkey = tls_load_file(Kflag, , 
> > > NULL)) == NULL)
> > > errx(1, "unable to load TLS key file %s", Kflag);
> > >
> > > -   if (pledge("stdio inet dns", NULL) == -1)
> > > +   if (Pflag) {
> > > +   if (pledge("stdio inet dns tty", NULL) == -1)
> > > +   err(1, "pledge");
> > > +   } else if (pledge("stdio inet dns", NULL) == -1)
> > > err(1, "pledge");
> > >
> > > if (tls_init() == -1)
> > >
> > 

Re: ptrace PT_IO write bug

[Mathieu -]

Mark Kettenis wrote:
> The thing you guys are missing is that on some architectures making changes
> to instructions (PT_WRITE_I) requires some additional operations to
> guarantee that the CPU actually sees those updated instructions.  Typically
> this is the case on architectures with separate data and instruction caches,
> where the instruction cache doesn't snoop the data cache.  On such
> architectures (powerpc and arm are examples) you need to flush the data
> cache and invalidate the instruction cache.  That may be a somewhat
> expensive operation.
> As you probably guessed, pmap_proc_iflush() is the function that takes care
> of this.  Since you still call pmap_proc_iflush(), the diff isn't wrong from
> a correctness point of view, but I think we should keep the optimization of
> not calling pmap_proc_iflush() for PT_WRITE_D.

Well his makes sense. I pondered while making this change whether
or not I should change the second condition (the iflush one), obviously
I shouldn't have. Heh, I pay my lack of knowledge on !x86 arch.

Thanks for the input!

> As for the original issue.  Adding UVM_IO_FIXPROT for PT_WRITE_D as well,
> means that it will now be able to make changes to read-only data.  That is
> probably corrrect.
> 
> So I think the only thing that should be changed is the following bit:
> 
> >@@ -734,11 +724,11 @@ process_domem(struct proc *curp, struct proc *p,
> >struct uio *uio, int req)
> > vm->vm_refcnt++;
> >
> > error = uvm_io(>vm_map, uio,
> >-(req == PT_WRITE_I) ? UVM_IO_FIXPROT : 0);
> >+(uio->uio_rw == UIO_WRITE) ? UVM_IO_FIXPROT : 0);
> >
> 
> Is that indeed enough to fix the original problem?

Yes this is indeed enough, and I just confirmed it with my test program.
So here is (hopefully) the latest version of this diff, which is now way
shorter.

diff --git a/sys/kern/sys_process.c b/sys/kern/sys_process.c
index 60ec50e..09e56b9 100644
--- a/sys/kern/sys_process.c
+++ b/sys/kern/sys_process.c
@@ -734,7 +734,7 @@ process_domem(struct proc *curp, struct proc *p, struct uio 
*uio, int req)
vm->vm_refcnt++;
 
error = uvm_io(>vm_map, uio,
-   (req == PT_WRITE_I) ? UVM_IO_FIXPROT : 0);
+   (uio->uio_rw == UIO_WRITE) ? UVM_IO_FIXPROT : 0);
 
uvmspace_free(vm);
 

Re: [patch] netcat: don't call report_connect() when family == AF_UNIX

[attila]

Bob Beck  writes:

>> Hi tech@,
>> 
>> I just ran into this while fiddling with some netcat-based plumbing.
>> When I connect to the specified socket from another shell, the
>> listening nc dies if it is AF_UNIX:
>> 
>> $ tail -10f /var/log/messages | nc -vkU -l ~/.xlog_socket
>> nc: getnameinfo: Invalid argument
>> 
>> The attached patch fixes the issue:
>> 
>> $ tail -10f /var/log/messages | \
>> /usr/obj/usr.bin/nc/nc -vkU -l ~/.xlog_socket
>> Connection on /home/attila/.xlog_socket received!
>> 
>> Feedback, comments most welcome.
>> 
>> Pax, -A
>
> How's this instead for you.. (Inspired by your diff.. thanks)

After I applied yours I saw something minor.  Attached is your patch
KNF'ed (80cols).  There appear to still be a few long lines elsewhere,
so maybe it's too much of a nit...

Pax, -A
--
http://haqistan.net/~attila | att...@stalphonsos.com | 0x62A729CF
Index: netcat.c
===
RCS file: /cvs/src/usr.bin/nc/netcat.c,v
retrieving revision 1.150
diff -u -p -r1.150 netcat.c
--- netcat.c	4 Jan 2016 02:18:31 -	1.150
+++ netcat.c	28 May 2016 20:04:14 -
@@ -133,7 +133,7 @@ int	unix_listen(char *);
 void	set_common_sockopts(int, int);
 int	map_tos(char *, int *);
 int	map_tls(char *, int *);
-void	report_connect(const struct sockaddr *, socklen_t);
+void	report_connect(const struct sockaddr *, socklen_t, char *);
 void	report_tls(struct tls *tls_ctx, char * host, char *tls_expectname);
 void	usage(int);
 ssize_t drainbuf(int, unsigned char *, size_t *, struct tls *);
@@ -516,7 +516,8 @@ main(int argc, char *argv[])
 	err(1, "connect");
 
 if (vflag)
-	report_connect((struct sockaddr *), len);
+	report_connect(
+	(struct sockaddr *), len, NULL);
 
 readwrite(s, NULL);
 			} else {
@@ -528,7 +529,9 @@ main(int argc, char *argv[])
 	err(1, "accept");
 }
 if (vflag)
-	report_connect((struct sockaddr *), len);
+	report_connect(
+	(struct sockaddr *), len,
+	family == AF_UNIX ? host : NULL);
 if ((usetls) &&
 (tls_cctx = tls_setup_server(tls_ctx, connfd, host)))
 	readwrite(connfd, tls_cctx);
@@ -1487,12 +1490,17 @@ report_tls(struct tls * tls_ctx, char * 
 }
 
 void
-report_connect(const struct sockaddr *sa, socklen_t salen)
+report_connect(const struct sockaddr *sa, socklen_t salen, char *path)
 {
 	char remote_host[NI_MAXHOST];
 	char remote_port[NI_MAXSERV];
 	int herr;
 	int flags = NI_NUMERICSERV;
+
+	if (path != NULL) {
+		fprintf(stderr, "Connection on %s received!\n", path);
+		return;
+	}
 
 	if (nflag)
 		flags |= NI_NUMERICHOST;

Re: Pledge failure in nc(1)

[Theo Buehler]

On Sat, May 28, 2016 at 01:47:22PM -0600, Bob Beck wrote:
> Nice catch, and the right analysis.. Thanks Anthony. I think that can
> be committed
> 

ok tb@

> I don't think we will get away from a bit of a maze there without
> deprecating nc options, so
> IMO this is fine.
> 
> 
> On Sat, May 28, 2016 at 12:36 PM, Anthony Coulter
>  wrote:
> > When nc(1) tries to connect through an HTTP proxy that requires
> > authentication, nc calls readpassphrase(3) and aborts. Pledging "tty"
> > fixes this problem, but you'll notice that the diff has a lot of nasty
> > branches. My failure to check Pflag when connecting over unix sockets
> > is not an oversight; nc does not support that configuration.
> >
> > To reproduce the failure without setting up a real HTTP proxy, open
> > two terminals and run nc as a coprocess in the first. The following
> > session causes a core dump:
> >   (tty1)$ nc -lk 8080 |&
> >   (tty2)$ nc -Xconnect -xlocalhost:8080 -Puser localhost 8081
> >   (tty1)$ print -np "HTTP/1.0 407 Authentication Required\r\n\r\n"
> >   (tty2) Abort trap (core dumped)
> >
> >
> > Index: netcat.c
> > ===
> > RCS file: /cvs/src/usr.bin/nc/netcat.c,v
> > retrieving revision 1.150
> > diff -u -p -r1.150 netcat.c
> > --- netcat.c4 Jan 2016 02:18:31 -   1.150
> > +++ netcat.c28 May 2016 18:33:30 -
> > @@ -323,7 +323,13 @@ main(int argc, char *argv[])
> > if (pledge("stdio rpath wpath cpath tmppath unix", NULL) == 
> > -1)
> > err(1, "pledge");
> > } else if (Fflag) {
> > -   if (pledge("stdio inet dns sendfd", NULL) == -1)
> > +   if (Pflag) {
> > +   if (pledge("stdio inet dns sendfd tty", NULL) == -1)
> > +   err(1, "pledge");
> > +   } else if (pledge("stdio inet dns sendfd", NULL) == -1)
> > +   err(1, "pledge");
> > +   } else if (Pflag) {
> > +   if (pledge("stdio inet dns tty", NULL) == -1)
> > err(1, "pledge");
> > } else if (usetls) {
> > if (pledge("stdio rpath inet dns", NULL) == -1)
> > @@ -434,7 +440,10 @@ main(int argc, char *argv[])
> > if (Kflag && (privkey = tls_load_file(Kflag, , 
> > NULL)) == NULL)
> > errx(1, "unable to load TLS key file %s", Kflag);
> >
> > -   if (pledge("stdio inet dns", NULL) == -1)
> > +   if (Pflag) {
> > +   if (pledge("stdio inet dns tty", NULL) == -1)
> > +   err(1, "pledge");
> > +   } else if (pledge("stdio inet dns", NULL) == -1)
> > err(1, "pledge");
> >
> > if (tls_init() == -1)
> >
> 

Re: Pledge failure in nc(1)

[Theo de Raadt]

> When nc(1) tries to connect through an HTTP proxy that requires
> authentication, nc calls readpassphrase(3) and aborts. Pledging "tty"
> fixes this problem, but you'll notice that the diff has a lot of nasty
> branches. My failure to check Pflag when connecting over unix sockets
> is not an oversight; nc does not support that configuration.
> 
> To reproduce the failure without setting up a real HTTP proxy, open
> two terminals and run nc as a coprocess in the first. The following
> session causes a core dump:
>   (tty1)$ nc -lk 8080 |&
>   (tty2)$ nc -Xconnect -xlocalhost:8080 -Puser localhost 8081
>   (tty1)$ print -np "HTTP/1.0 407 Authentication Required\r\n\r\n"
>   (tty2) Abort trap (core dumped)

The diff looks right to me.

Re: uvideo patches: Overview [0/4]

[Marcus Glocker]

On Fri, May 27, 2016 at 08:36:54AM +0200, Marcus Glocker wrote:

> On Thu, May 26, 2016 at 04:18:58PM -0700, patrick keshishian wrote:
> 
> > On Thu, May 26, 2016 at 10:55:47PM +0200, Marcus Glocker wrote:
> > > On Tue, May 17, 2016 at 05:15:51PM -0700, patrick keshishian wrote:
> > > 
> > > > Greetings,
> > > > 
> > > > I have been looking at uvideo trying to model a new driver I'm
> > > > attempting to port over and found a few issues (or what I precive
> > > > as issues).
> > > > 
> > > > Since the list likes separate diffs for easier discussion, Here
> > > > is my attempt to break them up in four emails. I think, with
> > > > exception of one, all should apply and compile individually.
> > > > 
> > > > Here are description of patches in decreasing order of my
> > > > confidence in proposing them:
> > > > 
> > > > 1/4: Incorrect enum used for v4l2_buf.flags.
> > > >  This is a paste error I believe. Very simple diff
> > > > 
> > > > 2/4: Assumption on endpoint index to use in uvideo_vs_open() vs
> > > >  actual saved endpoint address.
> > > 
> > > Makes sense.  I also would change the following DPRINTF text to
> > > remove 'sc->sc_vs_cur->endpoint' since this will be same as
> > > 'ed->bEndpointAddress' and also remove 'sc->sc_vs_cur->psize'
> > > since this information is already printed in uvideo_vs_set_alt().
> > 
> > As long as it is understood that the actually packet size is
> > not the literal value of wMaxPacketSize, rather:
> > 
> > psize = UGETW(ed->wMaxPacketSize);
> > psize = UE_GET_SIZE(psize) * (1 + UE_GET_TRANS(psize));
> 
> Right, which is also the information printed in uvideo_vs_set_alt()
> already.  When we think a minute about what is done here with
> the retained endpoint descriptor in uvideo_vs_open(); mainly nothing.
> We just check if we can access the endpoint selected before by
> uvideo_vs_set_alt().  Therefore it's a kind of double check since
> latest at the next step when we try to open the pipe we would fail if
> the endpoint wouldn't be valid.
> 
> My proposal is, leave it in just as a double check with your
> fix to do it for the proper endpoint and print a short debug
> info including the endpoint number before doing the pipe
> opening.
>  
> > Fancy stuff this USB.
> 
> Sigh ...
> 
> 
> Index: uvideo.c
> ===
> RCS file: /cvs/src/sys/dev/usb/uvideo.c,v
> retrieving revision 1.187
> diff -u -p -u -p -r1.187 uvideo.c
> --- uvideo.c  26 May 2016 04:47:08 -  1.187
> +++ uvideo.c  27 May 2016 06:07:22 -
> @@ -1817,19 +1817,17 @@ uvideo_vs_open(struct uvideo_softc *sc)
>   return (error);
>   }
>  
> - ed = usbd_interface2endpoint_descriptor(sc->sc_vs_cur->ifaceh, 0);
> + /* double check if we can access the selected endpoint descriptor */
> + ed = usbd_get_endpoint_descriptor(sc->sc_vs_cur->ifaceh,
> + sc->sc_vs_cur->endpoint);
>   if (ed == NULL) {
>   printf("%s: no endpoint descriptor for VS iface\n",
>   DEVNAME(sc));
>   return (USBD_INVAL);
>   }
> - DPRINTF(1, "%s: open pipe for ", DEVNAME(sc));
> - DPRINTF(1, "bEndpointAddress=0x%02x (0x%02x), wMaxPacketSize=%d (%d)\n",
> - ed->bEndpointAddress,
> - sc->sc_vs_cur->endpoint,
> - UGETW(ed->wMaxPacketSize),
> - sc->sc_vs_cur->psize);
>  
> + DPRINTF(1, "%s: open pipe for bEndpointAddress=0x%02x",
> + DEVNAME(sc), sc->sc_vs_cur->endpoint);
>   error = usbd_open_pipe(
>   sc->sc_vs_cur->ifaceh,
>   sc->sc_vs_cur->endpoint,
> 

This one has been committed as well now.

Re: Pledge failure in nc(1)

[Bob Beck]

Nice catch, and the right analysis.. Thanks Anthony. I think that can
be committed

I don't think we will get away from a bit of a maze there without
deprecating nc options, so
IMO this is fine.


On Sat, May 28, 2016 at 12:36 PM, Anthony Coulter
 wrote:
> When nc(1) tries to connect through an HTTP proxy that requires
> authentication, nc calls readpassphrase(3) and aborts. Pledging "tty"
> fixes this problem, but you'll notice that the diff has a lot of nasty
> branches. My failure to check Pflag when connecting over unix sockets
> is not an oversight; nc does not support that configuration.
>
> To reproduce the failure without setting up a real HTTP proxy, open
> two terminals and run nc as a coprocess in the first. The following
> session causes a core dump:
>   (tty1)$ nc -lk 8080 |&
>   (tty2)$ nc -Xconnect -xlocalhost:8080 -Puser localhost 8081
>   (tty1)$ print -np "HTTP/1.0 407 Authentication Required\r\n\r\n"
>   (tty2) Abort trap (core dumped)
>
>
> Index: netcat.c
> ===
> RCS file: /cvs/src/usr.bin/nc/netcat.c,v
> retrieving revision 1.150
> diff -u -p -r1.150 netcat.c
> --- netcat.c4 Jan 2016 02:18:31 -   1.150
> +++ netcat.c28 May 2016 18:33:30 -
> @@ -323,7 +323,13 @@ main(int argc, char *argv[])
> if (pledge("stdio rpath wpath cpath tmppath unix", NULL) == 
> -1)
> err(1, "pledge");
> } else if (Fflag) {
> -   if (pledge("stdio inet dns sendfd", NULL) == -1)
> +   if (Pflag) {
> +   if (pledge("stdio inet dns sendfd tty", NULL) == -1)
> +   err(1, "pledge");
> +   } else if (pledge("stdio inet dns sendfd", NULL) == -1)
> +   err(1, "pledge");
> +   } else if (Pflag) {
> +   if (pledge("stdio inet dns tty", NULL) == -1)
> err(1, "pledge");
> } else if (usetls) {
> if (pledge("stdio rpath inet dns", NULL) == -1)
> @@ -434,7 +440,10 @@ main(int argc, char *argv[])
> if (Kflag && (privkey = tls_load_file(Kflag, , 
> NULL)) == NULL)
> errx(1, "unable to load TLS key file %s", Kflag);
>
> -   if (pledge("stdio inet dns", NULL) == -1)
> +   if (Pflag) {
> +   if (pledge("stdio inet dns tty", NULL) == -1)
> +   err(1, "pledge");
> +   } else if (pledge("stdio inet dns", NULL) == -1)
> err(1, "pledge");
>
> if (tls_init() == -1)
>

Pledge failure in nc(1)

[Anthony Coulter]

When nc(1) tries to connect through an HTTP proxy that requires
authentication, nc calls readpassphrase(3) and aborts. Pledging "tty"
fixes this problem, but you'll notice that the diff has a lot of nasty
branches. My failure to check Pflag when connecting over unix sockets
is not an oversight; nc does not support that configuration.

To reproduce the failure without setting up a real HTTP proxy, open
two terminals and run nc as a coprocess in the first. The following
session causes a core dump:
  (tty1)$ nc -lk 8080 |&
  (tty2)$ nc -Xconnect -xlocalhost:8080 -Puser localhost 8081
  (tty1)$ print -np "HTTP/1.0 407 Authentication Required\r\n\r\n"
  (tty2) Abort trap (core dumped)


Index: netcat.c
===
RCS file: /cvs/src/usr.bin/nc/netcat.c,v
retrieving revision 1.150
diff -u -p -r1.150 netcat.c
--- netcat.c4 Jan 2016 02:18:31 -   1.150
+++ netcat.c28 May 2016 18:33:30 -
@@ -323,7 +323,13 @@ main(int argc, char *argv[])
if (pledge("stdio rpath wpath cpath tmppath unix", NULL) == -1)
err(1, "pledge");
} else if (Fflag) {
-   if (pledge("stdio inet dns sendfd", NULL) == -1)
+   if (Pflag) {
+   if (pledge("stdio inet dns sendfd tty", NULL) == -1)
+   err(1, "pledge");
+   } else if (pledge("stdio inet dns sendfd", NULL) == -1)
+   err(1, "pledge");
+   } else if (Pflag) {
+   if (pledge("stdio inet dns tty", NULL) == -1)
err(1, "pledge");
} else if (usetls) {
if (pledge("stdio rpath inet dns", NULL) == -1)
@@ -434,7 +440,10 @@ main(int argc, char *argv[])
if (Kflag && (privkey = tls_load_file(Kflag, , 
NULL)) == NULL)
errx(1, "unable to load TLS key file %s", Kflag);
 
-   if (pledge("stdio inet dns", NULL) == -1)
+   if (Pflag) {
+   if (pledge("stdio inet dns tty", NULL) == -1)
+   err(1, "pledge");
+   } else if (pledge("stdio inet dns", NULL) == -1)
err(1, "pledge");
 
if (tls_init() == -1)

Re: fsck_msdofs and pledge disklabel

[Sebastien Marie]

ok semarie@

On Sat, May 28, 2016 at 07:33:25PM +0200, Theo Buehler wrote:
> Another low hanging fruit of the same sort:
> 
> $ ktrace fsck_msdofs /dev/tty
> Abort trap (core dumped)
> $ kdump | tail
>   4663 fsck_msdos RET   read 612/0x264
>   4663 fsck_msdos CALL  read(3,0x304fdfe4000,0x4000)
>   4663 fsck_msdos RET   read 0
>   4663 fsck_msdos CALL  open(0x3029c121880,0x2)
>   4663 fsck_msdos NAMI  "/dev/tty"
>   4663 fsck_msdos RET   open 4
>   4663 fsck_msdos CALL  ioctl(4,DIOCGPDINFO,0x7f7ee340)
>   4663 fsck_msdos PLDG  ioctl, "ioctl", errno 1 Operation not permitted
>   4663 fsck_msdos PSIG  SIGABRT SIG_DFL code <1870144632>
>   4663 fsck_msdos NAMI  "fsck_msdos.core"
> 
> Right after DIOCGPDINFO comes a pledge stdio.
> 
> Index: main.c
> ===
> RCS file: /var/cvs/src/sbin/fsck_msdos/main.c,v
> retrieving revision 1.22
> diff -u -p -r1.22 main.c
> --- main.c23 Nov 2015 19:19:30 -  1.22
> +++ main.c28 May 2016 17:24:18 -
> @@ -57,9 +57,6 @@ main(int argc, char *argv[])
>  {
>   int ch;
>  
> - if (pledge("stdio rpath wpath disklabel", NULL) == -1)
> - err(1, "pledge");
> -
>   while ((ch = getopt(argc, argv, "pynf")) != -1) {
>   switch (ch) {
>   case 'f':
> 
> 

-- 
Sebastien Marie

Re: iwm(4): Add support for Intel Wireless 3165

[Stefan Sperling]

On Sat, May 28, 2016 at 05:27:28PM +0200, Imre Vadasz wrote:
> Hi,
> 
> This adds support for the AC 3165 Intel wireless chipset.
> Tested on a HP x2 210 tablet/detachable:
> 
> iwm0 at pci1 dev 0 function 0 "Intel Dual Band Wireless AC 3165" rev 0x81, msi
> iwm0: hw rev 0x210, fw ver 16.242414.0, address e0:94:67:bf:6f:76

Committed, thanks.

Re: [patch] netcat: don't call report_connect() when family == AF_UNIX

[Bob Beck]

if you write regress ill happily take them

try to make a few for regular and tls connections and failures

On Saturday, 28 May 2016, attila  wrote:

>
> Bob Beck > writes:
>
> >> Hi tech@,
> >>
> >> I just ran into this while fiddling with some netcat-based plumbing.
> >> When I connect to the specified socket from another shell, the
> >> listening nc dies if it is AF_UNIX:
> >>
> >> $ tail -10f /var/log/messages | nc -vkU -l ~/.xlog_socket
> >> nc: getnameinfo: Invalid argument
> >>
> >> The attached patch fixes the issue:
> >>
> >> $ tail -10f /var/log/messages | \
> >> /usr/obj/usr.bin/nc/nc -vkU -l ~/.xlog_socket
> >> Connection on /home/attila/.xlog_socket received!
> >>
> >> Feedback, comments most welcome.
> >>
> >> Pax, -A
> >
> > How's this instead for you.. (Inspired by your diff.. thanks)
> >
>
> Preferable to my patch.  Thanks for looking at it!
>
> Also: I noticed there are no regression tests for netcat, so I'm
> writing one at least to test this case.  Any requests or thoughts on
> others?  I have a couple obvious ideas, input most welcome.
>
> > Index: netcat.c
> > ===
> > RCS file: /cvs/src/usr.bin/nc/netcat.c,v
> > retrieving revision 1.150
> > diff -u -p -u -p -r1.150 netcat.c
> > --- netcat.c  4 Jan 2016 02:18:31 -   1.150
> > +++ netcat.c  28 May 2016 17:32:01 -
> > @@ -133,7 +133,7 @@ int   unix_listen(char *);
> >  void set_common_sockopts(int, int);
> >  int  map_tos(char *, int *);
> >  int  map_tls(char *, int *);
> > -void report_connect(const struct sockaddr *, socklen_t);
> > +void report_connect(const struct sockaddr *, socklen_t, char *);
> >  void report_tls(struct tls *tls_ctx, char * host, char *tls_expectname);
> >  void usage(int);
> >  ssize_t drainbuf(int, unsigned char *, size_t *, struct tls *);
> > @@ -516,7 +516,7 @@ main(int argc, char *argv[])
> >   err(1, "connect");
> >
> >   if (vflag)
> > - report_connect((struct sockaddr
> *), len);
> > + report_connect((struct sockaddr
> *), len, NULL);
> >
> >   readwrite(s, NULL);
> >   } else {
> > @@ -528,7 +528,8 @@ main(int argc, char *argv[])
> >   err(1, "accept");
> >   }
> >   if (vflag)
> > - report_connect((struct sockaddr
> *), len);
> > + report_connect((struct sockaddr
> *), len,
> > + family == AF_UNIX ? host :
> NULL);
> >   if ((usetls) &&
> >   (tls_cctx = tls_setup_server(tls_ctx,
> connfd, host)))
> >   readwrite(connfd, tls_cctx);
> > @@ -1487,12 +1488,17 @@ report_tls(struct tls * tls_ctx, char *
> >  }
> >
> >  void
> > -report_connect(const struct sockaddr *sa, socklen_t salen)
> > +report_connect(const struct sockaddr *sa, socklen_t salen, char *path)
> >  {
> >   char remote_host[NI_MAXHOST];
> >   char remote_port[NI_MAXSERV];
> >   int herr;
> >   int flags = NI_NUMERICSERV;
> > +
> > + if (path != NULL) {
> > + fprintf(stderr, "Connection on %s received!\n", path);
> > + return;
> > + }
> >
> >   if (nflag)
> >   flags |= NI_NUMERICHOST;
>
> Pax, -A
> --
> http://haqistan.net/~attila | att...@stalphonsos.com  |
> 0x62A729CF
>
>

Re: [patch] netcat: don't call report_connect() when family == AF_UNIX

[attila]

Bob Beck  writes:

>> Hi tech@,
>> 
>> I just ran into this while fiddling with some netcat-based plumbing.
>> When I connect to the specified socket from another shell, the
>> listening nc dies if it is AF_UNIX:
>> 
>> $ tail -10f /var/log/messages | nc -vkU -l ~/.xlog_socket
>> nc: getnameinfo: Invalid argument
>> 
>> The attached patch fixes the issue:
>> 
>> $ tail -10f /var/log/messages | \
>> /usr/obj/usr.bin/nc/nc -vkU -l ~/.xlog_socket
>> Connection on /home/attila/.xlog_socket received!
>> 
>> Feedback, comments most welcome.
>> 
>> Pax, -A
>
> How's this instead for you.. (Inspired by your diff.. thanks)
>

Preferable to my patch.  Thanks for looking at it!

Also: I noticed there are no regression tests for netcat, so I'm
writing one at least to test this case.  Any requests or thoughts on
others?  I have a couple obvious ideas, input most welcome.

> Index: netcat.c
> ===
> RCS file: /cvs/src/usr.bin/nc/netcat.c,v
> retrieving revision 1.150
> diff -u -p -u -p -r1.150 netcat.c
> --- netcat.c  4 Jan 2016 02:18:31 -   1.150
> +++ netcat.c  28 May 2016 17:32:01 -
> @@ -133,7 +133,7 @@ int   unix_listen(char *);
>  void set_common_sockopts(int, int);
>  int  map_tos(char *, int *);
>  int  map_tls(char *, int *);
> -void report_connect(const struct sockaddr *, socklen_t);
> +void report_connect(const struct sockaddr *, socklen_t, char *);
>  void report_tls(struct tls *tls_ctx, char * host, char *tls_expectname);
>  void usage(int);
>  ssize_t drainbuf(int, unsigned char *, size_t *, struct tls *);
> @@ -516,7 +516,7 @@ main(int argc, char *argv[])
>   err(1, "connect");
>  
>   if (vflag)
> - report_connect((struct sockaddr *), 
> len);
> + report_connect((struct sockaddr *), 
> len, NULL);
>  
>   readwrite(s, NULL);
>   } else {
> @@ -528,7 +528,8 @@ main(int argc, char *argv[])
>   err(1, "accept");
>   }
>   if (vflag)
> - report_connect((struct sockaddr 
> *), len);
> + report_connect((struct sockaddr 
> *), len,
> + family == AF_UNIX ? host : NULL);
>   if ((usetls) &&
>   (tls_cctx = tls_setup_server(tls_ctx, 
> connfd, host)))
>   readwrite(connfd, tls_cctx);
> @@ -1487,12 +1488,17 @@ report_tls(struct tls * tls_ctx, char * 
>  }
>  
>  void
> -report_connect(const struct sockaddr *sa, socklen_t salen)
> +report_connect(const struct sockaddr *sa, socklen_t salen, char *path)
>  {
>   char remote_host[NI_MAXHOST];
>   char remote_port[NI_MAXSERV];
>   int herr;
>   int flags = NI_NUMERICSERV;
> +
> + if (path != NULL) {
> + fprintf(stderr, "Connection on %s received!\n", path);
> + return;
> + }
>  
>   if (nflag)
>   flags |= NI_NUMERICHOST;

Pax, -A
--
http://haqistan.net/~attila | att...@stalphonsos.com | 0x62A729CF

Re: fsck_msdofs and pledge disklabel

[Bob Beck]

Another ok for this

Although I am kind of wondering outloud is pledge disklabel is needed as such
or could be refined.. should we simply always read the disklabel outside of
pledge, and only need this to write one?

On Sat, May 28, 2016 at 07:33:25PM +0200, Theo Buehler wrote:
> Another low hanging fruit of the same sort:
> 
> $ ktrace fsck_msdofs /dev/tty
> Abort trap (core dumped)
> $ kdump | tail
>   4663 fsck_msdos RET   read 612/0x264
>   4663 fsck_msdos CALL  read(3,0x304fdfe4000,0x4000)
>   4663 fsck_msdos RET   read 0
>   4663 fsck_msdos CALL  open(0x3029c121880,0x2)
>   4663 fsck_msdos NAMI  "/dev/tty"
>   4663 fsck_msdos RET   open 4
>   4663 fsck_msdos CALL  ioctl(4,DIOCGPDINFO,0x7f7ee340)
>   4663 fsck_msdos PLDG  ioctl, "ioctl", errno 1 Operation not permitted
>   4663 fsck_msdos PSIG  SIGABRT SIG_DFL code <1870144632>
>   4663 fsck_msdos NAMI  "fsck_msdos.core"
> 
> Right after DIOCGPDINFO comes a pledge stdio.
> 
> Index: main.c
> ===
> RCS file: /var/cvs/src/sbin/fsck_msdos/main.c,v
> retrieving revision 1.22
> diff -u -p -r1.22 main.c
> --- main.c23 Nov 2015 19:19:30 -  1.22
> +++ main.c28 May 2016 17:24:18 -
> @@ -57,9 +57,6 @@ main(int argc, char *argv[])
>  {
>   int ch;
>  
> - if (pledge("stdio rpath wpath disklabel", NULL) == -1)
> - err(1, "pledge");
> -
>   while ((ch = getopt(argc, argv, "pynf")) != -1) {
>   switch (ch) {
>   case 'f':
> 

Re: fsck_msdofs and pledge disklabel

[Bob Beck]

On Sat, May 28, 2016 at 11:45:38AM -0600, Bob Beck wrote:
> Another ok for this
> 
> Although I am kind of wondering outloud is pledge disklabel is needed as such
> or could be refined.. should we simply always read the disklabel outside of
> pledge, and only need this to write one?

actually that thought Is stupid..  I'm thinking this because of the hack of
hoisting all this.

which wouldn't be needed if we can fix opendev() to not be so dumb..

> 
> On Sat, May 28, 2016 at 07:33:25PM +0200, Theo Buehler wrote:
> > Another low hanging fruit of the same sort:
> > 
> > $ ktrace fsck_msdofs /dev/tty
> > Abort trap (core dumped)
> > $ kdump | tail
> >   4663 fsck_msdos RET   read 612/0x264
> >   4663 fsck_msdos CALL  read(3,0x304fdfe4000,0x4000)
> >   4663 fsck_msdos RET   read 0
> >   4663 fsck_msdos CALL  open(0x3029c121880,0x2)
> >   4663 fsck_msdos NAMI  "/dev/tty"
> >   4663 fsck_msdos RET   open 4
> >   4663 fsck_msdos CALL  ioctl(4,DIOCGPDINFO,0x7f7ee340)
> >   4663 fsck_msdos PLDG  ioctl, "ioctl", errno 1 Operation not permitted
> >   4663 fsck_msdos PSIG  SIGABRT SIG_DFL code <1870144632>
> >   4663 fsck_msdos NAMI  "fsck_msdos.core"
> > 
> > Right after DIOCGPDINFO comes a pledge stdio.
> > 
> > Index: main.c
> > ===
> > RCS file: /var/cvs/src/sbin/fsck_msdos/main.c,v
> > retrieving revision 1.22
> > diff -u -p -r1.22 main.c
> > --- main.c  23 Nov 2015 19:19:30 -  1.22
> > +++ main.c  28 May 2016 17:24:18 -
> > @@ -57,9 +57,6 @@ main(int argc, char *argv[])
> >  {
> > int ch;
> >  
> > -   if (pledge("stdio rpath wpath disklabel", NULL) == -1)
> > -   err(1, "pledge");
> > -
> > while ((ch = getopt(argc, argv, "pynf")) != -1) {
> > switch (ch) {
> > case 'f':
> > 

Re: ptrace PT_IO write bug

[Mark Kettenis]

Mathieu - schreef op 2016-05-28 13:05:

Martin Natano wrote:

The diff reads fine to me, however it is incomplete. There are some
callers of process_domem() in arch/. They will need to be changed too.
req seems to be in sync with uio_rw in all the cases, so just removing
the last argument should do it.



Thanks for the feedback. The missing callers where an overlook on my
part, sorry for that.

Here is a regenerated diff including all the call site. As a side note,
obviously every one of them was using PT_WRITE_I, that's why it went
unnoticed.


The thing you guys are missing is that on some architectures making 
changes to instructions (PT_WRITE_I) requires some additional operations 
to guarantee that the CPU actually sees those updated instructions.  
Typically this is the case on architectures with separate data and 
instruction caches, where the instruction cache doesn't snoop the data 
cache.  On such architectures (powerpc and arm are examples) you need to 
flush the data cache and invalidate the instruction cache.  That may be 
a somewhat expensive operation.
As you probably guessed, pmap_proc_iflush() is the function that takes 
care of this.  Since you still call pmap_proc_iflush(), the diff isn't 
wrong from a correctness point of view, but I think we should keep the 
optimization of not calling pmap_proc_iflush() for PT_WRITE_D.


As for the original issue.  Adding UVM_IO_FIXPROT for PT_WRITE_D as 
well, means that it will now be able to make changes to read-only data.  
That is probably corrrect.


So I think the only thing that should be changed is the following bit:


@@ -734,11 +724,11 @@ process_domem(struct proc *curp, struct proc *p,
struct uio *uio, int req)
vm->vm_refcnt++;

error = uvm_io(>vm_map, uio,
-   (req == PT_WRITE_I) ? UVM_IO_FIXPROT : 0);
+   (uio->uio_rw == UIO_WRITE) ? UVM_IO_FIXPROT : 0);



Is that indeed enough to fix the original problem?

Re: [patch] netcat: don't call report_connect() when family == AF_UNIX

[Bob Beck]

> Hi tech@,
> 
> I just ran into this while fiddling with some netcat-based plumbing.
> When I connect to the specified socket from another shell, the
> listening nc dies if it is AF_UNIX:
> 
> $ tail -10f /var/log/messages | nc -vkU -l ~/.xlog_socket
> nc: getnameinfo: Invalid argument
> 
> The attached patch fixes the issue:
> 
> $ tail -10f /var/log/messages | \
> /usr/obj/usr.bin/nc/nc -vkU -l ~/.xlog_socket
> Connection on /home/attila/.xlog_socket received!
> 
> Feedback, comments most welcome.
> 
> Pax, -A

How's this instead for you.. (Inspired by your diff.. thanks)

Index: netcat.c
===
RCS file: /cvs/src/usr.bin/nc/netcat.c,v
retrieving revision 1.150
diff -u -p -u -p -r1.150 netcat.c
--- netcat.c4 Jan 2016 02:18:31 -   1.150
+++ netcat.c28 May 2016 17:32:01 -
@@ -133,7 +133,7 @@ int unix_listen(char *);
 void   set_common_sockopts(int, int);
 intmap_tos(char *, int *);
 intmap_tls(char *, int *);
-void   report_connect(const struct sockaddr *, socklen_t);
+void   report_connect(const struct sockaddr *, socklen_t, char *);
 void   report_tls(struct tls *tls_ctx, char * host, char *tls_expectname);
 void   usage(int);
 ssize_t drainbuf(int, unsigned char *, size_t *, struct tls *);
@@ -516,7 +516,7 @@ main(int argc, char *argv[])
err(1, "connect");
 
if (vflag)
-   report_connect((struct sockaddr *), 
len);
+   report_connect((struct sockaddr *), 
len, NULL);
 
readwrite(s, NULL);
} else {
@@ -528,7 +528,8 @@ main(int argc, char *argv[])
err(1, "accept");
}
if (vflag)
-   report_connect((struct sockaddr 
*), len);
+   report_connect((struct sockaddr 
*), len,
+   family == AF_UNIX ? host : NULL);
if ((usetls) &&
(tls_cctx = tls_setup_server(tls_ctx, 
connfd, host)))
readwrite(connfd, tls_cctx);
@@ -1487,12 +1488,17 @@ report_tls(struct tls * tls_ctx, char * 
 }
 
 void
-report_connect(const struct sockaddr *sa, socklen_t salen)
+report_connect(const struct sockaddr *sa, socklen_t salen, char *path)
 {
char remote_host[NI_MAXHOST];
char remote_port[NI_MAXSERV];
int herr;
int flags = NI_NUMERICSERV;
+
+   if (path != NULL) {
+   fprintf(stderr, "Connection on %s received!\n", path);
+   return;
+   }
 
if (nflag)
flags |= NI_NUMERICHOST;

fsck_msdofs and pledge disklabel

[Theo Buehler]

Another low hanging fruit of the same sort:

$ ktrace fsck_msdofs /dev/tty
Abort trap (core dumped)
$ kdump | tail
  4663 fsck_msdos RET   read 612/0x264
  4663 fsck_msdos CALL  read(3,0x304fdfe4000,0x4000)
  4663 fsck_msdos RET   read 0
  4663 fsck_msdos CALL  open(0x3029c121880,0x2)
  4663 fsck_msdos NAMI  "/dev/tty"
  4663 fsck_msdos RET   open 4
  4663 fsck_msdos CALL  ioctl(4,DIOCGPDINFO,0x7f7ee340)
  4663 fsck_msdos PLDG  ioctl, "ioctl", errno 1 Operation not permitted
  4663 fsck_msdos PSIG  SIGABRT SIG_DFL code <1870144632>
  4663 fsck_msdos NAMI  "fsck_msdos.core"

Right after DIOCGPDINFO comes a pledge stdio.

Index: main.c
===
RCS file: /var/cvs/src/sbin/fsck_msdos/main.c,v
retrieving revision 1.22
diff -u -p -r1.22 main.c
--- main.c  23 Nov 2015 19:19:30 -  1.22
+++ main.c  28 May 2016 17:24:18 -
@@ -57,9 +57,6 @@ main(int argc, char *argv[])
 {
int ch;
 
-   if (pledge("stdio rpath wpath disklabel", NULL) == -1)
-   err(1, "pledge");
-
while ((ch = getopt(argc, argv, "pynf")) != -1) {
switch (ch) {
case 'f':

Re: fsirand and pledge disklabel

[Bob Beck]

OK


On Sat, May 28, 2016 at 07:02:44PM +0200, Theo Buehler wrote:
> $ ktrace fsirand -p /altroot
> Abort trap (core dumped)
> $ kdump | tail
>802 fsirand  CALL  pledge(0x16b2d3119a5e,0)
>802 fsirand  STRU  pledge promise="stdio rpath wpath disklabel"
>802 fsirand  RET   pledge 0
>802 fsirand  CALL  open(0x16b2d341f060,0)
>802 fsirand  NAMI  "/altroot"
>802 fsirand  RET   open 3
>802 fsirand  CALL  ioctl(3,DIOCGDINFO,0x7f7e6b10)
>802 fsirand  PLDG  ioctl, "ioctl", errno 1 Operation not permitted
>802 fsirand  PSIG  SIGABRT SIG_DFL code <537541152>
>802 fsirand  NAMI  "fsirand.core"
> 
> 
> Hence let's remove the pledge before opendev opendev and the DIOCGDINFO
> ioctl. Note that fsirand pledges "stdio" right after the ioctl.
> 
> Index: fsirand.c
> ===
> RCS file: /var/cvs/src/sbin/fsirand/fsirand.c,v
> retrieving revision 1.37
> diff -u -p -r1.37 fsirand.c
> --- fsirand.c 23 Nov 2015 19:19:30 -  1.37
> +++ fsirand.c 28 May 2016 16:58:03 -
> @@ -106,9 +106,6 @@ fsirand(char *device)
>   u_int32_t bsize = DEV_BSIZE;
>   struct disklabel label;
>  
> - if (pledge("stdio rpath wpath disklabel", NULL) == -1)
> - err(1, "pledge");
> -
>   if ((devfd = opendev(device, printonly ? O_RDONLY : O_RDWR,
>   0, )) < 0) {
>   warn("Can't open %s", devpath);
> 

fsirand and pledge disklabel

[Theo Buehler]

$ ktrace fsirand -p /altroot
Abort trap (core dumped)
$ kdump | tail
   802 fsirand  CALL  pledge(0x16b2d3119a5e,0)
   802 fsirand  STRU  pledge promise="stdio rpath wpath disklabel"
   802 fsirand  RET   pledge 0
   802 fsirand  CALL  open(0x16b2d341f060,0)
   802 fsirand  NAMI  "/altroot"
   802 fsirand  RET   open 3
   802 fsirand  CALL  ioctl(3,DIOCGDINFO,0x7f7e6b10)
   802 fsirand  PLDG  ioctl, "ioctl", errno 1 Operation not permitted
   802 fsirand  PSIG  SIGABRT SIG_DFL code <537541152>
   802 fsirand  NAMI  "fsirand.core"


Hence let's remove the pledge before opendev opendev and the DIOCGDINFO
ioctl. Note that fsirand pledges "stdio" right after the ioctl.

Index: fsirand.c
===
RCS file: /var/cvs/src/sbin/fsirand/fsirand.c,v
retrieving revision 1.37
diff -u -p -r1.37 fsirand.c
--- fsirand.c   23 Nov 2015 19:19:30 -  1.37
+++ fsirand.c   28 May 2016 16:58:03 -
@@ -106,9 +106,6 @@ fsirand(char *device)
u_int32_t bsize = DEV_BSIZE;
struct disklabel label;
 
-   if (pledge("stdio rpath wpath disklabel", NULL) == -1)
-   err(1, "pledge");
-
if ((devfd = opendev(device, printonly ? O_RDONLY : O_RDWR,
0, )) < 0) {
warn("Can't open %s", devpath);

Re: Allow top(1) to search arguments (again)

[Edd Barrett]

On Wed, May 11, 2016 at 02:28:51PM +0200, Michal Mazurek wrote:
> As discussed off list, "if (!term)" is redundant, as the caller does the
> check.
> 
> Also fix whitespace in some unrelated places.

Can you split out the "term" change from style changes?

You can do a KNF whack in a separate commit (later).

-- 
Best Regards
Edd Barrett

http://www.theunixzoo.co.uk

Re: Pledge failure in disklabel(8)

[Bob Beck]

On Sat, May 28, 2016 at 10:02:27AM -0600, Theo de Raadt wrote:
> fstat will not help:  disklabel /dev/tty
> 

Ok, apply against current and this does help.

I've moved the readlabel() call before the pledge.

ok?


Index: disklabel.c
===
RCS file: /cvs/src/sbin/disklabel/disklabel.c,v
retrieving revision 1.216
diff -u -p -u -p -B -r1.216 disklabel.c
--- disklabel.c 28 May 2016 16:00:19 -  1.216
+++ disklabel.c 28 May 2016 16:29:20 -
@@ -198,6 +198,7 @@ main(int argc, char *argv[])
);
if (f < 0)
err(4, "%s", specname);
+   readlabel(f);
 
if (op == EDIT || op == EDITOR || aflag) {
if (pledge("stdio rpath wpath cpath disklabel proc exec", NULL) 
== -1)
@@ -221,19 +222,16 @@ main(int argc, char *argv[])
case EDIT:
if (argc != 1)
usage();
-   readlabel(f);
error = edit(, f);
break;
case EDITOR:
if (argc != 1)
usage();
-   readlabel(f);
error = editor(f);
break;
case READ:
if (argc != 1)
usage();
-   readlabel(f);
 
if (pledge("stdio", NULL) == -1)
err(1, "pledge");
@@ -247,7 +245,6 @@ main(int argc, char *argv[])
case RESTORE:
if (argc < 2 || argc > 3)
usage();
-   readlabel(f);
if (!(t = fopen(argv[1], "r")))
err(4, "%s", argv[1]);
error = getasciilabel(t, );
@@ -263,9 +260,7 @@ main(int argc, char *argv[])
fclose(t);
break;
case WRITE:
-   if (dflag || aflag) {
-   readlabel(f);
-   } else if (argc < 2 || argc > 3)
+   if ((!(dflag || aflag)) && (argc < 2 || argc > 3))
usage();
else
makelabel(argv[1], argc == 3 ? argv[2] : NULL, );

[patch] netcat: don't call report_connect() when family == AF_UNIX

[attila]

Hi tech@,

I just ran into this while fiddling with some netcat-based plumbing.
When I connect to the specified socket from another shell, the
listening nc dies if it is AF_UNIX:

$ tail -10f /var/log/messages | nc -vkU -l ~/.xlog_socket
nc: getnameinfo: Invalid argument

The attached patch fixes the issue:

$ tail -10f /var/log/messages | \
/usr/obj/usr.bin/nc/nc -vkU -l ~/.xlog_socket
Connection on /home/attila/.xlog_socket received!

Feedback, comments most welcome.

Pax, -A
--
http://haqistan.net/~attila | att...@stalphonsos.com | 0x62A729CF
Index: netcat.c
===
RCS file: /cvs/src/usr.bin/nc/netcat.c,v
retrieving revision 1.150
diff -u -p -r1.150 netcat.c
--- netcat.c	4 Jan 2016 02:18:31 -	1.150
+++ netcat.c	28 May 2016 16:31:27 -
@@ -527,8 +527,16 @@ main(int argc, char *argv[])
 	/* For now, all errnos are fatal */
 	err(1, "accept");
 }
-if (vflag)
-	report_connect((struct sockaddr *), len);
+if (vflag) {
+	if (family == AF_UNIX)
+		fprintf(stderr,
+		"Connection on %s "
+		"received!\n", host);
+	else
+		report_connect(
+		(struct sockaddr *),
+		len);
+}
 if ((usetls) &&
 (tls_cctx = tls_setup_server(tls_ctx, connfd, host)))
 	readwrite(connfd, tls_cctx);

Re: Pledge failure in disklabel(8)

[Theo de Raadt]

fstat will not help:  disklabel /dev/tty

Re: Pledge failure in disklabel(8)

[Anthony Coulter]

Oops, I did not check the return value of fstat. Good catch.

But when I tested my change without yours, disklabel did not abort. Why 
then does opendev need to occur before the pledge? Is there another 
usage of disklabel that causes a different failure pattern?



On 05/28/2016 11:31 AM, Theo de Raadt wrote:

If you try to run disklabel(8) on a file that is not a device, it aborts
aborts for want of pledge("ioctl"). This diff prints an error message
and exits cleanly. I return exit code 1 but note that sometimes
disklabel returns 4; the man page doesn't explain the distinction
anywhere.

   $ disklabel /
   Abort trap (core dumped)
   $ obj/disklabel /
   disklabel: / is not a device


Indeed your diff is also needed on top of mine.  Let's try this.

Re: Pledge failure in disklabel(8)

[Theo de Raadt]

> If you try to run disklabel(8) on a file that is not a device, it aborts
> aborts for want of pledge("ioctl"). This diff prints an error message
> and exits cleanly. I return exit code 1 but note that sometimes
> disklabel returns 4; the man page doesn't explain the distinction
> anywhere.
> 
>   $ disklabel /
>   Abort trap (core dumped)
>   $ obj/disklabel /
>   disklabel: / is not a device

Indeed your diff is also needed on top of mine.  Let's try this.

Index: disklabel.c
===
RCS file: /cvs/src/sbin/disklabel/disklabel.c,v
retrieving revision 1.214
diff -u -p -u -r1.214 disklabel.c
--- disklabel.c 25 Nov 2015 17:17:38 -  1.214
+++ disklabel.c 28 May 2016 15:30:27 -
@@ -119,6 +119,7 @@ main(int argc, char *argv[])
int ch, f, error = 0;
FILE *t;
char *autotable = NULL;
+   struct stat st;
 
getphysmem();
 
@@ -191,14 +192,6 @@ main(int argc, char *argv[])
argc -= optind;
argv += optind;
 
-   if (op == EDIT || op == EDITOR || aflag) {
-   if (pledge("stdio rpath wpath cpath disklabel proc exec", NULL) 
== -1)
-   err(1, "pledge");
-   } else {
-   if (pledge("stdio rpath wpath disklabel", NULL) == -1)
-   err(1, "pledge");
-   }
-
if (op == UNSPEC)
op = READ;
 
@@ -211,6 +204,18 @@ main(int argc, char *argv[])
);
if (f < 0)
err(4, "%s", specname);
+   if (fstat(f, ) == -1)
+   errx(1, "fstat");
+   if (!S_ISBLK(st.st_mode) && !S_ISCHR(st.st_mode))
+   errx(1, "%s is not a device", dkname);
+
+   if (op == EDIT || op == EDITOR || aflag) {
+   if (pledge("stdio rpath wpath cpath disklabel proc exec", NULL) 
== -1)
+   err(1, "pledge");
+   } else {
+   if (pledge("stdio rpath wpath disklabel", NULL) == -1)
+   err(1, "pledge");
+   }
 
if (autotable != NULL)
parse_autotable(autotable);

Re: rbootd: simplify signal handling

[Theo de Raadt]

Jeremie Courreges-Anglas writes:
> ok

ok deraadt also.

> I doubt you'll get test reports.  Shouldn't this kind of software be
> moved to the ports tree, eventually?  I fear that the percentage of
> users that actually need rbootd is fairly small...

Well, I dunno about moving it to ports.  When someone needs this,
it is a hurdle for them to get it out of ports.  On the other hand
if someone isn't using it, then the presense of the daemon causes no
harm.

And... any relevant changes to other parts of the stack are correctly
adapted, such as natano's recent bpf changes.  Also, this daemon could
gain OpenBSD-specific BPF read/write locking if someone felt like it :)

Re: Pledge failure in disklabel(8)

[Bob Beck]

On Sat, May 28, 2016 at 09:27:09AM -0600, Theo de Raadt wrote:
> > If you try to run disklabel(8) on a file that is not a device, it aborts
> > aborts for want of pledge("ioctl"). This diff prints an error message
> > and exits cleanly. I return exit code 1 but note that sometimes
> > disklabel returns 4; the man page doesn't explain the distinction
> > anywhere.
> > 
> >   $ disklabel /
> >   Abort trap (core dumped)
> >   $ obj/disklabel /
> >   disklabel: / is not a device
> 
> Surprisingly, your fix won't help.  The problem is that opendev() is
> after pledge.
> 
> That is incorrect.  The pledge should occur after opendev.

Try this instead
Index: disklabel.c
===
RCS file: /cvs/src/sbin/disklabel/disklabel.c,v
retrieving revision 1.214
diff -u -p -u -p -r1.214 disklabel.c
--- disklabel.c 25 Nov 2015 17:17:38 -  1.214
+++ disklabel.c 28 May 2016 15:32:11 -
@@ -191,6 +191,12 @@ main(int argc, char *argv[])
argc -= optind;
argv += optind;
 
+   dkname = argv[0];
+   f = opendev(dkname, (op == READ ? O_RDONLY : O_RDWR), OPENDEV_PART,
+   );
+   if (f < 0)
+   err(4, "%s", specname);
+
if (op == EDIT || op == EDITOR || aflag) {
if (pledge("stdio rpath wpath cpath disklabel proc exec", NULL) 
== -1)
err(1, "pledge");
@@ -205,12 +211,6 @@ main(int argc, char *argv[])
if (argc < 1 || (fstabfile && !(op == EDITOR || op == RESTORE ||
aflag)))
usage();
-
-   dkname = argv[0];
-   f = opendev(dkname, (op == READ ? O_RDONLY : O_RDWR), OPENDEV_PART,
-   );
-   if (f < 0)
-   err(4, "%s", specname);
 
if (autotable != NULL)
parse_autotable(autotable);

iwm(4): Add support for Intel Wireless 3165

[Imre Vadasz]

Hi,

This adds support for the AC 3165 Intel wireless chipset.
Tested on a HP x2 210 tablet/detachable:

iwm0 at pci1 dev 0 function 0 "Intel Dual Band Wireless AC 3165" rev 0x81, msi
iwm0: hw rev 0x210, fw ver 16.242414.0, address e0:94:67:bf:6f:76

Index: sys/dev/pci/if_iwm.c
===
RCS file: /cvs/src/sys/dev/pci/if_iwm.c,v
retrieving revision 1.84
diff -u -p -r1.84 if_iwm.c
--- sys/dev/pci/if_iwm.c28 May 2016 08:34:17 -  1.84
+++ sys/dev/pci/if_iwm.c28 May 2016 15:05:30 -
@@ -7659,6 +7659,8 @@ typedef void *iwm_match_t;
 static const struct pci_matchid iwm_devices[] = {
{ PCI_VENDOR_INTEL, PCI_PRODUCT_INTEL_WL_3160_1 },
{ PCI_VENDOR_INTEL, PCI_PRODUCT_INTEL_WL_3160_2 },
+   { PCI_VENDOR_INTEL, PCI_PRODUCT_INTEL_WL_3165_1 },
+   { PCI_VENDOR_INTEL, PCI_PRODUCT_INTEL_WL_3165_2 },
{ PCI_VENDOR_INTEL, PCI_PRODUCT_INTEL_WL_7260_1 },
{ PCI_VENDOR_INTEL, PCI_PRODUCT_INTEL_WL_7260_2 },
{ PCI_VENDOR_INTEL, PCI_PRODUCT_INTEL_WL_7265_1 },
@@ -7822,6 +7824,13 @@ iwm_attach(struct device *parent, struct
case PCI_PRODUCT_INTEL_WL_3160_2:
sc->sc_fwname = "iwm-3160-16";
sc->host_interrupt_operation_mode = 1;
+   sc->sc_device_family = IWM_DEVICE_FAMILY_7000;
+   sc->sc_fwdmasegsz = IWM_FWDMASEGSZ;
+   break;
+   case PCI_PRODUCT_INTEL_WL_3165_1:
+   case PCI_PRODUCT_INTEL_WL_3165_2:
+   sc->sc_fwname = "iwm-7265-16";
+   sc->host_interrupt_operation_mode = 0;
sc->sc_device_family = IWM_DEVICE_FAMILY_7000;
sc->sc_fwdmasegsz = IWM_FWDMASEGSZ;
break;
Index: share/man/man4/iwm.4
===
RCS file: /cvs/src/share/man/man4/iwm.4,v
retrieving revision 1.17
diff -u -p -r1.17 iwm.4
--- share/man/man4/iwm.428 May 2016 08:22:16 -  1.17
+++ share/man/man4/iwm.428 May 2016 15:05:17 -
@@ -28,7 +28,7 @@ The
 .Nm
 driver provides support for
 .Tn Intel
-Wireless 7260, 7265, 3160, and 8260 PCIe Mini Card network adapters.
+Wireless 7260, 7265, 3160, 3165, and 8260 PCIe Mini Card network adapters.
 .Pp
 These are the modes the
 .Nm

Re: Pledge failure in disklabel(8)

[Theo de Raadt]

> If you try to run disklabel(8) on a file that is not a device, it aborts
> aborts for want of pledge("ioctl"). This diff prints an error message
> and exits cleanly. I return exit code 1 but note that sometimes
> disklabel returns 4; the man page doesn't explain the distinction
> anywhere.
> 
>   $ disklabel /
>   Abort trap (core dumped)
>   $ obj/disklabel /
>   disklabel: / is not a device

Surprisingly, your fix won't help.  The problem is that opendev() is
after pledge.

That is incorrect.  The pledge should occur after opendev.

> Index: disklabel.c
> ===
> RCS file: /cvs/src/sbin/disklabel/disklabel.c,v
> retrieving revision 1.214
> diff -u -p -r1.214 disklabel.c
> --- disklabel.c   25 Nov 2015 17:17:38 -  1.214
> +++ disklabel.c   27 May 2016 15:13:25 -
> @@ -119,6 +119,7 @@ main(int argc, char *argv[])
>   int ch, f, error = 0;
>   FILE *t;
>   char *autotable = NULL;
> + struct stat sb;
>  
>   getphysmem();
>  
> @@ -211,6 +212,9 @@ main(int argc, char *argv[])
>   );
>   if (f < 0)
>   err(4, "%s", specname);
> + fstat(f, );
> + if (!S_ISBLK(sb.st_mode) && !S_ISCHR(sb.st_mode))
> + errx(1, "%s is not a device", dkname);
>  
>   if (autotable != NULL)
>   parse_autotable(autotable);
> 

Pledge failure in disklabel(8)

[Anthony Coulter]

If you try to run disklabel(8) on a file that is not a device, it aborts
aborts for want of pledge("ioctl"). This diff prints an error message
and exits cleanly. I return exit code 1 but note that sometimes
disklabel returns 4; the man page doesn't explain the distinction
anywhere.

  $ disklabel /
  Abort trap (core dumped)
  $ obj/disklabel /
  disklabel: / is not a device


Index: disklabel.c
===
RCS file: /cvs/src/sbin/disklabel/disklabel.c,v
retrieving revision 1.214
diff -u -p -r1.214 disklabel.c
--- disklabel.c 25 Nov 2015 17:17:38 -  1.214
+++ disklabel.c 27 May 2016 15:13:25 -
@@ -119,6 +119,7 @@ main(int argc, char *argv[])
int ch, f, error = 0;
FILE *t;
char *autotable = NULL;
+   struct stat sb;
 
getphysmem();
 
@@ -211,6 +212,9 @@ main(int argc, char *argv[])
);
if (f < 0)
err(4, "%s", specname);
+   fstat(f, );
+   if (!S_ISBLK(sb.st_mode) && !S_ISCHR(sb.st_mode))
+   errx(1, "%s is not a device", dkname);
 
if (autotable != NULL)
parse_autotable(autotable);

Re: rbootd: simplify signal handling

[Jeremie Courreges-Anglas]

Philip Guenther  writes:

> Actually, the signal handling was simplified 14(!) years ago by miod@ when 
> he changed rbootd to do all processing from its main loop, with signals 
> just setting flags tested there.  This diff just removes the blocking and 
> unblocking that was rendered superfluous by that previous change.
>
> Back when signal were processed directly from the signal handler, opening 
> and closing the debug output file and reparsing the config, the "normal" 
> code had to block those signals when it wanted to keep those actions from 
> happening.  After miod's changes, the signal handler just sets a flag, so 
> there's no need to mask signals across sections of code.  While here, 
> delete some unnecessary #includes.
>
> I believe this removes the last uses of the obsolete sigblock(), 
> sigsetmask(), and sigmask() from non-GNU base.
>
> ok?

ok

I doubt you'll get test reports.  Shouldn't this kind of software be
moved to the ports tree, eventually?  I fear that the percentage of
users that actually need rbootd is fairly small...

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

Re: mklocale(1): ignore blanks after VARIABLE

[Jeremie Courreges-Anglas]

"Todd C. Miller"  writes:

> Currently, mklocale only ignores the first blank after the VARIABLE
> definition.  This means that we store the variable definition along
> with the leading blanks.
>
> The lexer should eat the blanks after VARIABLE before storing the
> variable definition.

ok

The manpage documents the existing constraint, thus it should probably
be amended.

Index: mklocale.1
===
RCS file: /cvs/src/usr.bin/mklocale/mklocale.1,v
retrieving revision 1.5
diff -u -p -p -u -r1.5 mklocale.1
--- mklocale.1  8 May 2016 15:25:44 -   1.5
+++ mklocale.1  28 May 2016 14:54:02 -
@@ -154,7 +154,7 @@ vendors of
 systems.
 .El
 .It Dv VARIABLE
-This keyword must be followed by a single tab or space character,
+This keyword must be followed by whitespace,
 after which encoding specific data is placed.
 Currently only the
 .Dv "EUC"


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

[patch]: httpd: extend "include" to support glob(3) patterns.

[Fabian Raetz]

Hi,

the patch below teaches httpd's "include" keyword how to handle file
patterns via glob(3) by introducing a new function "pushglob()".

This allows something like the following in httpd.conf:
include "/etc/httpd/sites-enabled/*.conf"

If the pattern passed to pushglob() contains no globbing characters
(e.g. a file path), the absence of the file is an error. In contrast, 
if the pattern contains globbing character and the pattern matches no
files, it is NOT an error.

pushglob() also supports the "secret" flag though it is not used in httpd. 
This should make it reuseable in other parse.y files in the tree.
If secret equals true all matched files must be "valid" or pushglob()
returns NULL.

No change in existing behaviour intended.

If you like this feature, i can prepare a diff for httpd.conf(5).

Cheers,
Fabian

Index: parse.y
===
RCS file: /cvs/src/usr.sbin/httpd/parse.y,v
retrieving revision 1.77
diff -u -p -r1.77 parse.y
--- parse.y 22 Nov 2015 13:27:13 -  1.77
+++ parse.y 28 May 2016 13:38:03 -
@@ -42,6 +42,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -62,6 +63,7 @@ static struct file {
int  lineno;
int  errors;
 } *file, *topfile;
+struct file*pushglob(const char *, int, int *);
 struct file*pushfile(const char *, int);
 int popfile(void);
 int check_file_secrecy(int, const char *);
@@ -157,15 +159,19 @@ grammar   : /* empty */
 
 include: INCLUDE STRING{
struct file *nfile;
+   int  magchar, secret = 0;
 
-   if ((nfile = pushfile($2, 0)) == NULL) {
-   yyerror("failed to include file %s", $2);
-   free($2);
-   YYERROR;
-   }
-   free($2);
+   if ((nfile = pushglob($2, secret, )) == NULL) {
+   if (secret || !magchar) {
+   yyerror("failed to include %s %s",
+   magchar ? "pattern" : "file", $2);
+   free($2);
+   YYERROR;
+   }
+   } else
+   file = nfile;
 
-   file = nfile;
+   free($2);
lungetc('\n');
}
;
@@ -1471,6 +1477,33 @@ check_file_secrecy(int fd, const char *f
return (-1);
}
return (0);
+}
+
+struct file *
+pushglob(const char *pattern, int secret, int *magchar)
+{
+   struct file *nfile = NULL, *tmp;
+   glob_t   g;
+   int  failed = 0, i;
+
+   if (glob(pattern, GLOB_NOCHECK, NULL, ) != 0) {
+   log_warn("cannot glob %s", pattern);
+   *magchar = 0;
+   return (NULL);
+   }
+
+   for (i = 0; i < g.gl_matchc; i++) {
+   tmp = pushfile(g.gl_pathv[i], secret);
+   if (tmp == NULL) {
+   if (secret)
+   failed = 1;
+   } else
+   nfile = tmp;
+   }
+
+   *magchar = g.gl_flags & GLOB_MAGCHAR ? 1 : 0;
+   globfree();
+   return (failed ? NULL : nfile);
 }
 
 struct file *

Re: rcmdsh(3): use getaddrinfo() not gethostbyname2()

[Jeremie Courreges-Anglas]

"Todd C. Miller"  writes:

> rmcd(3) was converted ages ago but apparently this was missed.

ok jca@

Note that the manpage still refers to gethostbyname(3).

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

Re: ptrace PT_IO write bug

[Jeremie Courreges-Anglas]

Mathieu -  writes:

> Martin Natano wrote:
>> The diff reads fine to me, however it is incomplete. There are some
>> callers of process_domem() in arch/. They will need to be changed too.
>> req seems to be in sync with uio_rw in all the cases, so just removing
>> the last argument should do it.
>> 

Thanks, well spotted Martin.

> Thanks for the feedback. The missing callers where an overlook on my
> part, sorry for that.
>
> Here is a regenerated diff including all the call site. As a side note,
> obviously every one of them was using PT_WRITE_I, that's why it went
> unnoticed.

Looks even more correct. :)

ok / objections?

> Mathieu-
>
>
> diff --git a/sys/arch/alpha/alpha/process_machdep.c 
> b/sys/arch/alpha/alpha/process_machdep.c
> index 6fe711e..291e06e 100644
> --- a/sys/arch/alpha/alpha/process_machdep.c
> +++ b/sys/arch/alpha/alpha/process_machdep.c
> @@ -181,7 +181,7 @@ ptrace_read_int(struct proc *p, vaddr_t addr, u_int32_t 
> *v)
>   uio.uio_segflg = UIO_SYSSPACE;
>   uio.uio_rw = UIO_READ;
>   uio.uio_procp = p;
> - return process_domem(curproc, p, , PT_READ_I);
> + return process_domem(curproc, p, );
>  }
>  
>  int
> @@ -199,7 +199,7 @@ ptrace_write_int(struct proc *p, vaddr_t addr, u_int32_t 
> v)
>   uio.uio_segflg = UIO_SYSSPACE;
>   uio.uio_rw = UIO_WRITE;
>   uio.uio_procp = p;
> - return process_domem(curproc, p, , PT_WRITE_I);
> + return process_domem(curproc, p, );
>  }
>  
>  u_int64_t
> diff --git a/sys/arch/hppa/hppa/trap.c b/sys/arch/hppa/hppa/trap.c
> index e86e636..805a924 100644
> --- a/sys/arch/hppa/hppa/trap.c
> +++ b/sys/arch/hppa/hppa/trap.c
> @@ -690,7 +690,7 @@ ss_get_value(struct proc *p, vaddr_t addr, u_int *value)
>   uio.uio_segflg = UIO_SYSSPACE;
>   uio.uio_rw = UIO_READ;
>   uio.uio_procp = curproc;
> - return (process_domem(curproc, p, , PT_READ_I));
> + return (process_domem(curproc, p, ));
>  }
>  
>  int
> @@ -708,7 +708,7 @@ ss_put_value(struct proc *p, vaddr_t addr, u_int value)
>   uio.uio_segflg = UIO_SYSSPACE;
>   uio.uio_rw = UIO_WRITE;
>   uio.uio_procp = curproc;
> - return (process_domem(curproc, p, , PT_WRITE_I));
> + return (process_domem(curproc, p, ));
>  }
>  
>  void
> diff --git a/sys/arch/m88k/m88k/trap.c b/sys/arch/m88k/m88k/trap.c
> index d11f8ca..734743d 100644
> --- a/sys/arch/m88k/m88k/trap.c
> +++ b/sys/arch/m88k/m88k/trap.c
> @@ -1447,7 +1447,7 @@ ss_get_value(struct proc *p, vaddr_t addr, u_int *value)
>   uio.uio_segflg = UIO_SYSSPACE;
>   uio.uio_rw = UIO_READ;
>   uio.uio_procp = curproc;
> - return (process_domem(curproc, p, , PT_READ_I));
> + return (process_domem(curproc, p, ));
>  }
>  
>  int
> @@ -1465,7 +1465,7 @@ ss_put_value(struct proc *p, vaddr_t addr, u_int value)
>   uio.uio_segflg = UIO_SYSSPACE;
>   uio.uio_rw = UIO_WRITE;
>   uio.uio_procp = curproc;
> - return (process_domem(curproc, p, , PT_WRITE_I));
> + return (process_domem(curproc, p, ));
>  }
>  
>  /*
> diff --git a/sys/arch/mips64/mips64/trap.c b/sys/arch/mips64/mips64/trap.c
> index 0bd71e5..9e81952 100644
> --- a/sys/arch/mips64/mips64/trap.c
> +++ b/sys/arch/mips64/mips64/trap.c
> @@ -1021,7 +1021,7 @@ ptrace_read_insn(struct proc *p, vaddr_t va, uint32_t 
> *insn)
>   uio.uio_segflg = UIO_SYSSPACE;
>   uio.uio_rw = UIO_READ;
>   uio.uio_procp = p;
> - return process_domem(p, p, , PT_READ_I);
> + return process_domem(p, p, );
>  }
>  
>  int
> @@ -1039,7 +1039,7 @@ ptrace_write_insn(struct proc *p, vaddr_t va, uint32_t 
> insn)
>   uio.uio_segflg = UIO_SYSSPACE;
>   uio.uio_rw = UIO_WRITE;
>   uio.uio_procp = p;
> - return process_domem(p, p, , PT_WRITE_I);
> + return process_domem(p, p, );
>  }
>  
>  /*
> diff --git a/sys/kern/sys_process.c b/sys/kern/sys_process.c
> index 60ec50e..4d589e7 100644
> --- a/sys/kern/sys_process.c
> +++ b/sys/kern/sys_process.c
> @@ -368,8 +368,7 @@ sys_ptrace(struct proc *p, void *v, register_t *retval)
>   uio.uio_segflg = UIO_SYSSPACE;
>   uio.uio_rw = write ? UIO_WRITE : UIO_READ;
>   uio.uio_procp = p;
> - error = process_domem(p, t, , write ? PT_WRITE_I :
> - PT_READ_I);
> + error = process_domem(p, t, );
>   if (write == 0)
>   *retval = temp;
>   return (error);
> @@ -387,23 +386,14 @@ sys_ptrace(struct proc *p, void *v, register_t *retval)
>   uio.uio_procp = p;
>   switch (piod.piod_op) {
>   case PIOD_READ_I:
> - req = PT_READ_I;
> - uio.uio_rw = UIO_READ;
> - break;
>   case PIOD_READ_D:
> - req = PT_READ_D;
>   uio.uio_rw = UIO_READ;
>   break;
>   case PIOD_WRITE_I:
> - req = PT_WRITE_I;
> -

Re: ptrace PT_IO write bug

[Mathieu -]

Martin Natano wrote:
> The diff reads fine to me, however it is incomplete. There are some
> callers of process_domem() in arch/. They will need to be changed too.
> req seems to be in sync with uio_rw in all the cases, so just removing
> the last argument should do it.
> 

Thanks for the feedback. The missing callers where an overlook on my
part, sorry for that.

Here is a regenerated diff including all the call site. As a side note,
obviously every one of them was using PT_WRITE_I, that's why it went
unnoticed.

Mathieu-


diff --git a/sys/arch/alpha/alpha/process_machdep.c 
b/sys/arch/alpha/alpha/process_machdep.c
index 6fe711e..291e06e 100644
--- a/sys/arch/alpha/alpha/process_machdep.c
+++ b/sys/arch/alpha/alpha/process_machdep.c
@@ -181,7 +181,7 @@ ptrace_read_int(struct proc *p, vaddr_t addr, u_int32_t *v)
uio.uio_segflg = UIO_SYSSPACE;
uio.uio_rw = UIO_READ;
uio.uio_procp = p;
-   return process_domem(curproc, p, , PT_READ_I);
+   return process_domem(curproc, p, );
 }
 
 int
@@ -199,7 +199,7 @@ ptrace_write_int(struct proc *p, vaddr_t addr, u_int32_t v)
uio.uio_segflg = UIO_SYSSPACE;
uio.uio_rw = UIO_WRITE;
uio.uio_procp = p;
-   return process_domem(curproc, p, , PT_WRITE_I);
+   return process_domem(curproc, p, );
 }
 
 u_int64_t
diff --git a/sys/arch/hppa/hppa/trap.c b/sys/arch/hppa/hppa/trap.c
index e86e636..805a924 100644
--- a/sys/arch/hppa/hppa/trap.c
+++ b/sys/arch/hppa/hppa/trap.c
@@ -690,7 +690,7 @@ ss_get_value(struct proc *p, vaddr_t addr, u_int *value)
uio.uio_segflg = UIO_SYSSPACE;
uio.uio_rw = UIO_READ;
uio.uio_procp = curproc;
-   return (process_domem(curproc, p, , PT_READ_I));
+   return (process_domem(curproc, p, ));
 }
 
 int
@@ -708,7 +708,7 @@ ss_put_value(struct proc *p, vaddr_t addr, u_int value)
uio.uio_segflg = UIO_SYSSPACE;
uio.uio_rw = UIO_WRITE;
uio.uio_procp = curproc;
-   return (process_domem(curproc, p, , PT_WRITE_I));
+   return (process_domem(curproc, p, ));
 }
 
 void
diff --git a/sys/arch/m88k/m88k/trap.c b/sys/arch/m88k/m88k/trap.c
index d11f8ca..734743d 100644
--- a/sys/arch/m88k/m88k/trap.c
+++ b/sys/arch/m88k/m88k/trap.c
@@ -1447,7 +1447,7 @@ ss_get_value(struct proc *p, vaddr_t addr, u_int *value)
uio.uio_segflg = UIO_SYSSPACE;
uio.uio_rw = UIO_READ;
uio.uio_procp = curproc;
-   return (process_domem(curproc, p, , PT_READ_I));
+   return (process_domem(curproc, p, ));
 }
 
 int
@@ -1465,7 +1465,7 @@ ss_put_value(struct proc *p, vaddr_t addr, u_int value)
uio.uio_segflg = UIO_SYSSPACE;
uio.uio_rw = UIO_WRITE;
uio.uio_procp = curproc;
-   return (process_domem(curproc, p, , PT_WRITE_I));
+   return (process_domem(curproc, p, ));
 }
 
 /*
diff --git a/sys/arch/mips64/mips64/trap.c b/sys/arch/mips64/mips64/trap.c
index 0bd71e5..9e81952 100644
--- a/sys/arch/mips64/mips64/trap.c
+++ b/sys/arch/mips64/mips64/trap.c
@@ -1021,7 +1021,7 @@ ptrace_read_insn(struct proc *p, vaddr_t va, uint32_t 
*insn)
uio.uio_segflg = UIO_SYSSPACE;
uio.uio_rw = UIO_READ;
uio.uio_procp = p;
-   return process_domem(p, p, , PT_READ_I);
+   return process_domem(p, p, );
 }
 
 int
@@ -1039,7 +1039,7 @@ ptrace_write_insn(struct proc *p, vaddr_t va, uint32_t 
insn)
uio.uio_segflg = UIO_SYSSPACE;
uio.uio_rw = UIO_WRITE;
uio.uio_procp = p;
-   return process_domem(p, p, , PT_WRITE_I);
+   return process_domem(p, p, );
 }
 
 /*
diff --git a/sys/kern/sys_process.c b/sys/kern/sys_process.c
index 60ec50e..4d589e7 100644
--- a/sys/kern/sys_process.c
+++ b/sys/kern/sys_process.c
@@ -368,8 +368,7 @@ sys_ptrace(struct proc *p, void *v, register_t *retval)
uio.uio_segflg = UIO_SYSSPACE;
uio.uio_rw = write ? UIO_WRITE : UIO_READ;
uio.uio_procp = p;
-   error = process_domem(p, t, , write ? PT_WRITE_I :
-   PT_READ_I);
+   error = process_domem(p, t, );
if (write == 0)
*retval = temp;
return (error);
@@ -387,23 +386,14 @@ sys_ptrace(struct proc *p, void *v, register_t *retval)
uio.uio_procp = p;
switch (piod.piod_op) {
case PIOD_READ_I:
-   req = PT_READ_I;
-   uio.uio_rw = UIO_READ;
-   break;
case PIOD_READ_D:
-   req = PT_READ_D;
uio.uio_rw = UIO_READ;
break;
case PIOD_WRITE_I:
-   req = PT_WRITE_I;
-   uio.uio_rw = UIO_WRITE;
-   break;
case PIOD_WRITE_D:
-   req = PT_WRITE_D;
uio.uio_rw = UIO_WRITE;
break;
case PIOD_READ_AUXV:
-   

Re: rcmdsh(3): use getaddrinfo() not gethostbyname2()

[Florian Obser]

OK florian@

On Fri, May 27, 2016 at 06:09:40PM -0600, Todd C. Miller wrote:
> rmcd(3) was converted ages ago but apparently this was missed.
> 
>  - todd
> 
> Index: lib/libc/net/rcmdsh.c
> ===
> RCS file: /cvs/src/lib/libc/net/rcmdsh.c,v
> retrieving revision 1.18
> diff -u -p -u -r1.18 rcmdsh.c
> --- lib/libc/net/rcmdsh.c 24 Nov 2015 22:03:33 -  1.18
> +++ lib/libc/net/rcmdsh.c 28 May 2016 00:06:24 -
> @@ -38,6 +38,7 @@
>  #include  
>  #include  
>  #include  
> +#include  
>  #include  
>  #include  
>  #include  
> @@ -55,7 +56,8 @@ int
>  rcmdsh(char **ahost, int rport, const char *locuser, const char *remuser,
>  const char *cmd, char *rshprog)
>  {
> - struct hostent *hp;
> + static char hbuf[HOST_NAME_MAX+1];
> + struct addrinfo hint, *res;
>   int sp[2];
>   pid_t cpid;
>   char *p, pwbuf[_PW_BUF_LEN];
> @@ -74,9 +76,16 @@ rcmdsh(char **ahost, int rport, const ch
>  
>   /* Validate remote hostname. */
>   if (strcmp(*ahost, "localhost") != 0) {
> - if ((hp = gethostbyname2(*ahost, AF_INET)) ||
> - (hp = gethostbyname2(*ahost, AF_INET6)))
> - *ahost = hp->h_name;
> + memset(, 0, sizeof(hint));
> + hint.ai_family = PF_UNSPEC;
> + hint.ai_flags = AI_CANONNAME;
> + if (getaddrinfo(*ahost, NULL, , ) == 0) {
> + if (res->ai_canonname) {
> + strlcpy(hbuf, res->ai_canonname, sizeof(hbuf));
> + *ahost = hbuf;
> + }
> + freeaddrinfo(res);
> + }
>   }
>  
>   /* Get a socketpair we'll use for stdin and stdout. */
> 

-- 
I'm not entirely sure you are real.

Re: MBIM Patch - Part 1 of 4

[Stuart Henderson]

On 2016/05/28 09:18, Stefan Sperling wrote:
> Could mbim in theory support IPv6?

The MBIM spec allows for it.

> Don't worry about breaking existing umsm(4) setups.
> Since this driver provides a much simpler user interface than umsm coupled
> with pppd(8) I'd suggest to eventually prefer attaching mbim instead of umsm
> where possible.

Agreed.

One caveat though, some people may still need the serial port
exposed for SMS, so it's helpful to make sure that a simple
"config -ef /bsd", "disable umb", "quit" will allow that.

Re: ptrace PT_IO write bug

[Martin Natano]

On Fri, May 27, 2016 at 10:15:39PM +0200, Jeremie Courreges-Anglas wrote:
> Mathieu -  writes:
> 
> > Hello everyone,
> >
> > While playing a bit with ptrace to do some debugging I stumbled upon
> > something that looks like a bug.
> > While trying to write to the ptrace'd process using PT_IO in combinaison
> > with PIOD_WRITE_D I kept getting EFAULTs.
> > PIOD_READ_D would work fine on the same address though, and even more
> > weirdly PIOD_WRITE_I would also work fine on the same address.
> > Even more strange, PT_WRITE_D works fine too on the same address.
> > So in effect, only PT_IO + PIOD_WRITE_D would EFAULT on this address.
> >
> > If this is the expected behavior (I really doubt it), then the man page
> > is wrong because it states clearly that *_I and *_D are equivalent.
> >
> > Digging a bit on the implementation I traced it back to rev 1.33 of
> > sys_process.c.
> > The old implementation used procfs_domem to do the uvm_io call, and
> > based the decision as to use UVM_IO_FIXPROT or not on the uio.uio_rw
> > field (UIO_WRITE meant FIXPROT vs UIO_READ).
> > However the new implementation, in process_domem takes a third
> > parameter, req, the ptrace request and would use UVM_IO_FIXPROT only in
> > the PT_WRITE_I case (rings any bell?).
> > That's why PT_WRITE_D will EFAULT in any case.
> > Oh and PT_WRITE_I and PT_WRITE_D work because they both use PT_WRITE_I
> > as the req parameter :
> >  process_domem(p, t, , write ? PT_WRITE_I : 
> >
> > So I came up with the following diff (kind of the big hammer approach),
> > which gets rid of the req parameters and base the UVM_IO_FIXPROT
> > decision on the uio.uio_rw field as the previous code (10 years ago!)
> > was doing.
> 
> This looks correct to me.  ok / can I get another review?
> 
> -- 
> jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF  DDCC 0DFA 74AE 1524 E7EE
> 

The diff reads fine to me, however it is incomplete. There are some
callers of process_domem() in arch/. They will need to be changed too.
req seems to be in sync with uio_rw in all the cases, so just removing
the last argument should do it.

natano

Re: iwm(4): The valid_{rx,tx}_ant values only be read from nvm for 8000 family

[Stefan Sperling]

On Thu, May 26, 2016 at 05:30:07PM +0200, Imre Vadasz wrote:
> Hi,
> 
> We only need to set the data->valid_tx_ant and data->valid_rx_ant
> values in iwm_parse_nvm_data() for the 8000 family chips.
> This matches what iwl_set_radio_cfg() in iwl-nvm-parse.c from Linux's
> iwlwifi does.
> 
> Also the "if (!data->valid_tx_ant || !data->valid_rx_ant) {" check should
> be removed again matching the iwlwifi code.

Thanks. I've rebased and committed this on top of my 8260 diff.

> That check actually got
> triggered when trying out the updated iwm code with a 3165 chipset (which
> seems to work correctly by just adding the pci ids, and treating it like
> the 7265 chipset).

Could you send a patch that adds 3165 support? I don't have this hardware.

Re: MBIM Patch - Part 1 of 4

[Stefan Sperling]

On Mon, May 23, 2016 at 03:54:36PM +0200, Martin Pieuchot wrote:
> I'd like to bikeshed early, we're trying to prefix all new USB driver
> name with 'u'.  So I'd suggest a rename when this goes in tree.

umb(4) ("USB Mobile Broadband") is available.

Re: MBIM Patch - Part 1 of 4

[Stefan Sperling]

On Mon, May 23, 2016 at 03:37:32PM +0200, Gerhard Roth wrote:
> I have this stuff around quite a while now, but since patrick@ asked
> me repeatedly, I think it is time to share this now.
> 
> This is a patch that adds support for the "Mobile Broadband Interface
> Model" (MBIM) from http://www.usb.org/. It allows to attach UMTS/LTE
> USB devices as a network interface.
> 
> I'm not asking for OKs at this time, but hope that some people will
> give it test. I tested with two different Sierra Wireless modules
> (EM8805, MC8305) and each one behaved a little bit different.
> 
> In order to configure the network interface, this is what you do:
> 
>   # ifconfig mbim0 pin 1234 apn internet.t-mobile
>   # ifconfig mbim0 inet 0.0.0.1 0.0.0.2
>   # route delete default
>   # route add -ifp mbim0 default 0.0.0.2
>   # ifconfig mbim0 up
> 
> The mbim interface is a point-to-point type interface and will
> update the default route, once it is registered in the network.

As others have already said, I think the problem with these diffs
is that you're trying to solve too many problems at once.

I believe the problems this driver should solve first and foremost are getting
a network link and getting an IP assigned to the interface so people can use
that IP to set up whatever routing they want.

Could mbim in theory support IPv6? We don't need to do anything about
this now. But the design shouldn't assume it will only support IPv4.

Don't worry about breaking existing umsm(4) setups.
Since this driver provides a much simpler user interface than umsm coupled
with pppd(8) I'd suggest to eventually prefer attaching mbim instead of umsm
where possible. I hate having to copy pppd config files around for umsm.
If I lost those files I'd have to spend a day or so to get the setup working
again because I don't remember (and don't want to remember) all the AT and
up/down scripts I used to get umsm to work. That's why I think mbim is 
promising.

> To get extended information on the interface use:
> 
>   # ifconfig mbim0 devinfo

I don't see the need to add a new subcommand for this.
If this information is important why is it not shown as part of the
default 'ifconfig mbim0' output? If it is not important, why show it?

I agree with Theo that the ifconfig diff is too large. And it seems you're
exposing too many details at this layer. How much of it can be removed while
still solving the problem of getting a network link and an IP assigned?
Can you compress status information on the 'status:' line, instead of printing
seperate lines for data items such as pin, error state, rssi, ber and so on?
How much of this information do users really need?

Re: replace bpf open loops

[Martin Natano]

On Fri, May 27, 2016 at 10:03:37PM +0200, Jeremie Courreges-Anglas wrote:
> Martin Natano  writes:
> 
> > I think it's time to get rid of all the bpf open() loops in base.
> > dhclient and libpcap do a plain open("/dev/bpf0", ...) since a couple of
> > weeks now and the upgrade issue (/dev/bpf vs. /dev/bpf0) has been fixed.
> > I didn't hear any other complaints in the meantime.
> >
> > Ok? Too soon?
> 
> Looks fine, and I don't see any reason to delay this.  Did you have
> a particular issue in mind?

No.

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