Re: pledge route(8) with '-n' flag

[Theo de Raadt]

> On 16/11/15(Mon) 10:03, Ricardo Mestre wrote:
> > Hello!
> > 
> > Like Benoit said, monitor still needs dns all the time, but since pledge
> > was being called there again with dns pledge then I thought it wouldn't
> > abort. Taking that into consideration and looking at it a little bit
> > more, how about this?
> 
> IMHO this doesn't make sense if I explicitly say "route -n monitor"  I'd
> prefer not to have any address resolved
> 
> Otherwise I just say "route monitor".
> 
> I'd add that on the list of bugs discovered by pledge(2).

Yes... the monitor command should honour the -n flag.

Re: Reducing compilation warnings in imsg.c on FreeSBD

[Michael McConville]

Craig Rodrigues wrote:
> Recently, I imported imsg.c from OpenBSD to the
> FreeBSD base system's libopenbsd:
> 
> https://svnweb.freebsd.org/changeset/base/290375
> 
> When compiling on FreeBSD, we get a compiler warning with clang:
> 
> cc  -O2 -pipe   -I/opt2/branches/head2/lib/libopenbsd -std=gnu99
> -fstack-protector-strong -Wsystem-headers -Wall -Wno-format-y2k -W
> -Wno-unused-parameter -Wstrict-prototypes -Wmissing-prototypes
> -Wpointer-arith -Wno-uninitialized -Wno-pointer-sign -Wno-empty-body
> -Wno-string-plus-int -Wno-unused-const-variable -Wno-tautological-compare
> -Wno-unused-value -Wno-parentheses-equality -Wno-unused-function
> -Wno-enum-conversion -Wno-unused-local-typedef -Qunused-arguments -c
> /opt2/branches/head2/lib/libopenbsd/imsg.c -o imsg.o
> /opt2/branches/head2/lib/libopenbsd/imsg.c:78:6: warning: comparison of
> integers of different signs: 'unsigned long' and 'int' [-Wsign-compare]
> >= getdtablesize()) {
> ^  ~~~
> 1 warning generated.
> 
> I can certainly patch the code with a cast on FreeBSD to get rid of
> the compiler warning.  However, I was wondering if there is a good fix
> that we can share between OpenBSD and FreeBSD?

Both OSs define getdtablesize() as sysconf(_SC_OPEN_MAX). sysconf(3)
returns a long, so it seems more correct to make getdtablesize(3) return
a long or ssize_t. The return value has to be signed because sysconf(3)
is specified by POSIX to return -1 and set errno on failure.

pledge newsyslog

[Sebastian Benoit]

hi,

this is pledge() in newsyslog.

please check & test...

and is someone using monitormode, please say so ;)

(oh, and oks?)

diff --git usr.bin/newsyslog/newsyslog.c usr.bin/newsyslog/newsyslog.c
index 761da36..acfd871 100644
--- usr.bin/newsyslog/newsyslog.c
+++ usr.bin/newsyslog/newsyslog.c
@@ -191,11 +191,20 @@ main(int argc, char **argv)
struct pidinfo *pidlist, *pl;
int status, listlen;
char **av;
+
+   if (pledge("stdio rpath wpath cpath fattr exec proc", NULL) == -1)
+   err(1,"pledge");

parse_args(argc, argv);
argc -= optind;
argv += optind;
 
+   if (noaction && pledge("stdio rpath", NULL) == -1)
+   err(1,"pledge");
+   else if (!monitormode && pledge("stdio rpath wpath cpath fattr proc",
+   NULL) == -1)
+   err(1,"pledge");
+   
if (needroot && getuid() && geteuid())
errx(1, "You must be root.");
 

Re: Reducing compilation warnings in imsg.c on FreeSBD

[Theo de Raadt]

> Both OSs define getdtablesize() as sysconf(_SC_OPEN_MAX). sysconf(3)
> returns a long, so it seems more correct to make getdtablesize(3) return
> a long or ssize_t. The return value has to be signed because sysconf(3)
> is specified by POSIX to return -1 and set errno on failure.

Don't be ridiculous, getdtablesize() is older than you are.  We are
not changing the type it returns.

Re: pledge route(8) with '-n' flag

[Ricardo Mestre]

Hi Martin,

Using the same logic of my previous patch, now monitor works with
"stdio" when -n is used and "stdio rpath dns" otherwhise. Unfortunately
for newroute (add/delete/change) it still needs the 3 of them since when
it detects it's a host (instead of an IP address) it will always contact
the resolver, no matter -n is used or not:

Index: route.c
===
RCS file: /cvs/src/sbin/route/route.c,v
retrieving revision 1.179
diff -u -p -u -r1.179 route.c
--- route.c 25 Oct 2015 09:37:08 -  1.179
+++ route.c 16 Nov 2015 11:07:24 -
@@ -224,17 +224,6 @@ main(int argc, char **argv)
case K_FLUSH:
exit(flushroutes(argc, argv));
break;
-   }
-   
-   if (nflag) {
-   if (pledge("stdio rpath dns", NULL) == -1)
-   err(1, "pledge");
-   } else {
-   if (pledge("stdio rpath dns", NULL) == -1)
-   err(1, "pledge");
-   }
-
-   switch (kw) {
case K_GET:
uid = 0;
/* FALLTHROUGH */
@@ -330,7 +319,7 @@ flushroutes(int argc, char **argv)
}

