ipsec panic early

2017-05-11 Thread Alexander Bluhm 
Hi,

Instead of printing a debug message at the end, panic early if the
IPsec security protocol is unknown.

ok?

bluhm

Index: netinet/ipsec_input.c
===
RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/ipsec_input.c,v
retrieving revision 1.149
diff -u -p -r1.149 ipsec_input.c
--- netinet/ipsec_input.c   11 May 2017 12:14:43 -  1.149
+++ netinet/ipsec_input.c   11 May 2017 22:14:41 -
@@ -172,15 +172,22 @@ ipsec_common_input(struct mbuf *m, int s
}
 
/* Retrieve the SPI from the relevant IPsec header */
-   if (sproto == IPPROTO_ESP)
+   switch (sproto) {
+   case IPPROTO_ESP:
m_copydata(m, skip, sizeof(u_int32_t), (caddr_t) );
-   else if (sproto == IPPROTO_AH)
+   break;
+   case IPPROTO_AH:
m_copydata(m, skip + sizeof(u_int32_t), sizeof(u_int32_t),
(caddr_t) );
-   else if (sproto == IPPROTO_IPCOMP) {
+   break;
+   case IPPROTO_IPCOMP:
m_copydata(m, skip + sizeof(u_int16_t), sizeof(u_int16_t),
(caddr_t) );
spi = ntohl(htons(cpi));
+   break;
+   default:
+   panic("%s: unknown/unsupported security protocol %d",
+   __func__, sproto);
}
 
/*
@@ -526,7 +533,8 @@ ipsec_common_input_cb(struct mbuf *m, st
m_tag_prepend(m, mtag);
}
 
-   if (sproto == IPPROTO_ESP) {
+   switch (sproto) {
+   case IPPROTO_ESP:
/* Packet is confidential ? */
if (tdbp->tdb_encalgxform)
m->m_flags |= M_CONF;
@@ -534,10 +542,16 @@ ipsec_common_input_cb(struct mbuf *m, st
/* Check if we had authenticated ESP. */
if (tdbp->tdb_authalgxform)
m->m_flags |= M_AUTH;
-   } else if (sproto == IPPROTO_AH) {
+   break;
+   case IPPROTO_AH:
m->m_flags |= M_AUTH;
-   } else if (sproto == IPPROTO_IPCOMP) {
+   break;
+   case IPPROTO_IPCOMP:
m->m_flags |= M_COMP;
+   break;
+   default:
+   panic("%s: unknown/unsupported security protocol %d",
+   __func__, sproto);
}
 
 #if NPF > 0
@@ -566,18 +580,6 @@ ipsec_common_input_cb(struct mbuf *m, st
}
}
 #endif
-
-   switch (sproto) {
-   case IPPROTO_ESP:
-   case IPPROTO_AH:
-   case IPPROTO_IPCOMP:
-   break;
-   default:
-   DPRINTF(("ipsec_common_input_cb(): unknown/unsupported"
-   " security protocol %d\n", sproto));
-   m_freem(m);
-   return;
-   }
 
