Re: malloc.conf.5: minor correction

2017-12-29 Thread Otto Moerbeek
On Sat, Dec 30, 2017 at 06:16:44AM +, kshe wrote:

> Hi,
> 
> The `F' option no longer disables delayed freeing.
> 
> Also, I think documenting implementation details like the exact value
> of the junk bytes is not very useful.

When you are debugging, it is good to see if a buffer is filled with
0xdb (not inited) or 0xdf (use after free). So I only took the F part.

-Otto


> 
> Index: malloc.conf.5
> ===
> RCS file: /cvs/src/share/man/man5/malloc.conf.5,v
> retrieving revision 1.12
> diff -u -p -r1.12 malloc.conf.5
> --- malloc.conf.5 23 Sep 2017 15:13:34 -  1.12
> +++ malloc.conf.5 28 Dec 2017 20:56:27 -
> @@ -73,13 +73,10 @@ Increase the junk level by one if it is 
>  .Dq Less junking .
>  Decrease the junk level by one if it is larger than 0.
>  Junking writes some junk bytes into the area allocated.
> -Currently junk is bytes of 0xdb when allocating;
> -freed chunks are filled with 0xdf.
>  By default the junk level is 1: small chunks are always junked
>  and the first part of pages is junked after free.
> -After a delay (if not switched off by the F option),
> -the filling pattern is validated and the process is aborted if the pattern
> -was modified.
> +After a delay, the filling pattern is validated
> +and the process is aborted if the pattern was modified.
>  If the junk level is zero, no junking is performed.
>  For junk level 2, junking is done without size restrictions.
>  .It Cm R
> 
> Regards,
> 
> kshe



Re: sh.1: backslash within double quotes

2017-12-29 Thread Jason McIntyre
On Sat, Dec 30, 2017 at 06:01:56AM +, kshe wrote:
> Hi,
> 
> Within double quotes, backslashes also escape newlines.  This is
> correctly documented in ksh.1, but not in sh.1.
> 

fixed, thanks.
jmc

> Index: sh.1
> ===
> RCS file: /cvs/src/bin/ksh/sh.1,v
> retrieving revision 1.145
> diff -u -p -r1.145 sh.1
> --- sh.1  15 Dec 2017 20:51:28 -  1.145
> +++ sh.1  28 Dec 2017 20:44:07 -
> @@ -1165,7 +1165,7 @@ if the user wants to indicate to the she
>  The following characters need quoting if their literal meaning is desired:
>  .Bd -literal -offset indent
>  | & ; < > ( ) $ \` \e " \(aq   
> -* ?  [ # ~ = %
> +* ? [ # ~ = %
>  .Ed
>  .Pp
>  A backslash
> @@ -1190,7 +1190,7 @@ A backslash
>  .Pq \e
>  within double quotes retains its special meaning,
>  but only when followed by a backquote, dollar sign,
> -double quote, or another backslash.
> +double quote, newline, or another backslash.
>  An at sign
>  .Pq @
>  within double quotes has a special meaning
> 
> Regards,
> 
> kshe
> 



Re: jot(1): one-byte overflow in error path

2017-12-29 Thread Theo Buehler
On Sat, Dec 30, 2017 at 05:55:57AM +, kshe wrote:
> Hi,
> 
> If the format string ends in an invalid specifier like `%l', p will
> already point to the trailing NUL upon entering the switch, wherein the
> instruction
> 
>   *++p = '\0';
> 
> will write another NUL after it, but there is no guarantee that the
> buffer extends beyond that first NUL; thus, in the rare case where it
> does not, this assignment will write one byte past its end.
> 
> While here, this diff also simplifies the said switch by removing some
> unneeded cases.

committed, thank you!



malloc.conf.5: minor correction

2017-12-29 Thread kshe
Hi,