if (nflag) {
-   if (pledge("stdio rpath dns", NULL) == -1)
+   if (pledge("stdio", NULL) == -1)
err(1, "pledge");
} else {
if (pledge("stdio rpath dns", NULL) == -1)
@@ -446,6 +435,9 @@ newroute(int argc, char **argv)
uint8_t prio = 0;
struct hostent *hp = NULL;

+   if (pledge("stdio rpath dns", NULL) == -1)
+   err(1, "pledge");
+
if (uid)
errx(1, "must be root to alter routing table");
cmd = argv[0];
@@ -1090,8 +1082,13 @@ monitor(int argc, char *argv[])
char msg[2048];
time_t now;

-   if (pledge("stdio rpath dns", NULL) == -1)
-   err(1, "pledge");
+   if (nflag) {
+   if (pledge("stdio", NULL) == -1)
+   err(1, "pledge");
+   } else {
+   if (pledge("stdio rpath dns", NULL) == -1)
+   err(1, "pledge");
+   }

verbose = 1;
if (debugonly) {
Index: show.c
===
RCS file: /cvs/src/sbin/route/show.c,v
retrieving revision 1.102
diff -u -p -u -r1.102 show.c
--- show.c  23 Oct 2015 15:03:25 -  1.102
+++ show.c  16 Nov 2015 11:07:33 -
@@ -146,7 +146,7 @@ p_rttables(int af, u_int tableid, int ha
}

if (nflag) {
-   if (pledge("stdio rpath dns", NULL) == -1)
+   if (pledge("stdio", NULL) == -1)
err(1, "pledge");
} else {
if (pledge("stdio rpath dns", NULL) == -1)

On 16/11/2015 10:22, Martin Pieuchot wrote:
> On 16/11/15(Mon) 10:03, Ricardo Mestre wrote:
>> Hello!
>>
>> Like Benoit said, monitor still needs dns all the time, but since pledge
>> was being called there again with dns pledge then I thought it wouldn't
>> abort. Taking that into consideration and looking at it a little bit
>> more, how about this?
> 
> IMHO this doesn't make sense if I explicitly say "route -n monitor"  I'd
> prefer not to have any address resolved
> 
> Otherwise I just say "route monitor".
> 
> I'd add that on the list of bugs discovered by pledge(2).
> 

Re: PC Engines APU2 coming soon - how is OpenBSD's support so far?

[Chris Cappuccio]

Noth [nothingn...@citycable.ch] wrote:
> Hi again,
> 
>   I think I've found a bug: if I have a console session open using minicom
> or cu when rebooting, the machine hangs. This doesn't happen with either
> CentOS Linux 7 or FreeBSD 10.2 / 11. I can reproduce the problem. Anyone
> else have this issue ?
> 

The boot loader output is emulated video output until it switches to com0.
Once it reads 'stty com0 115200; set tty com0' from /etc/boot.conf, it 
switches from video output to serial output.

During the switch, some contents comes out of the boot loader that is at
the wrong baud rate (or something similar). This causes some high characters
to come out, which your terminal emulator might interpret as a terminal command
and it could even send a short reply back. If it does, this reply stops the
auto-boot process and you sit at boot> indefinitely.

The solution is to not leave a terminal emulator connected during the boot
(or fix the boot loader to not output high characters)

Re: httpd URL rewrite support patch

[Stanislaw Adaszewski]

Some more fixes for style and parser syntax.

Best,

S.

On Sun, Nov 15, 2015 at 07:46:35PM +0100, Reyk Floeter wrote:
> On Sun, Nov 15, 2015 at 06:41:49PM +0100, Stanislaw Adaszewski wrote:
> > Sorry again, I'm trying now with with patch as an attachment -
> > when I did it the first time I think it wasn't accepted by the
> > list.
> > 
> > On Fri, Nov 13, 2015 at 05:22:22PM +0100, Bret Lambert wrote:
> > > tech@ accepts attachments; don't make this harder for us than
> > > it needs to be, please.
> > > 
> > > On Fri, Nov 13, 2015 at 05:06:47PM +0100, Stanislaw Adaszewski wrote:
> > > > Excuse me, the indentation was removed because message wasn't plain 
> > > > text.
> > > > 
> > > > Patch: http://pastebin.com/XnZLEPEk
> > > > 
> > > > httpd.conf: http://pastebin.com/gGp3NqCD
> > > > 
> > > > rewr.php: http://pastebin.com/prA1NJXC
> > > > 
> 
> OK, thanks, now I got the diff.
> 
> It is an interesting and simple approach, but neither florian@ nor me
> had the time to look at it yet.  See comments below.
> 
> > Index: parse.y
> > ===
> > RCS file: /cvs/src/usr.sbin/httpd/parse.y,v
> > retrieving revision 1.73
> > diff -u -p -u -r1.73 parse.y
> > --- parse.y 19 Jul 2015 05:17:27 -  1.73
> > +++ parse.y 13 Nov 2015 08:29:10 -
> > @@ -907,7 +907,7 @@ filter  : block RETURN NUMBER optstring 
> >  
> > if ($4 != NULL) {
> > /* Only for 3xx redirection headers */
> > -   if ($3 < 300 || $3 > 399) {
> > +   if ($3 != 200 && ($3 < 300 || $3 > 399)) {
> 
> I'd add it as "pass rewrite" instead.  It is not blocking the connection.
> 
> > yyerror("invalid return code for "
> > "location URI");
> > free($4);
> > Index: server_http.c
> > ===
> > RCS file: /cvs/src/usr.sbin/httpd/server_http.c,v
> > retrieving revision 1.96
> > diff -u -p -u -r1.96 server_http.c
> > --- server_http.c   31 Jul 2015 00:10:51 -  1.96
> > +++ server_http.c   13 Nov 2015 08:29:10 -
> > @@ -1054,6 +1054,7 @@ server_response(struct httpd *httpd, str
> > int  portval = -1, ret;
> > char*hostval;
> > const char  *errstr = NULL;
> > +   charbuf[IBUF_READ_SIZE];
> >  
> > /* Canonicalize the request path */
> > if (desc->http_path == NULL ||
> > @@ -1154,11 +1155,44 @@ server_response(struct httpd *httpd, str
> > resp->http_method = desc->http_method;
> > if ((resp->http_version = strdup(desc->http_version)) == NULL)
> > goto fail;
> > +   
> > +#ifdef DEBUG
> > +   DPRINTF("%s: http_path: %s, http_path_alias: %s\n", __func__, 
> > desc->http_path, desc->http_path_alias);
> 
> No need to put DPRINTF in DEBUG - it is already only compiled with DEBUG.
> 
> > +#endif
> >  
> > /* Now search for the location */
> > srv_conf = server_getlocation(clt, desc->http_path);
> >  
> > -   if (srv_conf->flags & SRVFLAG_BLOCK) {
> > +
> > +   if ((srv_conf->flags & SRVFLAG_BLOCK) && srv_conf->return_code == 200) {
> > +#ifdef DEBUG
> > +   DPRINTF("%s: Rewrite from: %s to %s\n", __func__, 
> > desc->http_path, srv_conf->return_uri);
> 
> Same here.
> 
> Hint: when sending patches, developers appreciate it when they already
> follow style(9).  This makes it easier for us to read.  So wrap the
> lines at 80 chars, don't use '//' comments as below, ...
> 
> > +#endif
> > +   if (desc->http_path != NULL) {
> > +   free(desc->http_path);
> > +   desc->http_path = NULL;
> 
> Oh, this will break some of the macros in server_expand_http()!
> 
> > +   }
> > +   if (server_expand_http(clt, srv_conf->return_uri, buf, 
> > sizeof(buf)) == NULL) {
> > +   server_abort_http(clt, 500, strerror(errno));
> > +   return (-1);
> > +   }
> > +   desc->http_path = strdup(buf);
> > +   if (desc->http_path == NULL) {
> > +   server_abort_http(clt, 500, strerror(errno));
> > +   }
> > +   desc->http_query = strchr(desc->http_path, '?');
> > +   if (desc->http_query != NULL) {
> > +   *desc->http_query++ = '\0';
> > +   desc->http_query = strdup(desc->http_query);
> > +   if (desc->http_query == NULL) {
> > +   server_abort_http(clt, 500, strerror(errno));
> > +   }
> > +   }
> > +   srv_conf = server_getlocation(clt, desc->http_path); // again
> > +   }
> > +
> > +
> > +   if ((srv_conf->flags & SRVFLAG_BLOCK) && srv_conf->return_code != 200) {
> 
> This logic is not so nice.
> 
> But what about
> 
>   if (srv_conf->flags & 

Re: initial 802.11n implementation

[Stefan Sperling]

On Mon, Nov 16, 2015 at 11:56:27AM +0100, Martin Pieuchot wrote:
> I'm not saying that's better.  I'm saying that the actual 11n driver/
> stack glue has been designed with iwn(4)'s behavior in mind.  So I
> think that's one argument more to do the same with iwm(4)...  At least
> at this stage.

But that's true for all of net80211, not just the 11n code.
net80211 assumes drivers can do anything in interrupt context.
It was written for early 11a/b/g hardware running in single core machines.
Current firmware based hardware essentially assumes the host OS has
interrupt threads which can sleep (like almost everyone else except us...)

This problem also exists with USB devices, and also with iwn(4).
iwn(4) just cheats around this limitation in a different way,
and not just in the block ack cases.

I won't mind switching iwm to iwn's behaviour but the diff I sent
to do this was not OK'd. I'd rather work on one complex problem at
a time so as long as iwm's behaviour doesn't cause unexpected problems
with 11n I'd rather work on that later. In my testing it works.

Eventually we will probably try moving net80211 processing into task
threads anyway, if not only for SMP. FreeBSD has already done that.

Re: pledge route(8) with '-n' flag

[Ricardo Mestre]

Hello again,

With the latest patch I sent all code paths honor -n and won't abort due
to pledge only to stdio, for now the only remaining it's newroute
(add/change/delete) that aborts if -n is used and pledge only to stdio.

Now, this one starts getting into netinet and I'm still not comfortable
with it, sorry :(

On 16/11/2015 16:06, Theo de Raadt wrote:
>> On 16/11/15(Mon) 10:03, Ricardo Mestre wrote:
>>> Hello!
>>>
>>> Like Benoit said, monitor still needs dns all the time, but since pledge
>>> was being called there again with dns pledge then I thought it wouldn't
>>> abort. Taking that into consideration and looking at it a little bit
>>> more, how about this?
>>
>> IMHO this doesn't make sense if I explicitly say "route -n monitor"  I'd
>> prefer not to have any address resolved
>>
>> Otherwise I just say "route monitor".
>>
>> I'd add that on the list of bugs discovered by pledge(2).
> 
> Yes... the monitor command should honour the -n flag.
> 

[trivial] int -> size_t in pax(1)

[Michael McConville]

ok?


Index: pax.h
===
RCS file: /cvs/src/bin/pax/pax.h,v
retrieving revision 1.27
diff -u -p -r1.27 pax.h
--- pax.h   19 Mar 2015 05:14:24 -  1.27
+++ pax.h   16 Nov 2015 18:23:24 -
@@ -79,7 +79,7 @@ typedef struct pattern {
char*pstr;  /* pattern to match, user supplied */
char*pend;  /* end of a prefix match */
char*chdname;   /* the dir to change to if not NULL.  */
-   int plen;   /* length of pstr */
+   size_t  plen;   /* length of pstr */
int flgs;   /* processing/state flags */
 #define MTCH   0x1 /* pattern has been matched */
 #define DIR_MTCH   0x2 /* pattern matched a directory */

Re: Reducing compilation warnings in imsg.c on FreeSBD

[Philip Guenther]

On Mon, Nov 16, 2015 at 8:41 AM, Michael McConville  wrote:
> Craig Rodrigues wrote:
>> Recently, I imported imsg.c from OpenBSD to the
>> FreeBSD base system's libopenbsd:
>>
>> https://svnweb.freebsd.org/changeset/base/290375
>>
>> When compiling on FreeBSD, we get a compiler warning with clang:
>>
>> cc  -O2 -pipe   -I/opt2/branches/head2/lib/libopenbsd -std=gnu99
>> -fstack-protector-strong -Wsystem-headers -Wall -Wno-format-y2k -W
>> -Wno-unused-parameter -Wstrict-prototypes -Wmissing-prototypes
>> -Wpointer-arith -Wno-uninitialized -Wno-pointer-sign -Wno-empty-body
>> -Wno-string-plus-int -Wno-unused-const-variable -Wno-tautological-compare
>> -Wno-unused-value -Wno-parentheses-equality -Wno-unused-function
>> -Wno-enum-conversion -Wno-unused-local-typedef -Qunused-arguments -c
>> /opt2/branches/head2/lib/libopenbsd/imsg.c -o imsg.o
>> /opt2/branches/head2/lib/libopenbsd/imsg.c:78:6: warning: comparison of
>> integers of different signs: 'unsigned long' and 'int' [-Wsign-compare]
>> >= getdtablesize()) {
>> ^  ~~~
>> 1 warning generated.

Out of curiousity, in your experience how often is that warning a real
issue versus a false positive?  In this case it's a false positive and
the code is safe.


>> I can certainly patch the code with a cast on FreeBSD to get rid of
>> the compiler warning.  However, I was wondering if there is a good fix
>> that we can share between OpenBSD and FreeBSD?
>
> Both OSs define getdtablesize() as sysconf(_SC_OPEN_MAX). sysconf(3)
> returns a long, so it seems more correct to make getdtablesize(3) return
> a long or ssize_t. The return value has to be signed because sysconf(3)
> is specified by POSIX to return -1 and set errno on failure.

No, fds are ints so returning a long from getdtablesize(3) would be
misleading and useless.

In the full expression:
if (getdtablecount() + imsg_fd_overhead +
(CMSG_SPACE(sizeof(int))-CMSG_SPACE(0))/sizeof(int)
>= getdtablesize()) {

the type of unsigned long just be from the CMSG_SPACE() term; if
keeping that warning option is felt to be a net positive, I would
suggest casting to int just the CMSG-CMSG/sizeof term.


Philip Guenther

Re: Reducing compilation warnings in imsg.c on FreeSBD

[Mark Kettenis]

> Date: Mon, 16 Nov 2015 11:41:09 -0500
> From: Michael McConville 
> 
> Craig Rodrigues wrote:
> > Recently, I imported imsg.c from OpenBSD to the
> > FreeBSD base system's libopenbsd:
> > 
> > https://svnweb.freebsd.org/changeset/base/290375
> > 
> > When compiling on FreeBSD, we get a compiler warning with clang:
> > 
> > cc  -O2 -pipe   -I/opt2/branches/head2/lib/libopenbsd -std=gnu99
> > -fstack-protector-strong -Wsystem-headers -Wall -Wno-format-y2k -W
> > -Wno-unused-parameter -Wstrict-prototypes -Wmissing-prototypes
> > -Wpointer-arith -Wno-uninitialized -Wno-pointer-sign -Wno-empty-body
> > -Wno-string-plus-int -Wno-unused-const-variable -Wno-tautological-compare
> > -Wno-unused-value -Wno-parentheses-equality -Wno-unused-function
> > -Wno-enum-conversion -Wno-unused-local-typedef -Qunused-arguments -c
> > /opt2/branches/head2/lib/libopenbsd/imsg.c -o imsg.o
> > /opt2/branches/head2/lib/libopenbsd/imsg.c:78:6: warning: comparison of
> > integers of different signs: 'unsigned long' and 'int' [-Wsign-compare]
> > >= getdtablesize()) {
> > ^  ~~~
> > 1 warning generated.
> > 
> > I can certainly patch the code with a cast on FreeBSD to get rid of
> > the compiler warning.  However, I was wondering if there is a good fix
> > that we can share between OpenBSD and FreeBSD?
> 
> Both OSs define getdtablesize() as sysconf(_SC_OPEN_MAX). sysconf(3)
> returns a long, so it seems more correct to make getdtablesize(3) return
> a long or ssize_t. The return value has to be signed because sysconf(3)
> is specified by POSIX to return -1 and set errno on failure.

Changing an existing API like that would be really bad.

The code in question looks weird; imsg_fd_overhead is initialized to
0, but never changed it seems.

Re: Reducing compilation warnings in imsg.c on FreeSBD

[Ted Unangst]

Craig Rodrigues wrote:
> I tried the following and was able to compile without warning:
> 
> Index: imsg.c
> ===
> --- imsg.c  (revision 290924)
> +++ imsg.c  (working copy)
> @@ -74,7 +74,7 @@
> 
>  again:
> if (getdtablecount() + imsg_fd_overhead +
> -   (CMSG_SPACE(sizeof(int))-CMSG_SPACE(0))/sizeof(int)
> +   (int)((CMSG_SPACE(sizeof(int))-CMSG_SPACE(0))/sizeof(int))
> >= getdtablesize()) {
> errno = EAGAIN;
> free(ifd);
> 
> The cast looks OK to me.  I can commit this to FreeBSD.
> Is something that OpenBSD would want to take?

I'm not a fan of the warning, but this seems reasonable. It's not especially
intrusive.

Re: Reducing compilation warnings in imsg.c on FreeSBD

[Craig Rodrigues]

On Mon, Nov 16, 2015 at 9:03 AM, Philip Guenther  wrote:

> >> integers of different signs: 'unsigned long' and 'int' [-Wsign-compare]
> >> >= getdtablesize()) {
> >> ^  ~~~
> >> 1 warning generated.
>
> Out of curiousity, in your experience how often is that warning a real
> issue versus a false positive?  In this case it's a false positive and
> the code is safe.
>
>
I don't encounter that warning very often, so I cannot say.
I agree with you that in this particular case it looks like a false
positive.


>
> In the full expression:
> if (getdtablecount() + imsg_fd_overhead +
> (CMSG_SPACE(sizeof(int))-CMSG_SPACE(0))/sizeof(int)
> >= getdtablesize()) {
>
> the type of unsigned long just be from the CMSG_SPACE() term; if
> keeping that warning option is felt to be a net positive, I would
> suggest casting to int just the CMSG-CMSG/sizeof term.



I tried the following and was able to compile without warning:

Index: imsg.c
===
--- imsg.c  (revision 290924)
+++ imsg.c  (working copy)
@@ -74,7 +74,7 @@

 again:
if (getdtablecount() + imsg_fd_overhead +
-   (CMSG_SPACE(sizeof(int))-CMSG_SPACE(0))/sizeof(int)
+   (int)((CMSG_SPACE(sizeof(int))-CMSG_SPACE(0))/sizeof(int))
>= getdtablesize()) {
errno = EAGAIN;
free(ifd);

The cast looks OK to me.  I can commit this to FreeBSD.
Is something that OpenBSD would want to take?
--
Craig

Signed indexing in bc(1)

[Michael McConville]

I may be missing something obvious here, but it seems that the below
indices should be unsigned. str_table has UCHAR_MAX elements, so it
expects to be indexed by chars > 127.

I'm currently digging through a bunch of segfaults found by American
Fuzzy Lop (afl). I don't think I've come across this in the results yet,
but it caught my eye.

Bounds checks may be necessary for the latter two hunks.

Thoughts?


Index: bc.y
===
RCS file: /cvs/src/usr.bin/bc/bc.y,v
retrieving revision 1.48
diff -u -p -r1.48 bc.y
--- bc.y10 Oct 2015 19:28:54 -  1.48
+++ bc.y16 Nov 2015 17:22:05 -
@@ -891,7 +891,7 @@ letter_node(char *str)
 
len = strlen(str);
if (len == 1 && str[0] != '_')
-   return cs(str_table[(int)str[0]]);
+   return cs(str_table[(u_char)str[0]]);
else
return lookup(str, len, 'L');
 }
@@ -903,7 +903,7 @@ array_node(char *str)
 
len = strlen(str);
if (len == 1 && str[0] != '_')
-   return cs(str_table[(int)str[0] - 'a' + ARRAY_CHAR]);
+   return cs(str_table[(u_char)str[0] - 'a' + ARRAY_CHAR]);
else
return lookup(str, len, 'A');
 }
@@ -915,7 +915,7 @@ function_node(char *str)
 
len = strlen(str);
if (len == 1 && str[0] != '_')
-   return cs(str_table[(int)str[0] - 'a' + FUNC_CHAR]);
+   return cs(str_table[(u_char)str[0] - 'a' + FUNC_CHAR]);
else
return lookup(str, len, 'F');
 }

Re: apmd(8) messages

[Michael McConville]

Jan Stary wrote:
> When we set perf policy to high, we say "high".
> So when we set to low, say "low", not "manual".
> (Both manual high and manual low are manual.)
> 
> And it's not "client reply", it's a reply to the client, right?

ok mmcc@

> Index: apmd.c
> ===
> RCS file: /cvs/src/usr.sbin/apmd/apmd.c,v
> retrieving revision 1.77
> diff -u -p -r1.77 apmd.c
> --- apmd.c11 Oct 2015 20:23:49 -  1.77
> +++ apmd.c16 Nov 2015 17:22:16 -
> @@ -277,7 +277,7 @@ handle_client(int sock_fd, int ctl_fd)
>   case SETPERF_LOW:
>   doperf = PERF_MANUAL;
>   reply.newstate = NORMAL;
> - syslog(LOG_NOTICE, "setting hw.perfpolicy to manual");
> + syslog(LOG_NOTICE, "setting hw.perfpolicy to low");
>   setperfpolicy("low");
>   break;
>   case SETPERF_HIGH:
> @@ -305,7 +305,7 @@ handle_client(int sock_fd, int ctl_fd)
>   reply.perfmode = doperf;
>   reply.vno = APMD_VNO;
>   if (send(cli_fd, , sizeof(reply), 0) != sizeof(reply))
> - syslog(LOG_INFO, "client reply botch");
> + syslog(LOG_INFO, "reply to client botched");
>   close(cli_fd);
>  
>   return reply.newstate;
> 

apmd(8) messages

[Jan Stary]

When we set perf policy to high, we say "high".
So when we set to low, say "low", not "manual".
(Both manual high and manual low are manual.)

And it's not "client reply", it's a reply to the client, right?

Jan


Index: apmd.c
===
RCS file: /cvs/src/usr.sbin/apmd/apmd.c,v
retrieving revision 1.77
diff -u -p -r1.77 apmd.c
--- apmd.c  11 Oct 2015 20:23:49 -  1.77
+++ apmd.c  16 Nov 2015 17:22:16 -
@@ -277,7 +277,7 @@ handle_client(int sock_fd, int ctl_fd)
case SETPERF_LOW:
doperf = PERF_MANUAL;
reply.newstate = NORMAL;
-   syslog(LOG_NOTICE, "setting hw.perfpolicy to manual");
+   syslog(LOG_NOTICE, "setting hw.perfpolicy to low");
setperfpolicy("low");
break;
case SETPERF_HIGH:
@@ -305,7 +305,7 @@ handle_client(int sock_fd, int ctl_fd)
reply.perfmode = doperf;
reply.vno = APMD_VNO;
if (send(cli_fd, , sizeof(reply), 0) != sizeof(reply))
-   syslog(LOG_INFO, "client reply botch");
+   syslog(LOG_INFO, "reply to client botched");
close(cli_fd);
 
return reply.newstate;

Re: Reducing compilation warnings in imsg.c on FreeSBD

[Theo de Raadt]

> > Both OSs define getdtablesize() as sysconf(_SC_OPEN_MAX). sysconf(3)
> > returns a long, so it seems more correct to make getdtablesize(3) return
> > a long or ssize_t. The return value has to be signed because sysconf(3)
> > is specified by POSIX to return -1 and set errno on failure.
> 
> No, fds are ints so returning a long from getdtablesize(3) would be
> misleading and useless.

I want to return to this point.

We cannot change the types of old fundamentals without incurring
many new problems.

  int dtablesize = getdtablesize();

Hello new compiler warning everytime.

Re: PC Engines APU2 coming soon - how is OpenBSD's support so far?

[Jason Barbier]

Looks like you need to email support and get the updated BIOS. I had the
same problem and there is an update to the bios to allow for mSATA boot
but not SD card boot yet.

-- 
Jason Barbier | E: jab...@serversave.us
GPG Key-ID: B5F75B47(http://kusuriya.devio.us/pubkey.asc)

On Sat, Nov 14, 2015, at 10:22 AM, Mathias Schmocker wrote:
> Le 14.11.15 17:02, Chris Cappuccio a écrit :
>  > Mathias Schmocker [s...@smat.ch] wrote:
>  >>
>  >> First tests with a bootable OpenBSD amd64 5.8-current USB stick and
>  >> installation on the 16GB mSata internal storage.
>  >> After reboot, BIOS could find mSata to boot on, but defaulted to the 
> memtest
>  >> boot payload
>  >>
>  >
>  > This is a setting you can change, although the default boot order 
> would put memtest last.
> 
> Not for me:
> 
> 1. USB MSC Drive SanDisk Ultra Fit 1.00
> 2. Payload [memtest]
> 3. Payload [setup]
> 
> I have now successfully installed and booted on a small 16GB SanDisk USB 
> key, root on sd1a (but I had a hard lockup once, with > 
> xhci_wait_for_command: Command ring still running ...)
> 
> But pressing F10 and 3 to access setup and placing mSata in front of all 
> the other does NOT work (even if sd0a has a bootable bsd.rd, /boot and 
> the proper /etc/boot.conf, and is accessible once OpenBSD runs out of 
> the USB key ...)
> 
> Which SeaBIOS, BUILD version, and option rom does your APU2 display when 
> booting up ?
> 
>  > I've had no problems with apu2 detecting msata so far..
>  >
>  >> Unable to abort memtest by pressing (esc) on the serial console, or 
> (c) for
>  >> configure.
>  >>
>  >
>  > You have to escape to the prompt before memtest starts.
>  >
>  >
> 

Re: [patch] gprof(1): fix incompatible pointer types

[Serguey Parkhomovsky]

Hi Philip,

Thanks for the detailed explanation on comparison functions for qsort. I
have looked through your changes, and have only found one issue:

> 2) totalcmp(A,B) and totalcmp(B,A) both return <0 if both A and B have 
>name==0 and cycleno!=0, and they both return >0 if both A and B have 
>naem==0 and cycleno==0, violating the consistency requirement.

The logic is still not quite right with totalcmp; it is still
inconsistent where both A and B have:
* name == 0 && cycleno == 0 (both return -1)
* name != 0 && cycleno == 0 (both return -1)
* name != 0 && cycleno != 0 (both return -1)

As well, if name == 0 && cycleno != 0, the code will drop through and
there will be a null pointer dereference:

if ( *(np1 -> name) != '_' && *(np2 -> name) == '_' )
return -1;
if ( *(np1 -> name) == '_' && *(np2 -> name) != '_' )
return 1;

What do you think of the following diff? I've put some of the boolean logic
into variables to enhance readability.

Index: arcs.c
===
RCS file: /cvs/src/usr.bin/gprof/arcs.c,v
retrieving revision 1.13
diff -u -p -u -r1.13 arcs.c
--- arcs.c  20 Aug 2015 22:32:41 -  1.13
+++ arcs.c  16 Nov 2015 17:41:55 -
@@ -95,9 +95,14 @@ addarc(nltype *parentp, nltype *childp, 
 nltype **topsortnlp;
 
 int
-topcmp(nltype **npp1, nltype **npp2)
+topcmp(const void *v1, const void *v2)
 {
-return (*npp1) -> toporder - (*npp2) -> toporder;
+const nltype * const *npp1 = v1;
+const nltype * const *npp2 = v2;
+
+if ((*npp1) -> toporder < (*npp2) -> toporder)
+   return -1;
+return (*npp1) -> toporder > (*npp2) -> toporder;
 }
 
 nltype **
Index: gprof.h
===
RCS file: /cvs/src/usr.bin/gprof/gprof.h,v
retrieving revision 1.14
diff -u -p -u -r1.14 gprof.h
--- gprof.h 19 Oct 2013 13:51:40 -  1.14
+++ gprof.h 16 Nov 2015 17:41:55 -
@@ -281,10 +281,10 @@ void  sortchildren(nltype *);
 void   sortmembers(nltype *);
 void   sortparents(nltype *);
 void   tally(struct rawarc *);
-inttimecmp(nltype **, nltype **);
+inttimecmp(const void *, const void *);
 void   timepropagate(nltype *);
-inttopcmp(nltype **, nltype **);
-inttotalcmp(nltype **, nltype **);
+inttopcmp(const void *, const void *);
+inttotalcmp(const void *, const void *);
 
 #defineLESSTHAN-1
 #defineEQUALTO 0
Index: printgprof.c
===
RCS file: /cvs/src/usr.bin/gprof/printgprof.c,v
retrieving revision 1.13
diff -u -p -u -r1.13 printgprof.c
--- printgprof.c20 Aug 2015 22:32:41 -  1.13
+++ printgprof.c16 Nov 2015 17:41:55 -
@@ -35,7 +35,7 @@
 #include "gprof.h"
 #include "pathnames.h"
 
-int namecmp(nltype **, nltype **);
+int namecmp(const void *, const void *);
 
 void
 printprof()
@@ -66,21 +66,19 @@ printprof()
 }
 
 int
-timecmp(nltype **npp1, nltype **npp2)
+timecmp(const void *v1, const void *v2)
 {
-double timediff;
-long   calldiff;
+const nltype * const *npp1 = v1;
+const nltype * const *npp2 = v2;
 
-timediff = (*npp2) -> time - (*npp1) -> time;
-if ( timediff > 0.0 )
+if ((*npp2) -> time < (*npp1) -> time)
+   return -1;
+if ((*npp2) -> time > (*npp1) -> time)
return 1 ;
-if ( timediff < 0.0 )
+if ((*npp2) -> ncall < (*npp1) -> ncall)
return -1;
-calldiff = (*npp2) -> ncall - (*npp1) -> ncall;
-if ( calldiff > 0 )
+if ((*npp2) -> ncall > (*npp1) -> ncall)
return 1;
-if ( calldiff < 0 )
-   return -1;
 return( strcmp( (*npp1) -> name , (*npp2) -> name ) );
 }
 
@@ -233,26 +231,37 @@ printgprof(nltype **timesortnlp)
  * all else being equal, sort by names.
  */
 int
-totalcmp(nltype **npp1, nltype **npp2)
+totalcmp(const void *v1, const void *v2)
 {
-nltype *np1 = *npp1;
-nltype *np2 = *npp2;
-double diff;
-
-diff =( np1 -> propself + np1 -> propchild )
-   - ( np2 -> propself + np2 -> propchild );
-if ( diff < 0.0 )
+const nltype *np1 = *(const nltype **)v1;
+const nltype *np2 = *(const nltype **)v2;
+double t1, t2;
+int np1noname, np2noname, np1cyclehdr, np2cyclehdr;
+
+t1 = np1 -> propself + np1 -> propchild;
+t2 = np2 -> propself + np2 -> propchild;
+if ( t2 > t1 )
return 1;
-if ( diff > 0.0 )
+if ( t2 < t1 )
return -1;
-if ( np1 -> name == 0 && np1 -> cycleno != 0 ) 
+
+np1noname = ( np1 -> name == 0 );
+np2noname = ( np2 -> name == 0 );
+np1cyclehdr = ( np1noname && np1 -> cycleno != 0 );
+np2cyclehdr = ( np2noname && np2 -> cycleno != 0 );
+
+if ( np1cyclehdr && !np2cyclehdr )
return -1;
-if ( np2 -> name == 0 && 

pledge mv(1)

[Ricardo Mestre]

Hello,

If only rename(2)'ing then it only needs "stdio rpath cpath",
nevertheless if we need to copy to a different partition it also needs
"wpath fattr" for writing and chmod/chown operations, and finally "proc
exec" are needed due to (extracted directly from mv(1)'s man page) ->
"Should the rename(2) call fail because the source and destination are
on different file systems, mv will use cp(1) and rm(1) to accomplish the
move.".

PS: It's not possible to reduce only to "stdio rpath cpath" since
rename(2) happens way before copy() or fastcopy() functions and if it
fails it will call copy() anyway which needs further permissions.

Index: mv.c
===
RCS file: /cvs/src/bin/mv/mv.c,v
retrieving revision 1.41
diff -u -p -u -r1.41 mv.c
--- mv.c6 Oct 2015 16:51:15 -   1.41
+++ mv.c16 Nov 2015 18:01:29 -
@@ -91,6 +91,9 @@ main(int argc, char *argv[])

stdin_ok = isatty(STDIN_FILENO);

+   if (pledge("stdio rpath wpath cpath fattr proc exec", NULL) == -1)
+   err(1, "pledge");
+
/*
 * If the stat on the target fails or the target isn't a directory,
 * try the move.  More than 2 arguments is an error in this case.
@@ -259,6 +262,9 @@ fastcopy(char *from, char *to, struct st
static char *bp;
int nread, from_fd, to_fd;
int badchown = 0, serrno = 0;
+
+   if (pledge("stdio rpath wpath cpath fattr", NULL) == -1)
+   err(1, "pledge");

if (!blen) {
blen = sbp->st_blksize;

Re: [patch] tail(1) follow multiple files

[Martijn van Duren]

Isn't anyone willing to take a stab at this patch?

On 11/09/15 12:56, Martijn van Duren wrote:

ping

On 11/04/15 23:29, Martijn van Duren wrote:

Hello tech@,

I got somewhat annoyed by the fact that OpenBSD's tail can't follow
multiple files and since the last attempt at it was from 2008 I thought
I'd give it a shot. I already sent an earlier version of this diff to
Landry who called it "a welcomed addition", but doesn't have time to
review my patch.

I've field tested this patch for a couple of days now and I didn't
experienced any problems. I also run it through
/usr/src/regress/usr.bin/tail/ both on UFS and NFS (v3), so it should
not suffer from the same issues as before.

The patch isn't based on any other follow implementation (at least not
by intent) and leans fully on kqueue for reading, deletion/renaming, and
truncation. When a file gets deleted I set a timeout to kevent(2) of 1
second and poll for the files individually, since the parent directory
might not always be available for kqueue monitoring.

Sincerely,

Martijn van Duren
Index: extern.h
===
--- extern.h	(revision 1)
+++ extern.h	(revision 34)
@@ -36,17 +36,23 @@
 	if (write(STDOUT_FILENO, p, size) != size) \
 		oerr();
 
+struct tailfile {
+	char		*fname;
+	FILE		*fp;
+	struct stat	 sb;
+};
+
 enum STYLE { NOTSET = 0, FBYTES, FLINES, RBYTES, RLINES, REVERSE };
 
-void forward(FILE *, enum STYLE, off_t, struct stat *);
-void reverse(FILE *, enum STYLE, off_t, struct stat *);
+void forward(struct tailfile *, int, enum STYLE, off_t);
+void reverse(struct tailfile *, int, enum STYLE, off_t);
 
-int bytes(FILE *, off_t);
-int lines(FILE *, off_t);
+int bytes(struct tailfile *, off_t);
+int lines(struct tailfile *, off_t);
 
-void ierr(void);
+void ierr(const char *);
 void oerr(void);
+void printfname(const char *);
 
 extern int fflag, rflag, rval;
-extern char *fname;
 extern int is_stdin;
Index: misc.c
===
--- misc.c	(revision 1)
+++ misc.c	(revision 34)
@@ -41,7 +41,7 @@
 #include "extern.h"
 
 void
-ierr(void)
+ierr(const char *fname)
 {
 	warn("%s", fname);
 	rval = 1;
@@ -52,3 +52,11 @@
 {
 	err(1, "stdout");
 }
+
+void printfname(const char *fname)
+{
+	static int first = 1;
+	(void)printf("%s==> %s <==\n", first ? "" : "\n", fname);
+	first = 0;
+	(void)fflush(stdout);
+}
Index: read.c
===
--- read.c	(revision 1)
+++ read.c	(revision 34)
@@ -59,7 +59,7 @@
  *
  */
 int
-bytes(FILE *fp, off_t off)
+bytes(struct tailfile *tf, off_t off)
 {
 	int ch;
 	size_t len, tlen;
@@ -73,7 +73,7 @@
 	if ((sp = p = malloc(off)) == NULL)
 		err(1, NULL);
 
-	for (wrap = 0, ep = p + off; (ch = getc(fp)) != EOF;) {
+	for (wrap = 0, ep = p + off; (ch = getc(tf->fp)) != EOF;) {
 		*p = ch;
 		if (++p == ep) {
 			wrap = 1;
@@ -80,8 +80,8 @@
 			p = sp;
 		}
 	}
-	if (ferror(fp)) {
-		ierr();
+	if (ferror(tf->fp)) {
+		ierr(tf->fname);
 		free(sp);
 		return(1);
 	}
@@ -135,7 +135,7 @@
  *
  */
 int
-lines(FILE *fp, off_t off)
+lines(struct tailfile *tf, off_t off)
 {
 	struct {
 		size_t blen;
@@ -156,7 +156,7 @@
 
 	blen = cnt = recno = wrap = 0;
 
-	while ((ch = getc(fp)) != EOF) {
+	while ((ch = getc(tf->fp)) != EOF) {
 		if (++cnt > blen) {
 			newsize = blen + 1024;
 			if ((newp = realloc(sp, newsize)) == NULL)
@@ -184,8 +184,8 @@
 			}
 		}
 	}
-	if (ferror(fp)) {
-		ierr();
+	if (ferror(tf->fp)) {
+		ierr(tf->fname);
 		rc = 1;
 		goto done;
 	}
Index: reverse.c
===
--- reverse.c	(revision 1)
+++ reverse.c	(revision 34)
@@ -43,12 +43,12 @@
 #include "extern.h"
 
 static void r_buf(FILE *);
-static int r_reg(FILE *, enum STYLE, off_t, struct stat *);
+static int r_reg(struct tailfile *, enum STYLE, off_t);
 
-#define COPYCHAR(fp, ch)\
+#define COPYCHAR(tf, ch)\
 	do {		\
-		if ((ch = getc(fp)) == EOF) {		\
-			ierr();\
+		if ((ch = getc(tf->fp)) == EOF) {	\
+			ierr(tf->fname);		\
 			return (0);			\
 		}	\
 		if (putchar(ch) == EOF) {		\
@@ -76,25 +76,35 @@
  *	NOREG	cyclically read input into a linked list of buffers
  */
 void
-reverse(FILE *fp, enum STYLE style, off_t off, struct stat *sbp)
+reverse(struct tailfile *tf, int nfiles, enum STYLE style, off_t off)
 {
+	int i;
+
 	if (style != REVERSE && off == 0)
 		return;
 
-	if (!S_ISREG(sbp->st_mode) || r_reg(fp, style, off, sbp) != 0)
-		switch(style) {
-		case FBYTES:
-		case RBYTES:
-			(void)bytes(fp, off);
-			break;
-		case FLINES:
-		case RLINES:
-			(void)lines(fp, off);
-			break;
-		case REVERSE:
-			r_buf(fp);
-			break;
+	for (i = 0; i < nfiles; i++) {
+		if (nfiles > 1)
+			printfname(tf[i].fname);
+		if (!S_ISREG(tf[i].sb.st_mode) ||
+		r_reg(&(tf[i]), style, off) != 0) {
+			switch(style) {
+			case FBYTES:
+			case RBYTES:
+(void)bytes(&(tf[i]), off);
+break;
+			case FLINES:
+			case 

Re: pledge mv(1)

[Theo de Raadt]

> If only rename(2)'ing then it only needs "stdio rpath cpath",
> nevertheless if we need to copy to a different partition it also needs
> "wpath fattr" for writing and chmod/chown operations, and finally "proc
> exec" are needed due to (extracted directly from mv(1)'s man page) ->
> "Should the rename(2) call fail because the source and destination are
> on different file systems, mv will use cp(1) and rm(1) to accomplish the
> move.".
> 
> PS: It's not possible to reduce only to "stdio rpath cpath" since
> rename(2) happens way before copy() or fastcopy() functions and if it
> fails it will call copy() anyway which needs further permissions.

We are going to skip doing mv, until other refactorings get done:

mv.c:   execl(_PATH_CP, "cp", "-PRp", "--", from, to, (char *)NULL);
mv.c:   execl(_PATH_RM, "rm", "-rf", "--", from, (char *)NULL);

It is quite horrible...

Re: Signed indexing in bc(1)

[Otto Moerbeek]

On Mon, Nov 16, 2015 at 12:45:08PM -0500, Michael McConville wrote:

> I may be missing something obvious here, but it seems that the below
> indices should be unsigned. str_table has UCHAR_MAX elements, so it
> expects to be indexed by chars > 127.
> 
> I'm currently digging through a bunch of segfaults found by American
> Fuzzy Lop (afl). I don't think I've come across this in the results yet,
> but it caught my eye.
> 
> Bounds checks may be necessary for the latter two hunks.
> 
> Thoughts?

Looking at the grammar and lexer, the functions letter_node(),
array_mode() and function_node() are called with the lexical value of
a LETTER symbol only, and these lexical values are restricted to
string of lower case alphabetic chars and numbers. So the actual
values alway fit into an int witjpou sign extension problems.

Still, you diff might make sense from a general type safety point of
view,

-Otto

> 
> 
> Index: bc.y
> ===
> RCS file: /cvs/src/usr.bin/bc/bc.y,v
> retrieving revision 1.48
> diff -u -p -r1.48 bc.y
> --- bc.y  10 Oct 2015 19:28:54 -  1.48
> +++ bc.y  16 Nov 2015 17:22:05 -
> @@ -891,7 +891,7 @@ letter_node(char *str)
>  
>   len = strlen(str);
>   if (len == 1 && str[0] != '_')
> - return cs(str_table[(int)str[0]]);
> + return cs(str_table[(u_char)str[0]]);
>   else
>   return lookup(str, len, 'L');
>  }
> @@ -903,7 +903,7 @@ array_node(char *str)
>  
>   len = strlen(str);
>   if (len == 1 && str[0] != '_')
> - return cs(str_table[(int)str[0] - 'a' + ARRAY_CHAR]);
> + return cs(str_table[(u_char)str[0] - 'a' + ARRAY_CHAR]);
>   else
>   return lookup(str, len, 'A');
>  }
> @@ -915,7 +915,7 @@ function_node(char *str)
>  
>   len = strlen(str);
>   if (len == 1 && str[0] != '_')
> - return cs(str_table[(int)str[0] - 'a' + FUNC_CHAR]);
> + return cs(str_table[(u_char)str[0] - 'a' + FUNC_CHAR]);
>   else
>   return lookup(str, len, 'F');
>  }

Re: [patch] tail(1) follow multiple files

[Ted Unangst]

Martijn van Duren wrote:
> Isn't anyone willing to take a stab at this patch?
> 
> On 11/09/15 12:56, Martijn van Duren wrote:
> > ping
> >
> > On 11/04/15 23:29, Martijn van Duren wrote:
> >> Hello tech@,
> >>
> >> I got somewhat annoyed by the fact that OpenBSD's tail can't follow
> >> multiple files and since the last attempt at it was from 2008 I thought
> >> I'd give it a shot. I already sent an earlier version of this diff to
> >> Landry who called it "a welcomed addition", but doesn't have time to
> >> review my patch.

hmm, i think "burned before" scares off many would be champions, but i think
it's worth a try as well.

Re: PC Engines APU2 coming soon - how is OpenBSD's support so far?

[Noth]

No it freezes after the "rebooting..." message appears. It isn't before 
the firmware restarts. Hopefully the next firmware release will some 
kind of fix for this.


On 16/11/15 16:00, Chris Cappuccio wrote:

Noth [nothingn...@citycable.ch] wrote:

Hi again,

   I think I've found a bug: if I have a console session open using minicom
or cu when rebooting, the machine hangs. This doesn't happen with either
CentOS Linux 7 or FreeBSD 10.2 / 11. I can reproduce the problem. Anyone
else have this issue ?


The boot loader output is emulated video output until it switches to com0.
Once it reads 'stty com0 115200; set tty com0' from /etc/boot.conf, it
switches from video output to serial output.

During the switch, some contents comes out of the boot loader that is at
the wrong baud rate (or something similar). This causes some high characters
to come out, which your terminal emulator might interpret as a terminal command
and it could even send a short reply back. If it does, this reply stops the
auto-boot process and you sit at boot> indefinitely.

The solution is to not leave a terminal emulator connected during the boot
(or fix the boot loader to not output high characters)

Re: PC Engines APU2 coming soon - how is OpenBSD's support so far?

[Chris Cappuccio]

Noth [nothingn...@citycable.ch] wrote:
> No it freezes after the "rebooting..." message appears. It isn't before the
> firmware restarts. Hopefully the next firmware release will some kind of fix
> for this.
> 

The non-ACPI kernel does this (bsd.rd). bsd should not do this

Re: give cron a sensible default max load_avg for batch jobs

[Craig Skinner]

On 2015-11-14 Sat 05:57 AM |, Todd C. Miller wrote:
> The quesion no one seems to be asking here is "who actually runs
> batch".  Anyone?
> 

I do, on small servers with an average uptime(1) load of ~0.2

Re: pledge for tetris

[Theo Buehler]

In its current form, tetris is a setgid program and needs a whopping

pledge("stdio rpath wpath cpath flock getpw id tty")

throughout its lifetime because of the score file in /var/games.

As discussed with Theo off-list, this is risk-only.  Thus, drop the
score file support, lose the setgid bit and make tetris a much more
reasonable pledge("stdio rpath tty") program for relaxed game play.

Index: games/tetris/Makefile
===
RCS file: /cvs/src/games/tetris/Makefile,v
retrieving revision 1.7
diff -u -p -r1.7 Makefile
--- games/tetris/Makefile   31 May 2002 03:46:35 -  1.7
+++ games/tetris/Makefile   17 Nov 2015 04:50:55 -
@@ -1,18 +1,9 @@
 #  $OpenBSD: Makefile,v 1.7 2002/05/31 03:46:35 pjanzen Exp $
 
 PROG=  tetris
-SRCS=  input.c screen.c shapes.c scores.c tetris.c
+SRCS=  input.c screen.c shapes.c tetris.c
 MAN=   tetris.6
 DPADD= ${LIBCURSES}
 LDADD= -lcurses
-BINMODE=2555
-
-beforeinstall:
-   @if [ ! -f ${DESTDIR}/var/games/tetris.scores ]; then \
-   ${INSTALL} ${INSTALL_COPY} -o ${BINOWN} -g ${BINGRP} -m 664 \
-   /dev/null ${DESTDIR}/var/games/tetris.scores ; \
-   else \
-   true ; \
-   fi
 
 .include 
Index: games/tetris/pathnames.h
===
RCS file: games/tetris/pathnames.h
diff -N games/tetris/pathnames.h
--- games/tetris/pathnames.h3 Jun 2003 03:01:41 -   1.3
+++ /dev/null   1 Jan 1970 00:00:00 -
@@ -1,38 +0,0 @@
-/* $OpenBSD: pathnames.h,v 1.3 2003/06/03 03:01:41 millert Exp $   */
-/* $NetBSD: pathnames.h,v 1.2 1995/04/22 07:42:37 cgd Exp $*/
-
-/*-
- * Copyright (c) 1992, 1993
- * The Regents of the University of California.  All rights reserved.
- *
- * This code is derived from software contributed to Berkeley by
- * Chris Torek and Darren F. Provine.
- *
- * Redistribution and use in source and binary forms, with or without
- * modification, are permitted provided that the following conditions
- * are met:
- * 1. Redistributions of source code must retain the above copyright
- *notice, this list of conditions and the following disclaimer.
- * 2. Redistributions in binary form must reproduce the above copyright
- *notice, this list of conditions and the following disclaimer in the
- *documentation and/or other materials provided with the distribution.
- * 3. Neither the name of the University nor the names of its contributors
- *may be used to endorse or promote products derived from this software
- *without specific prior written permission.
- *
- * THIS SOFTWARE IS PROVIDED BY THE REGENTS AND CONTRIBUTORS ``AS IS'' AND
- * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
- * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
- * ARE DISCLAIMED.  IN NO EVENT SHALL THE REGENTS OR CONTRIBUTORS BE LIABLE
- * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
- * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS
- * OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
- * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
- * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY
- * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
- * SUCH DAMAGE.
- *
- * @(#)pathnames.h 8.1 (Berkeley) 5/31/93
- */
-
-#define _PATH_SCOREFILE"/var/games/tetris.scores"
Index: games/tetris/scores.c
===
RCS file: games/tetris/scores.c
diff -N games/tetris/scores.c
--- games/tetris/scores.c   16 Nov 2014 04:49:49 -  1.12
+++ /dev/null   1 Jan 1970 00:00:00 -
@@ -1,444 +0,0 @@
-/* $OpenBSD: scores.c,v 1.12 2014/11/16 04:49:49 guenther Exp $*/
-/* $NetBSD: scores.c,v 1.2 1995/04/22 07:42:38 cgd Exp $   */
-
-/*-
- * Copyright (c) 1992, 1993
- * The Regents of the University of California.  All rights reserved.
- *
- * This code is derived from software contributed to Berkeley by
- * Chris Torek and Darren F. Provine.
- *
- * Redistribution and use in source and binary forms, with or without
- * modification, are permitted provided that the following conditions
- * are met:
- * 1. Redistributions of source code must retain the above copyright
- *notice, this list of conditions and the following disclaimer.
- * 2. Redistributions in binary form must reproduce the above copyright
- *notice, this list of conditions and the following disclaimer in the
- *documentation and/or other materials provided with the distribution.
- * 3. Neither the name of the University nor the names of its contributors
- *may be used to endorse or promote products derived from this software
- *without specific prior written permission.
- *
- * THIS SOFTWARE IS PROVIDED BY THE REGENTS AND CONTRIBUTORS 

Re: newsyslog -r

[Jan Stary]

ping

On Nov 12 22:21:39, h...@stare.cz wrote:
> The -r option of newsyslog(8) removes the requirement
> that newsyslog runs as root. Would it also make sense
> to not try to send the SIGHUP to syslogd in that case?
> 
>   Jan
> 
> 
> Index: newsyslog.8
> ===
> RCS file: /cvs/src/usr.bin/newsyslog/newsyslog.8,v
> retrieving revision 1.52
> diff -u -p -u -p -r1.52 newsyslog.8
> --- newsyslog.8   16 Sep 2014 16:27:23 -  1.52
> +++ newsyslog.8   12 Nov 2015 21:20:52 -
> @@ -123,7 +123,7 @@ Removes the restriction that
>  must be running as root.
>  Note that in this mode
>  .Nm
> -will not be able to send a
> +will not try to send a
>  .Dv SIGHUP
>  signal to
>  .Xr syslogd 8 .
> Index: newsyslog.c
> ===
> RCS file: /cvs/src/usr.bin/newsyslog/newsyslog.c,v
> retrieving revision 1.95
> diff -u -p -u -p -r1.95 newsyslog.c
> --- newsyslog.c   20 Aug 2015 22:32:41 -  1.95
> +++ newsyslog.c   12 Nov 2015 21:20:52 -
> @@ -406,7 +406,7 @@ send_signal(char *pidfile, int signal)
>   warnx("%s pid file: %s", err, pidfile);
>   else if (noaction)
>   (void)printf("kill -%s %ld\n", sys_signame[signal], (long)pid);
> - else if (kill(pid, signal))
> + else if (needroot && kill(pid, signal))
>   warnx("warning - could not send SIG%s to PID from pid file %s",
>   sys_signame[signal], pidfile);
>  }

Re: pledge for tetris

[Ted Unangst]

Theo Buehler wrote:
> In its current form, tetris is a setgid program and needs a whopping
> 
> pledge("stdio rpath wpath cpath flock getpw id tty")
> 
> throughout its lifetime because of the score file in /var/games.
> 
> As discussed with Theo off-list, this is risk-only.  Thus, drop the
> score file support, lose the setgid bit and make tetris a much more
> reasonable pledge("stdio rpath tty") program for relaxed game play.

No way! this is critical functionality. :)

Can you just change it to save the scores in HOME/.tetrisscores?

Re: pledge for tetris

[Michael McConville]

Ted Unangst wrote:
> Theo Buehler wrote:
> > drop the score file support
> 
> No way! this is critical functionality. :)

Seconded.  :P

Re: pledge for tetris

[Theo Buehler]

On Tue, Nov 17, 2015 at 12:15:59AM -0500, Ted Unangst wrote:
> Theo Buehler wrote:
> > In its current form, tetris is a setgid program and needs a whopping
> > 
> > pledge("stdio rpath wpath cpath flock getpw id tty")
> > 
> > throughout its lifetime because of the score file in /var/games.
> > 
> > As discussed with Theo off-list, this is risk-only.  Thus, drop the
> > score file support, lose the setgid bit and make tetris a much more
> > reasonable pledge("stdio rpath tty") program for relaxed game play.
> 
> No way! this is critical functionality. :)
> 
> Can you just change it to save the scores in HOME/.tetrisscores?

I thought about that, but I'm not a fan.  This would still mean
"stdio rpath wpath cpath flock tty" and "getpw" if we want to keep the
current file format.

Re: pledge for tetris

[Ted Unangst]

Ted Unangst wrote:
> Theo Buehler wrote:
> > In its current form, tetris is a setgid program and needs a whopping
> > 
> > pledge("stdio rpath wpath cpath flock getpw id tty")
> > 
> > throughout its lifetime because of the score file in /var/games.
> > 
> > As discussed with Theo off-list, this is risk-only.  Thus, drop the
> > score file support, lose the setgid bit and make tetris a much more
> > reasonable pledge("stdio rpath tty") program for relaxed game play.
> 
> No way! this is critical functionality. :)
> 
> Can you just change it to save the scores in HOME/.tetrisscores?

The score code still makes a weak attempt to save unique users, which i may
relax, but this should let you get by with just basic stdio stuff.

Index: Makefile
===
RCS file: /cvs/src/games/tetris/Makefile,v
retrieving revision 1.7
diff -u -p -r1.7 Makefile
--- Makefile31 May 2002 03:46:35 -  1.7
+++ Makefile17 Nov 2015 05:35:40 -
@@ -5,14 +5,5 @@ SRCS=  input.c screen.c shapes.c scores.c
 MAN=   tetris.6
 DPADD= ${LIBCURSES}
 LDADD= -lcurses
-BINMODE=2555
-
-beforeinstall:
-   @if [ ! -f ${DESTDIR}/var/games/tetris.scores ]; then \
-   ${INSTALL} ${INSTALL_COPY} -o ${BINOWN} -g ${BINGRP} -m 664 \
-   /dev/null ${DESTDIR}/var/games/tetris.scores ; \
-   else \
-   true ; \
-   fi
 
 .include 
Index: pathnames.h
===
RCS file: pathnames.h
diff -N pathnames.h
--- pathnames.h 3 Jun 2003 03:01:41 -   1.3
+++ /dev/null   1 Jan 1970 00:00:00 -
@@ -1,38 +0,0 @@
-/* $OpenBSD: pathnames.h,v 1.3 2003/06/03 03:01:41 millert Exp $   */
-/* $NetBSD: pathnames.h,v 1.2 1995/04/22 07:42:37 cgd Exp $*/
-
-/*-
- * Copyright (c) 1992, 1993
- * The Regents of the University of California.  All rights reserved.
- *
- * This code is derived from software contributed to Berkeley by
- * Chris Torek and Darren F. Provine.
- *
- * Redistribution and use in source and binary forms, with or without
- * modification, are permitted provided that the following conditions
- * are met:
- * 1. Redistributions of source code must retain the above copyright
- *notice, this list of conditions and the following disclaimer.
- * 2. Redistributions in binary form must reproduce the above copyright
- *notice, this list of conditions and the following disclaimer in the
- *documentation and/or other materials provided with the distribution.
- * 3. Neither the name of the University nor the names of its contributors
- *may be used to endorse or promote products derived from this software
- *without specific prior written permission.
- *
- * THIS SOFTWARE IS PROVIDED BY THE REGENTS AND CONTRIBUTORS ``AS IS'' AND
- * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
- * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
- * ARE DISCLAIMED.  IN NO EVENT SHALL THE REGENTS OR CONTRIBUTORS BE LIABLE
- * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
- * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS
- * OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
- * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
- * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY
- * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
- * SUCH DAMAGE.
- *
- * @(#)pathnames.h 8.1 (Berkeley) 5/31/93
- */
-
-#define _PATH_SCOREFILE"/var/games/tetris.scores"
Index: scores.c
===
RCS file: /cvs/src/games/tetris/scores.c,v
retrieving revision 1.12
diff -u -p -r1.12 scores.c
--- scores.c16 Nov 2014 04:49:49 -  1.12
+++ scores.c17 Nov 2015 05:43:50 -
@@ -49,7 +49,6 @@
 #include 
 #include 
 #include 
-#include 
 #include 
 #include 
 #include 
@@ -57,7 +56,6 @@
 #include 
 #include 
 
-#include "pathnames.h"
 #include "screen.h"
 #include "scores.h"
 #include "tetris.h"
@@ -98,50 +96,44 @@ getscores(FILE **fpp)
 {
int sd, mint, lck, mask, i;
char *mstr, *human;
+   char scorepath[PATH_MAX];
FILE *sf;
 
if (fpp != NULL) {
-   mint = O_RDWR | O_CREAT;
+   mint = O_RDWR | O_CREAT | O_EXLOCK;
mstr = "r+";
human = "read/write";
-   lck = LOCK_EX;
+   *fpp = NULL;
} else {
-   mint = O_RDONLY;
+   mint = O_RDONLY | O_EXLOCK;
mstr = "r";
human = "reading";
-   lck = LOCK_SH;
}
-   setegid(egid);
+   if (!getenv("HOME"))
+   return;
mask = umask(S_IWOTH);
-   sd = open(_PATH_SCOREFILE, mint, 0666);
+   snprintf(scorepath, sizeof(scorepath), "%s/%s", getenv("HOME"), 

Reducing compilation warnings in imsg.c on FreeSBD

[Craig Rodrigues]

Hi,

Recently, I imported imsg.c from OpenBSD to the
FreeBSD base system's libopenbsd:

https://svnweb.freebsd.org/changeset/base/290375

When compiling on FreeBSD, we get a compiler warning with clang:

cc  -O2 -pipe   -I/opt2/branches/head2/lib/libopenbsd -std=gnu99
-fstack-protector-strong -Wsystem-headers -Wall -Wno-format-y2k -W
-Wno-unused-parameter -Wstrict-prototypes -Wmissing-prototypes
-Wpointer-arith -Wno-uninitialized -Wno-pointer-sign -Wno-empty-body
-Wno-string-plus-int -Wno-unused-const-variable -Wno-tautological-compare
-Wno-unused-value -Wno-parentheses-equality -Wno-unused-function
-Wno-enum-conversion -Wno-unused-local-typedef -Qunused-arguments -c
/opt2/branches/head2/lib/libopenbsd/imsg.c -o imsg.o
/opt2/branches/head2/lib/libopenbsd/imsg.c:78:6: warning: comparison of
integers of different signs: 'unsigned long' and 'int' [-Wsign-compare]
>= getdtablesize()) {
^  ~~~
1 warning generated.

I can certainly patch the code with a cast on FreeBSD to get rid of the
compiler
warning.  However, I was wondering if there is a good fix that we can share
between
OpenBSD and FreeBSD?

Thanks.
--
Craig

Re: pledge route(8) with '-n' flag

[Martin Pieuchot]

On 16/11/15(Mon) 10:03, Ricardo Mestre wrote:
> Hello!
> 
> Like Benoit said, monitor still needs dns all the time, but since pledge
> was being called there again with dns pledge then I thought it wouldn't
> abort. Taking that into consideration and looking at it a little bit
> more, how about this?

IMHO this doesn't make sense if I explicitly say "route -n monitor"  I'd
prefer not to have any address resolved

Otherwise I just say "route monitor".

I'd add that on the list of bugs discovered by pledge(2).

Re: pledge route(8) with '-n' flag

[Ricardo Mestre]

Hello!

Like Benoit said, monitor still needs dns all the time, but since pledge
was being called there again with dns pledge then I thought it wouldn't
abort. Taking that into consideration and looking at it a little bit
more, how about this?

I removed the first pledge in main() between the 2 switch cases and
reunified them, then considering the four codepaths below I applied the
pledges directly onto each one:

show -> "stdio" when -n, "stdio rpath dns" otherwise
flushroutes -> "stdio" when -n, "stdio rpath dns" otherwise
newroute (add/change/delete) -> always needs "stdio rpath dns"
monitor -> always needs "stdio rpath dns" (this pledge was already there
and I left it untouched)

I tried a few tests manually, and also the regress battery tests for
route and apart from a couple of "Network unreachable" none had aborted
due to incorrect pledging.

PS: I replied this directly only to Theo, but undoubtedly it needs to be
reviewed by a larger audience.

Index: route.c
===
RCS file: /cvs/src/sbin/route/route.c,v
retrieving revision 1.179
diff -u -p -u -r1.179 route.c
--- route.c 25 Oct 2015 09:37:08 -  1.179
+++ route.c 15 Nov 2015 18:06:02 -
@@ -224,17 +224,6 @@ main(int argc, char **argv)
case K_FLUSH:
exit(flushroutes(argc, argv));
break;
-   }
-   
-   if (nflag) {
-   if (pledge("stdio rpath dns", NULL) == -1)
-   err(1, "pledge");
-   } else {
-   if (pledge("stdio rpath dns", NULL) == -1)
-   err(1, "pledge");
-   }
-
-   switch (kw) {
case K_GET:
uid = 0;
/* FALLTHROUGH */
@@ -330,7 +319,7 @@ flushroutes(int argc, char **argv)
}

if (nflag) {
-   if (pledge("stdio rpath dns", NULL) == -1)
+   if (pledge("stdio", NULL) == -1)
err(1, "pledge");
} else {
if (pledge("stdio rpath dns", NULL) == -1)
@@ -445,6 +434,9 @@ newroute(int argc, char **argv)
int key;
uint8_t prio = 0;
struct hostent *hp = NULL;
+
+   if (pledge("stdio rpath dns", NULL) == -1)
+   err(1, "pledge");

if (uid)
errx(1, "must be root to alter routing table");
Index: show.c
===
RCS file: /cvs/src/sbin/route/show.c,v
retrieving revision 1.102
diff -u -p -u -r1.102 show.c
--- show.c  23 Oct 2015 15:03:25 -  1.102
+++ show.c  15 Nov 2015 18:06:09 -
@@ -146,7 +146,7 @@ p_rttables(int af, u_int tableid, int ha
}

if (nflag) {
-   if (pledge("stdio rpath dns", NULL) == -1)
+   if (pledge("stdio", NULL) == -1)
err(1, "pledge");
} else {
if (pledge("stdio rpath dns", NULL) == -1)

On 13/11/2015 19:08, Sebastian Benoit wrote:
> Ricardo Mestre(ser...@helheim.mooo.com) on 2015.11.13 18:00:11 +:
>> Hello,
>>
>> If '-n' argument is used on route(8) then nflag will be active and dns
>> transactions won't be needed, am I correct?
> 
> please find out yourself.
> 
> at least the pledge call in monitor will fail with -n and your diff,
> so some more work is required.
> 
>> Index: route.c
>> ===
>> RCS file: /cvs/src/sbin/route/route.c,v
>> retrieving revision 1.179
>> diff -u -p -u -r1.179 route.c
>> --- route.c  25 Oct 2015 09:37:08 -  1.179
>> +++ route.c  13 Nov 2015 15:37:37 -
>> @@ -227,7 +227,7 @@ main(int argc, char **argv)
>>  }
>>  
>>  if (nflag) {
>> -if (pledge("stdio rpath dns", NULL) == -1)
>> +if (pledge("stdio rpath", NULL) == -1)
>>  err(1, "pledge");
>>  } else {
>>  if (pledge("stdio rpath dns", NULL) == -1)
>> @@ -330,7 +330,7 @@ flushroutes(int argc, char **argv)
>>  }
>>  
>>  if (nflag) {
>> -if (pledge("stdio rpath dns", NULL) == -1)
>> +if (pledge("stdio rpath", NULL) == -1)
>>  err(1, "pledge");
>>  } else {
>>  if (pledge("stdio rpath dns", NULL) == -1)
>> Index: show.c
>> ===
>> RCS file: /cvs/src/sbin/route/show.c,v
>> retrieving revision 1.102
>> diff -u -p -u -r1.102 show.c
>> --- show.c   23 Oct 2015 15:03:25 -  1.102
>> +++ show.c   13 Nov 2015 15:37:37 -
>> @@ -146,7 +146,7 @@ p_rttables(int af, u_int tableid, int ha
>>  }
>>  
>>  if (nflag) {
>> -if (pledge("stdio rpath dns", NULL) == -1)
>> +if (pledge("stdio rpath", NULL) == -1)
>>  err(1, "pledge");
>>  } else {
>>  if (pledge("stdio rpath dns", NULL) == -1)
>>
>> Best regards,
>> Ricardo Mestre
>>
> 

Re: httpd URL rewrite support patch

[Stanislaw Adaszewski]

Please take a look at the modified patch. I've introduced the
"pass rewrite" syntax you proposed, added support for recursive
rewriting (disabled by default), fixed freeing of http_path and
created separate rewrite_uri field not to interfere with the
return_uri logic.

Best,

S.

On Sun, Nov 15, 2015 at 07:46:35PM +0100, Reyk Floeter wrote:
> On Sun, Nov 15, 2015 at 06:41:49PM +0100, Stanislaw Adaszewski wrote:
> > Sorry again, I'm trying now with with patch as an attachment -
> > when I did it the first time I think it wasn't accepted by the
> > list.
> > 
> > On Fri, Nov 13, 2015 at 05:22:22PM +0100, Bret Lambert wrote:
> > > tech@ accepts attachments; don't make this harder for us than
> > > it needs to be, please.
> > > 
> > > On Fri, Nov 13, 2015 at 05:06:47PM +0100, Stanislaw Adaszewski wrote:
> > > > Excuse me, the indentation was removed because message wasn't plain 
> > > > text.
> > > > 
> > > > Patch: http://pastebin.com/XnZLEPEk
> > > > 
> > > > httpd.conf: http://pastebin.com/gGp3NqCD
> > > > 
> > > > rewr.php: http://pastebin.com/prA1NJXC
> > > > 
> 
> OK, thanks, now I got the diff.
> 
> It is an interesting and simple approach, but neither florian@ nor me
> had the time to look at it yet.  See comments below.
> 
> > Index: parse.y
> > ===
> > RCS file: /cvs/src/usr.sbin/httpd/parse.y,v
> > retrieving revision 1.73
> > diff -u -p -u -r1.73 parse.y
> > --- parse.y 19 Jul 2015 05:17:27 -  1.73
> > +++ parse.y 13 Nov 2015 08:29:10 -
> > @@ -907,7 +907,7 @@ filter  : block RETURN NUMBER optstring 
> >  
> > if ($4 != NULL) {
> > /* Only for 3xx redirection headers */
> > -   if ($3 < 300 || $3 > 399) {
> > +   if ($3 != 200 && ($3 < 300 || $3 > 399)) {
> 
> I'd add it as "pass rewrite" instead.  It is not blocking the connection.
> 
> > yyerror("invalid return code for "
> > "location URI");
> > free($4);
> > Index: server_http.c
> > ===
> > RCS file: /cvs/src/usr.sbin/httpd/server_http.c,v
> > retrieving revision 1.96
> > diff -u -p -u -r1.96 server_http.c
> > --- server_http.c   31 Jul 2015 00:10:51 -  1.96
> > +++ server_http.c   13 Nov 2015 08:29:10 -
> > @@ -1054,6 +1054,7 @@ server_response(struct httpd *httpd, str
> > int  portval = -1, ret;
> > char*hostval;
> > const char  *errstr = NULL;
> > +   charbuf[IBUF_READ_SIZE];
> >  
> > /* Canonicalize the request path */
> > if (desc->http_path == NULL ||
> > @@ -1154,11 +1155,44 @@ server_response(struct httpd *httpd, str
> > resp->http_method = desc->http_method;
> > if ((resp->http_version = strdup(desc->http_version)) == NULL)
> > goto fail;
> > +   
> > +#ifdef DEBUG
> > +   DPRINTF("%s: http_path: %s, http_path_alias: %s\n", __func__, 
> > desc->http_path, desc->http_path_alias);
> 
> No need to put DPRINTF in DEBUG - it is already only compiled with DEBUG.
> 
> > +#endif
> >  
> > /* Now search for the location */
> > srv_conf = server_getlocation(clt, desc->http_path);
> >  
> > -   if (srv_conf->flags & SRVFLAG_BLOCK) {
> > +
> > +   if ((srv_conf->flags & SRVFLAG_BLOCK) && srv_conf->return_code == 200) {
> > +#ifdef DEBUG
> > +   DPRINTF("%s: Rewrite from: %s to %s\n", __func__, 
> > desc->http_path, srv_conf->return_uri);
> 
> Same here.
> 
> Hint: when sending patches, developers appreciate it when they already
> follow style(9).  This makes it easier for us to read.  So wrap the
> lines at 80 chars, don't use '//' comments as below, ...
> 
> > +#endif
> > +   if (desc->http_path != NULL) {
> > +   free(desc->http_path);
> > +   desc->http_path = NULL;
> 
> Oh, this will break some of the macros in server_expand_http()!
> 
> > +   }
> > +   if (server_expand_http(clt, srv_conf->return_uri, buf, 
> > sizeof(buf)) == NULL) {
> > +   server_abort_http(clt, 500, strerror(errno));
> > +   return (-1);
> > +   }
> > +   desc->http_path = strdup(buf);
> > +   if (desc->http_path == NULL) {
> > +   server_abort_http(clt, 500, strerror(errno));
> > +   }
> > +   desc->http_query = strchr(desc->http_path, '?');
> > +   if (desc->http_query != NULL) {
> > +   *desc->http_query++ = '\0';
> > +   desc->http_query = strdup(desc->http_query);
> > +   if (desc->http_query == NULL) {
> > +   server_abort_http(clt, 500, strerror(errno));
> > +   }
> > +   }
> > +   srv_conf = server_getlocation(clt, 

Re: if_isconnected()

[Martin Pieuchot]

On 12/11/15(Thu) 11:41, Martin Pieuchot wrote:
> This is basically a rewrite of in6_ifpprefix() in a more generic
> fashion.  The idea is to get rid of rt_ifp.  I'm also introducing
> if_isconnected() because I want to use it in ARP.

Updated diff to make use of the function in ARP, any ok?

Index: net/if.c
===
RCS file: /cvs/src/sys/net/if.c,v
retrieving revision 1.406
diff -u -p -r1.406 if.c
--- net/if.c13 Nov 2015 10:18:04 -  1.406
+++ net/if.c16 Nov 2015 11:16:18 -
@@ -945,6 +945,36 @@ if_detach(struct ifnet *ifp)
 }
 
 /*
+ * Returns true if ``ifp0'' is connected to the interface with index ``ifidx''.
+ */
+int
+if_isconnected(const struct ifnet *ifp0, unsigned int ifidx)
+{
+   struct ifnet *ifp;
+   int connected = 0;
+
+   ifp = if_get(ifidx);
+   if (ifp == NULL)
+   return (0);
+
+   if (ifp0->if_index == ifp->if_index)
+   connected = 1;
+
+#if NBRIDGE > 0
+   if (SAME_BRIDGE(ifp0->if_bridgeport, ifp->if_bridgeport))
+   connected = 1;
+#endif
+#if NCARP > 0
+   if ((ifp0->if_type == IFT_CARP && ifp0->if_carpdev == ifp) ||
+   (ifp->if_type == IFT_CARP && ifp->if_carpdev == ifp0))
+   connected = 1;
+#endif
+
+   if_put(ifp);
+   return (connected);
+}
+
+/*
  * Create a clone network interface.
  */
 int
Index: net/if_var.h
===
RCS file: /cvs/src/sys/net/if_var.h,v
retrieving revision 1.52
diff -u -p -r1.52 if_var.h
--- net/if_var.h11 Nov 2015 10:23:23 -  1.52
+++ net/if_var.h16 Nov 2015 11:15:01 -
@@ -426,6 +426,8 @@ struct  ifaddr *ifa_ifwithnet(struct sock
 struct ifaddr *ifaof_ifpforaddr(struct sockaddr *, struct ifnet *);
 void   ifafree(struct ifaddr *);
 
+intif_isconnected(const struct ifnet *, unsigned int);
+
 void   if_clone_attach(struct if_clone *);
 void   if_clone_detach(struct if_clone *);
 
Index: netinet/if_ether.c
===
RCS file: /cvs/src/sys/netinet/if_ether.c,v
retrieving revision 1.186
diff -u -p -r1.186 if_ether.c
--- netinet/if_ether.c  13 Nov 2015 10:18:04 -  1.186
+++ netinet/if_ether.c  16 Nov 2015 11:16:19 -
@@ -611,18 +611,7 @@ in_arpinput(struct mbuf *m)
}
changed = 1;
}
-   } else if (rt->rt_ifp != ifp &&
-#if NBRIDGE > 0
-   !SAME_BRIDGE(ifp->if_bridgeport,
-   rt->rt_ifp->if_bridgeport) &&
-#endif
-#if NCARP > 0
-   !(rt->rt_ifp->if_type == IFT_CARP &&
-   rt->rt_ifp->if_carpdev == ifp) &&
-   !(ifp->if_type == IFT_CARP &&
-   ifp->if_carpdev == rt->rt_ifp) &&
-#endif
-   1) {
+   } else if (!if_isconnected(ifp, rt->rt_ifidx)) {
inet_ntop(AF_INET, , addr, sizeof(addr));
log(LOG_WARNING,
"arp: attempt to add entry for %s "
Index: netinet6/in6.c
===
RCS file: /cvs/src/sys/netinet6/in6.c,v
retrieving revision 1.178
diff -u -p -r1.178 in6.c
--- netinet6/in6.c  2 Nov 2015 15:05:23 -   1.178
+++ netinet6/in6.c  16 Nov 2015 11:15:01 -
@@ -61,7 +61,6 @@
  * @(#)in.c8.2 (Berkeley) 11/15/93
  */
 
-#include "bridge.h"
 #include "carp.h"
 
 #include 
@@ -84,9 +83,6 @@
 
 #include 
 #include 
-#if NBRIDGE > 0
-#include 
-#endif
 
 #include 
 #include 
@@ -1448,44 +1444,6 @@ in6ifa_ifpwithaddr(struct ifnet *ifp, st
}
 
return (ifatoia6(ifa));
-}
-
-/*
- * Check whether an interface has a prefix by looking up the cloning route.
- */
-int
-in6_ifpprefix(const struct ifnet *ifp, const struct in6_addr *addr)
-{
-   struct sockaddr_in6 dst;
-   struct rtentry *rt;
-   u_int tableid = ifp->if_rdomain;
-
-   bzero(, sizeof(dst));
-   dst.sin6_len = sizeof(struct sockaddr_in6);
-   dst.sin6_family = AF_INET6;
-   dst.sin6_addr = *addr;
-   rt = rtalloc(sin6tosa(), 0, tableid);
-
-   if (rt == NULL)
-   return (0);
-   if ((rt->rt_flags & (RTF_CLONING | RTF_CLONED)) == 0 ||
-   (rt->rt_ifp != ifp &&
-#if NBRIDGE > 0
-   !SAME_BRIDGE(rt->rt_ifp->if_bridgeport, ifp->if_bridgeport) &&
-#endif
-#if NCARP > 0
-   (ifp->if_type != IFT_CARP || rt->rt_ifp != ifp->if_carpdev) &&
-   (rt->rt_ifp->if_type != IFT_CARP || rt->rt_ifp->if_carpdev != ifp)&&
-   (ifp->if_type != IFT_CARP || rt->rt_ifp->if_type != IFT_CARP ||
-   rt->rt_ifp->if_carpdev != ifp->if_carpdev) &&
-#endif
-   1)) {
-   rtfree(rt);
-   return (0);
-   }
-
-   rtfree(rt);
-   return (1);
 }
 
 /*
Index: netinet6/in6_var.h

Re: Fewer rt_ifp in sys/net

[Martin Pieuchot]

On 11/11/15(Wed) 13:00, Martin Pieuchot wrote:
> Mostly around rtrequest(9) code, ok?

Anybody?

> Index: net/route.c
> ===
> RCS file: /cvs/src/sys/net/route.c,v
> retrieving revision 1.270
> diff -u -p -r1.270 route.c
> --- net/route.c   11 Nov 2015 11:25:16 -  1.270
> +++ net/route.c   11 Nov 2015 11:40:55 -
> @@ -806,6 +806,7 @@ int
>  rtrequest(int req, struct rt_addrinfo *info, u_int8_t prio,
>  struct rtentry **ret_nrt, u_int tableid)
>  {
> + struct ifnet*ifp;
>   struct rtentry  *rt, *crt;
>   struct ifaddr   *ifa;
>   struct sockaddr *ndst;
> @@ -870,7 +871,12 @@ rtrequest(int req, struct rt_addrinfo *i
>   rt->rt_parent = NULL;
>  
>   rt->rt_flags &= ~RTF_UP;
> - rt->rt_ifp->if_rtrequest(rt->rt_ifp, RTM_DELETE, rt);
> +
> + ifp = if_get(rt->rt_ifidx);
> + KASSERT(ifp != NULL);
> + ifp->if_rtrequest(ifp, RTM_DELETE, rt);
> + if_put(ifp);
> +
>   atomic_inc_int();
>  
>   if (ret_nrt != NULL)
> @@ -911,8 +917,9 @@ rtrequest(int req, struct rt_addrinfo *i
>   if (info->rti_ifa == NULL && (error = rt_getifa(info, tableid)))
>   return (error);
>   ifa = info->rti_ifa;
> + ifp = ifa->ifa_ifp;
>   if (prio == 0)
> - prio = ifa->ifa_ifp->if_priority + RTP_STATIC;
> + prio = ifp->if_priority + RTP_STATIC;
>  
>   dlen = info->rti_info[RTAX_DST]->sa_len;
>   ndst = malloc(dlen, M_RTABLE, M_NOWAIT);
> @@ -941,8 +948,8 @@ rtrequest(int req, struct rt_addrinfo *i
>   /* Check the link state if the table supports it. */
>   if (rtable_mpath_capable(tableid, ndst->sa_family) &&
>   !ISSET(rt->rt_flags, RTF_LOCAL) &&
> - (!LINK_STATE_IS_UP(ifa->ifa_ifp->if_link_state) ||
> - !ISSET(ifa->ifa_ifp->if_flags, IFF_UP))) {
> + (!LINK_STATE_IS_UP(ifp->if_link_state) ||
> + !ISSET(ifp->if_flags, IFF_UP))) {
>   rt->rt_flags &= ~RTF_UP;
>   rt->rt_priority |= RTP_DOWN;
>   }
> @@ -989,7 +996,7 @@ rtrequest(int req, struct rt_addrinfo *i
>  
>   ifa->ifa_refcnt++;
>   rt->rt_ifa = ifa;
> - rt->rt_ifp = ifa->ifa_ifp;
> + rt->rt_ifp = ifp;
>   if (rt->rt_flags & RTF_CLONED) {
>   /*
>* If the ifa of the cloning route was stale, a
> @@ -998,16 +1005,21 @@ rtrequest(int req, struct rt_addrinfo *i
>* route.
>*/
>   if ((*ret_nrt)->rt_ifa->ifa_ifp == NULL) {
> - printf("rtrequest RTM_RESOLVE: wrong ifa (%p) "
> - "was (%p)\n", ifa, (*ret_nrt)->rt_ifa);
> - (*ret_nrt)->rt_ifp->if_rtrequest(rt->rt_ifp,
> - RTM_DELETE, *ret_nrt);
> + struct ifnet *ifp0;
> +
> + printf("%s RTM_RESOLVE: wrong ifa (%p) was (%p)"
> + "\n", __func__, ifa, (*ret_nrt)->rt_ifa);
> +
> + ifp0 = if_get((*ret_nrt)->rt_ifidx);
> + KASSERT(ifp0 != NULL);
> + ifp0->if_rtrequest(ifp0, RTM_DELETE, *ret_nrt);
>   ifafree((*ret_nrt)->rt_ifa);
> - (*ret_nrt)->rt_ifa = ifa;
> - (*ret_nrt)->rt_ifp = ifa->ifa_ifp;
> + if_put(ifp0);
> +
>   ifa->ifa_refcnt++;
> - (*ret_nrt)->rt_ifp->if_rtrequest(rt->rt_ifp,
> - RTM_ADD, *ret_nrt);
> + (*ret_nrt)->rt_ifa = ifa;
> + (*ret_nrt)->rt_ifp = ifp;
> + ifp->if_rtrequest(ifp, RTM_ADD, *ret_nrt);
>   }
>   /*
>* Copy both metrics and a back pointer to the cloned
> @@ -1056,7 +1068,7 @@ rtrequest(int req, struct rt_addrinfo *i
>   pool_put(_pool, rt);
>   return (EEXIST);
>   }
> - rt->rt_ifp->if_rtrequest(rt->rt_ifp, req, rt);
> + ifp->if_rtrequest(ifp, req, rt);
>  
>   if ((rt->rt_flags & RTF_CLONING) != 0) {
>   /* clean up any cloned children */
> Index: net/rtsock.c
> ===
> RCS file: /cvs/src/sys/net/rtsock.c,v
> retrieving revision 1.182
> diff -u -p -r1.182 rtsock.c
> --- net/rtsock.c  9 Nov 2015 10:26:26 -   1.182
> 

Re: initial 802.11n implementation

[Martin Pieuchot]

On 15/11/15(Sun) 21:32, Stefan Sperling wrote:
> On Sun, Nov 15, 2015 at 08:27:28PM +0100, Martin Pieuchot wrote:
> > I'm really concerned about the use of tasks in iwm_ampdu_rx_{start,stop}
> > 
> > By looking at iwn(4) which also has a lot of bits for handling A-MPDU
> > and A-MSDU frames, it seems to me that this go against damien@'s design.
> > This design might have its own set of problems, but I fear that working
> > around them might add another set of problems on top.
> 
> iwm(4) runs all fw commands in process context and checks for errors
> after tsleep(). This means we cannot return errors to the interrupt
> context which triggered the commands.
> Remember this? http://marc.info/?l=openbsd-tech=143463744006189=2
> 
> iwn(4) triggers fw commands in interrupt context and ignores errors.

I remember, that's why I'm raising the subject again.
  
> I'm still not sure which is better. Perhaps iwm(4) should do what iwn(4)
> does. Perhaps not. But this is a general problem with iwm(4) and it's not
> restricted to just this task. I'm simply following the current pattern.

I'm not saying that's better.  I'm saying that the actual 11n driver/
stack glue has been designed with iwn(4)'s behavior in mind.  So I
think that's one argument more to do the same with iwm(4)...  At least
at this stage.

> > Are you sure that when these functions are called it is safe to modify
> > the soft states of the Block Ack agreements before notifying the
> > hardware?
> 
> The soft state is one layer removed from the real action.
> 
> Based on my understanding so far, I believe it works like this:
>
> [...] 
>
> If the hardware fails to set up a block ack we agreed to, or if
> for some other reason the hardware isn't set up for block ack yet
> when we receive an A-MPDU, I think the worst case should be that
> the sending AP will time out its BA agreement, and perhaps it will
> see dropped frames and retry. net80211 will recognize duplicates
> based on their sequence numbers and drop them. Non-aggregated frames
> are always a valid fallback so the AP may choose to try negotiating
> block ack again, or it might not.

Then I think it's fine.