once rules fix

2019-03-03 Thread Petr Hoffmann

Hi,

I noticed that pfctl says 'once' can be used only with pass/block rules,
but it is not true - it can't for block but can for anchor rules:

--8<---
# echo 'block once' | pfctl -f -
stdin:1: 'once' only applies to pass/block rules
pfctl: Syntax error in config file: pf rules not loaded

# echo 'anchor xxx once' | pfctl -f -
# pfctl -sr
anchor "xxx" all once
--->8--

The patch below resolves the issue. Note that pf_rule.action has no
special constant for anchors. The patch uses pf_rule.anchor to recognize
an anchor rule. To do this, I moved a piece of code from pfctl_add_rule()
to parse.y so that the anchor is created early and pf_rule.anchor becomes
non-NULL. This further allowed tosimplify expand_rule() and
pfctl_add_rule() by shortening their parameterlists.

The patched pfctl responds as expected:

--8<---
# echo 'block once' | ./pfctl -f -
# pfctl -sr
block drop all once
# echo 'anchor xxx once' | ./pfctl -f -
stdin:1: 'once' only applies to pass/block rules
pfctl: Syntax error in config file: pf rules not loaded
--->8--


diff --git a/sbin/pfctl/parse.y b/sbin/pfctl/parse.y
index e8dd97f6222..ce866883b04 100644
--- a/sbin/pfctl/parse.y
+++ b/sbin/pfctl/parse.y
@@ -354,7 +354,7 @@ struct pfctl_watermarks  syncookie_opts;
 int         disallow_table(struct node_host *, const char *);
 int         disallow_urpf_failed(struct node_host *, const char *);
 int         disallow_alias(struct node_host *, const char *);
-int         rule_consistent(struct pf_rule *, int);
+int         rule_consistent(struct pf_rule *);
 int         process_tabledef(char *, struct table_opts *, int);
 void         expand_label_str(char *, size_t, const char *, const char *);
 void         expand_label_if(const char *, char *, size_t, const char *);
@@ -377,8 +377,7 @@ void         expand_rule(struct pf_rule *, int, 
struct node_if *,

         struct node_proto *,
         struct node_os *, struct node_host *, struct node_port *,
         struct node_host *, struct node_port *, struct node_uid *,
-            struct node_gid *, struct node_if *, struct node_icmp *,
-            const char *);
+            struct node_gid *, struct node_if *, struct node_icmp *);
 int         expand_queue(char *, struct node_if *, struct queue_opts *);
 int         expand_skip_interface(struct node_if *);

@@ -876,6 +875,7 @@ anchorrule    : ANCHOR anchorname dir quick 
interface af proto fromto

     {
         struct pf_rule    r;
         struct node_proto    *proto;
+            char    *p;

         memset(, 0, sizeof(r));
         if (pf->astack[pf->asd + 1]) {
@@ -913,7 +913,33 @@ anchorrule    : ANCHOR anchorname dir quick 
interface af proto fromto

                     "rules must specify a name");
                 YYERROR;
             }
+
+                /*
+                 * Don't make non-brace anchors part of the main anchor 
pool.

+                 */
+                if ((r.anchor = calloc(1, sizeof(*r.anchor))) == NULL) {
+                    err(1, "anchorrule: calloc");
+                }
+ pf_init_ruleset(>ruleset);
+                r.anchor->ruleset.anchor = r.anchor;
+                if (strlcpy(r.anchor->path, $2,
+                    sizeof(r.anchor->path)) >= sizeof(r.anchor->path)) {
+                   errx(1, "anchorrule: strlcpy");
+                }
+                if ((p = strrchr($2, '/')) != NULL) {
+                    if (strlen(p) == 1) {
+                        yyerror("anchorrule: bad anchor name %s",
+                            $2);
+                        YYERROR;
+                    }
+                } else
+                    p = (char *)$2;
+                if (strlcpy(r.anchor->name, p,
+                    sizeof(r.anchor->name)) >= sizeof(r.anchor->name)) {
+                   errx(1, "anchorrule: strlcpy");
+                }
         }