/* Call the appropriate IPsec transform callback. */
switch (af) {

[PATCH] typo in src/bin/ksh/README

2017-05-11 Thread Raf Czlonka 
Hi all,

I've got a small diff which fixes a typo in the README file.

While there, I have removed a hyphen from "PD-ksh" as it does not
appear in any of the other files.

Also, the first line has been changed on purpose to highlight the
fact that BOM[0] had been introduced here, most likely by accident,
in version 1.12 of the file[1].

[0] https://en.wikipedia.org/wiki/Byte_order_mark
[1] 
http://cvsweb.openbsd.org/cgi-bin/cvsweb/src/bin/ksh/README.diff?r1=1.11=1.12=h

Regards,

Raf

Index: bin/ksh/README
===
RCS file: /cvs/src/bin/ksh/README,v
retrieving revision 1.15
diff -u -p -r1.15 README
--- bin/ksh/README  5 Dec 2015 19:40:45 -   1.15
+++ bin/ksh/README  11 May 2017 16:00:03 -
@@ -1,13 +1,13 @@
-$OpenBSD: README,v 1.15 2015/12/05 19:40:45 mmcc Exp $
+$OpenBSD: README,v 1.15 2015/12/05 19:40:45 mmcc Exp $
 
 Last updated Jul '99 for pdksh-5.2.14.
 
-PD-ksh is a mostly complete AT ksh look-alike (see NOTES file for a list
+PDksh is a mostly complete AT ksh look-alike (see NOTES file for a list
 of things not supported).  Work is mostly finished to make it fully
 compatible with both POSIX and AT ksh (when the two don't conflict).
 
 PDksh was being maintained by Michael Rendell (mich...@cs.mun.ca),
-who took over from Simon J. Gerraty (s...@zen.void.oz.au) at the later's
+who took over from Simon J. Gerraty (s...@zen.void.oz.au) at the latter's
 suggestion.
 
 Files of interest:

Re: ksh(1): UTF-8 column count bug

2017-05-11 Thread Theo Buehler 
On Thu, May 11, 2017 at 03:36:17PM +0200, Ingo Schwarze wrote:
> Hi,
> 
> i checked Anton's analysis and believe it is completely correct.
> Any OKs to commit?

ok

Re: ksh(1): UTF-8 column count bug

2017-05-11 Thread Ingo Schwarze 
Hi,

i checked Anton's analysis and believe it is completely correct.
Any OKs to commit?

See below for details.

Yours,
  Ingo


Anton Lindqvist wrote on Thu, May 11, 2017 at 08:02:59AM +0200:

> I recently encountered a bug in ksh(1)'s emacs mode while running from
> xterm(1). How-to reproduce:
> 
> 1. Insert a sequence containing at least one UTF-8 character until the
>line exceeds the last column, triggering horizontal scroll.
> 
> 2. Moving backwards using any motion (^A, ^B, ^[b) fails when reaching
>the first visible character triggering backwards horizontal scroll.
> 
> It seems like the cause is due to how the column counter works.

You are right that the global variable "static int x_col" is the
current display column.

> Every byte in a UTF-8 character is assumed to occupy one column.
> The most reliable way to solve this is probably using wcwidth(3)
> but that would require extensive work.

Exactly, and not just here, but (at least) at all the places where
isu8cont() is currently used.

> However, the same logic found in the x_size function could instead be
> used in which a UTF-8 continuation byte is assumed to not occupy a
> column.

Exactly.  Right now, we only support single-width characters on
the command line.

> In the process of fixing this, I found another bug. This does only occur
> when the current line exceeds the number of columns and includes UTF-8
> characters. How-to reproduce:
> 
> 1. Use a 80 column wide terminal and ensure your prompt contains at
>least 3 characters:
> 
>PS1='xxx'
> 
> 2. Insert the following sequence, without the leading spaces:
> 
>ö a a a a a a a a a a a a a a a a a a a a a a a b b b b b b b b b b b b b 
> b b b z
> 
> 3. Press ^A (beginning-of-line), ^[f (forward-word), ^W (kill-region).
>At this point the prompt gets overwritten.
> 
> By invalidating xlp in the x_delete function prior calling x_zots in
> which x_lastcp is called causing xlp to be recalculated solves this.

Indeed.

The global variable "static char *xlp /* last byte visible on screen */"
is a pointer into the input buffer.  As long as you only delete a string
of ASCII characters before that point, the last position in the input 
buffer that fits on the screen does not change, even though the bytes
in that part of the buffer obviously change (they are shifted left).
The same bug can even be triggered by a simple ^D (x_del_char()),
you don't even need ^W.  What matters is that the line exceeds the
right marging of the screen.

For the fix, some might think it better to count the width of the
string being deleted and adjust xlp accordingly, which might arguably
require less computation.  But it would require handling a few
corner cases, so the code would be more complicated.  In particular,
there are complications if the deletion causes non-ASCII characters
that previously were outside the screen to move into the screen.
So, simply invalidating xlp looks like the right move; that pattern
is also used in other parts of the code.

Thanks for finding the two issues, understanding them, and providing
the patch!

> Index: emacs.c
> ===
> RCS file: /cvs/src/bin/ksh/emacs.c,v
> retrieving revision 1.66
> diff -u -p -r1.66 emacs.c
> --- emacs.c   9 Aug 2016 11:04:46 -   1.66
> +++ emacs.c   9 May 2017 17:52:37 -
> @@ -533,6 +533,7 @@ x_delete(int nc, int push)
>   }
>   memmove(xcp, xcp+nc, xep - xcp + 1);/* Copies the null */
>   x_adj_ok = 0;   /* don't redraw */
> + xlp_valid = false;
>   x_zots(xcp);
>   /*
>* if we are already filling the line,
> @@ -1909,7 +1910,8 @@ x_e_putc(int c)
>   x_col--;
>   break;
>   default:
> - x_col++;
> + if (!isu8cont(c))
> + x_col++;
>   break;
>   }
>   }

Re: IPsec forward policy check in ip6_input

2017-05-11 Thread Alexander Bluhm 
On Thu, May 11, 2017 at 01:36:51PM +0200, Mike Belopuhov wrote:
> Maybe we should move ip_input_ipsec_fwd_check into the ipsec_input.c
> and give it a better name like ipsec_forward_check?  This function
> doesn't do any IPv4 or IPv6 specific dances anyways.

There are more such functions:

ip_output_ipsec_lookup
ip_output_ipsec_send
ip6_output_ipsec_lookup
ip6_output_ipsec_send
ip_input_ipsec_fwd_check
ip_input_ipsec_ours_check

Some of them can be made independent of the address family.
Then we can move all of them to a better home.

bluhm

> But I agree with you in principle.

So would like commit this step.  ip_input_ipsec_ours_check() is
next on my list.  Currently IPv6 is less strict than IPv4.  But
tcp_input() and udp_input() do the same check gain, but without
calling a function.  That is the rason why tcp, tcp6, udp, udp6
behave like ping, but ping6 is different.

bluhm

Re: [PATCH] Clean up obsolete information in gettimeofday.2

2017-05-11 Thread Todd C. Miller 
On Sat, 29 Apr 2017 23:05:45 +0900, Bryan Linton wrote:

> Is this worth deleting now that struct timezone *tzp is no longer
> used?

Actually, gettimeofday(2) will fill in struct timezone *tzp if it
is non-NULL.  It is still possible to set the time zone via the
TIMEZONE kernel option.  However, it is only used when the system's
real time clock is not set to UTC.  See options(4) for info.

It is also possible to set the real time clock's UTC offset via
settimeofday(2).

I'll admit that this is not documented very well.

 - todd

Re: IPsec forward policy check in ip6_input

2017-05-11 Thread Mike Belopuhov 
On Thu, May 11, 2017 at 13:11 +0200, Alexander Bluhm wrote:
> Hi,
> 
> ipv4_input() checks the IPsec policy for forwarding and local
> delivery.  Such code is missing in IPv6, the behavior is different.
> 
> Start using the forwarding check also in ip6_input().  While there
> avoid an ugly #ifdef in ipv4_input().
> 
> ok?
>

Maybe we should move ip_input_ipsec_fwd_check into the ipsec_input.c
and give it a better name like ipsec_forward_check?  This function
doesn't do any IPv4 or IPv6 specific dances anyways.

But I agree with you in principle.

> bluhm
> 
> Index: netinet/ip_input.c
> ===
> RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/ip_input.c,v
> retrieving revision 1.298
> diff -u -p -r1.298 ip_input.c
> --- netinet/ip_input.c19 Apr 2017 15:21:54 -  1.298
> +++ netinet/ip_input.c10 May 2017 23:55:42 -
> @@ -130,7 +130,6 @@ void  ip_ours(struct mbuf *);
>  int  ip_dooptions(struct mbuf *, struct ifnet *);
>  int  in_ouraddr(struct mbuf *, struct ifnet *, struct rtentry **);
>  #ifdef IPSEC
> -int  ip_input_ipsec_fwd_check(struct mbuf *, int);
>  int  ip_input_ipsec_ours_check(struct mbuf *, int);
>  #endif /* IPSEC */
>   
> @@ -241,9 +240,6 @@ ipv4_input(struct mbuf *m)
>   struct rtentry  *rt = NULL;
>   struct ip   *ip;
>   int hlen, len;
> -#if defined(MROUTING) || defined(IPSEC)
> - int rv;
> -#endif
>   in_addr_t pfrdr = 0;
>  
>   ifp = if_get(m->m_pkthdr.ph_ifidx);
> @@ -377,6 +373,8 @@ ipv4_input(struct mbuf *m)
>  
>  #ifdef MROUTING
>   if (ipmforwarding && ip_mrouter[ifp->if_rdomain]) {
> + int rv;
> +
>   if (m->m_flags & M_EXT) {
>   if ((m = m_pullup(m, hlen)) == NULL) {
>   ipstat_inc(ips_toosmall);
> @@ -444,8 +442,10 @@ ipv4_input(struct mbuf *m)
>   }
>  #ifdef IPSEC
>   if (ipsec_in_use) {
> + int rv;
> +
>   KERNEL_LOCK();
> - rv = ip_input_ipsec_fwd_check(m, hlen);
> + rv = ip_input_ipsec_fwd_check(m, hlen, AF_INET);
>   KERNEL_UNLOCK();
>   if (rv != 0) {
>   ipstat_inc(ips_cantforward);
> @@ -675,7 +675,7 @@ in_ouraddr(struct mbuf *m, struct ifnet 
>  
>  #ifdef IPSEC
>  int
> -ip_input_ipsec_fwd_check(struct mbuf *m, int hlen)
> +ip_input_ipsec_fwd_check(struct mbuf *m, int hlen, int af)
>  {
>   struct tdb *tdb;
>   struct tdb_ident *tdbi;
> @@ -692,8 +692,7 @@ ip_input_ipsec_fwd_check(struct mbuf *m,
>   tdb = gettdb(tdbi->rdomain, tdbi->spi, >dst, tdbi->proto);
>   } else
>   tdb = NULL;
> - ipsp_spd_lookup(m, AF_INET, hlen, , IPSP_DIRECTION_IN, tdb, NULL,
> - 0);
> + ipsp_spd_lookup(m, af, hlen, , IPSP_DIRECTION_IN, tdb, NULL, 0);
>  
>   return error;
>  }
> Index: netinet/ip_var.h
> ===
> RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/ip_var.h,v
> retrieving revision 1.71
> diff -u -p -r1.71 ip_var.h
> --- netinet/ip_var.h  14 Apr 2017 20:46:31 -  1.71
> +++ netinet/ip_var.h  10 May 2017 23:12:25 -
> @@ -250,6 +250,7 @@ void   ip_savecontrol(struct inpcb *, str
>  void  ipintr(void);
>  void  ipv4_input(struct mbuf *);
>  void  ip_forward(struct mbuf *, struct ifnet *, struct rtentry *, int);
> +int   ip_input_ipsec_fwd_check(struct mbuf *, int, int);
>  int   rip_ctloutput(int, struct socket *, int, int, struct mbuf *);
>  void  rip_init(void);
>  int   rip_input(struct mbuf **, int *, int, int);
> Index: netinet6/ip6_input.c
> ===
> RCS file: /data/mirror/openbsd/cvs/src/sys/netinet6/ip6_input.c,v
> retrieving revision 1.184
> diff -u -p -r1.184 ip6_input.c
> --- netinet6/ip6_input.c  8 May 2017 08:46:39 -   1.184
> +++ netinet6/ip6_input.c  10 May 2017 23:21:00 -
> @@ -470,6 +470,24 @@ ip6_input(struct mbuf *m)
>   goto out;
>   }
>  
> +#ifdef IPSEC
> + if (ipsec_in_use) {
> + int rv;
> +
> + KERNEL_LOCK();
> + rv = ip_input_ipsec_fwd_check(m, off, AF_INET6);
> + KERNEL_UNLOCK();
> + if (rv != 0) {
> + ipstat_inc(ips_cantforward);
> + goto bad;
> + }
> + /*
> +  * Fall through, forward packet. Outbound IPsec policy
> +  * checking will occur in ip6_forward().
> +  */
> + }
> +#endif /* IPSEC */
> +
>   ip6_forward(m, rt, srcrt);
>   if_put(ifp);
>   return;
>

Re: IPv6 IPsec transport pf

2017-05-11 Thread Mike Belopuhov 
On Mon, May 08, 2017 at 20:22 +0200, Alexander Bluhm wrote:
> Hi,
> 
> IPv6 IPsec transport mode does not work if pf is enabled.  The
> problem is that the decrypted packets in the input path are not
> checked with pf(4).  So if you have stateful filtering on enc0 (the
> default) direction aware protocols like ping or TCP do not pass.
> Only the output packets are matched with the states.
> 
> Adding an explicit pf_test() in ipsec_common_input_cb() fixes this.
> In the IPv4 case the decrypted packet is enqueued again, so it hits
> pf_test() in IPv4_input().  In IPv6 the ip6_local() shortcut is
> taken.  I like to keep the shortcut as it corresponds to the idea
> of IPv6 header chains.
> 
> ok?
> 
> bluhm
> 

Looks good to me.  Can you please put a comment in there saying
that this is done for ipv6 only due to the ip6_local shortcut.
With that OK mikeb

Fix comment into sys/dev/acpi/acpibtn.c

2017-05-11 Thread David Coppa 

I think this comment was copy-pasted as is from the comment some
lines below, but this is about hibernation, not sleep.

Ok?

Index: acpibtn.c
===
RCS file: /cvs/src/sys/dev/acpi/acpibtn.c,v
retrieving revision 1.44
diff -u -p -u -p -r1.44 acpibtn.c
--- acpibtn.c   2 Mar 2017 10:38:10 -   1.44
+++ acpibtn.c   11 May 2017 11:10:21 -
@@ -236,7 +236,7 @@ acpibtn_notify(struct aml_node *node, in
goto sleep;
 #ifdef HIBERNATE
case 2:
-   /* Request to go to sleep */
+   /* Request hibernation */
if (acpi_record_event(sc->sc_acpi, 
APM_USER_HIBERNATE_REQ))
acpi_addtask(sc->sc_acpi, acpi_sleep_task,
sc->sc_acpi, ACPI_SLEEP_HIBERNATE);

IPsec forward policy check in ip6_input

2017-05-11 Thread Alexander Bluhm 
Hi,

ipv4_input() checks the IPsec policy for forwarding and local
delivery.  Such code is missing in IPv6, the behavior is different.

Start using the forwarding check also in ip6_input().  While there
avoid an ugly #ifdef in ipv4_input().

ok?

bluhm

Index: netinet/ip_input.c
===
RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/ip_input.c,v
retrieving revision 1.298
diff -u -p -r1.298 ip_input.c
--- netinet/ip_input.c  19 Apr 2017 15:21:54 -  1.298
+++ netinet/ip_input.c  10 May 2017 23:55:42 -
@@ -130,7 +130,6 @@ voidip_ours(struct mbuf *);
 intip_dooptions(struct mbuf *, struct ifnet *);
 intin_ouraddr(struct mbuf *, struct ifnet *, struct rtentry **);
 #ifdef IPSEC
-intip_input_ipsec_fwd_check(struct mbuf *, int);
 intip_input_ipsec_ours_check(struct mbuf *, int);
 #endif /* IPSEC */

@@ -241,9 +240,6 @@ ipv4_input(struct mbuf *m)
struct rtentry  *rt = NULL;
struct ip   *ip;
int hlen, len;
-#if defined(MROUTING) || defined(IPSEC)
-   int rv;
-#endif
in_addr_t pfrdr = 0;
 
ifp = if_get(m->m_pkthdr.ph_ifidx);
@@ -377,6 +373,8 @@ ipv4_input(struct mbuf *m)
 
 #ifdef MROUTING
if (ipmforwarding && ip_mrouter[ifp->if_rdomain]) {
+   int rv;
+
if (m->m_flags & M_EXT) {
if ((m = m_pullup(m, hlen)) == NULL) {
ipstat_inc(ips_toosmall);
@@ -444,8 +442,10 @@ ipv4_input(struct mbuf *m)
}
 #ifdef IPSEC
if (ipsec_in_use) {
+   int rv;
+
KERNEL_LOCK();
-   rv = ip_input_ipsec_fwd_check(m, hlen);
+   rv = ip_input_ipsec_fwd_check(m, hlen, AF_INET);
KERNEL_UNLOCK();
if (rv != 0) {
ipstat_inc(ips_cantforward);
@@ -675,7 +675,7 @@ in_ouraddr(struct mbuf *m, struct ifnet 
 
 #ifdef IPSEC
 int
-ip_input_ipsec_fwd_check(struct mbuf *m, int hlen)
+ip_input_ipsec_fwd_check(struct mbuf *m, int hlen, int af)
 {
struct tdb *tdb;
struct tdb_ident *tdbi;
@@ -692,8 +692,7 @@ ip_input_ipsec_fwd_check(struct mbuf *m,
tdb = gettdb(tdbi->rdomain, tdbi->spi, >dst, tdbi->proto);
} else
tdb = NULL;
-   ipsp_spd_lookup(m, AF_INET, hlen, , IPSP_DIRECTION_IN, tdb, NULL,
-   0);
+   ipsp_spd_lookup(m, af, hlen, , IPSP_DIRECTION_IN, tdb, NULL, 0);
 
return error;
 }
Index: netinet/ip_var.h
===
RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/ip_var.h,v
retrieving revision 1.71
diff -u -p -r1.71 ip_var.h
--- netinet/ip_var.h14 Apr 2017 20:46:31 -  1.71
+++ netinet/ip_var.h10 May 2017 23:12:25 -
@@ -250,6 +250,7 @@ void ip_savecontrol(struct inpcb *, str
 voidipintr(void);
 voidipv4_input(struct mbuf *);
 voidip_forward(struct mbuf *, struct ifnet *, struct rtentry *, int);
+int ip_input_ipsec_fwd_check(struct mbuf *, int, int);
 int rip_ctloutput(int, struct socket *, int, int, struct mbuf *);
 voidrip_init(void);
 int rip_input(struct mbuf **, int *, int, int);
Index: netinet6/ip6_input.c
===
RCS file: /data/mirror/openbsd/cvs/src/sys/netinet6/ip6_input.c,v
retrieving revision 1.184
diff -u -p -r1.184 ip6_input.c
--- netinet6/ip6_input.c8 May 2017 08:46:39 -   1.184
+++ netinet6/ip6_input.c10 May 2017 23:21:00 -
@@ -470,6 +470,24 @@ ip6_input(struct mbuf *m)
goto out;
}
 
+#ifdef IPSEC
+   if (ipsec_in_use) {
+   int rv;
+
+   KERNEL_LOCK();
+   rv = ip_input_ipsec_fwd_check(m, off, AF_INET6);
+   KERNEL_UNLOCK();
+   if (rv != 0) {
+   ipstat_inc(ips_cantforward);
+   goto bad;
+   }
+   /*
+* Fall through, forward packet. Outbound IPsec policy
+* checking will occur in ip6_forward().
+*/
+   }
+#endif /* IPSEC */
+
ip6_forward(m, rt, srcrt);
if_put(ifp);
return;

Convert explicit_bzero+free to freezero on smtpd(8)

2017-05-11 Thread Ricardo Mestre 
Hi,

This converts explicit_bzero+free to freezero on smtpd(8).

OK?

Index: ca.c
===
RCS file: /cvs/src/usr.sbin/smtpd/ca.c,v
retrieving revision 1.26
diff -u -p -u -r1.26 ca.c
--- ca.c9 Jan 2017 09:53:23 -   1.26
+++ ca.c11 May 2017 10:16:47 -
@@ -142,8 +142,7 @@ ca_init(void)
 
pki->pki_pkey = pkey;
 
-   explicit_bzero(pki->pki_key, pki->pki_key_len);
-   free(pki->pki_key);
+   freezero(pki->pki_key, pki->pki_key_len);
pki->pki_key = NULL;
}
 }
Index: config.c
===
RCS file: /cvs/src/usr.sbin/smtpd/config.c,v
retrieving revision 1.37
diff -u -p -u -r1.37 config.c
--- config.c1 Sep 2016 10:54:25 -   1.37
+++ config.c11 May 2017 10:16:48 -
@@ -70,12 +70,8 @@ purge_config(uint8_t what)
}
if (what & PURGE_PKI) {
while (dict_poproot(env->sc_pki_dict, (void **))) {
-   explicit_bzero(p->pki_cert, p->pki_cert_len);
-   free(p->pki_cert);
-   if (p->pki_key) {
-   explicit_bzero(p->pki_key, p->pki_key_len);
-   free(p->pki_key);
-   }
+   freezero(p->pki_cert, p->pki_cert_len);
+   freezero(p->pki_key, p->pki_key_len);
if (p->pki_pkey)
EVP_PKEY_free(p->pki_pkey);
free(p);
@@ -86,14 +82,10 @@ purge_config(uint8_t what)
iter_dict = NULL;
while (dict_iter(env->sc_pki_dict, _dict, ,
(void **))) {
-   explicit_bzero(p->pki_cert, p->pki_cert_len);
-   free(p->pki_cert);
+   freezero(p->pki_cert, p->pki_cert_len);
p->pki_cert = NULL;
-   if (p->pki_key) {
-   explicit_bzero(p->pki_key, p->pki_key_len);
-   free(p->pki_key);
-   p->pki_key = NULL;
-   }
+   freezero(p->pki_key, p->pki_key_len);
+   p->pki_key = NULL;
if (p->pki_pkey)
EVP_PKEY_free(p->pki_pkey);
p->pki_pkey = NULL;
Index: mta_session.c
===
RCS file: /cvs/src/usr.sbin/smtpd/mta_session.c,v
retrieving revision 1.96
diff -u -p -u -r1.96 mta_session.c
--- mta_session.c   30 Nov 2016 17:43:32 -  1.96
+++ mta_session.c   11 May 2017 10:16:50 -
@@ -341,8 +341,7 @@ mta_session_imsg(struct mproc *p, struct
fatal("mta: ssl_mta_init");
io_start_tls(s->io, ssl);
 
-   explicit_bzero(resp_ca_cert->cert, resp_ca_cert->cert_len);
-   free(resp_ca_cert->cert);
+   freezero(resp_ca_cert->cert, resp_ca_cert->cert_len);
free(resp_ca_cert);
return;
 
Index: smtp_session.c
===
RCS file: /cvs/src/usr.sbin/smtpd/smtp_session.c,v
retrieving revision 1.302
diff -u -p -u -r1.302 smtp_session.c
--- smtp_session.c  30 Nov 2016 17:43:32 -  1.302
+++ smtp_session.c  11 May 2017 10:16:54 -
@@ -962,8 +962,7 @@ smtp_session_imsg(struct mproc *p, struc
io_set_read(s->io);
io_start_tls(s->io, ssl);
 
-   explicit_bzero(resp_ca_cert->cert, resp_ca_cert->cert_len);
-   free(resp_ca_cert->cert);
+   freezero(resp_ca_cert->cert, resp_ca_cert->cert_len);
free(resp_ca_cert);
return;

Re: Atomic copyin(9)/copyout(9) for amd64

2017-05-11 Thread Mark Kettenis 
> Date: Tue, 2 May 2017 15:52:56 +
> From: Visa Hankala 
> 
> On Mon, May 01, 2017 at 06:02:24PM +0200, Mark Kettenis wrote:
> > The futex(2) syscall needs to be able to atomically copy the futex in
> > and out of userland.  The current implementation uses copyin(9) and
> > copyout(9) for that.  The futex is a 32-bit integer, and currently our
> > copyin(9) and copyout(9) don't guarantee an atomic 32-bit access.
> > Previously mpi@ and I discussed implementing new interfaces that do
> > guarantee the required atomicity.  However, it oocurred to me that we
> > could simply change our copyin implementations such that they
> > guarantee atomicity of a properly aligned 32-bit copyin and copyout.
> > 
> > The i386 version of these calls uses "rep movsl", which means it is
> > already atomic.  At least that is how I interpret 8.2.4 in Volume 3A
> > of the Intel SDM.  The diff below makes the amd64 version safe as
> > well.  This does introduce a few additional instructions in the loop.
> > Apparently modern Intel CPUs optimize the string loops.  If we can
> > rely on the hardware to turn 32-bit moves into 64-bit moves, we could
> > simplify the code by using "rep movsl" instead of "rep movsq".
> > 
> > Thoughts?
> 
> I would add separate routines for atomic copying because they should
> fail if atomicity cannot be guaranteed. copyin(9) and copyout(9) do
> not catch memory aligment issues, for example. Forcing them to handle
> a special case like this does not look nice.

Hmm, that's not a very convincing argument.  Normal loads and stores
are only guaranteed to be atomic if they happend to be properly
aligned as well.

ksh(1): UTF-8 column count bug

2017-05-11 Thread Anton Lindqvist 
Hi,
I recently encountered a bug in ksh(1)'s emacs mode while running from
xterm(1). How-to reproduce:

1. Insert a sequence containing at least one UTF-8 character until the
   line exceeds the last column, triggering horizontal scroll.

2. Moving backwards using any motion (^A, ^B, ^[b) fails when reaching
   the first visible character triggering backwards horizontal scroll.

It seems like the cause is due to how the column counter works. Every
byte in a UTF-8 character is assumed to occupy one column. The most
reliable way to solve this is probably using wcwidth(3) but that would
require extensive work. However, the same logic found in the x_size
function could instead be used in which a UTF-8 continuation byte is
assumed to not occupy a column.

In the process of fixing this, I found another bug. This does only occur
when the current line exceeds the number of columns and includes UTF-8
characters. How-to reproduce:

1. Use a 80 column wide terminal and ensure your prompt contains at
   least 3 characters:

   PS1='xxx'

2. Insert the following sequence, without the leading spaces:

   ö a a a a a a a a a a a a a a a a a a a a a a a b b b b b b b b b b b b b b 
b b z

3. Press ^A (beginning-of-line), ^[f (forward-word), ^W (kill-region).
   At this point the prompt gets overwritten.

By invalidating xlp in the x_delete function prior calling x_zots in
which x_lastcp is called causing xlp to be recalculated solves this.

Comments and feedback are much appreciated.

Index: emacs.c
===
RCS file: /cvs/src/bin/ksh/emacs.c,v
retrieving revision 1.66
diff -u -p -r1.66 emacs.c
--- emacs.c 9 Aug 2016 11:04:46 -   1.66
+++ emacs.c 9 May 2017 17:52:37 -
@@ -533,6 +533,7 @@ x_delete(int nc, int push)
}
memmove(xcp, xcp+nc, xep - xcp + 1);/* Copies the null */
x_adj_ok = 0;   /* don't redraw */
+   xlp_valid = false;
x_zots(xcp);
/*
 * if we are already filling the line,
@@ -1909,7 +1910,8 @@ x_e_putc(int c)
x_col--;
break;
default:
-   x_col++;
+   if (!isu8cont(c))
+   x_col++;
break;
}
}