The `F' option no longer disables delayed freeing.

Also, I think documenting implementation details like the exact value
of the junk bytes is not very useful.

Index: malloc.conf.5
===
RCS file: /cvs/src/share/man/man5/malloc.conf.5,v
retrieving revision 1.12
diff -u -p -r1.12 malloc.conf.5
--- malloc.conf.5   23 Sep 2017 15:13:34 -  1.12
+++ malloc.conf.5   28 Dec 2017 20:56:27 -
@@ -73,13 +73,10 @@ Increase the junk level by one if it is 
 .Dq Less junking .
 Decrease the junk level by one if it is larger than 0.
 Junking writes some junk bytes into the area allocated.
-Currently junk is bytes of 0xdb when allocating;
-freed chunks are filled with 0xdf.
 By default the junk level is 1: small chunks are always junked
 and the first part of pages is junked after free.
-After a delay (if not switched off by the F option),
-the filling pattern is validated and the process is aborted if the pattern
-was modified.
+After a delay, the filling pattern is validated
+and the process is aborted if the pattern was modified.
 If the junk level is zero, no junking is performed.
 For junk level 2, junking is done without size restrictions.
 .It Cm R

Regards,

kshe



Re: malloc cleanup and small optimization (step 2)

2017-12-29 Thread kshe
Hi,

Looking at this diff and the previous one, I found some more possible
cleanups for malloc.c (the patch below is to be applied after both of
them, even if the second one has not been committed yet):

1.  In malloc_bytes(), use ffs(3) instead of manual loops, which on many
architectures boils down to merely one or two instructions (for example,
see src/lib/libc/arch/amd64/string/ffs.S).

2.  Slightly reorder find_chunksize() to avoid checking twice for the
same condition.

3.  Remove the outdated comment above omalloc_poolinit().

4.  Remove extra braces enclosing a switch case, made unnecessary by
revision 1.233.

5.  Some miscellaneous style and whitespace fixes.

--- malloc.c.orig   Fri Dec 29 00:06:24 2017
+++ malloc.cFri Dec 29 06:02:04 2017
@@ -376,12 +376,11 @@ omalloc_parseopt(char opt)
case 'X':
mopts.malloc_xmalloc = 1;
break;
-   default: {
+   default:
dprintf(STDERR_FILENO, "malloc() warning: "
-"unknown char in MALLOC_OPTIONS\n");
+   "unknown char in MALLOC_OPTIONS\n");
break;
}
-   }
 }
 
 static void
@@ -448,9 +447,6 @@ omalloc_init(void)
;
 }
 
-/*
- * Initialize a dir_info, which should have been cleared by caller
- */
 static void
 omalloc_poolinit(struct dir_info **dp)
 {
@@ -502,7 +498,7 @@ omalloc_grow(struct dir_info *d)
size_t i;
struct region_info *p;
 
-   if (d->regions_total > SIZE_MAX / sizeof(struct region_info) / 2 )
+   if (d->regions_total > SIZE_MAX / sizeof(struct region_info) / 2)
return 1;
 
newtotal = d->regions_total * 2;
@@ -828,7 +824,7 @@ alloc_chunk_info(struct dir_info *d, int bits)
return NULL;
STATS_ADD(d->malloc_used, MALLOC_PAGESIZE);
count = MALLOC_PAGESIZE / size;
-   
+
if (bits == 0) {
shift = 1;
i = MALLOC_MINSIZE - 1;
@@ -903,23 +899,21 @@ err:
 static int
 find_chunksize(size_t size)
 {
-   int i, j;
+   int r;
 
-   /* Don't bother with anything less than this */
-   /* unless we have a malloc(0) requests */
-   if (size != 0 && size < MALLOC_MINSIZE)
+   /* malloc(0) is special */
+   if (size == 0)
+   return 0;
+
+   if (size < MALLOC_MINSIZE)
size = MALLOC_MINSIZE;
+   size--;
 
-   /* Find the right bucket */
-   if (size == 0)
-   j = 0;
-   else {
-   j = MALLOC_MINSHIFT;
-   i = (size - 1) >> (MALLOC_MINSHIFT - 1);
-   while (i >>= 1)
-   j++;
-   }
-   return j;
+   r = MALLOC_MINSHIFT;
+   while (size >> r != 0)
+   r++;
+
+   return r;
 }
 
 static void
@@ -941,7 +935,7 @@ malloc_bytes(struct dir_info *d, size_t size, void *f)
u_int i, r;
int j, listnum;
size_t k;
-   u_short u, b, *lp;
+   u_short *lp;
struct chunk_info *bp;
void *p;
 
@@ -970,15 +964,12 @@ malloc_bytes(struct dir_info *d, size_t size, void *f)
/* start somewhere in a short */
lp = >bits[i / MALLOC_BITS];
if (*lp) {
-   b = *lp;
-   k = i % MALLOC_BITS;
-   u = 1 << k;
-   while (k < MALLOC_BITS) {
-   if (b & u)
-   goto found;
-   k++;
-   u <<= 1;
-   } 
+   j = i % MALLOC_BITS;
+   k = ffs(*lp >> j);
+   if (k != 0) {
+   k += j - 1;
+   goto found;
+   }
}
/* no bit halfway, go to next full short */
i /= MALLOC_BITS;
@@ -986,16 +977,10 @@ malloc_bytes(struct dir_info *d, size_t size, void *f)
if (++i >= bp->total / MALLOC_BITS)
i = 0;
lp = >bits[i];
-   if (*lp) {
-   b = *lp;
-   k = 0;
-   u = 1;
-   for (;;) {
-   if (b & u)
-   goto found;
-   k++;
-   u <<= 1;
-   }
+   k = ffs(*lp);
+   if (k != 0) {
+   k--;
+   break;
}
}
 found:
@@ -1006,10 +991,10 @@ found:
}
 #endif
 
-   *lp ^= u;
+   *lp ^= 1 << k;
 
/* If there are no more free, remove from free-list */
-   if (!--bp->free)
+   if (--bp->free == 0)
LIST_REMOVE(bp, entries);
 
/* Adjust to the real offset of that chunk */
@@ -1045,7 +1030,7 @@ validate_canary(struct dir_info *d, u_char *ptr, size_
if (*p != SOME_JUNK) {
  

sh.1: backslash within double quotes

2017-12-29 Thread kshe
Hi,

Within double quotes, backslashes also escape newlines.  This is
correctly documented in ksh.1, but not in sh.1.

Index: sh.1
===
RCS file: /cvs/src/bin/ksh/sh.1,v
retrieving revision 1.145
diff -u -p -r1.145 sh.1
--- sh.115 Dec 2017 20:51:28 -  1.145
+++ sh.128 Dec 2017 20:44:07 -
@@ -1165,7 +1165,7 @@ if the user wants to indicate to the she
 The following characters need quoting if their literal meaning is desired:
 .Bd -literal -offset indent
 | & ; < > ( ) $ \` \e " \(aq   
-* ?  [ # ~ = %
+* ? [ # ~ = %
 .Ed
 .Pp
 A backslash
@@ -1190,7 +1190,7 @@ A backslash
 .Pq \e
 within double quotes retains its special meaning,
 but only when followed by a backquote, dollar sign,
-double quote, or another backslash.
+double quote, newline, or another backslash.
 An at sign
 .Pq @
 within double quotes has a special meaning

Regards,

kshe



jot(1): one-byte overflow in error path

2017-12-29 Thread kshe
Hi,

If the format string ends in an invalid specifier like `%l', p will
already point to the trailing NUL upon entering the switch, wherein the
instruction

*++p = '\0';

will write another NUL after it, but there is no guarantee that the
buffer extends beyond that first NUL; thus, in the rare case where it
does not, this assignment will write one byte past its end.

While here, this diff also simplifies the said switch by removing some
unneeded cases.

Index: jot.c
===
RCS file: /cvs/src/usr.bin/jot/jot.c,v
retrieving revision 1.39
diff -u -p -r1.39 jot.c
--- jot.c   15 Dec 2017 14:20:52 -  1.39
+++ jot.c   29 Dec 2017 03:51:50 -
@@ -406,8 +406,7 @@ getformat(void)
if (*p == 'l') {
longdata = true;
if (*++p == 'l') {
-   if (p[1] != '\0')
-   p++;
+   p++;
goto fmt_broken;
}
}
@@ -439,9 +438,6 @@ getformat(void)
chardata = true;
break;
}
-   /* FALLTHROUGH */
-   case 'h': case 'n': case 'p': case 'q': case 's': case 'L':
-   case '$': case '*':
goto fmt_broken;
case 'f': case 'e': case 'g': case 'E': case 'G':
if (!longdata)
@@ -449,7 +445,8 @@ getformat(void)
/* FALLTHROUGH */
default:
 fmt_broken:
-   *++p = '\0';
+   if (*p != '\0')
+   p[1] = '\0';
errx(1, "illegal or unsupported format '%s'", p2);
}
while (*++p != '\0')

Regards,

kshe