+
         r.direction = $3;
         r.quick = $4.quick;
         r.af = $6;
@@ -955,8 +981,7 @@ anchorrule    : ANCHOR anchorname dir quick 
interface af proto fromto


         expand_rule(, 0, $5, NULL, NULL, NULL, $7, $8.src_os,
             $8.src.host, $8.src.port, $8.dst.host, $8.dst.port,
-                $9.uid, $9.gid, $9.rcv, $9.icmpspec,
-                pf->astack[pf->asd + 1] ? pf->alast->name : $2);
+                $9.uid, $9.gid, $9.rcv, $9.icmpspec);
         free($2);
         pf->astack[pf->asd + 1] = NULL;
     }
@@ -1110,7 +1135,7 @@ antispoof    : ANTISPOOF logquick antispoof_ifspc 
af antispoof_opts {

             if (h != NULL)
                 expand_rule(, 0, j, NULL, NULL, NULL,
                     NULL, NULL, h, NULL, NULL, NULL,
-                        NULL, 

Re: [patch] improve strptime(3) %z timezone parsing

2019-03-03 Thread Hiltjo Posthuma
On Sun, Feb 24, 2019 at 01:11:39PM +0100, Hiltjo Posthuma wrote:
> Hi,
> 
> I noticed some things in the strptime(3) implementing when parsing timezone
> strings using the %z format string.
> 
> 1. I noticed the tm_gmtoff value is not correctly set in some cases. It should
> set tm_gmtoff to the offset from UTC in seconds (as described in mktime(3)).
> 
> 2. The military/nautical UTC offsets are also reversed. This was also actually
> a bug in RFC822:
> 
> RFC5322 (referenced in strptime(3) man page):
> https://tools.ietf.org/html/rfc5322
> Section 4.3, page 34 says:
> "
>The 1 character military time zones were defined in a non-standard
>way in [RFC0822] and are therefore unpredictable in their meaning.
>The original definitions of the military zones "A" through "I" are
>equivalent to "+0100" through "+0900", respectively; "K", "L", and
>"M" are equivalent to "+1000", "+1100", and "+1200", respectively;
>"N" through "Y" are equivalent to "-0100" through "-1200".
>respectively; and "Z" is equivalent to "+".  However, because of
>the error in [RFC0822], they SHOULD all be considered equivalent to
>"-" unless there is out-of-band information confirming their
>meaning.
> "
> 
> While comparing the other BSD and Linux code-bases I noticed NetBSD has fixed
> this on 2017-08-24. It is not in NetBSD 8 stable yet, but in NetBSD-current:
> http://cvsweb.netbsd.org/bsdweb.cgi/src/lib/libc/time/strptime.c
> 
> 3. The military zone J is also ambiguous. Some standards define it as unused,
> while others define it as "local observed time". NetBSD handles it as local
> observed time, but OpenBSD returns NULL in strptime(3). I left this as is.
> 
> 4. While at it I also fixed some trailing whitespaces.
> 
> Thanks for not falling asleep and reading the long text :)
> 
> Patch and test program below:
> 
> diff --git lib/libc/time/strptime.c lib/libc/time/strptime.c
> index eaf182dc773..86a0d5353ee 100644
> --- lib/libc/time/strptime.c
> +++ lib/libc/time/strptime.c
> @@ -116,7 +116,7 @@ _strptime(const char *buf, const char *fmt, struct tm 
> *tm, int initialize)
>   fmt++;
>   continue;
>   }
> - 
> +
>   if ((c = *fmt++) != '%')
>   goto literal;
>  
> @@ -142,7 +142,7 @@ literal:
>   _LEGAL_ALT(0);
>   alt_format |= _ALT_O;
>   goto again;
> - 
> +
>   /*
>* "Complex" conversion rules, implemented through recursion.
>*/
> @@ -380,7 +380,7 @@ literal:
>* number but without the century.
>*/
>   if (!(_conv_num(, , 0, 99)))
> - return (NULL);  
> + return (NULL);
>   continue;
>  
>   case 'G':   /* The year corresponding to the ISO week
> @@ -500,7 +500,7 @@ literal:
>   ep = _find_string(bp, , nast, NULL, 4);
>   if (ep != NULL) {
>  #ifdef TM_GMTOFF
> - tm->TM_GMTOFF = -5 - i;
> + tm->TM_GMTOFF = (-5 - i) * SECSPERHOUR;
>  #endif
>  #ifdef TM_ZONE
>   tm->TM_ZONE = (char *)nast[i];
> @@ -512,7 +512,7 @@ literal:
>   if (ep != NULL) {
>   tm->tm_isdst = 1;
>  #ifdef TM_GMTOFF
> - tm->TM_GMTOFF = -4 - i;
> + tm->TM_GMTOFF = (-4 - i) * SECSPERHOUR;
>  #endif
>  #ifdef TM_ZONE
>   tm->TM_ZONE = (char *)nadt[i];
> @@ -527,11 +527,12 @@ literal:
>   /* Argh! No 'J'! */
>   if (*bp >= 'A' && *bp <= 'I')
>   tm->TM_GMTOFF =
> - ('A' - 1) - (int)*bp;
> + (int)*bp - ('A' - 1);
>   else if (*bp >= 'L' && *bp <= 'M')
> - tm->TM_GMTOFF = 'A' - (int)*bp;
> + tm->TM_GMTOFF = (int)*bp - 'A';
>   else if (*bp >= 'N' && *bp <= 'Y')
> - tm->TM_GMTOFF = (int)*bp - 'M';
> + tm->TM_GMTOFF = 'M' - (int)*bp;
> + tm->TM_GMTOFF *= SECSPERHOUR;
>  #endif
>  #ifdef TM_ZONE
>   tm->TM_ZONE = NULL; /* XXX */
> @@ -556,14 +557,15 @@ literal:
>   }
>   switch (i) {
>

Re: nm(1): return on malloc error

2019-03-03 Thread Ingo Schwarze
Hi,

Benjamin Baier wrote on Sat, Mar 02, 2019 at 10:10:40AM +0100:

> On malloc error symtab is unmapped, so proceeding on will lead
> to a NULL pointer dereference.
> When malloc fails we should return like the MMAP case does.

Committed.

Thanks for the patch (and to those who checked it).
  Ingo


> Index: nm.c
> ===
> RCS file: /cvs/src/usr.bin/nm/nm.c,v
> retrieving revision 1.53
> diff -u -p -u -C10 -r1.53 nm.c
> *** nm.c  27 Oct 2017 16:47:08 -  1.53
> --- nm.c  20 Feb 2019 17:34:01 -
> *** show_symtab(off_t off, u_long len, const
> *** 374,393 
> --- 374,394 
>   restore = ftello(fp);
> 
>   MMAP(symtab, len, PROT_READ, MAP_PRIVATE|MAP_FILE, fileno(fp), off);
>   if (symtab == MAP_FAILED)
>   return (1);
> 
>   namelen = sizeof(ar_head.ar_name);
>   if ((p = malloc(sizeof(ar_head.ar_name))) == NULL) {
>   warn("%s: malloc", name);
>   MUNMAP(symtab, len);
> + return (1);
>   }
> 
>   printf("\nArchive index:\n");
>   num = betoh32(*symtab);
>   strtab = (char *)(symtab + num + 1);
>   for (ps = symtab + 1; num--; ps++, strtab += strlen(strtab) + 1) {
>   if (fseeko(fp, betoh32(*ps), SEEK_SET)) {
>   warn("%s: fseeko", name);
>   rval = 1;
>   break;
> 



Re: nm(1): return on malloc error

2019-03-03 Thread Otto Moerbeek
On Sun, Mar 03, 2019 at 04:23:53PM +0100, Ingo Schwarze wrote:

> Hi,
> 
> Benjamin Baier wrote on Sat, Mar 02, 2019 at 10:10:40AM +0100:
> 
> > On malloc error symtab is unmapped, so proceeding on will lead
> > to a NULL pointer dereference.
> > When malloc fails we should return like the MMAP case does.
> 
> while i'm certainly not experienced with nm(1), this change looks
> correct to me.  OK to commit?
> 
> Note that returning implies that the program might attempt to process
> further files, which is of dubious value: it is likely to fail, too,
> and in the unlikely case of success, that's maybe even worse: the
> output from subsequent files might cause the user to miss the error
> message about the malloc failure...  But given that mmap(2) failure
> already behaves like that, switching to just err(3) out on resource
> exhaustion looks like a larger change which i'm not planning to
> push for, even though it would make sense to me.
> 
> Here is the patch again, in standard format.

ok,

-Otto

> 
> Yours
>   Ingo
> 
> 
> Index: nm.c
> ===
> RCS file: /cvs/src/usr.bin/nm/nm.c,v
> retrieving revision 1.53
> diff -p -U8 -r1.53 nm.c
> --- nm.c  27 Oct 2017 16:47:08 -  1.53
> +++ nm.c  3 Mar 2019 15:12:28 -
> @@ -376,16 +376,17 @@ show_symtab(off_t off, u_long len, const
>   MMAP(symtab, len, PROT_READ, MAP_PRIVATE|MAP_FILE, fileno(fp), off);
>   if (symtab == MAP_FAILED)
>   return (1);
>  
>   namelen = sizeof(ar_head.ar_name);
>   if ((p = malloc(sizeof(ar_head.ar_name))) == NULL) {
>   warn("%s: malloc", name);
>   MUNMAP(symtab, len);
> + return (1);
>   }
>  
>   printf("\nArchive index:\n");
>   num = betoh32(*symtab);
>   strtab = (char *)(symtab + num + 1);
>   for (ps = symtab + 1; num--; ps++, strtab += strlen(strtab) + 1) {
>   if (fseeko(fp, betoh32(*ps), SEEK_SET)) {
>   warn("%s: fseeko", name);
> 



Re: nm(1): return on malloc error

2019-03-03 Thread Ingo Schwarze
Hi,

Benjamin Baier wrote on Sat, Mar 02, 2019 at 10:10:40AM +0100:

> On malloc error symtab is unmapped, so proceeding on will lead
> to a NULL pointer dereference.
> When malloc fails we should return like the MMAP case does.

while i'm certainly not experienced with nm(1), this change looks
correct to me.  OK to commit?

Note that returning implies that the program might attempt to process
further files, which is of dubious value: it is likely to fail, too,
and in the unlikely case of success, that's maybe even worse: the
output from subsequent files might cause the user to miss the error
message about the malloc failure...  But given that mmap(2) failure
already behaves like that, switching to just err(3) out on resource
exhaustion looks like a larger change which i'm not planning to
push for, even though it would make sense to me.

Here is the patch again, in standard format.

Yours
  Ingo


Index: nm.c
===
RCS file: /cvs/src/usr.bin/nm/nm.c,v
retrieving revision 1.53
diff -p -U8 -r1.53 nm.c
--- nm.c27 Oct 2017 16:47:08 -  1.53
+++ nm.c3 Mar 2019 15:12:28 -
@@ -376,16 +376,17 @@ show_symtab(off_t off, u_long len, const
MMAP(symtab, len, PROT_READ, MAP_PRIVATE|MAP_FILE, fileno(fp), off);
if (symtab == MAP_FAILED)
return (1);
 
namelen = sizeof(ar_head.ar_name);
if ((p = malloc(sizeof(ar_head.ar_name))) == NULL) {
warn("%s: malloc", name);
MUNMAP(symtab, len);
+   return (1);
}
 
printf("\nArchive index:\n");
num = betoh32(*symtab);
strtab = (char *)(symtab + num + 1);
for (ps = symtab + 1; num--; ps++, strtab += strlen(strtab) + 1) {
if (fseeko(fp, betoh32(*ps), SEEK_SET)) {
warn("%s: fseeko", name);



update xserver to version 1.19.7

2019-03-03 Thread Matthieu Herrb
Hi,

the patch below updates the X server to version 1.19.7. It's a bug-fix
release. You'll find the change log at the begining of the patch.

To test, apply the patch with patch -p0 -E in /usr/xenocara/xserver,
and then re build xenocara as documented in release(8).

The patch is also available at https://herrb.eu/xserver-1.19.7.diff

Test reports and/or Oks welcome,

Thanks,

Index: ChangeLog
===
RCS file: /cvs/OpenBSD/xenocara/xserver/ChangeLog,v
retrieving revision 1.31
diff -u -p -u -r1.31 ChangeLog
--- ChangeLog   18 Feb 2018 17:16:37 -  1.31
+++ ChangeLog   3 Mar 2019 09:25:45 -
@@ -1,3 +1,547 @@
+commit 937391523eef6459d1f8b1ae25fe7e1f77b8a12a
+Author: Kevin Brace 
+Date:   Sat Mar 2 14:13:20 2019 -0800
+
+xserver 1.19.7
+
+Signed-off-by: Kevin Brace 
+
+commit a93f8f74b54accfb94a8c56357e566db76c24b22
+Author: Kevin Brace 
+Date:   Sat Mar 2 14:10:41 2019 -0800
+
+Update configure.ac bug URL for gitlab migration
+
+It is based on Alan Coopersmith's commit for various fd.o projects.
+
+Signed-off-by: Kevin Brace 
+
+commit af63efe470417cde8a64068b1e6965b2677d92d9
+Author: Kevin Brace 
+Date:   Thu Dec 13 22:32:27 2018 -0600
+
+Add 24-bit color support to exaGetPixmapFirstPixel
+
+It appears that people who developed EXA forgot that there used to be
+graphics devices that used 24-bits (3 bytes) instead of 32-bits (4 bytes)
+in order to display one pixel. The lack of 24-bit color support inside
+exaGetPixmapFirstPixel causes SiS 6326 to crash when running Xfce since
+SiS 6326 does not support 32-bit color.
+
+Signed-off-by: Kevin Brace 
+
+commit 56547b196660e246e37132960723819972b99c8c
+Author: Mario Kleiner 
+Date:   Mon Feb 5 11:20:41 2018 +0100
+
+glx: Only assign 8 bpc fbconfigs for composite visuals.
+
+Commit 91c42093b248 ("glx: Duplicate relevant fbconfigs for
+compositing visuals") adds many new depth 32 fbconfigs as
+composite visuals. On a X-Screen running at depth 24, this
+also adds bgra 10-10-10-2 fbconigs, as they also have
+config.rgbBits == 32, but these are not displayable on a
+depth 24 screen, leading to visually corrupted desktops
+under some compositors, e.g., fdo bug 104597 "Compton
+weird colors" when running compton with
+"compton --backend glx".
+
+Be more conservative for now and only select fbconfigs with
+8 bpc red, green, blue components for composite visuals.
+
+Fixes: 91c42093b248 ("glx: Duplicate relevant fbconfigs for
+  compositing visuals")
+Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=104597
+Signed-off-by: Mario Kleiner 
+Reviewed-by: Thomas Hellstrom 
+Reviewed-by: Adam Jackson 
+(cherry picked from commit bebcc8477c8070ade9dd4be7299c718baeab3d7a)
+
+commit e96bd477395af3c2c3157ebda0f55ea4b672a114
+Author: Lyude Paul 
+Date:   Tue Feb 6 12:41:47 2018 -0500
+
+xwayland: Don't process cursor warping without an xwl_seat
+
+Unfortunately, on my machine Xwayland immediately crashes when I try to
+start it. gdb backtrace:
+
+ #0  0x774f0e79 in wl_proxy_marshal () from 
target:/lib64/libwayland-client.so.0
+ #1  0x00413172 in zwp_confined_pointer_v1_destroy 
(zwp_confined_pointer_v1=0x7)
+ at 
hw/xwayland/Xwayland@exe/pointer-constraints-unstable-v1-client-protocol.h:612
+ #2  0x00418bc0 in xwl_seat_destroy_confined_pointer 
(xwl_seat=0x8ba2a0)
+ at /home/lyudess/Projects/xserver/hw/xwayland/xwayland-input.c:2839
+ #3  0x00418c09 in xwl_seat_unconfine_pointer (xwl_seat=0x8ba2a0)
+ at /home/lyudess/Projects/xserver/hw/xwayland/xwayland-input.c:2849
+ #4  0x00410d97 in xwl_cursor_confined_to (device=0xa5a000, 
screen=0x8b9d80, window=0x9bdb70)
+ at /home/lyudess/Projects/xserver/hw/xwayland/xwayland.c:328
+ #5  0x004a8571 in ConfineCursorToWindow (pDev=0xa5a000, 
pWin=0x9bdb70, generateEvents=1,
+ confineToScreen=0) at /home/lyudess/Projects/xserver/dix/events.c:900
+ #6  0x004a94b7 in ScreenRestructured (pScreen=0x8b9d80)
+ at /home/lyudess/Projects/xserver/dix/events.c:1387
+ #7  0x00502386 in RRScreenSizeNotify (pScreen=0x8b9d80)
+ at /home/lyudess/Projects/xserver/randr/rrscreen.c:160
+ #8  0x0041a83c in update_screen_size (xwl_output=0x8e7670, 
width=3840, height=2160)
+ at /home/lyudess/Projects/xserver/hw/xwayland/xwayland-output.c:203
+ #9  0x0041a9f0 in apply_output_change (xwl_output=0x8e7670)
+ at /home/lyudess/Projects/xserver/hw/xwayland/xwayland-output.c:252
+ #10 0x0041aaeb in xdg_output_handle_done (data=0x8e7670, 
xdg_output=0x8e7580)
+ at /home/lyudess/Projects/xserver/hw/xwayland/xwayland-output.c:307
+ #11 0x750e9d1e in ffi_call_unix64 () at 

Re: fix bgpd unittests

2019-03-03 Thread Claudio Jeker
On Sun, Mar 03, 2019 at 07:12:19AM +0100, mb...@mbuhl.me wrote:
> Hi,
> the bgpd unittests are currently failing because the signature of sa2addr
> changed.
> 
> > Wed Feb 27 04:31:56 2019 UTC (4 days, 1 hour ago) by claudio 
> > Convert the remote and local addresses in struct peer to be bgpd_addrs
> > instead of sockaddr_storage. This again helps protability and simplifies
> > some code. sa2addr now takes an optional pointer to return the port of
> > the sockaddr.
> > OK benno@
> 
> Index: regress/usr.sbin/bgpd/unittests/rde_trie_test.c
> ===
> RCS file: 
> /mount/openbsd/cvs/src/regress/usr.sbin/bgpd/unittests/rde_trie_test.c,v
> retrieving revision 1.9
> diff -u -p -r1.9 rde_trie_test.c
> --- regress/usr.sbin/bgpd/unittests/rde_trie_test.c   1 Nov 2018 14:20:41 
> -   1.9
> +++ regress/usr.sbin/bgpd/unittests/rde_trie_test.c   3 Mar 2019 05:54:08 
> -
> @@ -48,7 +48,7 @@ host_ip(const char *s, struct bgpd_addr 
>   hints.ai_flags = AI_NUMERICHOST;
>   if (getaddrinfo(s, NULL, , ) == 0) {
>   *len = res->ai_family == AF_INET6 ? 128 : 32;
> - sa2addr(res->ai_addr, h);
> + sa2addr(res->ai_addr, h, 0);
>   freeaddrinfo(res);
>   } else {/* ie. for 10/8 parsing */
>   if ((bits = inet_net_pton(AF_INET, s, >v4, sizeof(h->v4))) 
> == -1)
> 

I committed this with NULL instead of 0. Since the third argument is a
pointer. Thanks for the report.

-- 
:wq Claudio