uninitialised variable crashes in bgpd config parser

[Jonathan Gray]

Index: parse.y
===
RCS file: /cvs/src/usr.sbin/bgpd/parse.y,v
retrieving revision 1.315
diff -u -p -r1.315 parse.y
--- parse.y 21 Aug 2017 14:41:22 -  1.315
+++ parse.y 19 Oct 2017 05:32:03 -
@@ -3181,7 +3181,7 @@ parseextcommunity(struct filter_extcommu
switch (type) {
case -1:
if ((p = strchr(s, ':')) == NULL) {
-   yyerror("Bad ext-community %s is %s", s, errstr);
+   yyerror("Bad ext-community %s", s);
return (-1);
}
*p++ = '\0';
@@ -3239,7 +3239,7 @@ parseextcommunity(struct filter_extcommu
else if (strcmp(s, "not-found") == 0)
c->data.ext_opaq = EXT_COMMUNITY_OVS_NOTFOUND;
else {
-   yyerror("Bad ext-community %s is %s", s, errstr);
+   yyerror("Bad ext-community %s", s);
return (-1);
}
break;

fix memory handling in acme-client config parser

[Jonathan Gray]

Use after free and a memory leak.

Index: parse.y
===
RCS file: /cvs/src/usr.sbin/acme-client/parse.y,v
retrieving revision 1.17
diff -u -p -U4 -r1.17 parse.y
--- parse.y 23 Mar 2017 12:59:32 -  1.17
+++ parse.y 19 Oct 2017 04:50:29 -
@@ -224,10 +224,10 @@ domain: DOMAIN STRING {
char *s;
if ((s = strdup($2)) == NULL)
err(EXIT_FAILURE, "strdup");
if (!domain_valid(s)) {
-   free(s);
yyerror("%s: bad domain syntax", s);
+   free(s);
YYERROR;
}
if ((domain = conf_new_domain(conf, s)) == NULL) {
free(s);
@@ -335,8 +335,9 @@ domainoptsl : ALTERNATIVE NAMES '{' altn
if ((s = strdup($3)) == NULL)
err(EXIT_FAILURE, "strdup");
if (authority_find(conf, s) == NULL) {
yyerror("use: unknown authority");
+   free(s);
YYERROR;
}
domain->auth = s;
}

Re: ksh: skip long history lines

[Ori Bernstein]

On Thu, 19 Oct 2017 00:22:51 +0200
Jeremie Courreges-Anglas  wrote:

> Those variables are static so that error messages are only printed once
> in the shell lifetime.  history reload happens each time the shell
> detects that the histfile has been modified, which can happen often if
> like me you have multiple interactives shells running at the same time.
 
Makes sense, although I had been thinking that it could lead to lost
warnings when changing histfile; I'd hope that a corrupt histfile isn't
a persistent condition.

In any case, this looks pretty good to me.

> > Maybe hoist this outside of the loop.
> 
> I don't see a reason for this.
> 
> >> +  /* no trailing newline? */
> >> +  if (strlen(p) != sizeof(encoded) - 1) {
> >> +  if (!corrupt) {
> >> +  corrupt = 1;
> >> +  bi_errorf("history file is corrupt");
> >> +  }
> >> +  return;
> >
> > Why does `corrupt` need to be recorded at all? There's a
> > return right after it.
> 
> Also to limit the amount of crap printed on screen, though maybe we
> should warn every time in this case.  Here's an updated diff that does
> so and hopefully makes the intents clearer.  It also adds a comment
> suggested by anton@
> 
> If your histfile ends with an overlong line which is also not terminated
> by a newline, the do...while loop would silently ignore this last line.
> I think that's acceptable, but YMMV.
> 
> 
> Index: history.c
> ===
> RCS file: /d/cvs/src/bin/ksh/history.c,v
> retrieving revision 1.72
> diff -u -p -p -u -r1.72 history.c
> --- history.c 18 Oct 2017 15:41:25 -  1.72
> +++ history.c 18 Oct 2017 22:16:43 -
> @@ -739,24 +739,42 @@ history_close(void)
>  static void
>  history_load(Source *s)
>  {
> - char*p, encoded[LINE + 1], line[LINE + 1];
> + char*nl, *p, encoded[LINE + 1], line[LINE + 1];
>  
>   rewind(histfh);
> + line_co = 1;
>  
>   /* just read it all; will auto resize history upon next command */
> - for (line_co = 1; ; line_co++) {
> - p = fgets(encoded, sizeof(encoded), histfh);
> - if (p == NULL || feof(histfh) || ferror(histfh))
> - break;
> - if ((p = strchr(encoded, '\n')) == NULL) {
> - bi_errorf("history file is corrupt");
> - return;
> + while ((p = fgets(encoded, sizeof(encoded), histfh)) != NULL) {
> + if ((nl = strchr(encoded, '\n')) == NULL) {
> + static int warned;
> +
> + /* no trailing newline? */
> + if (strlen(p) != sizeof(encoded) - 1) {
> + bi_errorf("history file is corrupt");
> + return;
> + }
> +
> + if (!warned) {
> + warned = 1;
> + bi_errorf(
> + "ignoring history line(s) longer than %d"
> + " bytes", LINE);
> + }
> +
> + /* discard overlong line */
> + do {
> + p = fgets(encoded, sizeof(encoded), histfh);
> + } while (p != NULL && strchr(p, '\n') == NULL);
> +
> + continue;
>   }
> - *p = '\0';
> + *nl = '\0';
>   s->line = line_co;
>   s->cmd_offset = line_co;
>   strunvis(line, encoded);
>   histsave(line_co, line, 0);
> + line_co++;
>   }
>  
>   history_write();
> 
> -- 
> jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF  DDCC 0DFA 74AE 1524 E7EE
> 


-- 
Ori Bernstein 

ksh: extra rewind call

[Jeremie Courreges-Anglas]

This call was added along with the magic check, but it not actually
needed: history_load already calls rewind(3).  I feel like the magic
check should also be in history_load(), so that a ksh process can
recover at runtime from a binary->plaintext history file migration.
Worth the trouble?

ok for the diff below?


Index: history.c
===
RCS file: /d/cvs/src/bin/ksh/history.c,v
retrieving revision 1.72
diff -u -p -p -u -r1.72 history.c
--- history.c   18 Oct 2017 15:41:25 -  1.72
+++ history.c   18 Oct 2017 22:25:23 -
@@ -799,8 +799,6 @@ hist_init(Source *s)
return;
}
 
-   rewind(histfh);
-
history_load(s);
 
history_lock(LOCK_UN);


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

Re: ksh: skip long history lines

[Jeremie Courreges-Anglas]

Hi Ori,

thanks for your feedback.  Reply and updated diff below,

On Wed, Oct 18 2017, Ori Bernstein  wrote:
> On Wed, 18 Oct 2017 22:33:51 +0200
> Jeremie Courreges-Anglas  wrote:
>
>> 
>> It would be nice to support arbitrarily long lines, but a first step
>> would be to skip them gracefuly.
>> 
>> The code modifies the loop condition: no tests against ferror(3)/feof(3)
>> are performed any more (I don't see their point).
>> 
>> We could print a warning on ferror(), though, but that would be another
>> diff.
>> 
>> ok?
>> 
>> 
>> Index: history.c
>> ===
>> RCS file: /d/cvs/src/bin/ksh/history.c,v
>> retrieving revision 1.72
>> diff -u -p -p -u -r1.72 history.c
>> --- history.c18 Oct 2017 15:41:25 -  1.72
>> +++ history.c18 Oct 2017 20:18:24 -
>> @@ -739,24 +739,45 @@ history_close(void)
>>  static void
>>  history_load(Source *s)
>>  {
>> -char*p, encoded[LINE + 1], line[LINE + 1];
>> +char*nl, *p, encoded[LINE + 1], line[LINE + 1];
>>  
>>  rewind(histfh);
>> +line_co = 1;
>>  
>>  /* just read it all; will auto resize history upon next command */
>> -for (line_co = 1; ; line_co++) {
>> -p = fgets(encoded, sizeof(encoded), histfh);
>> -if (p == NULL || feof(histfh) || ferror(histfh))
>> -break;
>> -if ((p = strchr(encoded, '\n')) == NULL) {
>> -bi_errorf("history file is corrupt");
>> -return;
>> +for (p = fgets(encoded, sizeof(encoded), histfh); p != NULL;
>> + p = fgets(encoded, sizeof(encoded), histfh)) {
>
> The redundant calls prevent this code from scanning as well as it
> could for me. Perhaps:
>
>   for(;;) {
>   if ((p = fgets(...) == NULL)
>   break;
>
> Just a minor cosmetic nitpick, it's not a big deal either way.

I must admit that the code is needlessly redundant, but I prefer to have
the loop condition inside... the loop condition.

>> +if ((nl = strchr(encoded, '\n')) == NULL) {
>> +static int corrupt, toolong;
>
> I'm not sure why these need to be static. I'm not an expert on
> the code, but it looks like histsave can be called multiple
> times as the shell runs, which triggers a history reload.
> Keeping this state seems like it would be wrong.

Those variables are static so that error messages are only printed once
in the shell lifetime.  history reload happens each time the shell
detects that the histfile has been modified, which can happen often if
like me you have multiple interactives shells running at the same time.

> Maybe hoist this outside of the loop.

I don't see a reason for this.

>> +/* no trailing newline? */
>> +if (strlen(p) != sizeof(encoded) - 1) {
>> +if (!corrupt) {
>> +corrupt = 1;
>> +bi_errorf("history file is corrupt");
>> +}
>> +return;
>
> Why does `corrupt` need to be recorded at all? There's a
> return right after it.

Also to limit the amount of crap printed on screen, though maybe we
should warn every time in this case.  Here's an updated diff that does
so and hopefully makes the intents clearer.  It also adds a comment
suggested by anton@

If your histfile ends with an overlong line which is also not terminated
by a newline, the do...while loop would silently ignore this last line.
I think that's acceptable, but YMMV.


Index: history.c
===
RCS file: /d/cvs/src/bin/ksh/history.c,v
retrieving revision 1.72
diff -u -p -p -u -r1.72 history.c
--- history.c   18 Oct 2017 15:41:25 -  1.72
+++ history.c   18 Oct 2017 22:16:43 -
@@ -739,24 +739,42 @@ history_close(void)
 static void
 history_load(Source *s)
 {
-   char*p, encoded[LINE + 1], line[LINE + 1];
+   char*nl, *p, encoded[LINE + 1], line[LINE + 1];
 
rewind(histfh);
+   line_co = 1;
 
/* just read it all; will auto resize history upon next command */
-   for (line_co = 1; ; line_co++) {
-   p = fgets(encoded, sizeof(encoded), histfh);
-   if (p == NULL || feof(histfh) || ferror(histfh))
-   break;
-   if ((p = strchr(encoded, '\n')) == NULL) {
-   bi_errorf("history file is corrupt");
-   return;
+   while ((p = fgets(encoded, sizeof(encoded), histfh)) != NULL) {
+   if ((nl = strchr(encoded, '\n')) == NULL) {
+   static int warned;
+
+   /* no trailing newline? */
+   if (strlen(p) != sizeof(encoded) - 1) {
+   bi_errorf("history 

Re: queue packet in loopback

[Alexandr Nedvedicky]

Hello,

On Wed, Oct 18, 2017 at 08:13:09PM +0200, Alexander Bluhm wrote:
> On Wed, Oct 18, 2017 at 10:48:30AM +0200, Alexandr Nedvedicky wrote:
> > I think you also want to add if_ih_remove() to loop_clone_destroy().
> 
> Yes, thanks for catching that.
> 
> > I feel it should be called after if_detach().
> 
> I disagree.  Other pseudo-interfaces call it before.  But more
> importantly, if_ih_remove() calls refcnt_finalize() which sleeps
> until all references are gone.  We should not detach the interface
> before doing that.
> 
> correct?

yes, it makes sense. I was perhaps mistaken by bridge_clone_destroy(),
which calls if_deactivate(ifp); first:

 224 
 225 /* Undo pseudo-driver changes. */
 226 if_deactivate(ifp);
 227 
 228 if_ih_remove(ifp, ether_input, NULL);
 229 
 230 KASSERT(SRPL_EMPTY_LOCKED(>if_inputs));
 231
 232 if_detach(ifp);

my concern was to call if_ih_remove() _after_ if_deactivate().
but it looks like it does not matter. your way is consistent with
mpw_clone_destroy(). Thank you for fixing me. your patch looks good
to me.

OK sashan

Re: Httpd support for internal redirects.

[Ori Bernstein]

Pinging this patch.

On Tue, 10 Oct 2017 21:31:20 -0700
Ori Bernstein  wrote:

> My website generator is a little stupid at times. It generates
> files with .html suffixes, but urls without them.
> 
> I worked around this with some redirects, but it never felt
> quite right doing an extra round trip. Therefore, I added
> internal redirects, processing the rewrite before responding to
> an http request.
> 
> This introduces new syntax to the config file, allowing you to
> do:
> 
>   location match "^(/foo/bar/[%w]+)$" {
>   rewrite-to "/baz/%1.html"
>   }
> 
> Because we don't know what the paths should be relative
> to, all paths rewritten must be absolute.
> 
> It seems like someone else may find it useful[1], so
> I'm submitting it. I've been running a slightly older
> version of this on https://myrlang.org for the last
> day or two, and it's been uneventful. The difference
> is that the syntax used to piggy back off the "block"
> action => 'block internal return 302 "path"'.
> 
> This doesn't currently support chained rewrites. I think
> that it wouldn't be hard to add if it's needed.
> 
> Ok?
> 
> [1] https://github.com/reyk/httpd/issues/27
> ==
> 
> 
> diff --git usr.sbin/httpd/config.c usr.sbin/httpd/config.c
> index 3c31c3d4cd3..7d2982af7b9 100644
> --- usr.sbin/httpd/config.c
> +++ usr.sbin/httpd/config.c
> @@ -448,7 +448,7 @@ config_getserver_config(struct httpd *env, struct server 
> *srv,
>   sizeof(srv_conf->errorlog));
>   }
>  
> - f = SRVFLAG_BLOCK|SRVFLAG_NO_BLOCK;
> + f = SRVFLAG_BLOCK|SRVFLAG_NO_BLOCK|SRVFLAG_REWRITE;
>   if ((srv_conf->flags & f) == 0) {
>   free(srv_conf->return_uri);
>   srv_conf->flags |= parent->flags & f;
> diff --git usr.sbin/httpd/httpd.conf.5 usr.sbin/httpd/httpd.conf.5
> index a3c97629de3..3a00a750537 100644
> --- usr.sbin/httpd/httpd.conf.5
> +++ usr.sbin/httpd/httpd.conf.5
> @@ -454,6 +454,14 @@ instead of the log files.
>  Disable any previous
>  .Ic block
>  in a location.
> +.It Ic rewrite-to Ar path
> +The current request path is rewritten to
> +.Ar  path .
> +using the same macro expansions as
> +.Cm block return
> +rules. After macros are substituted, the resulting paths must be
> +absolute, beginning with a slash.  Rewriting is not done
> +recursively.
>  .It Ic root Ar option
>  Configure the document root and options for the request path.
>  Valid options are:
> diff --git usr.sbin/httpd/httpd.h usr.sbin/httpd/httpd.h
> index 05cbb8e3550..477115ec92d 100644
> --- usr.sbin/httpd/httpd.h
> +++ usr.sbin/httpd/httpd.h
> @@ -394,6 +394,7 @@ SPLAY_HEAD(client_tree, client);
>  #define SRVFLAG_SERVER_MATCH 0x0020
>  #define SRVFLAG_SERVER_HSTS  0x0040
>  #define SRVFLAG_DEFAULT_TYPE 0x0080
> +#define SRVFLAG_REWRITE  0x0100
>  
>  #define SRVFLAG_BITS \
>   "\10\01INDEX\02NO_INDEX\03AUTO_INDEX\04NO_AUTO_INDEX"   \
> diff --git usr.sbin/httpd/parse.y usr.sbin/httpd/parse.y
> index fcf1938c42d..4072ee5b532 100644
> --- usr.sbin/httpd/parse.y
> +++ usr.sbin/httpd/parse.y
> @@ -134,7 +134,7 @@ typedef struct {
>  %token   LISTEN LOCATION LOG LOGDIR MATCH MAXIMUM NO NODELAY OCSP ON 
> PORT PREFORK
>  %token   PROTOCOLS REQUESTS ROOT SACK SERVER SOCKET STRIP STYLE SYSLOG 
> TCP TICKET
>  %token   TIMEOUT TLS TYPE TYPES HSTS MAXAGE SUBDOMAINS DEFAULT PRELOAD 
> REQUEST
> -%token   ERROR INCLUDE AUTHENTICATE WITH BLOCK DROP RETURN PASS
> +%token   ERROR INCLUDE AUTHENTICATE WITH BLOCK DROP RETURN REWRITE PASS
>  %token STRING
>  %token NUMBER
>  %typeport
> @@ -986,6 +986,11 @@ filter   : block RETURN NUMBER optstring {
>   srv_conf->return_uri_len = strlen($4) + 1;
>   }
>   }
> + | REWRITE STRING {
> + srv_conf->flags |= SRVFLAG_REWRITE;
> + srv_conf->return_uri = $2;
> + srv_conf->return_uri_len = strlen($2) + 1;
> + }
>   | block DROP{
>   /* No return code, silently drop the connection */
>   srv_conf->return_code = 0;
> @@ -1255,6 +1260,7 @@ lookup(char *s)
>   { "request",REQUEST },
>   { "requests",   REQUESTS },
>   { "return", RETURN },
> + { "rewrite-to", REWRITE },
>   { "root",   ROOT },
>   { "sack",   SACK },
>   { "server", SERVER },
> diff --git usr.sbin/httpd/server_http.c usr.sbin/httpd/server_http.c
> index e64de0d2f9c..c9ea4771037 100644
> --- usr.sbin/httpd/server_http.c
> +++ usr.sbin/httpd/server_http.c
> @@ -1162,10 +1162,34 @@ 

Re: tftpd(8): diff for ip path rewrite

[Jan Klemkow]

On Wed, Oct 18, 2017 at 08:37:48PM +, Jason McIntyre wrote:
> On Wed, Oct 18, 2017 at 10:25:13PM +0200, Jan Klemkow wrote:
> > This diff adds an option for client IP address path prefixes to the
> > tftpd(8).  First, I used the -r rewrite socket for this, but...
> > 
> > If you use the rewrite socket feature, the tftpd(8) will exit with an
> > error when the rewrite socket is closed.  A reopen of the socket is not
> > possible, if its outside of the chroot directory.  And a privilege
> > separated tftpd(8) is a bit overkill for a stable per client path
> > rewrite feature.  This story led me to this change here.
> > 
> > Any suggestions or objections are welcome. :-)
> 
> evening. some comments inline:

Thanks.  Fixed diff:

Index: tftpd.8
===
RCS file: /mount/openbsd/cvs/src/usr.sbin/tftpd/tftpd.8,v
retrieving revision 1.5
diff -u -p -r1.5 tftpd.8
--- tftpd.8 18 Jul 2015 05:32:56 -  1.5
+++ tftpd.8 18 Oct 2017 21:12:52 -
@@ -37,7 +37,7 @@
 .Nd DARPA Trivial File Transfer Protocol daemon
 .Sh SYNOPSIS
 .Nm tftpd
-.Op Fl 46cdv
+.Op Fl 46cdiv
 .Op Fl l Ar address
 .Op Fl p Ar port
 .Op Fl r Ar socket
@@ -100,6 +100,11 @@ If this option is specified,
 .Nm
 will run in the foreground and log
 the client IP, type of request, and filename to stderr.
+.It Fl i
+Use the client's IP address as a subdirectory prefix for all requested
+filenames.
+This option can not be combined with
+.Fl r .
 .It Fl l Ar address
 Listen on the specified address.
 By default
@@ -126,6 +131,8 @@ before the TFTP request will continue.
 By default
 .Nm
 does not use filename rewriting.
+This option can not be combined with
+.Fl i .
 .It Fl v
 Log the client IP, type of request, and filename.
 .It Ar directory
Index: tftpd.c
===
RCS file: /mount/openbsd/cvs/src/usr.sbin/tftpd/tftpd.c,v
retrieving revision 1.39
diff -u -p -r1.39 tftpd.c
--- tftpd.c 26 May 2017 17:38:46 -  1.39
+++ tftpd.c 18 Oct 2017 21:16:25 -
@@ -282,7 +282,7 @@ __dead void
 usage(void)
 {
extern char *__progname;
-   fprintf(stderr, "usage: %s [-46cdv] [-l address] [-p port] [-r socket]"
+   fprintf(stderr, "usage: %s [-46cdiv] [-l address] [-p port] [-r socket]"
" directory\n", __progname);
exit(1);
 }
@@ -290,6 +290,7 @@ usage(void)
 int  cancreate = 0;
 int  verbose = 0;
 int  debug = 0;
+int  iflag = 0;
 
 int
 main(int argc, char *argv[])
@@ -307,7 +308,7 @@ main(int argc, char *argv[])
int family = AF_UNSPEC;
int devnull = -1;
 
-   while ((c = getopt(argc, argv, "46cdl:p:r:v")) != -1) {
+   while ((c = getopt(argc, argv, "46cdil:p:r:v")) != -1) {
switch (c) {
case '4':
family = AF_INET;
@@ -321,6 +322,11 @@ main(int argc, char *argv[])
case 'd':
verbose = debug = 1;
break;
+   case 'i':
+   if (rewrite != NULL)
+   usage();
+   iflag = 1;
+   break;
case 'l':
addr = optarg;
break;
@@ -328,6 +334,8 @@ main(int argc, char *argv[])
port = optarg;
break;
case 'r':
+   if (iflag)
+   usage();
rewrite = optarg;
break;
case 'v':
@@ -903,7 +911,13 @@ again:
 
if (rwmap != NULL)
rewrite_map(client, filename);
-   else
+   else if (iflag) {
+   char nfilename[PATH_MAX];
+
+   snprintf(nfilename, sizeof nfilename, "%s/%s",
+   getip(>ss), filename);
+   tftp_open(client, nfilename);
+   } else
tftp_open(client, filename);
 
return;

Re: ksh: skip long history lines

[Ori Bernstein]

On Wed, 18 Oct 2017 22:33:51 +0200
Jeremie Courreges-Anglas  wrote:

> 
> It would be nice to support arbitrarily long lines, but a first step
> would be to skip them gracefuly.
> 
> The code modifies the loop condition: no tests against ferror(3)/feof(3)
> are performed any more (I don't see their point).
> 
> We could print a warning on ferror(), though, but that would be another
> diff.
> 
> ok?
> 
> 
> Index: history.c
> ===
> RCS file: /d/cvs/src/bin/ksh/history.c,v
> retrieving revision 1.72
> diff -u -p -p -u -r1.72 history.c
> --- history.c 18 Oct 2017 15:41:25 -  1.72
> +++ history.c 18 Oct 2017 20:18:24 -
> @@ -739,24 +739,45 @@ history_close(void)
>  static void
>  history_load(Source *s)
>  {
> - char*p, encoded[LINE + 1], line[LINE + 1];
> + char*nl, *p, encoded[LINE + 1], line[LINE + 1];
>  
>   rewind(histfh);
> + line_co = 1;
>  
>   /* just read it all; will auto resize history upon next command */
> - for (line_co = 1; ; line_co++) {
> - p = fgets(encoded, sizeof(encoded), histfh);
> - if (p == NULL || feof(histfh) || ferror(histfh))
> - break;
> - if ((p = strchr(encoded, '\n')) == NULL) {
> - bi_errorf("history file is corrupt");
> - return;
> + for (p = fgets(encoded, sizeof(encoded), histfh); p != NULL;
> +  p = fgets(encoded, sizeof(encoded), histfh)) {

The redundant calls prevent this code from scanning as well as it
could for me. Perhaps:

for(;;) {
if ((p = fgets(...) == NULL)
break;

Just a minor cosmetic nitpick, it's not a big deal either way.

> + if ((nl = strchr(encoded, '\n')) == NULL) {
> + static int corrupt, toolong;

I'm not sure why these need to be static. I'm not an expert on
the code, but it looks like histsave can be called multiple
times as the shell runs, which triggers a history reload.
Keeping this state seems like it would be wrong.

Maybe hoist this outside of the loop.

> + /* no trailing newline? */
> + if (strlen(p) != sizeof(encoded) - 1) {
> + if (!corrupt) {
> + corrupt = 1;
> + bi_errorf("history file is corrupt");
> + }
> + return;

Why does `corrupt` need to be recorded at all? There's a
return right after it.

> + }
> +
> + if (!toolong) {
> + toolong = 1;
> + bi_errorf(
> + "ignoring history line(s) longer than %d"
> + " bytes", LINE);
> + }
> +
> + do {
> + p = fgets(encoded, sizeof(encoded), histfh);
> + } while (p != NULL && strchr(p, '\n') == NULL);
> +
> + continue;
>   }
> - *p = '\0';
> + *nl = '\0';
>   s->line = line_co;
>   s->cmd_offset = line_co;
>   strunvis(line, encoded);
>   histsave(line_co, line, 0);
> + line_co++;
>   }
>  
>   history_write();
> 
> -- 
> jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF  DDCC 0DFA 74AE 1524 E7EE
> 


-- 
Ori Bernstein 

ksh: skip long history lines

[Jeremie Courreges-Anglas]

It would be nice to support arbitrarily long lines, but a first step
would be to skip them gracefuly.

The code modifies the loop condition: no tests against ferror(3)/feof(3)
are performed any more (I don't see their point).

We could print a warning on ferror(), though, but that would be another
diff.

ok?


Index: history.c
===
RCS file: /d/cvs/src/bin/ksh/history.c,v
retrieving revision 1.72
diff -u -p -p -u -r1.72 history.c
--- history.c   18 Oct 2017 15:41:25 -  1.72
+++ history.c   18 Oct 2017 20:18:24 -
@@ -739,24 +739,45 @@ history_close(void)
 static void
 history_load(Source *s)
 {
-   char*p, encoded[LINE + 1], line[LINE + 1];
+   char*nl, *p, encoded[LINE + 1], line[LINE + 1];
 
rewind(histfh);
+   line_co = 1;
 
/* just read it all; will auto resize history upon next command */
-   for (line_co = 1; ; line_co++) {
-   p = fgets(encoded, sizeof(encoded), histfh);
-   if (p == NULL || feof(histfh) || ferror(histfh))
-   break;
-   if ((p = strchr(encoded, '\n')) == NULL) {
-   bi_errorf("history file is corrupt");
-   return;
+   for (p = fgets(encoded, sizeof(encoded), histfh); p != NULL;
+p = fgets(encoded, sizeof(encoded), histfh)) {
+   if ((nl = strchr(encoded, '\n')) == NULL) {
+   static int corrupt, toolong;
+
+   /* no trailing newline? */
+   if (strlen(p) != sizeof(encoded) - 1) {
+   if (!corrupt) {
+   corrupt = 1;
+   bi_errorf("history file is corrupt");
+   }
+   return;
+   }
+
+   if (!toolong) {
+   toolong = 1;
+   bi_errorf(
+   "ignoring history line(s) longer than %d"
+   " bytes", LINE);
+   }
+
+   do {
+   p = fgets(encoded, sizeof(encoded), histfh);
+   } while (p != NULL && strchr(p, '\n') == NULL);
+
+   continue;
}
-   *p = '\0';
+   *nl = '\0';
s->line = line_co;
s->cmd_offset = line_co;
strunvis(line, encoded);
histsave(line_co, line, 0);
+   line_co++;
}
 
history_write();

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

tftpd(8): diff for ip path rewrite

[Jan Klemkow]

Hi,

This diff adds an option for client IP address path prefixes to the
tftpd(8).  First, I used the -r rewrite socket for this, but...

If you use the rewrite socket feature, the tftpd(8) will exit with an
error when the rewrite socket is closed.  A reopen of the socket is not
possible, if its outside of the chroot directory.  And a privilege
separated tftpd(8) is a bit overkill for a stable per client path
rewrite feature.  This story led me to this change here.

Any suggestions or objections are welcome. :-)

Bye,
Jan

Index: tftpd.8
===
RCS file: /mount/openbsd/cvs/src/usr.sbin/tftpd/tftpd.8,v
retrieving revision 1.5
diff -u -p -r1.5 tftpd.8
--- tftpd.8 18 Jul 2015 05:32:56 -  1.5
+++ tftpd.8 18 Oct 2017 19:58:40 -
@@ -40,7 +40,7 @@
 .Op Fl 46cdv
 .Op Fl l Ar address
 .Op Fl p Ar port
-.Op Fl r Ar socket
+.Op Fl i | Fl r Ar socket
 .Ar directory
 .Sh DESCRIPTION
 .Nm
@@ -113,6 +113,12 @@ listens on the port indicated in the
 .Ql tftp
 service description; see
 .Xr services 5 .
+.It Fl i
+.Nm
+will use the clients IP address as subdirectory prefix for all requested
+filenames.
+This option can not be combined with
+.Fl r .
 .It Fl r Ar socket
 Issue filename rewrite requests to the specified UNIX domain socket.
 .Nm
@@ -126,6 +132,8 @@ before the TFTP request will continue.
 By default
 .Nm
 does not use filename rewriting.
+This option can not be combined with
+.Fl i .
 .It Fl v
 Log the client IP, type of request, and filename.
 .It Ar directory
Index: tftpd.c
===
RCS file: /mount/openbsd/cvs/src/usr.sbin/tftpd/tftpd.c,v
retrieving revision 1.39
diff -u -p -r1.39 tftpd.c
--- tftpd.c 26 May 2017 17:38:46 -  1.39
+++ tftpd.c 18 Oct 2017 19:59:40 -
@@ -282,14 +282,15 @@ __dead void
 usage(void)
 {
extern char *__progname;
-   fprintf(stderr, "usage: %s [-46cdv] [-l address] [-p port] [-r socket]"
-   " directory\n", __progname);
+   fprintf(stderr, "usage: %s [-46cdv] [-l address] [-p port]"
+   " [-i | -r socket] directory\n", __progname);
exit(1);
 }
 
 int  cancreate = 0;
 int  verbose = 0;
 int  debug = 0;
+int  iflag = 0;
 
 int
 main(int argc, char *argv[])
@@ -307,7 +308,7 @@ main(int argc, char *argv[])
int family = AF_UNSPEC;
int devnull = -1;
 
-   while ((c = getopt(argc, argv, "46cdl:p:r:v")) != -1) {
+   while ((c = getopt(argc, argv, "46cdil:p:r:v")) != -1) {
switch (c) {
case '4':
family = AF_INET;
@@ -321,6 +322,11 @@ main(int argc, char *argv[])
case 'd':
verbose = debug = 1;
break;
+   case 'i':
+   if (rewrite != NULL)
+   usage();
+   iflag = 1;
+   break;
case 'l':
addr = optarg;
break;
@@ -328,6 +334,8 @@ main(int argc, char *argv[])
port = optarg;
break;
case 'r':
+   if (iflag)
+   usage();
rewrite = optarg;
break;
case 'v':
@@ -903,7 +911,13 @@ again:
 
if (rwmap != NULL)
rewrite_map(client, filename);
-   else
+   else if (iflag) {
+   char nfilename[PATH_MAX];
+
+   snprintf(nfilename, sizeof nfilename, "%s/%s",
+   getip(>ss), filename);
+   tftp_open(client, nfilename);
+   } else
tftp_open(client, filename);
 
return;

ksh: remove the deprecated emacs-usemeta option

[Jeremie Courreges-Anglas]

This would make ''set -o emacs-usemeta'' a fatal error, which means that
subsequent lines in your kshrc will not be run.  I think people were
given enough time to cope with this (6.2 users get a warning).

ok?


Index: misc.c
===
RCS file: /d/cvs/src/bin/ksh/misc.c,v
retrieving revision 1.59
diff -u -p -r1.59 misc.c
--- misc.c  30 Aug 2017 17:15:36 -  1.59
+++ misc.c  18 Oct 2017 19:41:04 -
@@ -129,7 +129,6 @@ const struct option options[] = {
{ "csh-history",  0,OF_ANY }, /* non-standard */
 #ifdef EMACS
{ "emacs",0,OF_ANY },
-   { "emacs-usemeta",  0,  OF_ANY }, /* XXX delete after 6.2 */
 #endif
{ "errexit",'e',OF_ANY },
 #ifdef EMACS
@@ -185,13 +184,8 @@ option(const char *n)
int i;
 
for (i = 0; i < NELEM(options); i++)
-   if (options[i].name && strcmp(options[i].name, n) == 0) {
-#ifdef EMACS
-   if (i == FEMACSUSEMETA)
-   warningf(true, "%s: deprecated option", n);
-#endif
+   if (options[i].name && strcmp(options[i].name, n) == 0)
return i;
-   }
 
return -1;
 }
@@ -232,10 +226,6 @@ printoptions(int verbose)
shprintf("Current option settings\n");
 
for (i = n = oi.opt_width = 0; i < NELEM(options); i++) {
-#ifdef EMACS
-   if (i == FEMACSUSEMETA)
-   continue;
-#endif
if (options[i].name) {
len = strlen(options[i].name);
oi.opts[n].name = options[i].name;
@@ -250,10 +240,6 @@ printoptions(int verbose)
/* short version ala ksh93 */
shprintf("set");
for (i = 0; i < NELEM(options); i++) {
-#ifdef EMACS
-   if (i == FEMACSUSEMETA)
-   continue;
-#endif
if (options[i].name)
shprintf(" %co %s",
 Flag(i) ? '-' : '+',
Index: sh.h
===
RCS file: /d/cvs/src/bin/ksh/sh.h,v
retrieving revision 1.64
diff -u -p -r1.64 sh.h
--- sh.h3 Sep 2017 11:52:01 -   1.64
+++ sh.h18 Oct 2017 16:12:59 -
@@ -139,7 +139,6 @@ enum sh_flag {
FCSHHISTORY,/* csh-style history enabled */
 #ifdef EMACS
FEMACS, /* emacs command editing */
-   FEMACSUSEMETA,  /* XXX delete after 6.2 */
 #endif
FERREXIT,   /* -e: quit on error */
 #ifdef EMACS


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

Re: queue packet in loopback

[Alexander Bluhm]

On Wed, Oct 18, 2017 at 10:48:30AM +0200, Alexandr Nedvedicky wrote:
> I think you also want to add if_ih_remove() to loop_clone_destroy().

Yes, thanks for catching that.

> I feel it should be called after if_detach().

I disagree.  Other pseudo-interfaces call it before.  But more
importantly, if_ih_remove() calls refcnt_finalize() which sleeps
until all references are gone.  We should not detach the interface
before doing that.

correct?

Parts of the diff have been commited, so here is the rest with
if_ih_remove() added.

bluhm

Index: net/if_loop.c
===
RCS file: /data/mirror/openbsd/cvs/src/sys/net/if_loop.c,v
retrieving revision 1.81
diff -u -p -r1.81 if_loop.c
--- net/if_loop.c   19 Apr 2017 15:21:54 -  1.81
+++ net/if_loop.c   18 Oct 2017 17:15:29 -
@@ -143,6 +143,7 @@
 intloioctl(struct ifnet *, u_long, caddr_t);
 void   loopattach(int);
 void   lortrequest(struct ifnet *, int, struct rtentry *);
+intloinput(struct ifnet *, struct mbuf *, void *);
 intlooutput(struct ifnet *,
struct mbuf *, struct sockaddr *, struct rtentry *);
 
@@ -191,6 +192,7 @@ loop_clone_create(struct if_clone *ifc, 
 #if NBPFILTER > 0
bpfattach(>if_bpf, ifp, DLT_LOOP, sizeof(u_int32_t));
 #endif
+   if_ih_insert(ifp, loinput, NULL);
return (0);
 }
 
@@ -200,6 +202,7 @@ loop_clone_destroy(struct ifnet *ifp)
if (ifp->if_index == rtable_loindex(ifp->if_rdomain))
return (EPERM);
 
+   if_ih_remove(ifp, loinput, NULL);
if_detach(ifp);
 
free(ifp, M_DEVBUF, sizeof(*ifp));
@@ -207,11 +210,26 @@ loop_clone_destroy(struct ifnet *ifp)
 }
 
 int
+loinput(struct ifnet *ifp, struct mbuf *m, void *cookie)
+{
+   int error;
+
+   if ((m->m_flags & M_PKTHDR) == 0)
+   panic("%s: no header mbuf", __func__);
+
+   error = if_input_local(ifp, m, m->m_pkthdr.ph_family);
+   if (error)
+   ifp->if_ierrors++;
+
+   return (1);
+}
+
+int
 looutput(struct ifnet *ifp, struct mbuf *m, struct sockaddr *dst,
 struct rtentry *rt)
 {
if ((m->m_flags & M_PKTHDR) == 0)
-   panic("looutput: no header mbuf");
+   panic("%s: no header mbuf", __func__);
 
if (rt && rt->rt_flags & (RTF_REJECT|RTF_BLACKHOLE)) {
m_freem(m);
@@ -219,7 +237,16 @@ looutput(struct ifnet *ifp, struct mbuf 
rt->rt_flags & RTF_HOST ? EHOSTUNREACH : ENETUNREACH);
}
 
-   return (if_input_local(ifp, m, dst->sa_family));
+   /* Use the quick path only once to avoid stack overflow. */
+   if ((m->m_flags & M_LOOP) == 0)
+   return (if_input_local(ifp, m, dst->sa_family));
+
+   m->m_pkthdr.ph_family = dst->sa_family;
+   if (mq_enqueue(>if_inputqueue, m))
+   return ENOBUFS;
+   task_add(softnettq, ifp->if_inputtask);
+
+   return (0);
 }
 
 void
Index: sys/mbuf.h
===
RCS file: /data/mirror/openbsd/cvs/src/sys/sys/mbuf.h,v
retrieving revision 1.231
diff -u -p -r1.231 mbuf.h
--- sys/mbuf.h  23 Jun 2017 11:18:12 -  1.231
+++ sys/mbuf.h  18 Oct 2017 17:03:14 -
@@ -134,6 +134,7 @@ struct  pkthdr {
u_intph_rtableid;   /* routing table id */
u_intph_ifidx;  /* rcv interface index */
u_int8_t ph_loopcnt;/* mbuf is looping in kernel */
+   u_int8_t ph_family; /* af, used when queueing */
struct pkthdr_pf pf;
 };
 

Re: KAME ioctl leftovers

[Claudio Jeker]

On Wed, Oct 18, 2017 at 01:34:27PM +0200, Martin Pieuchot wrote:
> Kill ioctl(2) added with original KAME import that have never been used.
> FreeBSD also stopped supporting them in 2013.
> 
> ok?

Agreed, if it builds OK claudio. The amount of - is shocking...
 
> Index: sys/sockio.h
> ===
> RCS file: /cvs/src/sys/sys/sockio.h,v
> retrieving revision 1.70
> diff -u -p -r1.70 sockio.h
> --- sys/sockio.h  27 Jun 2017 22:18:24 -  1.70
> +++ sys/sockio.h  18 Oct 2017 11:28:25 -
> @@ -64,12 +64,6 @@
>  #define  SIOCGIFDATA _IOWR('i', 27, struct ifreq)/* get if_data 
> */
>  #define  SIOCSIFLLADDR   _IOW('i', 31, struct ifreq) /* set link 
> level addr */
>  
> -/* KAME IPv6 */
> -/* SIOCAIFALIAS? */
> -#define SIOCALIFADDR  _IOW('i', 28, struct if_laddrreq) /* add IF addr */
> -#define SIOCGLIFADDR _IOWR('i', 29, struct if_laddrreq) /* get IF addr */
> -#define SIOCDLIFADDR  _IOW('i', 30, struct if_laddrreq) /* delete IF addr */
> -
>  #define  SIOCADDMULTI _IOW('i', 49, struct ifreq)/* add m'cast 
> addr */
>  #define  SIOCDELMULTI _IOW('i', 50, struct ifreq)/* del m'cast 
> addr */
>  #define  SIOCGETVIFCNT   _IOWR('u', 51, struct sioc_vif_req)/* vif pkt 
> cnt */
> Index: netinet/in.c
> ===
> RCS file: /cvs/src/sys/netinet/in.c,v
> retrieving revision 1.140
> diff -u -p -r1.140 in.c
> --- netinet/in.c  11 Aug 2017 19:53:02 -  1.140
> +++ netinet/in.c  18 Oct 2017 11:29:04 -
> @@ -83,7 +83,6 @@
>  
>  
>  void in_socktrim(struct sockaddr_in *);
> -int in_lifaddr_ioctl(u_long, caddr_t, struct ifnet *, int);
>  
>  void in_purgeaddr(struct ifaddr *);
>  int in_addhost(struct in_ifaddr *, struct sockaddr_in *);
> @@ -182,9 +181,6 @@ in_nam2sin(const struct mbuf *nam, struc
>   return 0;
>  }
>  
> -/*
> - * Generic internet control operations (ioctl's).
> - */
>  int
>  in_control(struct socket *so, u_long cmd, caddr_t data, struct ifnet *ifp)
>  {
> @@ -194,25 +190,13 @@ in_control(struct socket *so, u_long cmd
>   if ((so->so_state & SS_PRIV) != 0)
>   privileged++;
>  
> - switch (cmd) {
>  #ifdef MROUTING
> + switch (cmd) {
>   case SIOCGETVIFCNT:
>   case SIOCGETSGCNT:
>   return (mrt_ioctl(so, cmd, data));
> -#endif /* MROUTING */
> - case SIOCALIFADDR:
> - case SIOCDLIFADDR:
> - if (!privileged)
> - return (EPERM);
> - /* FALLTHROUGH */
> - case SIOCGLIFADDR:
> - if (ifp == NULL)
> - return (EINVAL);
> - return in_lifaddr_ioctl(cmd, data, ifp, privileged);
> - default:
> - if (ifp == NULL)
> - return (EOPNOTSUPP);
>   }
> +#endif /* MROUTING */
>  
>   return (in_ioctl(cmd, data, ifp, privileged));
>  }
> @@ -228,6 +212,9 @@ in_ioctl(u_long cmd, caddr_t data, struc
>   int error;
>   int newifaddr;
>  
> + if (ifp == NULL)
> + return (EOPNOTSUPP);
> +
>   NET_ASSERT_LOCKED();
>  
>   TAILQ_FOREACH(ifa, >if_addrlist, ifa_list) {
> @@ -413,187 +400,6 @@ in_ioctl(u_long cmd, caddr_t data, struc
>   }
>   return (0);
>  }
> -
> -/*
> - * SIOC[GAD]LIFADDR.
> - *   SIOCGLIFADDR: get first address. (???)
> - *   SIOCGLIFADDR with IFLR_PREFIX:
> - *   get first address that matches the specified prefix.
> - *   SIOCALIFADDR: add the specified address.
> - *   SIOCALIFADDR with IFLR_PREFIX:
> - *   EINVAL since we can't deduce hostid part of the address.
> - *   SIOCDLIFADDR: delete the specified address.
> - *   SIOCDLIFADDR with IFLR_PREFIX:
> - *   delete the first address that matches the specified prefix.
> - * return values:
> - *   EINVAL on invalid parameters
> - *   EADDRNOTAVAIL on prefix match failed/specified address not found
> - *   other values may be returned from in_ioctl()
> - */
> -int
> -in_lifaddr_ioctl(u_long cmd, caddr_t data, struct ifnet *ifp, int privileged)
> -{
> - struct if_laddrreq *iflr = (struct if_laddrreq *)data;
> - struct ifaddr *ifa;
> - struct sockaddr *sa;
> -
> - /* sanity checks */
> - if (!data || !ifp) {
> - panic("invalid argument to in_lifaddr_ioctl");
> - /*NOTRECHED*/
> - }
> -
> - switch (cmd) {
> - case SIOCGLIFADDR:
> - /* address must be specified on GET with IFLR_PREFIX */
> - if ((iflr->flags & IFLR_PREFIX) == 0)
> - break;
> - /*FALLTHROUGH*/
> - case SIOCALIFADDR:
> - case SIOCDLIFADDR:
> - /* address must be specified on ADD and DELETE */
> - sa = sstosa(>addr);
> - if (sa->sa_family != AF_INET)
> - return EINVAL;
> - if (sa->sa_len != sizeof(struct sockaddr_in))
> - return EINVAL;
> -  

Re: spamdb: allow keys to be specified in list mode

[Jason McIntyre]

On Wed, Oct 18, 2017 at 10:22:00AM -0600, Todd C. Miller wrote:
> I often want to query a specific key instead of dumping out the
> entire database.  This lets you do things like:
> 
> $ spamdb 180.124.41.143
> TRAPPED|180.124.41.143|1508373987
> 
> or:
> 
> $ spamdb 74.125.198.26 180.124.41.143 107.161.26.205
> WHITE|74.125.198.26|||1506223558|1506223558|1509333958|1|0
> TRAPPED|180.124.41.143|1508373987
> 
> No error is displayed for missing addresses but the exit value will
> be 1 instead of 0 in this case.
> 
>  - todd
> 

the man page bits are ok me.

personally i like your diff, since it makes spamdb usage appear simpler,
and saves having to run grep on something that is fairly common.

jmc

> Index: usr.sbin/spamdb/spamdb.8
> ===
> RCS file: /cvs/src/usr.sbin/spamdb/spamdb.8,v
> retrieving revision 1.19
> diff -u -p -u -r1.19 spamdb.8
> --- usr.sbin/spamdb/spamdb.8  11 Oct 2017 18:25:07 -  1.19
> +++ usr.sbin/spamdb/spamdb.8  18 Oct 2017 16:17:41 -
> @@ -22,10 +22,8 @@
>  .Nd spamd database tool
>  .Sh SYNOPSIS
>  .Nm spamdb
> -.Oo Oo Fl Tt Oc
> -.Fl a Ar keys Oc
> -.Oo Oo Fl GTt Oc
> -.Fl d Ar keys Oc
> +.Op Fl adGTt
> +.Op Ar keys ...
>  .Sh DESCRIPTION
>  .Nm
>  manipulates the spamd database in
> @@ -35,7 +33,7 @@ used for
>  .Pp
>  The options are as follows:
>  .Bl -tag -width Ds
> -.It Fl a Ar keys
> +.It Fl a
>  Add or update the entries for
>  .Ar keys .
>  This can be used to whitelist one or more IP addresses
> @@ -46,7 +44,7 @@ If any
>  specified match entries already in the spamd database,
>  .Nm
>  updates the entry's time last seen to now.
> -.It Fl d Ar keys
> +.It Fl d
>  Delete entries for
>  .Ar keys .
>  .It Fl G
> @@ -90,9 +88,12 @@ Otherwise
>  .Ar keys
>  must be numerical IP addresses.
>  .Ss DATABASE OUTPUT FORMAT
> -If invoked without any arguments,
> +If invoked without any options,
>  .Nm
>  lists the contents of the database in a text format.
> +If one or more
> +.Ar keys
> +are specified, only matching entries will be printed.
>  .Pp
>  For SPAMTRAP entries the format is:
>  .Pp
> Index: usr.sbin/spamdb/spamdb.c
> ===
> RCS file: /cvs/src/usr.sbin/spamdb/spamdb.c,v
> retrieving revision 1.33
> diff -u -p -u -r1.33 spamdb.c
> --- usr.sbin/spamdb/spamdb.c  12 Oct 2017 09:28:56 -  1.33
> +++ usr.sbin/spamdb/spamdb.c  18 Oct 2017 16:21:21 -
> @@ -186,10 +186,81 @@ dbupdate(DB *db, char *ip, int add, int 
>  }
>  
>  int
> +print_entry(DBT *dbk, DBT *dbd)
> +{
> + struct gdata gd;
> + char *a, *cp;
> +
> + if ((dbk->size < 1) || gdcopyin(dbd, ) == -1) {
> + warnx("bogus size db entry - bad db file?");
> + return (1);
> + }
> + a = malloc(dbk->size + 1);
> + if (a == NULL)
> + err(1, "malloc");
> + memcpy(a, dbk->data, dbk->size);
> + a[dbk->size]='\0';
> + cp = strchr(a, '\n');
> + if (cp == NULL) {
> + /* this is a non-greylist entry */
> + switch (gd.pcount) {
> + case -1: /* spamtrap hit, with expiry time */
> + printf("TRAPPED|%s|%lld\n", a,
> + (long long)gd.expire);
> + break;
> + case -2: /* spamtrap address */
> + printf("SPAMTRAP|%s\n", a);
> + break;
> + default: /* whitelist */
> + printf("WHITE|%s|||%lld|%lld|%lld|%d|%d\n", a,
> + (long long)gd.first, (long long)gd.pass,
> + (long long)gd.expire, gd.bcount,
> + gd.pcount);
> + break;
> + }
> + } else {
> + char *helo, *from, *to;
> +
> + /* greylist entry */
> + *cp = '\0';
> + helo = cp + 1;
> + from = strchr(helo, '\n');
> + if (from == NULL) {
> + warnx("No from part in grey key %s", a);
> + free(a);
> + return (1);
> + }
> + *from = '\0';
> + from++;
> + to = strchr(from, '\n');
> + if (to == NULL) {
> + /* probably old format - print it the
> +  * with an empty HELO field instead 
> +  * of erroring out.
> +  */   
> + printf("GREY|%s|%s|%s|%s|%lld|%lld|%lld|%d|%d\n",
> + a, "", helo, from, (long long)gd.first,
> + (long long)gd.pass, (long long)gd.expire,
> + gd.bcount, gd.pcount);
> + 
> + } else {
> + *to = '\0';
> + to++;
> + printf("GREY|%s|%s|%s|%s|%lld|%lld|%lld|%d|%d\n",
> + a, helo, from, to, (long 

spamdb: allow keys to be specified in list mode

[Todd C. Miller]

I often want to query a specific key instead of dumping out the
entire database.  This lets you do things like:

$ spamdb 180.124.41.143
TRAPPED|180.124.41.143|1508373987

or:

$ spamdb 74.125.198.26 180.124.41.143 107.161.26.205
WHITE|74.125.198.26|||1506223558|1506223558|1509333958|1|0
TRAPPED|180.124.41.143|1508373987

No error is displayed for missing addresses but the exit value will
be 1 instead of 0 in this case.

 - todd

Index: usr.sbin/spamdb/spamdb.8
===
RCS file: /cvs/src/usr.sbin/spamdb/spamdb.8,v
retrieving revision 1.19
diff -u -p -u -r1.19 spamdb.8
--- usr.sbin/spamdb/spamdb.811 Oct 2017 18:25:07 -  1.19
+++ usr.sbin/spamdb/spamdb.818 Oct 2017 16:17:41 -
@@ -22,10 +22,8 @@
 .Nd spamd database tool
 .Sh SYNOPSIS
 .Nm spamdb
-.Oo Oo Fl Tt Oc
-.Fl a Ar keys Oc
-.Oo Oo Fl GTt Oc
-.Fl d Ar keys Oc
+.Op Fl adGTt
+.Op Ar keys ...
 .Sh DESCRIPTION
 .Nm
 manipulates the spamd database in
@@ -35,7 +33,7 @@ used for
 .Pp
 The options are as follows:
 .Bl -tag -width Ds
-.It Fl a Ar keys
+.It Fl a
 Add or update the entries for
 .Ar keys .
 This can be used to whitelist one or more IP addresses
@@ -46,7 +44,7 @@ If any
 specified match entries already in the spamd database,
 .Nm
 updates the entry's time last seen to now.
-.It Fl d Ar keys
+.It Fl d
 Delete entries for
 .Ar keys .
 .It Fl G
@@ -90,9 +88,12 @@ Otherwise
 .Ar keys
 must be numerical IP addresses.
 .Ss DATABASE OUTPUT FORMAT
-If invoked without any arguments,
+If invoked without any options,
 .Nm
 lists the contents of the database in a text format.
+If one or more
+.Ar keys
+are specified, only matching entries will be printed.
 .Pp
 For SPAMTRAP entries the format is:
 .Pp
Index: usr.sbin/spamdb/spamdb.c
===
RCS file: /cvs/src/usr.sbin/spamdb/spamdb.c,v
retrieving revision 1.33
diff -u -p -u -r1.33 spamdb.c
--- usr.sbin/spamdb/spamdb.c12 Oct 2017 09:28:56 -  1.33
+++ usr.sbin/spamdb/spamdb.c18 Oct 2017 16:21:21 -
@@ -186,10 +186,81 @@ dbupdate(DB *db, char *ip, int add, int 
 }
 
 int
+print_entry(DBT *dbk, DBT *dbd)
+{
+   struct gdata gd;
+   char *a, *cp;
+
+   if ((dbk->size < 1) || gdcopyin(dbd, ) == -1) {
+   warnx("bogus size db entry - bad db file?");
+   return (1);
+   }
+   a = malloc(dbk->size + 1);
+   if (a == NULL)
+   err(1, "malloc");
+   memcpy(a, dbk->data, dbk->size);
+   a[dbk->size]='\0';
+   cp = strchr(a, '\n');
+   if (cp == NULL) {
+   /* this is a non-greylist entry */
+   switch (gd.pcount) {
+   case -1: /* spamtrap hit, with expiry time */
+   printf("TRAPPED|%s|%lld\n", a,
+   (long long)gd.expire);
+   break;
+   case -2: /* spamtrap address */
+   printf("SPAMTRAP|%s\n", a);
+   break;
+   default: /* whitelist */
+   printf("WHITE|%s|||%lld|%lld|%lld|%d|%d\n", a,
+   (long long)gd.first, (long long)gd.pass,
+   (long long)gd.expire, gd.bcount,
+   gd.pcount);
+   break;
+   }
+   } else {
+   char *helo, *from, *to;
+
+   /* greylist entry */
+   *cp = '\0';
+   helo = cp + 1;
+   from = strchr(helo, '\n');
+   if (from == NULL) {
+   warnx("No from part in grey key %s", a);
+   free(a);
+   return (1);
+   }
+   *from = '\0';
+   from++;
+   to = strchr(from, '\n');
+   if (to == NULL) {
+   /* probably old format - print it the
+* with an empty HELO field instead 
+* of erroring out.
+*/   
+   printf("GREY|%s|%s|%s|%s|%lld|%lld|%lld|%d|%d\n",
+   a, "", helo, from, (long long)gd.first,
+   (long long)gd.pass, (long long)gd.expire,
+   gd.bcount, gd.pcount);
+   
+   } else {
+   *to = '\0';
+   to++;
+   printf("GREY|%s|%s|%s|%s|%lld|%lld|%lld|%d|%d\n",
+   a, helo, from, to, (long long)gd.first,
+   (long long)gd.pass, (long long)gd.expire,
+   gd.bcount, gd.pcount);
+   }
+   }
+   free(a);
+
+   return (0);
+}
+
+int
 dblist(DB *db)
 {
DBT dbk, dbd;
-   struct gdatagd;
int r;
 
/* walk db, list in text format */
@@ -197,80 +268,52 @@ dblist(DB *db)
   

Re: libfuse: patch to prevent fuse-zip from dumping core

[Helg Bredow]

On Wed, 18 Oct 2017 15:03:21 +0200
Martin Pieuchot  wrote:

> On 18/10/17(Wed) 12:51, Helg Bredow wrote:
> > On Wed, 18 Oct 2017 10:04:07 +0200
> > Martin Pieuchot  wrote:
> > 
> > > On 17/10/17(Tue) 15:30, Helg Bredow wrote:
> > > > If you execute "fuse-zip -V" it prints the version and then dumps core. 
> > > > This is because fuse-zip does not initialise the mount point pointer to 
> > > > NULL. This patch ensures that it's always initialised to NULL.
> > > 
> > > It's hard to understand your fix if you don't explain what "dumps core".
> > > 
> > > I had to install the package and look at the trace myself.  You could
> > > save me these tasks by either posting the backtrace, saying that free(3)
> > > is call on an uninitialized memory or both.
> > > 
> > > That said, I'd suggest different fix.  Initializing `mp' in fuse_setup()
> > > is very fragile.  Instead I'd declare a local variable and don't use
> > > `mp' at all in these function.
> > > In case of sucsses, just before returning the "struct fuse" pointer I'd
> > > assign *mp, if not NULL, to the local variable.
> > > 
> > > By the way, what does "mp" stand for?  I find the name confusing.
> > > 
> > 
> > Sorry, I've been looking at this for so long I assumed the diff was 
> > self-explanatory. Thanks for your patience. I like your suggestion and have 
> > modified the patch accordingly. A NULL test before the assignment is 
> > superfluous since the code won't be reached if dir is NULL.
> 
> But what if mp is NULL?

Good point; I hadn't considered that. I've added the check.

> 
> > `mp' is the directory where the fuse file system will be mounted (mount 
> > point). I've named the new variable dir to be consistent with mount(2).
> 
> Nice.


Index: fuse.c
===
RCS file: /cvs/src/lib/libfuse/fuse.c,v
retrieving revision 1.29
diff -u -p -r1.29 fuse.c
--- fuse.c  21 Aug 2017 21:41:13 -  1.29
+++ fuse.c  18 Oct 2017 14:30:08 -
@@ -466,14 +466,16 @@ fuse_setup(int argc, char **argv, const 
struct fuse_args args = FUSE_ARGS_INIT(argc, argv);
struct fuse_chan *fc;
struct fuse *fuse;
+   char *dir;
int fg;
 
-   if (fuse_parse_cmdline(, mp, mt, ))
+   dir = NULL;
+   if (fuse_parse_cmdline(, , mt, ))
goto err;
 
fuse_daemonize(0);
 
-   if ((fc = fuse_mount(*mp, NULL)) == NULL)
+   if ((fc = fuse_mount(dir, NULL)) == NULL)
goto err;
 
if ((fuse = fuse_new(fc, NULL, ops, size, data)) == NULL) {
@@ -481,9 +483,12 @@ fuse_setup(int argc, char **argv, const 
goto err;
}
 
+   if (mp != NULL)
+   *mp = dir;
+
return (fuse);
 err:
-   free(*mp);
+   free(dir);
return (NULL);
 }
 

Re: libfuse: patch to prevent fuse-zip from dumping core

[Martin Pieuchot]

On 18/10/17(Wed) 12:51, Helg Bredow wrote:
> On Wed, 18 Oct 2017 10:04:07 +0200
> Martin Pieuchot  wrote:
> 
> > On 17/10/17(Tue) 15:30, Helg Bredow wrote:
> > > If you execute "fuse-zip -V" it prints the version and then dumps core. 
> > > This is because fuse-zip does not initialise the mount point pointer to 
> > > NULL. This patch ensures that it's always initialised to NULL.
> > 
> > It's hard to understand your fix if you don't explain what "dumps core".
> > 
> > I had to install the package and look at the trace myself.  You could
> > save me these tasks by either posting the backtrace, saying that free(3)
> > is call on an uninitialized memory or both.
> > 
> > That said, I'd suggest different fix.  Initializing `mp' in fuse_setup()
> > is very fragile.  Instead I'd declare a local variable and don't use
> > `mp' at all in these function.
> > In case of sucsses, just before returning the "struct fuse" pointer I'd
> > assign *mp, if not NULL, to the local variable.
> > 
> > By the way, what does "mp" stand for?  I find the name confusing.
> > 
> 
> Sorry, I've been looking at this for so long I assumed the diff was 
> self-explanatory. Thanks for your patience. I like your suggestion and have 
> modified the patch accordingly. A NULL test before the assignment is 
> superfluous since the code won't be reached if dir is NULL.

But what if mp is NULL?

> `mp' is the directory where the fuse file system will be mounted (mount 
> point). I've named the new variable dir to be consistent with mount(2).

Nice.

Re: libfuse: patch to prevent fuse-zip from dumping core

[Helg Bredow]

On Wed, 18 Oct 2017 10:04:07 +0200
Martin Pieuchot  wrote:

> On 17/10/17(Tue) 15:30, Helg Bredow wrote:
> > If you execute "fuse-zip -V" it prints the version and then dumps core. 
> > This is because fuse-zip does not initialise the mount point pointer to 
> > NULL. This patch ensures that it's always initialised to NULL.
> 
> It's hard to understand your fix if you don't explain what "dumps core".
> 
> I had to install the package and look at the trace myself.  You could
> save me these tasks by either posting the backtrace, saying that free(3)
> is call on an uninitialized memory or both.
> 
> That said, I'd suggest different fix.  Initializing `mp' in fuse_setup()
> is very fragile.  Instead I'd declare a local variable and don't use
> `mp' at all in these function.
> In case of sucsses, just before returning the "struct fuse" pointer I'd
> assign *mp, if not NULL, to the local variable.
> 
> By the way, what does "mp" stand for?  I find the name confusing.
> 

Sorry, I've been looking at this for so long I assumed the diff was 
self-explanatory. Thanks for your patience. I like your suggestion and have 
modified the patch accordingly. A NULL test before the assignment is 
superfluous since the code won't be reached if dir is NULL.

`mp' is the directory where the fuse file system will be mounted (mount point). 
I've named the new variable dir to be consistent with mount(2).

Index: fuse.c
===
RCS file: /cvs/src/lib/libfuse/fuse.c,v
retrieving revision 1.29
diff -u -p -r1.29 fuse.c
--- fuse.c  21 Aug 2017 21:41:13 -  1.29
+++ fuse.c  18 Oct 2017 12:39:12 -
@@ -466,14 +466,16 @@ fuse_setup(int argc, char **argv, const 
struct fuse_args args = FUSE_ARGS_INIT(argc, argv);
struct fuse_chan *fc;
struct fuse *fuse;
+   char *dir;
int fg;
 
-   if (fuse_parse_cmdline(, mp, mt, ))
+   dir = NULL;
+   if (fuse_parse_cmdline(, , mt, ))
goto err;
 
fuse_daemonize(0);
 
-   if ((fc = fuse_mount(*mp, NULL)) == NULL)
+   if ((fc = fuse_mount(dir, NULL)) == NULL)
goto err;
 
if ((fuse = fuse_new(fc, NULL, ops, size, data)) == NULL) {
@@ -481,9 +483,10 @@ fuse_setup(int argc, char **argv, const 
goto err;
}
 
+   *mp = dir;
return (fuse);
 err:
-   free(*mp);
+   free(dir);
return (NULL);
 }
 

umb/ppp/gre: dead ioctls

[Martin Pieuchot]

These ioctl(2)s are handled by ifioctl() and never passed down to the
drivers.

ok?

Index: net/if_gre.c
===
RCS file: /cvs/src/sys/net/if_gre.c,v
retrieving revision 1.87
diff -u -p -r1.87 if_gre.c
--- net/if_gre.c11 Aug 2017 21:24:19 -  1.87
+++ net/if_gre.c18 Oct 2017 12:36:35 -
@@ -456,12 +456,6 @@ gre_ioctl(struct ifnet *ifp, u_long cmd,
}
ifp->if_mtu = ifr->ifr_mtu;
break;
-   case SIOCGIFMTU:
-   ifr->ifr_mtu = sc->sc_if.if_mtu;
-   break;
-   case SIOCGIFHARDMTU:
-   ifr->ifr_hardmtu = sc->sc_if.if_hardmtu;
-   break;
case SIOCADDMULTI:
case SIOCDELMULTI:
break;
Index: net/if_spppsubr.c
===
RCS file: /cvs/src/sys/net/if_spppsubr.c,v
retrieving revision 1.172
diff -u -p -r1.172 if_spppsubr.c
--- net/if_spppsubr.c   15 Aug 2017 06:08:52 -  1.172
+++ net/if_spppsubr.c   18 Oct 2017 12:36:32 -
@@ -864,12 +864,6 @@ sppp_ioctl(struct ifnet *ifp, u_long cmd
}
ifp->if_mtu = ifr->ifr_mtu;
break;
-   case SIOCGIFMTU:
-   ifr->ifr_mtu = ifp->if_mtu;
-   break;
-   case SIOCGIFHARDMTU:
-   ifr->ifr_hardmtu = ifp->if_hardmtu;
-   break;
case SIOCADDMULTI:
case SIOCDELMULTI:
break;
Index: dev/usb/if_umb.c
===
RCS file: /cvs/src/sys/dev/usb/if_umb.c,v
retrieving revision 1.15
diff -u -p -r1.15 if_umb.c
--- dev/usb/if_umb.c11 Aug 2017 21:24:19 -  1.15
+++ dev/usb/if_umb.c18 Oct 2017 12:36:42 -
@@ -734,12 +734,6 @@ umb_ioctl(struct ifnet *ifp, u_long cmd,
}
ifp->if_mtu = ifr->ifr_mtu;
break;
-   case SIOCGIFMTU:
-   ifr->ifr_mtu = ifp->if_mtu;
-   break;
-   case SIOCGIFHARDMTU:
-   ifr->ifr_hardmtu = ifp->if_hardmtu;
-   break;
case SIOCSIFADDR:
case SIOCAIFADDR:
case SIOCSIFDSTADDR:

Re: KAME ioctl leftovers

[Florian Obser]

On Wed, Oct 18, 2017 at 11:34:27AM +, Martin Pieuchot wrote:
> Kill ioctl(2) added with original KAME import that have never been used.
> FreeBSD also stopped supporting them in 2013.

usr.sbin/bind is looking at SIOCGLIFADDR, but it looks like it's
properly guarded by #ifdef. debian codesearch also finds a few things,
a cursory look suggests that it's properly guarded.

> 
> ok?

if it survived a base build OK florian@

> 
> Index: sys/sockio.h
> ===
> RCS file: /cvs/src/sys/sys/sockio.h,v
> retrieving revision 1.70
> diff -u -p -r1.70 sockio.h
> --- sys/sockio.h  27 Jun 2017 22:18:24 -  1.70
> +++ sys/sockio.h  18 Oct 2017 11:28:25 -
> @@ -64,12 +64,6 @@
>  #define  SIOCGIFDATA _IOWR('i', 27, struct ifreq)/* get if_data 
> */
>  #define  SIOCSIFLLADDR   _IOW('i', 31, struct ifreq) /* set link 
> level addr */
>  
> -/* KAME IPv6 */
> -/* SIOCAIFALIAS? */
> -#define SIOCALIFADDR  _IOW('i', 28, struct if_laddrreq) /* add IF addr */
> -#define SIOCGLIFADDR _IOWR('i', 29, struct if_laddrreq) /* get IF addr */
> -#define SIOCDLIFADDR  _IOW('i', 30, struct if_laddrreq) /* delete IF addr */
> -
>  #define  SIOCADDMULTI _IOW('i', 49, struct ifreq)/* add m'cast 
> addr */
>  #define  SIOCDELMULTI _IOW('i', 50, struct ifreq)/* del m'cast 
> addr */
>  #define  SIOCGETVIFCNT   _IOWR('u', 51, struct sioc_vif_req)/* vif pkt 
> cnt */
> Index: netinet/in.c
> ===
> RCS file: /cvs/src/sys/netinet/in.c,v
> retrieving revision 1.140
> diff -u -p -r1.140 in.c
> --- netinet/in.c  11 Aug 2017 19:53:02 -  1.140
> +++ netinet/in.c  18 Oct 2017 11:29:04 -
> @@ -83,7 +83,6 @@
>  
>  
>  void in_socktrim(struct sockaddr_in *);
> -int in_lifaddr_ioctl(u_long, caddr_t, struct ifnet *, int);
>  
>  void in_purgeaddr(struct ifaddr *);
>  int in_addhost(struct in_ifaddr *, struct sockaddr_in *);
> @@ -182,9 +181,6 @@ in_nam2sin(const struct mbuf *nam, struc
>   return 0;
>  }
>  
> -/*
> - * Generic internet control operations (ioctl's).
> - */
>  int
>  in_control(struct socket *so, u_long cmd, caddr_t data, struct ifnet *ifp)
>  {
> @@ -194,25 +190,13 @@ in_control(struct socket *so, u_long cmd
>   if ((so->so_state & SS_PRIV) != 0)
>   privileged++;
>  
> - switch (cmd) {
>  #ifdef MROUTING
> + switch (cmd) {
>   case SIOCGETVIFCNT:
>   case SIOCGETSGCNT:
>   return (mrt_ioctl(so, cmd, data));
> -#endif /* MROUTING */
> - case SIOCALIFADDR:
> - case SIOCDLIFADDR:
> - if (!privileged)
> - return (EPERM);
> - /* FALLTHROUGH */
> - case SIOCGLIFADDR:
> - if (ifp == NULL)
> - return (EINVAL);
> - return in_lifaddr_ioctl(cmd, data, ifp, privileged);
> - default:
> - if (ifp == NULL)
> - return (EOPNOTSUPP);
>   }
> +#endif /* MROUTING */
>  
>   return (in_ioctl(cmd, data, ifp, privileged));
>  }
> @@ -228,6 +212,9 @@ in_ioctl(u_long cmd, caddr_t data, struc
>   int error;
>   int newifaddr;
>  
> + if (ifp == NULL)
> + return (EOPNOTSUPP);
> +
>   NET_ASSERT_LOCKED();
>  
>   TAILQ_FOREACH(ifa, >if_addrlist, ifa_list) {
> @@ -413,187 +400,6 @@ in_ioctl(u_long cmd, caddr_t data, struc
>   }
>   return (0);
>  }
> -
> -/*
> - * SIOC[GAD]LIFADDR.
> - *   SIOCGLIFADDR: get first address. (???)
> - *   SIOCGLIFADDR with IFLR_PREFIX:
> - *   get first address that matches the specified prefix.
> - *   SIOCALIFADDR: add the specified address.
> - *   SIOCALIFADDR with IFLR_PREFIX:
> - *   EINVAL since we can't deduce hostid part of the address.
> - *   SIOCDLIFADDR: delete the specified address.
> - *   SIOCDLIFADDR with IFLR_PREFIX:
> - *   delete the first address that matches the specified prefix.
> - * return values:
> - *   EINVAL on invalid parameters
> - *   EADDRNOTAVAIL on prefix match failed/specified address not found
> - *   other values may be returned from in_ioctl()
> - */
> -int
> -in_lifaddr_ioctl(u_long cmd, caddr_t data, struct ifnet *ifp, int privileged)
> -{
> - struct if_laddrreq *iflr = (struct if_laddrreq *)data;
> - struct ifaddr *ifa;
> - struct sockaddr *sa;
> -
> - /* sanity checks */
> - if (!data || !ifp) {
> - panic("invalid argument to in_lifaddr_ioctl");
> - /*NOTRECHED*/
> - }
> -
> - switch (cmd) {
> - case SIOCGLIFADDR:
> - /* address must be specified on GET with IFLR_PREFIX */
> - if ((iflr->flags & IFLR_PREFIX) == 0)
> - break;
> - /*FALLTHROUGH*/
> - case SIOCALIFADDR:
> - case SIOCDLIFADDR:
> - /* address must be specified on ADD and DELETE */
> - sa = sstosa(>addr);
> - if 

if_ioctl is not NULL

[Martin Pieuchot]

All are drivers provides it and if_attach() now asserts that it is not
NULL.

Let's get rid of those checks, ok?

Index: netinet/in.c
===
RCS file: /cvs/src/sys/netinet/in.c,v
retrieving revision 1.140
diff -u -p -r1.140 in.c
--- netinet/in.c11 Aug 2017 19:53:02 -  1.140
+++ netinet/in.c18 Oct 2017 11:40:41 -
@@ -406,8 +406,6 @@ in_ioctl(u_long cmd, caddr_t data, struc
break;
 
default:
-   if (ifp->if_ioctl == NULL)
-   return (EOPNOTSUPP);
error = ((*ifp->if_ioctl)(ifp, cmd, data));
return (error);
}
@@ -826,9 +824,6 @@ in_addmulti(struct in_addr *ap, struct i
 */
++inm->inm_refcnt;
} else {
-   if (ifp->if_ioctl == NULL)
-   return (NULL);
-
/*
 * New address; allocate a new multicast record
 * and link it into the interface's multicast list.
Index: netinet6/in6.c
===
RCS file: /cvs/src/sys/netinet6/in6.c,v
retrieving revision 1.212
diff -u -p -r1.212 in6.c
--- netinet6/in6.c  16 Oct 2017 13:40:58 -  1.212
+++ netinet6/in6.c  18 Oct 2017 11:40:03 -
@@ -489,8 +489,6 @@ in6_ioctl(u_long cmd, caddr_t data, stru
break;
 
default:
-   if (ifp->if_ioctl == NULL)
-   return (EOPNOTSUPP);
error = ((*ifp->if_ioctl)(ifp, cmd, data));
return (error);
}
@@ -1247,11 +1245,6 @@ in6_addmulti(struct in6_addr *maddr6, st
 */
in6m->in6m_refcnt++;
} else {
-   if (ifp->if_ioctl == NULL) {
-   *errorp = ENXIO; /* XXX: appropriate? */
-   return (NULL);
-   }
-
/*
 * New address; allocate a new multicast record
 * and link it into the interface's multicast list.

KAME ioctl leftovers

[Martin Pieuchot]

Kill ioctl(2) added with original KAME import that have never been used.
FreeBSD also stopped supporting them in 2013.

ok?

Index: sys/sockio.h
===
RCS file: /cvs/src/sys/sys/sockio.h,v
retrieving revision 1.70
diff -u -p -r1.70 sockio.h
--- sys/sockio.h27 Jun 2017 22:18:24 -  1.70
+++ sys/sockio.h18 Oct 2017 11:28:25 -
@@ -64,12 +64,6 @@
 #defineSIOCGIFDATA _IOWR('i', 27, struct ifreq)/* get if_data 
*/
 #defineSIOCSIFLLADDR   _IOW('i', 31, struct ifreq) /* set link 
level addr */
 
-/* KAME IPv6 */
-/* SIOCAIFALIAS? */
-#define SIOCALIFADDR_IOW('i', 28, struct if_laddrreq) /* add IF addr */
-#define SIOCGLIFADDR   _IOWR('i', 29, struct if_laddrreq) /* get IF addr */
-#define SIOCDLIFADDR_IOW('i', 30, struct if_laddrreq) /* delete IF addr */
-
 #defineSIOCADDMULTI _IOW('i', 49, struct ifreq)/* add m'cast 
addr */
 #defineSIOCDELMULTI _IOW('i', 50, struct ifreq)/* del m'cast 
addr */
 #defineSIOCGETVIFCNT   _IOWR('u', 51, struct sioc_vif_req)/* vif pkt 
cnt */
Index: netinet/in.c
===
RCS file: /cvs/src/sys/netinet/in.c,v
retrieving revision 1.140
diff -u -p -r1.140 in.c
--- netinet/in.c11 Aug 2017 19:53:02 -  1.140
+++ netinet/in.c18 Oct 2017 11:29:04 -
@@ -83,7 +83,6 @@
 
 
 void in_socktrim(struct sockaddr_in *);
-int in_lifaddr_ioctl(u_long, caddr_t, struct ifnet *, int);
 
 void in_purgeaddr(struct ifaddr *);
 int in_addhost(struct in_ifaddr *, struct sockaddr_in *);
@@ -182,9 +181,6 @@ in_nam2sin(const struct mbuf *nam, struc
return 0;
 }
 
-/*
- * Generic internet control operations (ioctl's).
- */
 int
 in_control(struct socket *so, u_long cmd, caddr_t data, struct ifnet *ifp)
 {
@@ -194,25 +190,13 @@ in_control(struct socket *so, u_long cmd
if ((so->so_state & SS_PRIV) != 0)
privileged++;
 
-   switch (cmd) {
 #ifdef MROUTING
+   switch (cmd) {
case SIOCGETVIFCNT:
case SIOCGETSGCNT:
return (mrt_ioctl(so, cmd, data));
-#endif /* MROUTING */
-   case SIOCALIFADDR:
-   case SIOCDLIFADDR:
-   if (!privileged)
-   return (EPERM);
-   /* FALLTHROUGH */
-   case SIOCGLIFADDR:
-   if (ifp == NULL)
-   return (EINVAL);
-   return in_lifaddr_ioctl(cmd, data, ifp, privileged);
-   default:
-   if (ifp == NULL)
-   return (EOPNOTSUPP);
}
+#endif /* MROUTING */
 
return (in_ioctl(cmd, data, ifp, privileged));
 }
@@ -228,6 +212,9 @@ in_ioctl(u_long cmd, caddr_t data, struc
int error;
int newifaddr;
 
+   if (ifp == NULL)
+   return (EOPNOTSUPP);
+
NET_ASSERT_LOCKED();
 
TAILQ_FOREACH(ifa, >if_addrlist, ifa_list) {
@@ -413,187 +400,6 @@ in_ioctl(u_long cmd, caddr_t data, struc
}
return (0);
 }
-
-/*
- * SIOC[GAD]LIFADDR.
- * SIOCGLIFADDR: get first address. (???)
- * SIOCGLIFADDR with IFLR_PREFIX:
- * get first address that matches the specified prefix.
- * SIOCALIFADDR: add the specified address.
- * SIOCALIFADDR with IFLR_PREFIX:
- * EINVAL since we can't deduce hostid part of the address.
- * SIOCDLIFADDR: delete the specified address.
- * SIOCDLIFADDR with IFLR_PREFIX:
- * delete the first address that matches the specified prefix.
- * return values:
- * EINVAL on invalid parameters
- * EADDRNOTAVAIL on prefix match failed/specified address not found
- * other values may be returned from in_ioctl()
- */
-int
-in_lifaddr_ioctl(u_long cmd, caddr_t data, struct ifnet *ifp, int privileged)
-{
-   struct if_laddrreq *iflr = (struct if_laddrreq *)data;
-   struct ifaddr *ifa;
-   struct sockaddr *sa;
-
-   /* sanity checks */
-   if (!data || !ifp) {
-   panic("invalid argument to in_lifaddr_ioctl");
-   /*NOTRECHED*/
-   }
-
-   switch (cmd) {
-   case SIOCGLIFADDR:
-   /* address must be specified on GET with IFLR_PREFIX */
-   if ((iflr->flags & IFLR_PREFIX) == 0)
-   break;
-   /*FALLTHROUGH*/
-   case SIOCALIFADDR:
-   case SIOCDLIFADDR:
-   /* address must be specified on ADD and DELETE */
-   sa = sstosa(>addr);
-   if (sa->sa_family != AF_INET)
-   return EINVAL;
-   if (sa->sa_len != sizeof(struct sockaddr_in))
-   return EINVAL;
-   /* XXX need improvement */
-   sa = sstosa(>dstaddr);
-   if (sa->sa_family
-&& sa->sa_family != AF_INET)
-   return EINVAL;
-   if (sa->sa_len && sa->sa_len != sizeof(struct 

kqueue_scan() in parallel

[Martin Pieuchot]

Diff below is the last version of my kqueue diff to allow grabbing the
solock() and possibly sleeping, inside kqueue_scan().

Sleeping in kqueue_scan() is not a problem since threads are already
doing it when no event are available, in that case `kq_count' is 0.
When this happens, a thread going to sleep first remove its own marker
from the given kqueue's TAILQ.

The diff below allows threads to sleep *inside* the event collection
loop.  For that marker are now flagged with EVFILT_MARKER and thread
are taught how to skip other threads markers.

Now if a thread, tA, is sleeping inside `f_event' it "owns" the corres-
ponding knote.  That means the knote is no longer present in the kqueue's
TAILQ and kq_count no longer accounts for it.

However a sibling thread, tB, could decide it's a good time to close the
file descriptor associated to this knote.  So to make sure tA finished
processing the knote before tB frees it, we use the KN_PROCESSING flag.
That means fdrelease() will sleeps until it can acquires the knote.

This logic is taken from DragonflyBSD where it is used as part of a
finer grained locking scheme.  We still rely on the KERNEL_LOCK() for
serializing accesses to the data structures.

The sys_close() vs sys_kevent() race described above has been triggered
by abieber@ with www/gitea and homestead with previous version of this
diff.  Base doesn't seem to be good enough to exercise it.

Second part of the diff below enables the solock() inside the filters. 

Comments and tests welcome.

Index: kern/kern_event.c
===
RCS file: /cvs/src/sys/kern/kern_event.c,v
retrieving revision 1.81
diff -u -p -r1.81 kern_event.c
--- kern/kern_event.c   11 Oct 2017 08:06:56 -  1.81
+++ kern/kern_event.c   11 Oct 2017 10:40:10 -
@@ -84,6 +84,8 @@ void  knote_attach(struct knote *kn, stru
 void   knote_drop(struct knote *kn, struct proc *p, struct filedesc *fdp);
 void   knote_enqueue(struct knote *kn);
 void   knote_dequeue(struct knote *kn);
+intknote_acquire(struct knote *kn);
+intknote_release(struct knote *kn);
 #define knote_alloc() ((struct knote *)pool_get(_pool, PR_WAITOK))
 #define knote_free(kn) pool_put(_pool, (kn))
 
@@ -759,27 +761,43 @@ start:
goto done;
}
 
+   marker.kn_filter = EVFILT_MARKER;
+   marker.kn_status = KN_PROCESSING;
TAILQ_INSERT_TAIL(>kq_head, , kn_tqe);
while (count) {
kn = TAILQ_FIRST(>kq_head);
if (kn == ) {
-   TAILQ_REMOVE(>kq_head, kn, kn_tqe);
+   TAILQ_REMOVE(>kq_head, , kn_tqe);
splx(s);
if (count == maxevents)
goto retry;
goto done;
}
+   if (kn->kn_filter == EVFILT_MARKER) {
+   struct knote *other_marker = kn;
+
+   /* Move some other threads marker past this kn */
+   kn = TAILQ_NEXT(other_marker, kn_tqe);
+   TAILQ_REMOVE(>kq_head, kn, kn_tqe);
+   TAILQ_INSERT_BEFORE(other_marker, kn, kn_tqe);
+   continue;
+   }
+
+   if (!knote_acquire(kn))
+   continue;
 
TAILQ_REMOVE(>kq_head, kn, kn_tqe);
kq->kq_count--;
 
if (kn->kn_status & KN_DISABLED) {
kn->kn_status &= ~KN_QUEUED;
+   knote_release(kn);
continue;
}
if ((kn->kn_flags & EV_ONESHOT) == 0 &&
kn->kn_fop->f_event(kn, 0) == 0) {
kn->kn_status &= ~(KN_QUEUED | KN_ACTIVE);
+   knote_release(kn);
continue;
}
*kevp = kn->kn_kevent;
@@ -799,9 +817,11 @@ start:
if (kn->kn_flags & EV_DISPATCH)
kn->kn_status |= KN_DISABLED;
kn->kn_status &= ~(KN_QUEUED | KN_ACTIVE);
+   knote_release(kn);
} else {
TAILQ_INSERT_TAIL(>kq_head, kn, kn_tqe);
kq->kq_count++;
+   knote_release(kn);
}
count--;
if (nkev == KQ_NEVENTS) {
@@ -956,6 +976,41 @@ kqueue_wakeup(struct kqueue *kq)
 }
 
 /*
+ * Acquire a knote, return non-zero on success, 0 on failure.
+ *
+ * If we cannot acquire the knote we sleep and return 0.  The knote
+ * may be stale on return in this case and the caller must restart
+ * whatever loop they are in.
+ */
+int
+knote_acquire(struct knote *kn)
+{
+   if (kn->kn_status & KN_PROCESSING) {
+   kn->kn_status |= KN_WAITING;
+   tsleep(kn, 0, "kqepts", hz);
+   /* knote may be stale now */
+   

Re: queue packet in loopback

[Alexandr Nedvedicky]

Hello,



thanks for great explanation of the problem. I like your approach.

I have just two small nitpicks/questions see below.

> 
> Index: net/if_loop.c
> ===
> RCS file: /data/mirror/openbsd/cvs/src/sys/net/if_loop.c,v
> retrieving revision 1.81
> diff -u -p -r1.81 if_loop.c
> --- net/if_loop.c 19 Apr 2017 15:21:54 -  1.81
> +++ net/if_loop.c 16 Oct 2017 13:51:03 -
> @@ -143,6 +143,7 @@
>  int  loioctl(struct ifnet *, u_long, caddr_t);
>  void loopattach(int);
>  void lortrequest(struct ifnet *, int, struct rtentry *);
> +int  loinput(struct ifnet *, struct mbuf *, void *);
>  int  looutput(struct ifnet *,
>   struct mbuf *, struct sockaddr *, struct rtentry *);
>  
> @@ -191,6 +192,7 @@ loop_clone_create(struct if_clone *ifc, 
>  #if NBPFILTER > 0
>   bpfattach(>if_bpf, ifp, DLT_LOOP, sizeof(u_int32_t));
>  #endif
> + if_ih_insert(ifp, loinput, NULL);

I think you also want to add if_ih_remove() to loop_clone_destroy().

8<---8<---8<--8<
199 int
200 loop_clone_destroy(struct ifnet *ifp)
201 {
202 if (ifp->if_index == rtable_loindex(ifp->if_rdomain))
203 return (EPERM);
204 
205 if_detach(ifp);
206 if_ih_remove(ifp, loinput, NULL);
207 
208 free(ifp, M_DEVBUF, sizeof(*ifp));
209 return (0);
210 }
211 
8<---8<---8<--8<
I feel it should be called after if_detach().


>  
> @@ -1168,9 +1174,6 @@ icmp6_reflect(struct mbuf *m, size_t off
>* Note that only echo and node information replies are affected,
>* since the length of ICMP6 errors is limited to the minimum MTU.
>*/
> -#if NPF > 0
> - pf_pkt_addr_changed(m);
> -#endif
>   ip6_send(m);
>   return;

Also I would like to kindly ask you to fix up my mess, when you are there.
the comment above should be moved to ip6_send_dispatch():

8<---8<---8<--8<
diff --git a/sys/netinet6/ip6_input.c b/sys/netinet6/ip6_input.c
index 4e588d83394..bc19e5d8eb6 100644
--- a/sys/netinet6/ip6_input.c
+++ b/sys/netinet6/ip6_input.c
@@ -1481,6 +1481,13 @@ ip6_send_dispatch(void *xmq)
 #endif /* IPSEC */
 
while ((m = ml_dequeue()) != NULL) {
+   /*
+* To avoid a "too big" situation at an intermediate router and
+* the path MTU discovery process, specify the IPV6_MINMTU
+* flag.  Note that only echo and node information replies are
+* affected, since the length of ICMP6 errors is limited to the
+* minimum MTU.
+*/
ip6_output(m, NULL, NULL, IPV6_MINMTU, NULL, NULL);
}
NET_UNLOCK();
8<---8<---8<--8<

I've forgot to move the comment, when turning ip*_send() functions to tasks
long time ago:
8<---8<---8<--8<
revision 1.152
date: 2015/12/03 21:11:54;  author: sashan;  state: Exp;  lines: +36 -1;\
  commitid: nhuzteWvIf6uiITt;
ip_send()/ip6_send() allow PF to send response packet in ipsoftnet task.
this avoids current recursion to pf_test() function. the change also
switches icmp_error()/icmp6_error() to use ip_send()/ip6_send() so
they are safe for PF.

The idea comes from Markus Friedl. bluhm, mikeb and mpi helped me
a lot to get it into shape.
8<---8<---8<--8<

thanks a lot in advance, for taking care about it.

The updated patch with two changes discussed above is below.

thanks and
regards
sasha

8<---8<---8<--8<
diff --git a/sys/net/if_loop.c b/sys/net/if_loop.c
index 47f88306699..e024aca74f4 100644
--- a/sys/net/if_loop.c
+++ b/sys/net/if_loop.c
@@ -143,6 +143,7 @@
 intloioctl(struct ifnet *, u_long, caddr_t);
 void   loopattach(int);
 void   lortrequest(struct ifnet *, int, struct rtentry *);
+intloinput(struct ifnet *, struct mbuf *, void *);
 intlooutput(struct ifnet *,
struct mbuf *, struct sockaddr *, struct rtentry *);
 
@@ -191,6 +192,7 @@ loop_clone_create(struct if_clone *ifc, int unit)
 #if NBPFILTER > 0
bpfattach(>if_bpf, ifp, DLT_LOOP, sizeof(u_int32_t));
 #endif
+   if_ih_insert(ifp, loinput, NULL);
return (0);
 }
 
@@ -201,17 +203,33 @@ loop_clone_destroy(struct ifnet *ifp)
return (EPERM);
 
if_detach(ifp);
+   if_ih_remove(ifp, loinput, NULL);
 
free(ifp, M_DEVBUF, sizeof(*ifp));
return (0);
 }
 
+int
+loinput(struct ifnet *ifp, struct mbuf *m, void *cookie)
+{
+   int error;
+
+   if ((m->m_flags & M_PKTHDR) == 0)
+   panic("%s: no header mbuf", __func__);
+
+   error = if_input_local(ifp, m, 

Re: libfuse: patch to prevent fuse-zip from dumping core

[Martin Pieuchot]

On 17/10/17(Tue) 15:30, Helg Bredow wrote:
> If you execute "fuse-zip -V" it prints the version and then dumps core. This 
> is because fuse-zip does not initialise the mount point pointer to NULL. This 
> patch ensures that it's always initialised to NULL.

It's hard to understand your fix if you don't explain what "dumps core".

I had to install the package and look at the trace myself.  You could
save me these tasks by either posting the backtrace, saying that free(3)
is call on an uninitialized memory or both.

That said, I'd suggest different fix.  Initializing `mp' in fuse_setup()
is very fragile.  Instead I'd declare a local variable and don't use
`mp' at all in these function.
In case of sucsses, just before returning the "struct fuse" pointer I'd
assign *mp, if not NULL, to the local variable.

By the way, what does "mp" stand for?  I find the name confusing.

> Index: fuse.c
> ===
> RCS file: /cvs/src/lib/libfuse/fuse.c,v
> retrieving revision 1.29
> diff -u -p -u -p -r1.29 fuse.c
> --- fuse.c21 Aug 2017 21:41:13 -  1.29
> +++ fuse.c17 Oct 2017 15:21:05 -
> @@ -468,6 +468,7 @@ fuse_setup(int argc, char **argv, const 
>   struct fuse *fuse;
>   int fg;
>  
> + *mp = NULL;
>   if (fuse_parse_cmdline(, mp, mt, ))
>   goto err;
>  
>