Re: pf state key linking mbuf assert

2017-12-29 Thread Alexandr Nedvedicky
Hello,

On Fri, Dec 29, 2017 at 11:04:07PM +0100, Alexander Bluhm wrote:
> On Thu, Dec 28, 2017 at 12:46:45AM +0100, Alexandr Nedvedicky wrote:
> > I like your change. You have my OK with small change here:
> 
> I have commitet my diff with a small addition.  I fixed a memory
> leak that I had introduced before.
> 
> > > -pf_pkt_state_key_ref(struct mbuf *m)
> > > +pf_mbuf_link_state_key(struct mbuf *m, struct pf_state_key *sk)
> > >  {
> > > - pf_state_key_ref(m->m_pkthdr.pf.statekey);
> > KASSERT(m->m_pkthdr.pf.statekey == NULL);
> > > + m->m_pkthdr.pf.statekey = pf_state_key_ref(sk);
> > >  }
> > 
> > I think we should have KASSERT() above as kind of check if
> > more safety belts are needed to mitigate potential memory leak.
> 
> It does not work that easily.  The only caller m_dup_pkthdr() does
> set the statekey before.
> 
> I tried to put an assert "to->m_pkthdr.pf.statekey == NULL" before
> the "to->m_pkthdr = from->m_pkthdr" in m_dup_pkthdr().  But the
> mbuf "to" is not initialized, it is m_dup_pkthdr()'s job to fill
> the memory.  So we cannot assume that to->m_pkthdr.pf.statekey
> contains anything useful.
> 
> All we could do is this:
> 
> ok?
> 

I have no better plan.

Thanks and OK

sashan



Re: pf state key linking mbuf assert

2017-12-29 Thread Alexander Bluhm
On Thu, Dec 28, 2017 at 12:46:45AM +0100, Alexandr Nedvedicky wrote:
> I like your change. You have my OK with small change here:

I have commitet my diff with a small addition.  I fixed a memory
leak that I had introduced before.

> > -pf_pkt_state_key_ref(struct mbuf *m)
> > +pf_mbuf_link_state_key(struct mbuf *m, struct pf_state_key *sk)
> >  {
> > -   pf_state_key_ref(m->m_pkthdr.pf.statekey);
>   KASSERT(m->m_pkthdr.pf.statekey == NULL);
> > +   m->m_pkthdr.pf.statekey = pf_state_key_ref(sk);
> >  }
> 
> I think we should have KASSERT() above as kind of check if
> more safety belts are needed to mitigate potential memory leak.

It does not work that easily.  The only caller m_dup_pkthdr() does
set the statekey before.

I tried to put an assert "to->m_pkthdr.pf.statekey == NULL" before
the "to->m_pkthdr = from->m_pkthdr" in m_dup_pkthdr().  But the
mbuf "to" is not initialized, it is m_dup_pkthdr()'s job to fill
the memory.  So we cannot assume that to->m_pkthdr.pf.statekey
contains anything useful.

All we could do is this:

ok?

bluhm

Index: kern/uipc_mbuf.c
===
RCS file: /data/mirror/openbsd/cvs/src/sys/kern/uipc_mbuf.c,v
retrieving revision 1.251
diff -u -p -r1.251 uipc_mbuf.c
--- kern/uipc_mbuf.c29 Dec 2017 17:05:25 -  1.251
+++ kern/uipc_mbuf.c29 Dec 2017 17:18:56 -
@@ -1325,6 +1325,7 @@ m_dup_pkthdr(struct mbuf *to, struct mbu
to->m_pkthdr = from->m_pkthdr;
 
 #if NPF > 0
+   to->m_pkthdr.pf.statekey = NULL;
pf_mbuf_link_state_key(to, from->m_pkthdr.pf.statekey);
 #endif /* NPF > 0 */
 
Index: net/pf.c
===
RCS file: /data/mirror/openbsd/cvs/src/sys/net/pf.c,v
retrieving revision 1.1053
diff -u -p -r1.1053 pf.c
--- net/pf.c29 Dec 2017 17:05:25 -  1.1053
+++ net/pf.c29 Dec 2017 17:11:47 -
@@ -7268,6 +7268,7 @@ pf_mbuf_unlink_state_key(struct mbuf *m)
 void
 pf_mbuf_link_state_key(struct mbuf *m, struct pf_state_key *sk)
 {
+   KASSERT(m->m_pkthdr.pf.statekey == NULL);
m->m_pkthdr.pf.statekey = pf_state_key_ref(sk);
 }
 



paste(1): default to stdin if no files given

2017-12-29 Thread Lauri Tirkkonen
Currently paste(1) silently does nothing if it's given no file
arguments:

% printf 'hello\nworld\n'|paste
%

I often do things like 'ps -p $(pgrep sh | paste -sd,)' and forget the
"-" argument. Some other systems (gnu coreutils, illumos, perhaps more)
default to reading from stdin if no files are given; I think that makes
sense, so diff to do that below.

Index: paste.c
===
RCS file: /cvs/src/usr.bin/paste/paste.c,v
retrieving revision 1.22
diff -u -p -r1.22 paste.c
--- paste.c 9 Dec 2015 19:39:10 -   1.22
+++ paste.c 29 Dec 2017 13:47:50 -
@@ -56,6 +56,7 @@ main(int argc, char *argv[])
extern char *optarg;
extern int optind;
int ch, seq;
+   char **files = (char *[]){ "-", NULL };
 
if (pledge("stdio rpath", NULL) == -1)
err(1, "pledge");
@@ -77,15 +78,18 @@ main(int argc, char *argv[])
argc -= optind;
argv += optind;
 
+   if (argc)
+   files = argv;
+
if (!delim) {
delimcnt = 1;
delim = "\t";
}
 
if (seq)
-   sequential(argv);
+   sequential(files);
else
-   parallel(argv);
+   parallel(files);
exit(0);
 }
 
-- 
Lauri Tirkkonen | lotheac @ IRCnet



Re: armv7 / A33 clock and pinctrl bindings

2017-12-29 Thread Kevin Lo
On Fri, Dec 29, 2017 at 02:04:32PM +0100, Mark Kettenis wrote:
> 
> > Date: Thu, 28 Dec 2017 10:07:12 +0800
> > From: Kevin Lo 
> > 
> > On Wed, Dec 27, 2017 at 10:48:16AM +0100, Mark Kettenis wrote:
> > > 
> > > > Date: Wed, 27 Dec 2017 09:37:12 +0800
> > > > From: Kevin Lo 
> > > > 
> > > > On Tue, Dec 26, 2017 at 06:20:59PM +0100, Mark Kettenis wrote:
> > > > > 
> > > > > > Date: Tue, 26 Dec 2017 22:51:38 +0800
> > > > > > From: Kevin Lo 
> > > > > > 
> > > > > > > Shouldn't be too difficult to add support for that one to 
> > > > > > > axppmic(4).
> > > > > > > Do you want me to create a diff for you to test?
> > > > > > 
> > > > > > That'd be great, thanks.  Testing is easier than writing code :)
> > > > > 
> > > > > Here's a diff.  Should give you a couple of sensors and working
> > > > > regulators.  That might make the on-board eMMC work.
> > > > > 
> > > > 
> > > > Thanks, but I keep getting these messages:
> > > > 
> > > > axppmic0 at sxirsb0 addr 0x3a3: AXP223
> > > > sxirsb0: RD8 failed for run-time address 0x2d
> > > > sxirsb0: RD8 failed for run-time address 0x2d
> > > > sxirsb0: WR8 failed for run-time address 0x2d
> > > > sxirsb0: RD8 failed for run-time address 0x2d
> > > > sxirsb0: RD8 failed for run-time address 0x2d
> > > 
> > > That suggests communication with the device over the RSB is failing.
> > > 
> > > Can you send me the output of
> > > 
> > > # eeprom -p
> > > 
> > > for that board?
> > 
> > Sure, here you go: http://ix.io/DsI
> 
> Ok.  Looks like a was a bit quick ok'ing that initial sxiccmu(4) diff.
> The PRCM clocks that you defined aren't actually being hooked up since
> the nodes in the device tree are subnodes of the "prcm" node.  They
> also don't have a "reg" property.  So hooking them up is slightly
> non-trivial.  Diff below adds code to do that, but I can't test this
> unfortunately.  Can you give it a go, and maybe add some printfs in
> sxiccmu_attach_clocks() to see wether the PRCM clocks are really
> registered?  Hopefully this will fix the RSB issue.

Thanks for the explanation.  Your diff does solve the rsb problem to me,
thanks again!

> > BTW, mine is BPI-M2 Magic, the board without eMMC [1].
> 
> A right.  But the device tree has the eMMC controller enabled, so that
> is why you get some error messages.
> 
> 
> Index: dev/fdt/sxiccmu.c
> ===
> RCS file: /cvs/src/sys/dev/fdt/sxiccmu.c,v
> retrieving revision 1.14
> diff -u -p -r1.14 sxiccmu.c
> --- dev/fdt/sxiccmu.c 28 Dec 2017 18:11:13 -  1.14
> +++ dev/fdt/sxiccmu.c 29 Dec 2017 12:57:01 -
> @@ -79,7 +79,7 @@ struct cfdriver sxiccmu_cd = {
>   NULL, "sxiccmu", DV_DULL
>  };
>  
> -void sxiccmu_attach_clock(struct sxiccmu_softc *, int);
> +void sxiccmu_attach_clock(struct sxiccmu_softc *, int, int);
>  
>  uint32_t sxiccmu_ccu_get_frequency(void *, uint32_t *);
>  int  sxiccmu_ccu_set_frequency(void *, uint32_t *, uint32_t);
> @@ -124,6 +124,7 @@ sxiccmu_match(struct device *parent, voi
>   return (OF_is_compatible(node, "allwinner,sun4i-a10-ccu") ||
>   OF_is_compatible(node, "allwinner,sun7i-a20-ccu") ||
>   OF_is_compatible(node, "allwinner,sun8i-a23-ccu") ||
> + OF_is_compatible(node, "allwinner,sun8i-a23-prcm") ||
>   OF_is_compatible(node, "allwinner,sun8i-a33-ccu") ||
>   OF_is_compatible(node, "allwinner,sun8i-h3-ccu") ||
>   OF_is_compatible(node, "allwinner,sun8i-h3-r-ccu") ||
> @@ -218,7 +219,7 @@ sxiccmu_attach(struct device *parent, st
>   sc->sc_set_frequency = sxiccmu_a64_set_frequency;
>   } else {
>   for (node = OF_child(node); node; node = OF_peer(node))
> - sxiccmu_attach_clock(sc, node);
> + sxiccmu_attach_clock(sc, node, faa->fa_nreg);
>   }
>  
>   if (sc->sc_gates) {
> @@ -259,6 +260,7 @@ struct sxiccmu_device {
>   int (*set_frequency)(void *, uint32_t *, uint32_t);
>   void(*enable)(void *, uint32_t *, int);
>   void(*reset)(void *, uint32_t *, int);
> + bus_size_t offset;
>  };
>  
>  uint32_t sxiccmu_gen_get_frequency(void *, uint32_t *);
> @@ -356,7 +358,8 @@ struct sxiccmu_device sxiccmu_devices[] 
>   },
>   {
>   .compat = "allwinner,sun6i-a31-clock-reset",
> - .reset = sxiccmu_reset
> + .reset = sxiccmu_reset,
> + .offset = 0x00b0
>   },
>   {
>   .compat = "allwinner,sun7i-a20-ahb-gates-clk",
> @@ -379,7 +382,8 @@ struct sxiccmu_device sxiccmu_devices[] 
>   },
>   {
>   .compat = "allwinner,sun8i-a23-apb0-clk",
> - .get_frequency = sxiccmu_apbs_get_frequency
> + .get_frequency = sxiccmu_apbs_get_frequency,
> + .offset = 0x000c
>   },
>   {
>   .compat = "allwinner,sun8i-a23-ahb1-gates-clk",
> @@ -389,7 +393,8 @@ struct sxiccmu_device 

Re: armv7 / A33 clock and pinctrl bindings

2017-12-29 Thread Mark Kettenis
> Date: Thu, 28 Dec 2017 10:07:12 +0800
> From: Kevin Lo 
> 
> On Wed, Dec 27, 2017 at 10:48:16AM +0100, Mark Kettenis wrote:
> > 
> > > Date: Wed, 27 Dec 2017 09:37:12 +0800
> > > From: Kevin Lo 
> > > 
> > > On Tue, Dec 26, 2017 at 06:20:59PM +0100, Mark Kettenis wrote:
> > > > 
> > > > > Date: Tue, 26 Dec 2017 22:51:38 +0800
> > > > > From: Kevin Lo 
> > > > > 
> > > > > > Shouldn't be too difficult to add support for that one to 
> > > > > > axppmic(4).
> > > > > > Do you want me to create a diff for you to test?
> > > > > 
> > > > > That'd be great, thanks.  Testing is easier than writing code :)
> > > > 
> > > > Here's a diff.  Should give you a couple of sensors and working
> > > > regulators.  That might make the on-board eMMC work.
> > > > 
> > > 
> > > Thanks, but I keep getting these messages:
> > > 
> > > axppmic0 at sxirsb0 addr 0x3a3: AXP223
> > > sxirsb0: RD8 failed for run-time address 0x2d
> > > sxirsb0: RD8 failed for run-time address 0x2d
> > > sxirsb0: WR8 failed for run-time address 0x2d
> > > sxirsb0: RD8 failed for run-time address 0x2d
> > > sxirsb0: RD8 failed for run-time address 0x2d
> > 
> > That suggests communication with the device over the RSB is failing.
> > 
> > Can you send me the output of
> > 
> > # eeprom -p
> > 
> > for that board?
> 
> Sure, here you go: http://ix.io/DsI

Ok.  Looks like a was a bit quick ok'ing that initial sxiccmu(4) diff.
The PRCM clocks that you defined aren't actually being hooked up since
the nodes in the device tree are subnodes of the "prcm" node.  They
also don't have a "reg" property.  So hooking them up is slightly
non-trivial.  Diff below adds code to do that, but I can't test this
unfortunately.  Can you give it a go, and maybe add some printfs in
sxiccmu_attach_clocks() to see wether the PRCM clocks are really
registered?  Hopefully this will fix the RSB issue.

> BTW, mine is BPI-M2 Magic, the board without eMMC [1].

A right.  But the device tree has the eMMC controller enabled, so that
is why you get some error messages.


Index: dev/fdt/sxiccmu.c
===
RCS file: /cvs/src/sys/dev/fdt/sxiccmu.c,v
retrieving revision 1.14
diff -u -p -r1.14 sxiccmu.c
--- dev/fdt/sxiccmu.c   28 Dec 2017 18:11:13 -  1.14
+++ dev/fdt/sxiccmu.c   29 Dec 2017 12:57:01 -
@@ -79,7 +79,7 @@ struct cfdriver sxiccmu_cd = {
NULL, "sxiccmu", DV_DULL
 };
 
-void sxiccmu_attach_clock(struct sxiccmu_softc *, int);
+void sxiccmu_attach_clock(struct sxiccmu_softc *, int, int);
 
 uint32_t sxiccmu_ccu_get_frequency(void *, uint32_t *);
 intsxiccmu_ccu_set_frequency(void *, uint32_t *, uint32_t);
@@ -124,6 +124,7 @@ sxiccmu_match(struct device *parent, voi
return (OF_is_compatible(node, "allwinner,sun4i-a10-ccu") ||
OF_is_compatible(node, "allwinner,sun7i-a20-ccu") ||
OF_is_compatible(node, "allwinner,sun8i-a23-ccu") ||
+   OF_is_compatible(node, "allwinner,sun8i-a23-prcm") ||
OF_is_compatible(node, "allwinner,sun8i-a33-ccu") ||
OF_is_compatible(node, "allwinner,sun8i-h3-ccu") ||
OF_is_compatible(node, "allwinner,sun8i-h3-r-ccu") ||
@@ -218,7 +219,7 @@ sxiccmu_attach(struct device *parent, st
sc->sc_set_frequency = sxiccmu_a64_set_frequency;
} else {
for (node = OF_child(node); node; node = OF_peer(node))
-   sxiccmu_attach_clock(sc, node);
+   sxiccmu_attach_clock(sc, node, faa->fa_nreg);
}
 
if (sc->sc_gates) {
@@ -259,6 +260,7 @@ struct sxiccmu_device {
int (*set_frequency)(void *, uint32_t *, uint32_t);
void(*enable)(void *, uint32_t *, int);
void(*reset)(void *, uint32_t *, int);
+   bus_size_t offset;
 };
 
 uint32_t sxiccmu_gen_get_frequency(void *, uint32_t *);
@@ -356,7 +358,8 @@ struct sxiccmu_device sxiccmu_devices[] 
},
{
.compat = "allwinner,sun6i-a31-clock-reset",
-   .reset = sxiccmu_reset
+   .reset = sxiccmu_reset,
+   .offset = 0x00b0
},
{
.compat = "allwinner,sun7i-a20-ahb-gates-clk",
@@ -379,7 +382,8 @@ struct sxiccmu_device sxiccmu_devices[] 
},
{
.compat = "allwinner,sun8i-a23-apb0-clk",
-   .get_frequency = sxiccmu_apbs_get_frequency
+   .get_frequency = sxiccmu_apbs_get_frequency,
+   .offset = 0x000c
},
{
.compat = "allwinner,sun8i-a23-ahb1-gates-clk",
@@ -389,7 +393,8 @@ struct sxiccmu_device sxiccmu_devices[] 
{
.compat = "allwinner,sun8i-a23-apb0-gates-clk",
.get_frequency = sxiccmu_gen_get_frequency,
-   .enable = sxiccmu_gate_enable
+   .enable = sxiccmu_gate_enable,
+   .offset = 0x0028
},
{
   

Re: armv7 unaligned access in libcrypto

2017-12-29 Thread Mark Kettenis
> Date: Fri, 29 Dec 2017 21:21:04 +1100
> From: Jonathan Gray 
> 
> On Fri, Dec 29, 2017 at 10:47:06AM +0100, Mark Kettenis wrote:
> > The Aarch32 assembly code in libcrypto assumes that armv7 supports
> > unaligned access.  It does, but only if you don't enable the bit that
> > makes it trap on unaligned access.  And we enable that bit on OpenBSD.
> > So doing a SHA256 of an unaligned buffer (something ftp(1) ends up
> > doing) you SIGBUS.
> > 
> > This currently isn't an issue since our base GCC does not advertise
> > that we're compiling for armv7 and up only.  It barely knows about
> > armv7 at all.  But with clang that is no longer true.  And we really
> > want to build for armv7 and up only because that gives us proper
> > atomic operations and such.
> > 
> > So here is a diff that avoids the unaligned access bits that matter
> > when compiling on OpenBSD.
> > 
> > ok?
> > 
> > P.S. Ports people might want to apply a similar diff to the OpenSSSL
> >  port if we still have it.
> 
> Any reason to not use __STRICT_ALIGNMENT for this?

The assembly code doesn't include the header file that defines
__STRICT_ALIGNMENT.  But I could have arm_arch.h define it and use it
instead of __OpenBSD__.  Makes things a little bit more obvious...

Any of the LibreSSL folks have an opinion about that?

> > Index: lib/libcrypto/aes/asm/aes-armv4.pl
> > ===
> > RCS file: /cvs/src/lib/libcrypto/aes/asm/aes-armv4.pl,v
> > retrieving revision 1.2
> > diff -u -p -r1.2 aes-armv4.pl
> > --- lib/libcrypto/aes/asm/aes-armv4.pl  9 Jul 2014 09:10:07 -   
> > 1.2
> > +++ lib/libcrypto/aes/asm/aes-armv4.pl  29 Dec 2017 09:38:11 -
> > @@ -172,7 +172,7 @@ AES_encrypt:
> > mov $rounds,r0  @ inp
> > mov $key,r2
> > sub $tbl,r3,#AES_encrypt-AES_Te @ Te
> > -#if __ARM_ARCH__<7
> > +#if __ARM_ARCH__<7 || defined(__OpenBSD__)
> > ldrb$s0,[$rounds,#3]@ load input data in endian-neutral
> > ldrb$t1,[$rounds,#2]@ manner...
> > ldrb$t2,[$rounds,#1]
> > @@ -216,7 +216,7 @@ AES_encrypt:
> > bl  _armv4_AES_encrypt
> >  
> > ldr $rounds,[sp],#4 @ pop out
> > -#if __ARM_ARCH__>=7
> > +#if __ARM_ARCH__>=7 && !defined(__OpenBSD__)
> >  #ifdef __ARMEL__
> > rev $s0,$s0
> > rev $s1,$s1
> > @@ -432,7 +432,7 @@ _armv4_AES_set_encrypt_key:
> > mov lr,r1   @ bits
> > mov $key,r2 @ key
> >  
> > -#if __ARM_ARCH__<7
> > +#if __ARM_ARCH__<7 || defined(__OpenBSD__)
> > ldrb$s0,[$rounds,#3]@ load input data in endian-neutral
> > ldrb$t1,[$rounds,#2]@ manner...
> > ldrb$t2,[$rounds,#1]
> > @@ -517,7 +517,7 @@ _armv4_AES_set_encrypt_key:
> > b   .Ldone
> >  
> >  .Lnot128:
> > -#if __ARM_ARCH__<7
> > +#if __ARM_ARCH__<7 || defined(__OpenBSD__)
> > ldrb$i2,[$rounds,#19]
> > ldrb$t1,[$rounds,#18]
> > ldrb$t2,[$rounds,#17]
> > @@ -588,7 +588,7 @@ _armv4_AES_set_encrypt_key:
> > b   .L192_loop
> >  
> >  .Lnot192:
> > -#if __ARM_ARCH__<7
> > +#if __ARM_ARCH__<7 || defined(__OpenBSD__)
> > ldrb$i2,[$rounds,#27]
> > ldrb$t1,[$rounds,#26]
> > ldrb$t2,[$rounds,#25]
> > @@ -888,7 +888,7 @@ AES_decrypt:
> > mov $rounds,r0  @ inp
> > mov $key,r2
> > sub $tbl,r3,#AES_decrypt-AES_Td @ Td
> > -#if __ARM_ARCH__<7
> > +#if __ARM_ARCH__<7 || defined(__OpenBSD__)
> > ldrb$s0,[$rounds,#3]@ load input data in endian-neutral
> > ldrb$t1,[$rounds,#2]@ manner...
> > ldrb$t2,[$rounds,#1]
> > @@ -932,7 +932,7 @@ AES_decrypt:
> > bl  _armv4_AES_decrypt
> >  
> > ldr $rounds,[sp],#4 @ pop out
> > -#if __ARM_ARCH__>=7
> > +#if __ARM_ARCH__>=7 && !defined(__OpenBSD__)
> >  #ifdef __ARMEL__
> > rev $s0,$s0
> > rev $s1,$s1
> > Index: lib/libcrypto/sha/asm/sha1-armv4-large.pl
> > ===
> > RCS file: /cvs/src/lib/libcrypto/sha/asm/sha1-armv4-large.pl,v
> > retrieving revision 1.1.1.4
> > diff -u -p -r1.1.1.4 sha1-armv4-large.pl
> > --- lib/libcrypto/sha/asm/sha1-armv4-large.pl   13 Apr 2014 15:16:35 
> > -  1.1.1.4
> > +++ lib/libcrypto/sha/asm/sha1-armv4-large.pl   29 Dec 2017 09:38:11 
> > -
> > @@ -95,7 +95,7 @@ ___
> >  sub BODY_00_15 {
> >  my ($a,$b,$c,$d,$e)=@_;
> >  $code.=<<___;
> > -#if __ARM_ARCH__<7
> > +#if __ARM_ARCH__<7 || defined(__OpenBSD__)
> > ldrb$t1,[$inp,#2]
> > ldrb$t0,[$inp,#3]
> > ldrb$t2,[$inp,#1]
> > Index: lib/libcrypto/sha/asm/sha256-armv4.pl
> > ===
> > RCS file: /cvs/src/lib/libcrypto/sha/asm/sha256-armv4.pl,v
> > retrieving revision 1.1.1.3
> > diff -u -p -r1.1.1.3 sha256-armv4.pl
> > --- 

Re: armv7 unaligned access in libcrypto

2017-12-29 Thread Jonathan Gray
On Fri, Dec 29, 2017 at 10:47:06AM +0100, Mark Kettenis wrote:
> The Aarch32 assembly code in libcrypto assumes that armv7 supports
> unaligned access.  It does, but only if you don't enable the bit that
> makes it trap on unaligned access.  And we enable that bit on OpenBSD.
> So doing a SHA256 of an unaligned buffer (something ftp(1) ends up
> doing) you SIGBUS.
> 
> This currently isn't an issue since our base GCC does not advertise
> that we're compiling for armv7 and up only.  It barely knows about
> armv7 at all.  But with clang that is no longer true.  And we really
> want to build for armv7 and up only because that gives us proper
> atomic operations and such.
> 
> So here is a diff that avoids the unaligned access bits that matter
> when compiling on OpenBSD.
> 
> ok?
> 
> P.S. Ports people might want to apply a similar diff to the OpenSSSL
>  port if we still have it.

Any reason to not use __STRICT_ALIGNMENT for this?

> 
> 
> Index: lib/libcrypto/aes/asm/aes-armv4.pl
> ===
> RCS file: /cvs/src/lib/libcrypto/aes/asm/aes-armv4.pl,v
> retrieving revision 1.2
> diff -u -p -r1.2 aes-armv4.pl
> --- lib/libcrypto/aes/asm/aes-armv4.pl9 Jul 2014 09:10:07 -   
> 1.2
> +++ lib/libcrypto/aes/asm/aes-armv4.pl29 Dec 2017 09:38:11 -
> @@ -172,7 +172,7 @@ AES_encrypt:
>   mov $rounds,r0  @ inp
>   mov $key,r2
>   sub $tbl,r3,#AES_encrypt-AES_Te @ Te
> -#if __ARM_ARCH__<7
> +#if __ARM_ARCH__<7 || defined(__OpenBSD__)
>   ldrb$s0,[$rounds,#3]@ load input data in endian-neutral
>   ldrb$t1,[$rounds,#2]@ manner...
>   ldrb$t2,[$rounds,#1]
> @@ -216,7 +216,7 @@ AES_encrypt:
>   bl  _armv4_AES_encrypt
>  
>   ldr $rounds,[sp],#4 @ pop out
> -#if __ARM_ARCH__>=7
> +#if __ARM_ARCH__>=7 && !defined(__OpenBSD__)
>  #ifdef __ARMEL__
>   rev $s0,$s0
>   rev $s1,$s1
> @@ -432,7 +432,7 @@ _armv4_AES_set_encrypt_key:
>   mov lr,r1   @ bits
>   mov $key,r2 @ key
>  
> -#if __ARM_ARCH__<7
> +#if __ARM_ARCH__<7 || defined(__OpenBSD__)
>   ldrb$s0,[$rounds,#3]@ load input data in endian-neutral
>   ldrb$t1,[$rounds,#2]@ manner...
>   ldrb$t2,[$rounds,#1]
> @@ -517,7 +517,7 @@ _armv4_AES_set_encrypt_key:
>   b   .Ldone
>  
>  .Lnot128:
> -#if __ARM_ARCH__<7
> +#if __ARM_ARCH__<7 || defined(__OpenBSD__)
>   ldrb$i2,[$rounds,#19]
>   ldrb$t1,[$rounds,#18]
>   ldrb$t2,[$rounds,#17]
> @@ -588,7 +588,7 @@ _armv4_AES_set_encrypt_key:
>   b   .L192_loop
>  
>  .Lnot192:
> -#if __ARM_ARCH__<7
> +#if __ARM_ARCH__<7 || defined(__OpenBSD__)
>   ldrb$i2,[$rounds,#27]
>   ldrb$t1,[$rounds,#26]
>   ldrb$t2,[$rounds,#25]
> @@ -888,7 +888,7 @@ AES_decrypt:
>   mov $rounds,r0  @ inp
>   mov $key,r2
>   sub $tbl,r3,#AES_decrypt-AES_Td @ Td
> -#if __ARM_ARCH__<7
> +#if __ARM_ARCH__<7 || defined(__OpenBSD__)
>   ldrb$s0,[$rounds,#3]@ load input data in endian-neutral
>   ldrb$t1,[$rounds,#2]@ manner...
>   ldrb$t2,[$rounds,#1]
> @@ -932,7 +932,7 @@ AES_decrypt:
>   bl  _armv4_AES_decrypt
>  
>   ldr $rounds,[sp],#4 @ pop out
> -#if __ARM_ARCH__>=7
> +#if __ARM_ARCH__>=7 && !defined(__OpenBSD__)
>  #ifdef __ARMEL__
>   rev $s0,$s0
>   rev $s1,$s1
> Index: lib/libcrypto/sha/asm/sha1-armv4-large.pl
> ===
> RCS file: /cvs/src/lib/libcrypto/sha/asm/sha1-armv4-large.pl,v
> retrieving revision 1.1.1.4
> diff -u -p -r1.1.1.4 sha1-armv4-large.pl
> --- lib/libcrypto/sha/asm/sha1-armv4-large.pl 13 Apr 2014 15:16:35 -  
> 1.1.1.4
> +++ lib/libcrypto/sha/asm/sha1-armv4-large.pl 29 Dec 2017 09:38:11 -
> @@ -95,7 +95,7 @@ ___
>  sub BODY_00_15 {
>  my ($a,$b,$c,$d,$e)=@_;
>  $code.=<<___;
> -#if __ARM_ARCH__<7
> +#if __ARM_ARCH__<7 || defined(__OpenBSD__)
>   ldrb$t1,[$inp,#2]
>   ldrb$t0,[$inp,#3]
>   ldrb$t2,[$inp,#1]
> Index: lib/libcrypto/sha/asm/sha256-armv4.pl
> ===
> RCS file: /cvs/src/lib/libcrypto/sha/asm/sha256-armv4.pl,v
> retrieving revision 1.1.1.3
> diff -u -p -r1.1.1.3 sha256-armv4.pl
> --- lib/libcrypto/sha/asm/sha256-armv4.pl 13 Oct 2012 21:23:43 -  
> 1.1.1.3
> +++ lib/libcrypto/sha/asm/sha256-armv4.pl 29 Dec 2017 09:38:11 -
> @@ -51,7 +51,7 @@ sub BODY_00_15 {
>  my ($i,$a,$b,$c,$d,$e,$f,$g,$h) = @_;
>  
>  $code.=<<___ if ($i<16);
> -#if __ARM_ARCH__>=7
> +#if __ARM_ARCH__>=7 && !defined(__OpenBSD__)
>   ldr $T1,[$inp],#4
>  #else
>   ldrb$T1,[$inp,#3]   @ $i
> @@ -70,7 +70,7 @@ $code.=<<___;
>   eor $t1,$f,$g
>  #if $i>=16
>   add   

armv7 unaligned access in libcrypto

2017-12-29 Thread Mark Kettenis
The Aarch32 assembly code in libcrypto assumes that armv7 supports
unaligned access.  It does, but only if you don't enable the bit that
makes it trap on unaligned access.  And we enable that bit on OpenBSD.
So doing a SHA256 of an unaligned buffer (something ftp(1) ends up
doing) you SIGBUS.

This currently isn't an issue since our base GCC does not advertise
that we're compiling for armv7 and up only.  It barely knows about
armv7 at all.  But with clang that is no longer true.  And we really
want to build for armv7 and up only because that gives us proper
atomic operations and such.

So here is a diff that avoids the unaligned access bits that matter
when compiling on OpenBSD.

ok?

P.S. Ports people might want to apply a similar diff to the OpenSSSL
 port if we still have it.


Index: lib/libcrypto/aes/asm/aes-armv4.pl
===
RCS file: /cvs/src/lib/libcrypto/aes/asm/aes-armv4.pl,v
retrieving revision 1.2
diff -u -p -r1.2 aes-armv4.pl
--- lib/libcrypto/aes/asm/aes-armv4.pl  9 Jul 2014 09:10:07 -   1.2
+++ lib/libcrypto/aes/asm/aes-armv4.pl  29 Dec 2017 09:38:11 -
@@ -172,7 +172,7 @@ AES_encrypt:
mov $rounds,r0  @ inp
mov $key,r2
sub $tbl,r3,#AES_encrypt-AES_Te @ Te
-#if __ARM_ARCH__<7
+#if __ARM_ARCH__<7 || defined(__OpenBSD__)
ldrb$s0,[$rounds,#3]@ load input data in endian-neutral
ldrb$t1,[$rounds,#2]@ manner...
ldrb$t2,[$rounds,#1]
@@ -216,7 +216,7 @@ AES_encrypt:
bl  _armv4_AES_encrypt
 
ldr $rounds,[sp],#4 @ pop out
-#if __ARM_ARCH__>=7
+#if __ARM_ARCH__>=7 && !defined(__OpenBSD__)
 #ifdef __ARMEL__
rev $s0,$s0
rev $s1,$s1
@@ -432,7 +432,7 @@ _armv4_AES_set_encrypt_key:
mov lr,r1   @ bits
mov $key,r2 @ key
 
-#if __ARM_ARCH__<7
+#if __ARM_ARCH__<7 || defined(__OpenBSD__)
ldrb$s0,[$rounds,#3]@ load input data in endian-neutral
ldrb$t1,[$rounds,#2]@ manner...
ldrb$t2,[$rounds,#1]
@@ -517,7 +517,7 @@ _armv4_AES_set_encrypt_key:
b   .Ldone
 
 .Lnot128:
-#if __ARM_ARCH__<7
+#if __ARM_ARCH__<7 || defined(__OpenBSD__)
ldrb$i2,[$rounds,#19]
ldrb$t1,[$rounds,#18]
ldrb$t2,[$rounds,#17]
@@ -588,7 +588,7 @@ _armv4_AES_set_encrypt_key:
b   .L192_loop
 
 .Lnot192:
-#if __ARM_ARCH__<7
+#if __ARM_ARCH__<7 || defined(__OpenBSD__)
ldrb$i2,[$rounds,#27]
ldrb$t1,[$rounds,#26]
ldrb$t2,[$rounds,#25]
@@ -888,7 +888,7 @@ AES_decrypt:
mov $rounds,r0  @ inp
mov $key,r2
sub $tbl,r3,#AES_decrypt-AES_Td @ Td
-#if __ARM_ARCH__<7
+#if __ARM_ARCH__<7 || defined(__OpenBSD__)
ldrb$s0,[$rounds,#3]@ load input data in endian-neutral
ldrb$t1,[$rounds,#2]@ manner...
ldrb$t2,[$rounds,#1]
@@ -932,7 +932,7 @@ AES_decrypt:
bl  _armv4_AES_decrypt
 
ldr $rounds,[sp],#4 @ pop out
-#if __ARM_ARCH__>=7
+#if __ARM_ARCH__>=7 && !defined(__OpenBSD__)
 #ifdef __ARMEL__
rev $s0,$s0
rev $s1,$s1
Index: lib/libcrypto/sha/asm/sha1-armv4-large.pl
===
RCS file: /cvs/src/lib/libcrypto/sha/asm/sha1-armv4-large.pl,v
retrieving revision 1.1.1.4
diff -u -p -r1.1.1.4 sha1-armv4-large.pl
--- lib/libcrypto/sha/asm/sha1-armv4-large.pl   13 Apr 2014 15:16:35 -  
1.1.1.4
+++ lib/libcrypto/sha/asm/sha1-armv4-large.pl   29 Dec 2017 09:38:11 -
@@ -95,7 +95,7 @@ ___
 sub BODY_00_15 {
 my ($a,$b,$c,$d,$e)=@_;
 $code.=<<___;
-#if __ARM_ARCH__<7
+#if __ARM_ARCH__<7 || defined(__OpenBSD__)
ldrb$t1,[$inp,#2]
ldrb$t0,[$inp,#3]
ldrb$t2,[$inp,#1]
Index: lib/libcrypto/sha/asm/sha256-armv4.pl
===
RCS file: /cvs/src/lib/libcrypto/sha/asm/sha256-armv4.pl,v
retrieving revision 1.1.1.3
diff -u -p -r1.1.1.3 sha256-armv4.pl
--- lib/libcrypto/sha/asm/sha256-armv4.pl   13 Oct 2012 21:23:43 -  
1.1.1.3
+++ lib/libcrypto/sha/asm/sha256-armv4.pl   29 Dec 2017 09:38:11 -
@@ -51,7 +51,7 @@ sub BODY_00_15 {
 my ($i,$a,$b,$c,$d,$e,$f,$g,$h) = @_;
 
 $code.=<<___ if ($i<16);
-#if __ARM_ARCH__>=7
+#if __ARM_ARCH__>=7 && !defined(__OpenBSD__)
ldr $T1,[$inp],#4
 #else
ldrb$T1,[$inp,#3]   @ $i
@@ -70,7 +70,7 @@ $code.=<<___;
eor $t1,$f,$g
 #if $i>=16
add $T1,$T1,$t3 @ from BODY_16_xx
-#elif __ARM_ARCH__>=7 && defined(__ARMEL__)
+#elif __ARM_ARCH__>=7 && defined(__ARMEL__) && !defined(__OpenBSD__)
rev $T1,$T1
 #endif
 #if $i==15
Index: lib/libcrypto/sha/asm/sha512-armv4.pl