Re: iwn0 firmware errors

2012-02-12 Thread Joachim Schipper
On Sun, Feb 12, 2012 at 08:58:31PM +, Edd Barrett wrote:
> On Sun, Feb 12, 2012 at 09:24:17PM +0100, Sebastian Benoit wrote:
> > i was using a x61s until a month ago. I had these errors too, but only with
> > certain wifi networks (specifically 2 events with 30+ access points and 
> > 1000+
> > wifi users) using cisco ap's. I never saw these errors somewhere else.
> 
> Hmm. I am seeing the error connecting to a soekris I set up myself. I am
> the only client on this. It has a ral card in hostap mode.
> 
> It may be related to power saving? Pure speculation ofcourse -- I know
> nothing about how these devices work internally.

I also have issues with iwn on my SL510, although only when stressing
the network (and not reliably). Sample:

iwn0 at pci4 dev 0 function 0 "Intel WiFi Link 1000" rev 0x00: msi, MIMO 1T2R, 
BGS, address 00:1e:64:6b:9a:7c
iwn0: fatal firmware error
firmware error log:
  error type  = "NMI_INTERRUPT_WDG" (0x0004)
  program counter = 0x06D0
  source line = 0x012E
  error data  = 0x00020363
  branch link = 0x059606D4
  interrupt link  = 0x0892839C
  time= 2431600062
driver status:
  tx ring  0: qid=0  cur=0   queued=1  
  tx ring  1: qid=1  cur=0   queued=0  
  tx ring  2: qid=2  cur=0   queued=0  
  tx ring  3: qid=3  cur=0   queued=0  
  tx ring  4: qid=4  cur=78  queued=0  
  tx ring  5: qid=5  cur=0   queued=0  
  tx ring  6: qid=6  cur=0   queued=0  
  tx ring  7: qid=7  cur=0   queued=0  
  tx ring  8: qid=8  cur=0   queued=0  
  tx ring  9: qid=9  cur=0   queued=0  
  tx ring 10: qid=10 cur=0   queued=0  
  tx ring 11: qid=11 cur=0   queued=0  
  tx ring 12: qid=12 cur=0   queued=0  
  tx ring 13: qid=13 cur=0   queued=0  
  tx ring 14: qid=14 cur=0   queued=0  
  tx ring 15: qid=15 cur=0   queued=0  
  tx ring 16: qid=16 cur=0   queued=0  
  tx ring 17: qid=17 cur=0   queued=0  
  tx ring 18: qid=18 cur=0   queued=0  
  tx ring 19: qid=19 cur=0   queued=0  
  rx ring: cur=35
  802.11 state 4

This occurs with a multitude of APs, so I'm not sure if it's the same
issue. I have the latest firmware, iwn-firmware-5.6p0.

Joachim



Xsearch(3) nit

2012-02-12 Thread Joachim Schipper
bsearch(3), tsearch(3) contains some superfluous spaces.

Joachim

Index: bsearch.3
===
RCS file: /usr/cvs/src/src/lib/libc/stdlib/bsearch.3,v
retrieving revision 1.7
diff -u -p -r1.7 bsearch.3
--- bsearch.3   31 May 2007 19:19:31 -  1.7
+++ bsearch.3   21 Apr 2011 16:03:10 -
@@ -40,7 +40,7 @@
 .Sh SYNOPSIS
 .Fd #include 
 .Ft void *
-.Fn bsearch "const void *key" "const void *base" "size_t nmemb" "size_t size" 
"int (*compar) (const void *, const void *)"
+.Fn bsearch "const void *key" "const void *base" "size_t nmemb" "size_t size" 
"int (*compar)(const void *, const void *)"
 .Sh DESCRIPTION
 The
 .Fn bsearch
Index: tsearch.3
===
RCS file: /usr/cvs/src/src/lib/libc/stdlib/tsearch.3,v
retrieving revision 1.16
diff -u -p -r1.16 tsearch.3
--- tsearch.3   31 May 2007 19:19:32 -  1.16
+++ tsearch.3   21 Apr 2011 16:03:10 -
@@ -26,13 +26,13 @@
 .Sh SYNOPSIS
 .Fd #include 
 .Ft void *
-.Fn tdelete "const void *key" "void **rootp" "int (*compar) (const void *, 
const void *)"
+.Fn tdelete "const void *key" "void **rootp" "int (*compar)(const void *, 
const void *)"
 .Ft void *
-.Fn tfind "const void *key" "void * const *rootp" "int (*compar) (const void 
*, const void *)"
+.Fn tfind "const void *key" "void * const *rootp" "int (*compar)(const void *, 
const void *)"
 .Ft void *
-.Fn tsearch "const void *key" "void **rootp" "int (*compar) (const void *, 
const void *)"
+.Fn tsearch "const void *key" "void **rootp" "int (*compar)(const void *, 
const void *)"
 .Ft void
-.Fn twalk "const void *root" "void (*action) (const void *, VISIT, int)"
+.Fn twalk "const void *root" "void (*action)(const void *, VISIT, int)"
 .Sh DESCRIPTION
 The
 .Fn tdelete ,



select(2) nit

2012-02-12 Thread Joachim Schipper
The sample code given in the BUGS section of select(2) contains an
unnecessary cast.

Joachim

Index: select.2
===
RCS file: /usr/cvs/src/src/lib/libc/sys/select.2,v
retrieving revision 1.28
diff -u -p -r1.28 select.2
--- select.231 May 2007 19:19:33 -  1.28
+++ select.212 Feb 2012 18:59:13 -
@@ -204,8 +204,7 @@ userland libraries:
 fd_set *fdsr;
 int max = fd;
 
-fdsr = (fd_set *)calloc(howmany(max+1, NFDBITS),
-sizeof(fd_mask));
+fdsr = calloc(howmany(max+1, NFDBITS), sizeof(fd_mask));
 if (fdsr == NULL) {
...
return (-1);



uname(3) return values

2011-04-18 Thread Joachim Schipper
The uname(3) man page suggests that checking the return value against -1
makes sense. That is not the case:

On Sun, Apr 03, 2011, Ingo Schwarze wrote to the mandoc mailing list:
> > Yuri Pankov wrote:
> >> uname(2) on Solaris (...) states:
> >>
> >> RETURN VALUES
> >>  Upon  successful  completion,  a   non-negative   value   is
> >>  returned.  Otherwise,  -1  is  returned  and errno is set to
> >>  indicate the error.
> 
> Hm, indeed, that is not just Solaris, but POSIX, see here:
> 
>  http://pubs.opengroup.org/onlinepubs/9699919799/functions/uname.html

The following trivial diff should prevent future accidents.

Joachim

Index: uname.3
===
RCS file: /usr/cvs/src/src/lib/libc/gen/uname.3,v
retrieving revision 1.12
diff -u -p -r1.12 uname.3
--- uname.3 31 May 2007 19:19:29 -  1.12
+++ uname.3 18 Apr 2011 16:46:45 -
@@ -65,7 +65,9 @@ Machine hardware platform.
 .Sh RETURN VALUES
 If
 .Fn uname
-is successful, 0 is returned; otherwise, \-1 is returned and
+is successful, 0 is returned; otherwise, a nonzero value (on 
+.Ox ,
+\-1) is returned and
 .Va errno
 is set appropriately.
 .Sh ERRORS



Re: sparc64, hardware timer, security/botan

2011-03-25 Thread Joachim Schipper
On Fri, Mar 25, 2011 at 01:36:13PM +0100, Mark Kettenis wrote:
> > Date: Tue, 15 Mar 2011 14:22:16 +0100
> > From: Aleksander Piotrowski 
> > i have signal 4, Illegal instruction crash on sparc64 with security/botan
> > (required by newer devel/monotone).  it looks like they are trying to get 
> > time
> > from hardware timer using some funny asm's.  sparc64 and asm are rocket 
> > science for me,
> > that's why i'm asking for your help :-)
> > 
> > here goes method body that, according to gdb, is our culprit.  and the 
> > exact line is
> > 
> > asm volatile("rd %%tick, %0" : "=r" (rtc));

> On OpenBSD we disable access to %tick from userland.  I think the idea
> is to make it harder for people to perform timing attacks, and
> therefore improve security.  But I don't consider myself enough of a
> security expert to be able to judge wethere that really helps.  So I
> CC'ed tech@ in the hope that a more knowledgable person will chime in.

I don't know much about SPARCs, but on i386/amd64 the pctr(4) device -
or, if you want to reduce the noise from switching into the kernel, the
rdtsc instruction - works just fine. See
http://www.openbsd.org/cgi-bin/man.cgi?query=pctr&apropos=0&sektion=4&manpath=OpenBSD+Current&arch=i386&format=html
and /usr/include/machine/pctr.h on an appropriate machine. (rdpmc may be
convenient for cache-based attacks; see the output of "pctr -l" on an
appropriate machine. Of course, you can infer cache misses from the
instruction count, too.)

So if this is for security, it's at least not universal.

Joachim



Re: Syslogd: Adding log sockets that are only writeable by a single group

2011-02-21 Thread Joachim Schipper
On Sat, Feb 19, 2011 at 10:17:21PM -0500, Eric wrote:
> On Sun, Feb 13, 2011 at 8:45 PM, Philip Guenther  wrote:
> > On Sun, Feb 13, 2011 at 8:27 AM, Eric  wrote:
> >> On (...) Philip Guenther  wrote:
> >>> (...) if you're intending that this should affect all
> >>> programs without any changes to the program themselves, then this will
> >>> require much care and verification that it doesn't bloat everything.
> >>> Consider that *every* C program on OpenBSD pulls in syslog_r() to
> >>> support the stack-protector check code.  If that starts pulling the
> >>> NIS code for getgrgid() to do the gid -> name mapping to find the
> >>> syslog socket, then many binaries will grow.
> >>
> >> I can only think of two ways to avoid having NIS linked into everything:
> >>
> >> - Only use the modified syslog functions though LD_PRELOAD
> >>
> >> - Make openlog/syslog/closelog system calls (this would also allow
> >>  us to ensure the accuracy of the pid and program name strings, and
> >>  we could filter by program name in syslogd).
> >
> > Yow!  Group names are strictly user-space right now; how exactly where
> > you planning on having the kernel get the group name for a process?
> >
> > Backing up a step, syslog_r() MUST remain async-signal-safe, so
> > whatever designs you ponder, please check them against that
> > requirement.  That would be true even of an LD_PRELOAD replacement...
> 
> I've decided to stay in user-space and stick with group-based
> controls, but I've come across a small problem with modifying openlog
> to find the current process' group:
> 
> Daemons like httpd perform openlog before they drop privileges,
> meaning that most daemons would end up logging to
> /dev/syslog_wheel/log.  Not that this is such a horrible outcome, but
> it would be nice i these daemons logged to their own sockets.
> 
> I'm not sure how to handle this problem, but I thought of two possible
> solutions:
> 
> - It might be alright to have the group or perhaps the path of the log
> socket passed as part of the struct syslog_data that's explcitly
> provided by programs that use openlog_r/syslog_r.  To get the daemons
> to use this option, we would have to modify each one to use the new
> struct member (many small changes).

syslog() should Just Work - your proposal requires modification of every
daemon and port in a way that only works on OpenBSD.

> - I noticed that the syslog code uses connect() and send() to send
> messages to syslog.  I could modify syslog to use sendto()
> instead--Would it be unreasonable to call getresgid and getgrgid for
> each call to syslog()?

getgrgid() isn't signal-safe, so this is not a good idea. It would also
make syslog() quite slow.

> - If each process only had to call getresgid, would that help me avoid
> enlarging each process (avoiding NIS by not using getgrgid/getgrnam?)
> If I did this, I could use the gid_t instead of the group name as the
> path, e.g. /dev/syslog_123/log.

I don't know. Note, however, that all of these things rely on every
process being nice and using syslog() - didn't your first message in
this thread contain the phrase "high integrity log files"? (To spell it
out, syslog() just writes a message to a socket; there are other ways to
do that.)

Backing up a step, pretty much every daemon already has an option to set
the path to /dev/log, and syslog already knows where its sockets are.
Why not just extend /etc/syslog.conf to accept syntax like this:

# Already supported
!sudo
*.* /var/log/sudo
# Extension
:/var/www/dev/log
*.* /var/log/apache
# You may also want to support
127.0.0.1:syslog
*.* /var/log/localhost

You could then do access control via permissions on the socket or the
directory the socket is in. In fact, I'm pretty sure that at least one
of the syslogd replacements in ports already does this.

I'm not convinced that what you're proposing is worth implementing, but
at least the above design is minimally intrusive and may even be
convenient for other uses.

Joachim



Re: IMPORTANT: video reposting diff

2011-02-18 Thread Joachim Schipper
On Sat, Feb 12, 2011 at 04:16:19AM +0200, Paul Irofti wrote:
> On Sat, Feb 12, 2011 at 03:33:35AM +0200, Paul Irofti wrote:
> > Please everyone test the following diff even if video reposting *works*
> > for you at the moment. And everyone with a non-working video reposting
> > card that *isn't* nvidia contact me!
> > 
> > AGAIN! If suspend/resume doesn't work for you and its not NVIDIA, contact 
> > me!
> 
> New diff including the amd64 bits which I missed in the former mail.
> Also remove wbinvd() call and comment what this is trying to accomplish.
> 
> Test on all machines, working or not! Nvidia is a don't care case.

Hi,

Without your patch I can switch to/from consoles, suspend/resume and
restart X without issues, without your patch; however, with your patch,
while switching to/from consoles and restarting X is fine, the system
powers off when resuming (that is, the lights briefly come on and then
the computer completely turns itself off.)

Note that this laptop has other ACPI issues, so it may not be your code
that is wrong: note
http://www.mail-archive.com/source-changes@openbsd.org/msg20575.html,
the other messages on resume (starting at the second line beginning with
'ahci0:'), and that I need to 'disable acpitz*' to boot.

I'll be happy to run some more tests for you tonight/this weekend, if
you'd like some. dmesg from an official kernel (with some
suspend/resumes) and ("your") Xorg.0.log below.

Joachim

OpenBSD 4.9-beta (GENERIC.MP) #803: Mon Feb 14 14:38:41 MST 2011
t...@amd64.openbsd.org:/usr/src/sys/arch/amd64/compile/GENERIC.MP
real mem = 2003517440 (1910MB)
avail mem = 1936162816 (1846MB)
mainbus0 at root
bios0 at mainbus0: SMBIOS rev. 2.5 @ 0xe0010 (44 entries)
bios0: vendor LENOVO version "6JET83WW (1.41 )" date 09/21/2010
bios0: LENOVO 28477LG
acpi0 at bios0: rev 4
acpi0: sleep states S0 S3 S4 S5
acpi0: tables DSDT FACP HPET MCFG APIC BOOT SLIC SSDT SSDT SSDT
acpi0: wakeup devices P0P2(S4) P0P1(S4) USB0(S3) USB1(S3) USB2(S3) USBR(S3) 
EHC1(S3) USB3(S3) USB4(S3) USB5(S3) EHC2(S3) HDEF(S4) PXSX(S4) RP01(S4) 
PXSX(S4) RP02(S4) PXSX(S4) RP03(S4) PXSX(S4) RP04(S4) PXSX(S4) RP05(S4) 
RP06(S4) BLAN(S4) LID_(S3) SLPB(S3)
acpitimer0 at acpi0: 3579545 Hz, 24 bits
acpihpet0 at acpi0: 14318179 Hz
acpimcfg0 at acpi0 addr 0xe000, bus 0-255
acpimadt0 at acpi0 addr 0xfee0: PC-AT compat
cpu0 at mainbus0: apid 0 (boot processor)
cpu0: Intel(R) Core(TM)2 Duo CPU T6570 @ 2.10GHz, 6494.14 MHz
cpu0: 
FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,DS,ACPI,MMX,FXSR,SSE,SSE2,SS,HTT,TM,SBF,SSE3,MWAIT,DS-CPL,VMX,EST,TM2,SSSE3,CX16,xTPR,PDCM,SSE4.1,XSAVE,NXE,LONG
cpu0: 2MB 64b/line 8-way L2 cache
cpu0: apic clock running at 199MHz
cpu1 at mainbus0: apid 1 (application processor)
cpu1: Intel(R) Core(TM)2 Duo CPU T6570 @ 2.10GHz, 2094.75 MHz
cpu1: 
FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,DS,ACPI,MMX,FXSR,SSE,SSE2,SS,HTT,TM,SBF,SSE3,MWAIT,DS-CPL,VMX,EST,TM2,SSSE3,CX16,xTPR,PDCM,SSE4.1,XSAVE,NXE,LONG
cpu1: 2MB 64b/line 8-way L2 cache
ioapic0 at mainbus0: apid 2 pa 0xfec0, version 20, 24 pins
acpiprt0 at acpi0: bus 0 (PCI0)
acpiprt1 at acpi0: bus -1 (P0P2)
acpiprt2 at acpi0: bus 9 (P0P1)
acpiprt3 at acpi0: bus 2 (RP01)
acpiprt4 at acpi0: bus 3 (RP02)
acpiprt5 at acpi0: bus 4 (RP03)
acpiprt6 at acpi0: bus 5 (RP04)
acpiprt7 at acpi0: bus 6 (RP05)
acpiprt8 at acpi0: bus 8 (RP06)
acpiec0 at acpi0
acpicpu0 at acpi0: C2, C1, PSS
acpicpu1 at acpi0: C2, C1, PSS
acpitz at acpi0 not configured
acpithinkpad0 at acpi0
acpiac0 at acpi0: AC unit online
acpibat0 at acpi0: BAT1 model "42T4751" serial 14540 type LION oem "SANYO"
acpibtn0 at acpi0: LID_
acpibtn1 at acpi0: PWRB
acpibtn2 at acpi0: SLPB
cpu0: Enhanced SpeedStep 2094 MHz: speeds: 2101, 2100, 1600, 1200 MHz
pci0 at mainbus0 bus 0
pchb0 at pci0 dev 0 function 0 "Intel GM45 Host" rev 0x07
vga1 at pci0 dev 2 function 0 "Intel GM45 Video" rev 0x07
wsdisplay0 at vga1 mux 1: console (80x25, vt100 emulation)
wsdisplay0: screen 1-5 added (80x25, vt100 emulation)
intagp0 at vga1
agp0 at intagp0: aperture at 0xd000, size 0x1000
inteldrm0 at vga1: apic 2 int 16 (irq 10)
drm0 at inteldrm0
"Intel GM45 Video" rev 0x07 at pci0 dev 2 function 1 not configured
uhci0 at pci0 dev 26 function 0 "Intel 82801I USB" rev 0x03: apic 2 int 16 (irq 
10)
uhci1 at pci0 dev 26 function 1 "Intel 82801I USB" rev 0x03: apic 2 int 21 (irq 
10)
uhci2 at pci0 dev 26 function 2 "Intel 82801I USB" rev 0x03: apic 2 int 19 (irq 
10)
ehci0 at pci0 dev 26 function 7 "Intel 82801I USB" rev 0x03: apic 2 int 19 (irq 
10)
usb0 at ehci0: USB revision 2.0
uhub0 at usb0 "Intel EHCI root hub" rev 2.00/1.00 addr 1
azalia0 at pci0 dev 27 function 0 "Intel 82801I HD Audio" rev 0x03: apic 2 int 
22 (irq 10)
azalia0: codecs: Realtek ALC269, Intel/0x2802, using Realtek ALC269
audio0 at azalia0
ppb0 at pci0 dev 28 function 0 "Intel 82801I PCIE" rev 0x03: apic 2 int 17 (irq 
11)
pci1 at ppb0 bus 2
"JMicron SD/MMC" rev 0x0

Re: Allegations regarding OpenBSD IPSEC

2010-12-23 Thread Joachim Schipper
On Thu, Dec 23, 2010 at 11:39:59AM +0100, Kurt Knochner wrote:
> 2010/12/22 Marsh Ray :
> > In any case, generic statistical tests might detect really horrible
> > brokenness but they're are not the thing to certify CSRNGs with.
> 
> Really? So, how do you certify the IMPLEMENTATION (bold, not shouting)
> of a CSRNG,  (not the theoretical design)?

There's no really good way to do this at the moment. Yes, that's a
problem.

Joachim



Re: Allegations regarding OpenBSD IPSEC

2010-12-22 Thread Joachim Schipper
On Wed, Dec 22, 2010 at 04:29:59PM +0100, Kurt Knochner wrote:
> 2010/12/22 Theo de Raadt :
> > Go ahead, do a FIPS check on it.  You will be doing a FIPS check on
> > 4096 bytes here, then a gap of unknown length, then 4096 bytes here,
> > then a gap of unknown length, then 4096 bytes here, then a gap of
> > unknown length, 

> Do you have a hint, how I could emit the random values from arc4random
> in a "clever" way?

This isn't even remotely clever, but printf() and some base64 encoding
should work fine for a one-off experiment. There *is* a limit to how
much you can print before you fill up the dmesg; if insufficient, try
compiling with a CONFIG.MP_LARGEBUF like this:

---
include "arch/amd64/conf/GENERIC.MP"

option  MSGBUFSIZE=131072
---

You may wish to look at misc/ent.

Joachim



Re: Improving early randomness (was: Allegations regarding OpenBSD IPSEC)

2010-12-21 Thread Joachim Schipper
On Tue, Dec 21, 2010 at 01:24:55PM -0700, Kjell Wooding wrote:
> >"MD5(time), MD5(time + 1ns), MD5(time + 2ns) etc into the buffer. This
>  does not increase entropy, but having more-or-less uncorrelated data
>  in the entire buffer should make attacks more difficult."
> 
> No. Unless you know something I don't, This is voodoo. To do it once might
> add something, but to do it multiple times, with strongly correlated inputs
> seems potentially dangerous. Especially since you are XORing them. Does
> anyone elsewhere in the cryptographic world do something like this?
> 
> Can you prove there are no statistical weaknesses in MD5 for such inputs?

Note, as has been pointed out to me, that the kernel only relies on the
entropy of nanotime() until we can get some actual data in, i.e. for a
*very* short time. Thus, this whole discussion is probably moot.

Of course I can't prove that MD5 works, but there *is* some actual
reasoning behind the code I sent:

- random XOR anything_uncorrelated is random, so this shouldn't hurt;
- the output of MD5(time) and MD5(time + 1ns) should look very
  different for (practical) hash functions. To the best of my knowledge,
  no vulnerabilities *of this kind* are known in MD5;
- spreading the entropy over the entire key should be preferable to
  concentrating it in a few bits.

That said, the last "should" is not a very strong argument.

I'm not aware what others do; certainly, no cryptographer will be happy
with a PRNG seeded by a timestamp, so this is not exactly best practice
(probably the best we can do at that time, though.)

Joachim



Re: Allegations regarding OpenBSD IPSEC

2010-12-21 Thread Joachim Schipper
On Tue, Dec 21, 2010 at 01:33:46PM -0700, Theo de Raadt wrote:
> I do not understand what hashing principle you are basing this on.

On closer reflection, neither do I ("MD5 in CTR mode"? Cute, but not
necessarily a good idea). Can we just pretend I never sent that message?

Joachim



Re: Allegations regarding OpenBSD IPSEC

2010-12-21 Thread Joachim Schipper
On Tue, Dec 21, 2010 at 01:33:46PM -0700, Theo de Raadt wrote:
> > - Instead of XOR'ing the results of nanotime into the buffer, XOR
> >   MD5(time), MD5(time + 1ns), MD5(time + 2ns) etc into the buffer. This
> >   does not increase entropy, but having more-or-less uncorrelated data
> >   in the entire buffer should make attacks more difficult.
> 
> I do not understand what hashing principle you are basing this on.
> 
> In essence, md5 doesn't care what is in the buffer, or where it is.
> Placing it at the front, vs massaging it in by hand... Fundamentally
> there is no difference... or is there?

This was based on the following intuition, which has very little to do
with hashing at all:

If our RC4 state is , an attacker may be able to
predict *most* of the RC4 state through the first couple of rounds
(until  sufficiently interferes with the known state).

It *seems harder* (but I'm not an expert on this kind of thing!) to
predict the first couple of rounds if  is hashed (which
means that you have to re-do the complete calculation for each possible
, which may not necessarily be the case above), and if
this hashing is used to distribute the noise over the entire initial
state of the cipher (so that no known portion exists).

Again, though, this is just intuition, and it's not wise to trust our
intuition in this kind of thing. I actually *am* a cryptographer, but
I'm quite new at it and a mathematician specializing in a very different
area, so don't take this as gospel. (I'd be willing to spend some more
time looking into this if we consider it important.)

Joachim



Re: Allegations regarding OpenBSD IPSEC

2010-12-21 Thread Joachim Schipper
On Tue, Dec 21, 2010 at 08:27:49PM +0100, Kurt Knochner wrote:
> 2010/12/21 Joachim Schipper :
> > +   get_random_bytes(buf, sizeof(buf));
> > +   nanotime(&ts);
> > +   for (i = 0; i < sizeof(ts); i++)
> > +   buf[i] ^= ((uint8_t *) &ts)[i];
> 
> I like the idea of using XOR. However, there are two issues:
> 
> 1.) if nanotime() was called because of not enough random data from
> get_random_bytes() then you could end up with only the value &ts in
> buf. Why not use a md5 hash of the time value (better than the plain
> time value) or better just don't use the time value at all.

New diff.

Improvements:
- document the "why" of this loop (from Otto's message)
- Instead of XOR'ing the results of nanotime into the buffer, XOR
  MD5(time), MD5(time + 1ns), MD5(time + 2ns) etc into the buffer. This
  does not increase entropy, but having more-or-less uncorrelated data
  in the entire buffer should make attacks more difficult.

Joachim

Index: ../../dev/rnd.c
===
RCS file: /usr/cvs/src/src/sys/dev/rnd.c,v
retrieving revision 1.104
diff -u -p -r1.104 rnd.c
--- ../../dev/rnd.c 21 Nov 2010 22:58:40 -  1.104
+++ ../../dev/rnd.c 21 Dec 2010 20:01:02 -
@@ -779,13 +779,29 @@ get_random_bytes(void *buf, size_t nbyte
 static void
 arc4_stir(void)
 {
-   u_int8_t buf[256];
-   int len;
+   u_int8_t buf[256], md5_buf[MD5_DIGEST_LENGTH];
+   MD5_CTX  md5_ctx;
+   struct timespec  ts;
+   int  i, j;
 
-   nanotime((struct timespec *) buf);
-   len = sizeof(buf) - sizeof(struct timespec);
-   get_random_bytes(buf + sizeof (struct timespec), len);
-   len += sizeof(struct timespec);
+   get_random_bytes(buf, sizeof(buf));
+
+   /*
+* extract_entropy(), and thus get_random_bytes(), may not actually be
+* very random early in the startup sequence of some machines. This is
+* a desperate attempt to increase the randomness in the pool by mixing
+* in nanotime().
+*/
+   nanotime(&ts);
+   KDASSERT(sizeof(buf) % MD5_DIGEST_LENGTH == 0);
+   for (i = 0; i < sizeof(buf); i += MD5_DIGEST_LENGTH) {
+   ts.tv_nsec = (ts.tv_nsec + 1) % (1000 * 1000 * 1000);
+   MD5Init(&md5_ctx);
+   MD5Update(&md5_ctx, (u_int8_t *) &ts, sizeof(ts));
+   MD5Final(md5_buf, &md5_ctx);
+   for (j = 0; j < MD5_DIGEST_LENGTH; j++)
+   buf[i + j] ^= md5_buf[j];
+   }
 
mtx_enter(&rndlock);
if (rndstats.arc4_nstirs > 0)
@@ -793,7 +809,7 @@ arc4_stir(void)
 
rc4_keysetup(&arc4random_state, buf, sizeof(buf));
arc4random_count = 0;
-   rndstats.arc4_stirs += len;
+   rndstats.arc4_stirs += sizeof(buf);
rndstats.arc4_nstirs++;
 
/*



Re: Allegations regarding OpenBSD IPSEC

2010-12-21 Thread Joachim Schipper
On Tue, Dec 21, 2010 at 06:51:03PM +0100, Otto Moerbeek wrote:
> On Tue, Dec 21, 2010 at 05:59:33PM +0100, Kurt Knochner wrote:
> > OpenBSD uses arc4random() and arc4random_buf() all over the code to generate
> > random numbers. This code is defined in src/sys/dev/rnd.c.

> > [arc4stir] initializes the RC4 context with some random data, gathered by 
> > system
> > enthropy, that is mainly done by get_random_bytes().
> > 
> > ==> Bug #1
> > 
> > HOWEVER: Have a look at the buffer that's beeing used as a seed for the RC4
> > key setup. It's beeing filled with the random data, BUT at the beginning it
> > will be filled with just the value of nanotime().
> > 
> > >nanotime((struct timespec *) buf);
> > >len = sizeof(buf) - sizeof(struct timespec);
> > >get_random_bytes(buf + sizeof (struct timespec), len);
> > >len += sizeof(struct timespec);
> > 
> > 
> > So, there is a lot of effort in get_random_bytes() to get "real random" data
> > for the buffer and then the value of nanotime() is prepended to the buffer?
> > That does not look right. Please consider: this buffer will be used as key
> > for  rc4_keysetup() and thus it should contain unrelated and unpredictable
> > data.
> 
> I don't know the answer to this question, but my guess is that the
> buffer is filled by nanotime() to cover the case that
> get_random_bytes() does not have enough entropy available, so at least
> some non-constant data is used. 

Wouldn't it be better to XOR this into the data returned from
get_random_bytes(), though? That'd get you optimal entropy in either
case.

Sample diff (which also removes the len variable):

Index: rnd.c
===
RCS file: /usr/cvs/src/src/sys/dev/rnd.c,v
retrieving revision 1.104
diff -u -p -r1.104 rnd.c
--- rnd.c   21 Nov 2010 22:58:40 -  1.104
+++ rnd.c   21 Dec 2010 18:19:35 -
@@ -779,13 +779,14 @@ get_random_bytes(void *buf, size_t nbyte
 static void
 arc4_stir(void)
 {
-   u_int8_t buf[256];
-   int len;
+   u_int8_t buf[256];
+   struct timespec  ts;
+   int  i;
 
-   nanotime((struct timespec *) buf);
-   len = sizeof(buf) - sizeof(struct timespec);
-   get_random_bytes(buf + sizeof (struct timespec), len);
-   len += sizeof(struct timespec);
+   get_random_bytes(buf, sizeof(buf));
+   nanotime(&ts);
+   for (i = 0; i < sizeof(ts); i++)
+   buf[i] ^= ((uint8_t *) &ts)[i];
 
mtx_enter(&rndlock);
if (rndstats.arc4_nstirs > 0)
@@ -793,7 +794,7 @@ arc4_stir(void)
 
rc4_keysetup(&arc4random_state, buf, sizeof(buf));
arc4random_count = 0;
-   rndstats.arc4_stirs += len;
+   rndstats.arc4_stirs += sizeof(buf);
rndstats.arc4_nstirs++;
 
/*

> > ==> Bug #2
> > 
> > The function rc4_crypt() get's called as soon as rndstats.arc4_nstirs > 0.
> > This will be the case whenever arc4_stir get's called the second time (by
> > the timer reset - see above).
> > 
> > >if (rndstats.arc4_nstirs > 0)
> > >rc4_crypt(&arc4random_state, buf, buf, sizeof(buf));
> > 
> > >rc4_keysetup(&arc4random_state, buf, sizeof(buf));
> > >arc4random_count = 0;
> > >rndstats.arc4_stirs += len;
> > >rndstats.arc4_nstirs++;
> > 
> > HOWEVER, right after the call of rc4_crypt(), we call rc4_keysetup() with
> > the same 'arc4random_state'. This makes the call to rc4_crypt() useless, as
> > the data structure will be overwritten again with the init data of the RC4
> > function.
> 
> rc4_crypt() changes both the state and the contents of buf, since buf is
> used both as source and destination. That buf is used by rc4_keysetup()
> to create a new state. So indeed the state is overwritten, but the
> contents of buf produced by rc4_crypt() is used to do that.

Yes, that does seem right.

By the way, the arc4random_count variable doesn't seem to be used; is
there a reason not to garbage-collect it?

Joachim



Re: slower logins

2010-12-16 Thread Joachim Schipper
On Thu, Dec 16, 2010 at 08:14:06AM -0700, Bob Beck wrote:
> > Why not? An attacker can, after all, brute-force your password on a
> > machine of his choice. Silently decreasing the number of rounds on older
> > architectures surprises the user in a way that can lead to password
> > compromise ("My password was brute-forced because I used it on a sparc?!
> > I would have been fine on amd64? Huh? What happened to 'secure by
> > default'?!")
> 
> > .. and I only use new machines...
> 
> And that is exactly my point.  By your logic let's switch you to a
> 2^25 round blowfish on you dumbass. you'd be incredibly secure.
> 
> Show me colin percivals' peer reviewed paper about this new scheme,
> and where it has been compared to bcrypt.  then go read neil's paper
> on the subject please.

My message was too emotional, and for that I apologize. Sorry.

I share your concern for the newness of scrypt, and in fact I edited out
a sentence to that effect. With respect to the paper: are you talking
about "A Future-Adaptable Password Scheme" by Niels Provos and David
Mazihres? If not, my Google-fu failed me and I would, honestly,
appreciate a pointer to said paper.

Joachim



Re: slower logins

2010-12-16 Thread Joachim Schipper
On Wed, Dec 15, 2010 at 09:42:52PM -0700, Bob Beck wrote:
> I don't mind [increasing the number of Blowfish rounds] if the
> eventual goal is to think about diddling with it per arch..
> 
> I certainly do NOT want a 2^11 blowfish password when logging into my
> sparc

Why not? An attacker can, after all, brute-force your password on a
machine of his choice. Silently decreasing the number of rounds on older
architectures surprises the user in a way that can lead to password
compromise ("My password was brute-forced because I used it on a sparc?!
I would have been fine on amd64? Huh? What happened to 'secure by
default'?!")

One *could* consider using a memory- instead of CPU-bound function to
calculate the password hash. Since, historically, memory access times
have increased less than effective CPU speeds, this may give decent
security without penalizing old machines quite as much.

Colin Percival (FreeBSD security officer) has proposed scrypt
(security/scrypt; BSD license), which is supposed to be both memory- and
CPU-bound. Perhaps you could do some test runs on your sparc to see if
the above theory actually holds up? (Leaving open the question of
whether OpenBSD should switch - bcrypt *is* more battle-tested.)

FWIW, I've used 2^10 rounds for years and never had any problems. Then
again, I've only used machines made in the last 10 years...

Joachim



Re: Allegations regarding OpenBSD IPSEC

2010-12-16 Thread Joachim Schipper
On Wed, Dec 15, 2010 at 07:04:27PM +, Kevin Chadwick wrote:
> "Jason L. Wright"  wrote:
> >I cannot fathom his motivation for writing such falsehood

> >The real work on OCF did not begin in earnest until February 2000.
> 
> I can't see how this gives you credibility but maybe the people who
> worked with you at the time can understand how your evidence supports
> what you say.

While the whole thing is most likely FUD, Perry did say

  Jason Wright and several other developers were responsible for those
  backdoors, and you would be well advised to review any and all code
  commits by Wright as well as the other developers he worked with
  originating from NETSEC.

so it's not like Jason is the only one.

Joachim



Re: hotplug(4) r1.10 ignoring hotplug_put_event() prior to hotplugopen() [Was: hotplugd(8) ignoring devices attached before boot]

2010-12-14 Thread Joachim Schipper
On Mon, Dec 13, 2010 at 09:27:51PM +0100, Mark Kettenis wrote:
> > From: Ted Unangst 
> > On Mon, Dec 13, 2010 at 7:41 AM, Mark Kettenis  
> > wrote:
> > > (...) I don't really agree with Tedu that the
> > > changed behaviour is an improvement.  Say I have configured
> > > hotplugd(8) such that it automatically mounts things when I plug in my
> > > camera.  Now I reboot my machine, without unplugging the camera.
> > > Previously hotplugd(8) would remount things upon boot.  Now suddenly
> > > it doesn't and I have to unplug and replug the camera.
> > 
> > I think the solution to that is to make adding the duid to fstab work.
> >  At boot, if the duid exists, it's mounted.  If it doesn't, it doesn't
> > mount but also doesn't error out.  This may already work even, I
> > haven't tried it.
> 
> Sorry, but a FAT-formatted USB device (which most cameras effectively
> are) will never have a duid.

FAT has volume labels; I'm not saying that too much hackery would be
desirable, but it's not impossible to abuse these as a kind-of-duid.

Joachim



Re: apmd action scripts

2010-12-02 Thread Joachim Schipper
On Wed, Dec 01, 2010 at 11:05:03PM +0100, Holger Mikolon wrote:
> Hi tech@ !
> 
> A couple of times now I didn't notice when my laptop battery reached the 0% 
> remaining capacity. I am not aware of any "tool" in base that could issue
> a beep or nice sound in case of critical battery.
> 
> Currently, apmd reports battery and power events to syslog.
> Below is a patch to let apmd report battery state, battery life and
> power state as parameters to an action script (/etc/apm/powerchange).
> This makes the powerup and powerdown scripts obsolete.
> I also included my powerchange script as an example.

For your information and that of people reading the archives: the
current way to do this is sensorsd.

Still, this may be more obvious.

Joachim



Re: allow bioctl to read passphrase from stdin

2010-11-30 Thread Joachim Schipper
On Mon, Nov 29, 2010 at 02:22:35PM -0800, Chris Kuethe wrote:
> Currently bioctl invokes readpassphrase(3) with RPP_REQUIRE_TTY, which
> means that there must be a controlling tty to read the password from.
> This diff adds an option (-s) to force bioctl to read the passphrase
> from stdin. Without this option existing behavior is maintained.

As I recall, last time someone wanted such functionality (perhaps for
vnconfig?), the response was "this makes doing stupid things too easy,
and you should use -k if you want to mount without typing a passphrase".
(Presumably because a keydisk has better entropy than a passphrase.)

In short, what's the use?

Joachim

> Index: bioctl.8
> ===
> RCS file: /cvs/src/sbin/bioctl/bioctl.8,v
> retrieving revision 1.82
> diff -u -p -r1.82 bioctl.8
> --- bioctl.8  20 Nov 2010 17:46:24 -  1.82
> +++ bioctl.8  29 Nov 2010 22:17:03 -
> @@ -43,7 +43,7 @@
>  .Pp
>  .Nm bioctl
>  .Bk -words
> -.Op Fl dhiPqv
> +.Op Fl dhiPqsv
>  .Op Fl C Ar flag[,flag,...]
>  .Op Fl c Ar raidlevel
>  .Op Fl k Ar keydisk
> @@ -235,6 +235,11 @@ the PBKDF2 algorithm used to convert a p
>  Higher iteration counts take more time, but offer more resistance to key
>  guessing attacks.
>  The minimum is 1000 rounds and the default is 8192.
> +.It Fl s
> +Read the passphrase for the selected crypto volume from
> +.Pa /dev/stdin
> +rather than
> +.Pa /dev/tty .
>  .El
>  .Sh EXAMPLES
>  The following command, executed from the command line, would configure
> Index: bioctl.c
> ===
> RCS file: /cvs/src/sbin/bioctl/bioctl.c,v
> retrieving revision 1.97
> diff -u -p -r1.97 bioctl.c
> --- bioctl.c  10 Jul 2010 02:56:16 -  1.97
> +++ bioctl.c  29 Nov 2010 22:17:03 -
> @@ -86,6 +86,7 @@ int rflag = 8192;
>  char *password;
> 
>  struct bio_locatebl;
> +int rpp_flag = RPP_REQUIRE_TTY;
> 
>  int
>  main(int argc, char *argv[])
> @@ -106,7 +107,7 @@ main(int argc, char *argv[])
>   if (argc < 2)
>   usage();
> 
> - while ((ch = getopt(argc, argv, "a:b:C:c:dH:hik:l:Pp:qr:R:vu:")) !=
> + while ((ch = getopt(argc, argv, "a:b:C:c:dH:hik:l:Pp:qr:R:svu:")) !=
>   -1) {
>   switch (ch) {
>   case 'a': /* alarm */
> @@ -174,6 +175,9 @@ main(int argc, char *argv[])
>   ss_func = BIOC_SSREBUILD;
>   al_arg = optarg;
>   break;
> + case 's':
> + rpp_flag = RPP_STDIN;
> + break;
>   case 'v':
>   verbose = 1;
>   break;
> @@ -252,12 +256,12 @@ usage(void)
>   "[-R device | channel:target[.lun]\n"
>   "\t[-u channel:target[.lun]] "
>   "device\n"
> -"   %s [-dhiPqv] "
> -"[-C flag[,flag,...]] [-c raidlevel] [-k keydisk]\n"
> -"\t[-l special[,special,...]] [-p passfile]\n"
> -"\t[-R device | channel:target[.lun] [-r rounds] "
> + "   %s [-dhiPqsv] "
> + "[-C flag[,flag,...]] [-c raidlevel] [-k keydisk]\n"
> + "\t[-l special[,special,...]] [-p passfile]\n"
> + "\t[-R device | channel:target[.lun] [-r rounds] "
>   "device\n", __progname, __progname);
> - 
> +
>   exit(1);
>  }
> 
> @@ -1070,14 +1074,14 @@ derive_key_pkcs(int rounds, u_int8_t *ke
>   fclose(f);
>   } else {
>   if (readpassphrase(prompt, passphrase, sizeof(passphrase),
> - RPP_REQUIRE_TTY) == NULL)
> + rpp_flag) == NULL)
>   errx(1, "unable to read passphrase");
>   }
> 
>   if (verify) {
>   /* request user to re-type it */
>   if (readpassphrase("Re-type passphrase: ", verifybuf,
> - sizeof(verifybuf), RPP_REQUIRE_TTY) == NULL) {
> + sizeof(verifybuf), rpp_flag) == NULL) {
>   memset(passphrase, 0, sizeof(passphrase));
>   errx(1, "unable to read passphrase");
>   }



Re: better random devices

2010-10-01 Thread Joachim Schipper
On Fri, Oct 01, 2010 at 11:36:25AM -0400, Ted Unangst wrote:
> nobody should really be using srandom, but we provide it and it's a 
> tempting target, so they do.  let's give them arandom instead.  they'll 
> never know the difference, except it may actually work.  :)
> 
> i included a conversion for urandom on the same principle.

I'm not sure i like this more than a man page patch or symlink, but it's
certainly not my bikeshed. A few trivial comments inline.

> Index: rnd.c
> ===
> RCS file: /home/tedu/cvs/src/sys/dev/rnd.c,v
> retrieving revision 1.102
> diff -u -r1.102 rnd.c
> --- rnd.c 20 Apr 2010 22:05:41 -  1.102
> +++ rnd.c 30 Sep 2010 00:28:48 -

> @@ -1073,10 +1039,7 @@
>  
>   revents = events & (POLLOUT | POLLWRNORM);  /* always writable */
>   if (events & (POLLIN | POLLRDNORM)) {
> - if (minor(dev) == RND_SRND && random_state.entropy_count <= 0)
> - selrecord(p, &rnd_rsel);
> - else
> - revents |= events & (POLLIN | POLLRDNORM);
> + revents |= events & (POLLIN | POLLRDNORM);
>   }
>  
>   return (revents);

You don't need the if() here anymore, just replace the whole thing with

revents = events & (POLLIN | POLLOUT | POLLRDNORM | POLLWRNORM);

> @@ -1164,8 +1127,7 @@
>   }
>   }
>  
> - if ((minor(dev) == RND_ARND || minor(dev) == RND_ARND_OLD) &&
> - !ret)
> + if (!ret)
>   arc4random_initialized = 0;
>  
>   free(buf, M_TEMP);
> 

While you're there, you may also want to replace the 'n / 4' a few lines
above by 'n / sizeof(uint32_t)', for consistency.

I'm running with this patch now, and nothing is obviously broken.

Joachim



random(4) clarifications (was: How to use /dev/srandom)

2010-10-01 Thread Joachim Schipper
On Wed, Sep 29, 2010, Theo de Raadt wrote to m...@openbsd.org:
> [Ted Unangst wrote:  -- Joachim]
> > On Wed, Sep 29, 2010 at 12:49 PM, Kevin Chadwick  
> > wrote:
> > >> [Joachim Schipper wrote:  -- Joachim]
> > >> > And isn't srandom sometimes (very rarely!) appropriate? E.g. for
> > >> > generating encryption keys?
> > 
> > If arandom is somehow not appropriate for generating keys, it should
> > be fixed.  I'd be interested to hear more.
> 
> For those who don't want to go read the code, the algorith on the very back
> end is roughly this:
> 
> (a) collect entropy until there is a big enough buffer
> (b) fold it into the srandom buffer, eventually
> 
> That is just like the past.
> 
> But the front end is different.  From the kernel side:
> 
> (1) grab a srandom buffer and start a arc4 stream cipher on it
>(discarding the first bit, of course)
> (2) now the kernel starts taking data from this on every packet
>it sends, to modulate this, to modulate that, who knows.
> (3) lots of other subsystems get small chunks of random from the
>stream; deeply unpredictable when
> (4) on very interrupt, based on quality, the kernel injects something
>into (a)
> (5) re-seed the buffer as stated in (1) when needed
> 
> Simultaneously, userland programs need random data:
> 
> (i) libc does a sysctl to get a chunk from the rc4 buffer
> (ii) starts a arc4 buffer of it's own, in that program
> (iii) feeds data to the program, and re-seeds the buffer when needed
>  
> The arc4 stream ciphers get new entropy when they need. But the really
> neat architecture here is that a single stream cipher is *unpredictably*
> having entropy taken out of it, by hundreds of consumers.  In regular
> unix operating systems, there are only a few entropy consumers.  In OpenBSD
> there are hundreds and hundreds.  The entire system is full of random number
> readers, at every level.  That is why this works so well.
> 
> > > I notice arandom doesn't pause. Is arandom always better or only when
> > > there's enough entropy?
> > 
> > It is more efficient.  There is almost always enough entropy for
> > arandom, and if there isn't, you would have a hard time detecting
> > that.
> 
> There is always enough.  The generator will keep moving, until it has fetched
> too much, or too much time has gone by.  Then it reseeds; though I think
> it fundamentally does not care if the srandom buffer it feeds from is full
> or not.

My, how embarrassing. I could have figured that out.

Still, the man page does allow these misconceptions to persist. Perhaps
the following patch would improve matters?

- terminology: (A)RC4 is not a hash/message digest like MD5;
- clarify what srandom(4), urandom(4) and arandom(4) do and how they
  compare;
- "pauses while more of such data is collected" -> "pauses while more
  data is collected" (clear from the context and less awkward);
- clarify that arandom(4) is pretty much always the right choice.

This does not attempt to explain *why* arandom(4) continues to output
high-quality data even when the entropy pool runs low (the reasons,
(pseudorandom) "generator" and "simultaneous", *are* named); I couldn't
think of a few to express it concisely enough for a man page.

Joachim

Index: random.4
===
RCS file: /usr/cvs/src/src/share/man/man4/random.4,v
retrieving revision 1.22
diff -u -p -r1.22 random.4
--- random.410 Oct 2008 20:13:29 -  1.22
+++ random.430 Sep 2010 19:21:33 -
@@ -42,31 +42,30 @@ The various
 devices produce random output data with different random qualities.
 Entropy data is collected from system activity (like disk and
 network device interrupts and such), and then run through various
-hash or message digest functions to generate the output.
+algorithms to generate the output.
 .Bl -hang -width /dev/srandomX
 .It Pa /dev/random
 This device is reserved for future support of hardware
 random generators.
 .It Pa /dev/srandom
-Strong random data.
-This device returns reliable random data.
+Data directly from the entropy pool, converted using MD5.
 If sufficient entropy is not currently available (i.e., the entropy
-pool quality starts to run low), the driver pauses while more of
-such data is collected.
-The entropy pool data is converted into output data using MD5.
+pool quality starts to run low), the driver pauses while more
+data is collected.
 .It Pa /dev/urandom
-Same as above, but does not guarantee the data to be strong.
-The entropy pool data is converted into output data using MD5.
+Data directly from the entropy p

fuser(1): -MN are also extensions to POSIX

2010-09-24 Thread Joachim Schipper
The fuser(1) man page mentions that -ks are extensions to POSIX. This is
true, but so are -MN; add them.

(Compare e.g.
http://www.opengroup.org/onlinepubs/009695399/utilities/fuser.html.)

Joachim

Index: fuser.1
===
RCS file: /usr/cvs/src/src/usr.bin/fstat/fuser.1,v
retrieving revision 1.4
diff -u -p -r1.4 fuser.1
--- fuser.1 3 Sep 2010 11:09:28 -   1.4
+++ fuser.1 24 Sep 2010 19:30:09 -
@@ -142,5 +142,5 @@ utility is compliant with the
 specification.
 .Pp
 The flags
-.Op Fl ks
+.Op Fl kMNs
 are extensions to that specification.



Re: find(1) manpage patch (was: system/6462: find(1) -print wtf [non-bug])

2010-09-15 Thread Joachim Schipper
On Wed, Sep 15, 2010 at 01:11:55AM +0200, Ingo Schwarze wrote:
> So on the basis of Joachim's last diff, i'm trying to put everything
> together to get it off the table:
> 
>  * I do think that -or in the last example makes sense.  That example
>is not just about precedence in general, but it also makes the point
>that explicit final -print is different from implicit final -print:
>the former needs explicit parentheses, which the latter doesn't.
>  * find -print is not only dangerous with rm, but with practically
>all other programs, in particular the shell.
>Perhaps this even shorter wording is clearer:
>  "Passing output of find to other programs requires some care."
>  * Display the example file names in CAVEATS as suggested by Jason.

This is nice, but how about changing this

> +.Pp
> +Passing output of
> +.Nm
> +to other programs requires some care:
> +.Pp
> +.Dl "$ find . -name \e*.jpg | xargs rm"
> +or
> +.Dl "$ rm `find . -name \e*.jpg`"
> +.Pp
> +would, given file names
> +.Dq important\ .jpg
> +and
> +.Dq important ,
> +remove
> +.Dq important .
> +Use the
> +.Ic -print0
> +or
> +.Ic -exec
> +primaries instead.

to read '*files* "important .jpg" and "important"'?

Joachim



Re: find(1) manpage patch (was: system/6462: find(1) -print wtf [non-bug])

2010-09-14 Thread Joachim Schipper
On Tue, Sep 14, 2010 at 02:48:01PM +0100, Jason McIntyre wrote:
> On Tue, Sep 14, 2010 at 01:42:17PM +0200, Joachim Schipper wrote:
> > On Mon, Sep 13, 2010 at 08:01:28PM +0100, Jason McIntyre wrote:
> > > On Mon, Sep 13, 2010 at 02:49:58PM +0200, Joachim Schipper wrote:
> > > > The first diff changes the last example in find(1). This diff:
> > > > (...)
> > > > - adds handling of *.gif files to illustrate ordering;
> > > 
> > > but we already have an example that does this:
> > > 
> > >   Print out a list of all the files which are not both newer than
> > >   ``ttt'' and owned by ``wnj'':
> > > 
> > >   $ find / \! \( -newer ttt -user wnj \)
> > > 
> > > i think that covers it well enough.
> > 
> > In the message to bugs@ that started this thread, djm@ got the ordering
> > of -o, specifically, wrong. I agree that the order of "and" is obvious,
> > but reasonable people can clearly believe that
> > 
> > $ find . -name \*.jpg -o -name \*.gif -print0
> > 
> > will print both *.jpg and *.gif files.
> 
> the poster closed that bug report with the words (from memory) "i should
> have rtfm".

I don't disagree with you that this is already in the man page; but this
change will prevent people from making the same mistake (i.e. has an
upside) and doesn't appear to have any significant downsides (unlike the
second diff, the first adds very little text to the page.)

You seem a reasonable enough fellow, and I see you've worked on find.1
in the past. I don't see why you're so adamantly opposed. Care to
enlighten me?

> > There is merit in being concise, but:
> > - this should be under CAVEATS, no matter where else it appears (i.e. if
> >   you want to prevent duplication, remove it elsewhere);
> 
> where you put this information is a matter of taste. there is nothing,
> nothing, which says it must go in CAVEATS. it might turn out that that
> is the best place for it, but in that case the diff should subtract the
> information already contained.

I think this is something that people using find should be warned about,
hence CAVEATS. Putting it only in the discussion of -X, as in the
previous version, means it doesn't reach people who don't read the
entire page.

> > - getting this wrong is a somewhat serious security problem, and tons of
> >   people get it wrong. Repeating it doesn't hurt.
> 
> right. so please add this information five times, so that for sure
> people will read it.

Would you be happier with a version with less repetition? The following
diff is like the one I sent out earlier, but
- shorten the reference from "-X" to "-print0"
- removes the warning from the description of -print (people using
  "-print" instead of letting find(1) add it by default will probably
  look at CAVEATS too);

With the attached diff, the man page refers to this issue in "-X" and
"-print0" and CAVEATS; would you like to see it removed from one of
these places?

Joachim

Index: find.1
===
RCS file: /cvs/src/usr.bin/find/find.1,v
retrieving revision 1.78
diff -u -p -r1.78 find.1
--- find.1  15 Jul 2010 20:51:38 -  1.78
+++ find.1  14 Sep 2010 14:49:42 -
@@ -126,13 +126,9 @@ quotes, backslash
 space, tab, and newline
 .Pq Sq \en
 characters.
-Alternatively, the
+Consider using
 .Fl print0
-primary may be used in conjunction with the
-.Fl 0
-option to
-.Xr xargs 1 ,
-allowing all file names to be processed safely.
+instead.
 .It Fl x
 Prevents
 .Nm
@@ -386,7 +382,10 @@ character.
 .It Ic -print0
 This primary always evaluates to true.
 It prints the pathname of the current file to standard output, followed
-by a null character.
+by a null character, suitable for use with the
+.Fl 0
+option to
+.Xr xargs 1 .
 .It Ic -prune
 This primary always evaluates to true.
 It causes
@@ -525,11 +524,13 @@ ending in a dot and single digit, but sk
 .Pp
 .Dl "$ find /usr/src -path /usr/src/gnu -prune -or -name \e*.[0-9]"
 .Pp
-Find and remove all *.jpg files in the current working directory:
+Find and remove all *.jpg and *.gif files under the current working
+directory
+.Pq see also Sx CAVEATS :
 .Pp
-.Dl "$ find . -name \e*.jpg -exec rm {} \e;"
+.Dl "$ find . \e( -name \e*.jpg -o -name \e*.gif \e) -exec rm {} \e;
 or
-.Dl "$ find . -name \e*.jpg | xargs rm"
+.Dl "$ find . \e( -name \e*.jpg -o -name \e*.gif \e) -print0 | xargs -0r rm"
 .Sh SEE ALSO
 .Xr chflags 1 ,
 .Xr chmod 1 ,
@@ -637,6 +638,25 @@ In particular, the characters
 and
 .Ql \&;
 may have to be escaped from the shell.
+.Pp
+Using
+.Nm
+in combination with
+other programs requires some care:
+.Pp
+.Dl "$ find . -name \e*.jpg | xargs rm"
+or
+.Dl "$ rm `find . -name \e*.jpg`"
+.Pp
+would, given a file named
+.Pa "important .jpg"
+(with a space character in the middle), remove
+.Pa important .
+Use the
+.Ic -print0
+or
+.Ic -exec
+primaries instead.
 .Pp
 As there is no delimiter separating options and file names or file
 names and the



Re: find(1) manpage patch

2010-09-14 Thread Joachim Schipper
On Tue, Sep 14, 2010 at 04:35:24AM +0100, Stuart Henderson wrote:
> On 2010/09/14 01:48, Ingo Schwarze wrote:
> 
> I like most of this, one thing though,

I like those tweaks too; thanks Ingo!

> > +This is dangerous in conjunction with
> > +.Xr xargs 1 ,
> > +see
> > +.Sx CAVEATS .
> 
> This is also dangerous when using the output in some other
> ways, e.g.:  rm `find . -name *.jpg`

That's a good point.

> > @@ -637,6 +646,24 @@
> >  and
> >  .Ql \&;
> >  may have to be escaped from the shell.
> > +.Pp
> > +Using
> > +.Nm
> > +in combination with
> > +.Xr xargs 1
> > +requires some care:
> 
> "Using filenames output by .Nm requires some care:" ?
> 
> (I did think of "Using the -print flag requires some care:"
> but readers may then miss the fact that it is implicit).

How about the following? (First diff is relative to Ingo's diff; second
diff is relative to HEAD.)

I'm not too happy about enumerating mistakes like this, but these *are*
both quite common, and I can't think of any other common mistakes.

Joachim

--- find.1.ingo Tue Sep 14 13:32:26 2010
+++ find.1.mine Tue Sep 14 13:32:51 2010
@@ -383,9 +383,9 @@
 by a newline
 .Pq Ql \en
 character.
-This is dangerous in conjunction with
-.Xr xargs 1 ,
-see
+This is dangerous when used with other programs (like
+.Xr xargs 1
+or a shell), see
 .Sx CAVEATS .
 .It Ic -print0
 This primary always evaluates to true.
@@ -650,10 +650,11 @@
 Using
 .Nm
 in combination with
-.Xr xargs 1
-requires some care:
+other programs requires some care:
 .Pp
 .Dl "$ find . -name \e*.jpg | xargs rm"
+or
+.Dl "$ rm `find . -name \e*.jpg`"
 .Pp
 would, given a file named
 .Pa "important .jpg"


Index: find.1
===
RCS file: /cvs/src/usr.bin/find/find.1,v
retrieving revision 1.78
diff -u -p -r1.78 find.1
--- find.1  15 Jul 2010 20:51:38 -  1.78
+++ find.1  14 Sep 2010 11:32:14 -
@@ -383,10 +383,17 @@ It prints the pathname of the current fi
 by a newline
 .Pq Ql \en
 character.
+This is dangerous when used with other programs (like
+.Xr xargs 1
+or a shell), see
+.Sx CAVEATS .
 .It Ic -print0
 This primary always evaluates to true.
 It prints the pathname of the current file to standard output, followed
-by a null character.
+by a null character, suitable for use with the
+.Fl 0
+option to
+.Xr xargs 1 .
 .It Ic -prune
 This primary always evaluates to true.
 It causes
@@ -525,11 +532,13 @@ ending in a dot and single digit, but sk
 .Pp
 .Dl "$ find /usr/src -path /usr/src/gnu -prune -or -name \e*.[0-9]"
 .Pp
-Find and remove all *.jpg files in the current working directory:
+Find and remove all *.jpg and *.gif files under the current working
+directory
+.Pq see also Sx CAVEATS :
 .Pp
-.Dl "$ find . -name \e*.jpg -exec rm {} \e;"
+.Dl "$ find . \e( -name \e*.jpg -o -name \e*.gif \e) -exec rm {} \e;
 or
-.Dl "$ find . -name \e*.jpg | xargs rm"
+.Dl "$ find . \e( -name \e*.jpg -o -name \e*.gif \e) -print0 | xargs -0r rm"
 .Sh SEE ALSO
 .Xr chflags 1 ,
 .Xr chmod 1 ,
@@ -637,6 +646,25 @@ In particular, the characters
 and
 .Ql \&;
 may have to be escaped from the shell.
+.Pp
+Using
+.Nm
+in combination with
+other programs requires some care:
+.Pp
+.Dl "$ find . -name \e*.jpg | xargs rm"
+or
+.Dl "$ rm `find . -name \e*.jpg`"
+.Pp
+would, given a file named
+.Pa "important .jpg"
+(with a space character in the middle), remove
+.Pa important .
+Use the
+.Ic -print0
+or
+.Ic -exec
+primaries instead.
 .Pp
 As there is no delimiter separating options and file names or file
 names and the



Re: find(1) manpage patch (was: system/6462: find(1) -print wtf [non-bug])

2010-09-14 Thread Joachim Schipper
On Mon, Sep 13, 2010 at 08:01:28PM +0100, Jason McIntyre wrote:
> On Mon, Sep 13, 2010 at 02:49:58PM +0200, Joachim Schipper wrote:
> > The first diff changes the last example in find(1). This diff:
> > (...)
> > - adds handling of *.gif files to illustrate ordering;
> 
> but we already have an example that does this:
> 
>   Print out a list of all the files which are not both newer than
>   ``ttt'' and owned by ``wnj'':
> 
>   $ find / \! \( -newer ttt -user wnj \)
> 
> i think that covers it well enough.

In the message to bugs@ that started this thread, djm@ got the ordering
of -o, specifically, wrong. I agree that the order of "and" is obvious,
but reasonable people can clearly believe that

$ find . -name \*.jpg -o -name \*.gif -print0

will print both *.jpg and *.gif files.

> > - changes to -print0 | xargs -0r to illustrate proper form.
> 
> wouldn;t it be better to just use "find -X"? it is shorter, and the
> description of -X is probably the best place to start reading for most
> people.

find -X does solve some security problems, but only by erroring out. I
think -print0 is superior in every respect.

I agree that, in the current manpage, the description of -X is one of
the better places to point people.

> > The second diff is relative to the first diff, and adds the following
> > warning to CAVEATS:
> > "Using find in combination with xargs(1) requires some care:
> > 
> > $ find . -name \e*.jpg | xargs rm
> > 
> > would, given a file *important\n.jpg* (where *\n* is a newline),
> > remove *important*. Use the -print0 or -X flags, or use -exec
> > instead."
> > 
> > This is already addressed in the man page (e.g. under the description of
> > -print0 or -X), but many people (including the EXAMPLES) get it wrong;
> > hopefully the above warning (and the references from the description of
> > -print and the EXAMPLES section) prevents some people from making this
> > mistake.
> 
> i don;t instinctively like this. if we document something correctly, we
> shouldn;t be adding in duplicate text just in case it gets missed. there
> is enough reading to do already, and what if the second text section is
> also ignored/not understood by the reader?
> 
> i'd prefer to just concentrate on saying something well, once.

There is merit in being concise, but:
- this should be under CAVEATS, no matter where else it appears (i.e. if
  you want to prevent duplication, remove it elsewhere);
- getting this wrong is a somewhat serious security problem, and tons of
  people get it wrong. Repeating it doesn't hurt.

Joachim



find(1) manpage patch (was: system/6462: find(1) -print wtf [non-bug])

2010-09-13 Thread Joachim Schipper
On Mon, Sep 13, 2010 at 09:55:17AM +0200, Joachim Schipper wrote:
> On Thu, Sep 09, 2010 at 10:15:01PM -0600, Tim Chase wrote:
> > On 09/09/10 20:37, d...@mindrot.org wrote [lightly edited]:
> > > $ touch foo.orig foo.rej
> > > $ find . -type f -name \*.orig -or -name \*.rej
> > > ./foo.orig
> > > ./foo.rej
> > > $ find . -type f -name \*.orig -or -name \*.rej -print
> > > ./foo.rej
> > >
> > > [WTF?!]
> >  
> >  Not a bug, but rather order of operations & short-circuit logic 
> >  evaluation.
> 
> Perhaps we could help other people avoid this misunderstanding by
> changing the last EXAMPLE in find(1) to
> 
> $ find . \( -name \*.jpg -or -name \*.gif \) -print0 | xargs -0r rm
> 
> This would demonstrate proper form, too: use print0 when dealing with
> untrusted filenames (...).

[Please snip bugs@ and Stuart from replies.]

Here are two diffs to find(1).

The first diff changes the last example in find(1). This diff:
- makes it clear that it, in fact, works on all files *under* (not just
  "in") the current working directory;
- adds handling of *.gif files to illustrate ordering;
- changes to -print0 | xargs -0r to illustrate proper form.

The second diff is relative to the first diff, and adds the following
warning to CAVEATS:
"Using find in combination with xargs(1) requires some care:

$ find . -name \e*.jpg | xargs rm

would, given a file *important\n.jpg* (where *\n* is a newline),
remove *important*. Use the -print0 or -X flags, or use -exec
instead."

This is already addressed in the man page (e.g. under the description of
-print0 or -X), but many people (including the EXAMPLES) get it wrong;
hopefully the above warning (and the references from the description of
-print and the EXAMPLES section) prevents some people from making this
mistake.

Joachim

Index: find.1
===
RCS file: /cvs/src/usr.bin/find/find.1,v
retrieving revision 1.78
diff -u -p -r1.78 find.1
--- find.1  15 Jul 2010 20:51:38 -  1.78
+++ find.1  13 Sep 2010 12:29:56 -
@@ -525,11 +525,11 @@ ending in a dot and single digit, but sk
 .Pp
 .Dl "$ find /usr/src -path /usr/src/gnu -prune -or -name \e*.[0-9]"
 .Pp
-Find and remove all *.jpg files in the current working directory:
+Find and remove all *.jpg or *.gif files under the current working directory:
 .Pp
-.Dl "$ find . -name \e*.jpg -exec rm {} \e;"
+.Dl "$ find . \e( -name \e*.jpg -o -name \e*.gif \e) -exec rm {} \e;
 or
-.Dl "$ find . -name \e*.jpg | xargs rm"
+.Dl "$ find . \e( -name \e*.jpg -o -name \e*.gif \e) -print0 | xargs -0r rm"
 .Sh SEE ALSO
 .Xr chflags 1 ,
 .Xr chmod 1 ,

--- find.1  Mon Sep 13 14:12:53 2010
+++ find.1.new  Mon Sep 13 14:32:38 2010
@@ -382,7 +382,10 @@
 It prints the pathname of the current file to standard output, followed
 by a newline
 .Pq Ql \en
-character.
+character. This should not be used in conjunction with
+.Xr xargs 1 ,
+see
+.Sx CAVEATS .
 .It Ic -print0
 This primary always evaluates to true.
 It prints the pathname of the current file to standard output, followed
@@ -530,6 +533,13 @@
 .Dl "$ find . \e( -name \e*.jpg -o -name \e*.gif \e) -exec rm {} \e;
 or
 .Dl "$ find . \e( -name \e*.jpg -o -name \e*.gif \e) -print0 | xargs -0r rm"
+.Pp
+Note the use of
+.Ic -exec
+or
+.Ic -print0 ;
+see
+.Sx CAVEATS .
 .Sh SEE ALSO
 .Xr chflags 1 ,
 .Xr chmod 1 ,
@@ -637,6 +647,26 @@
 and
 .Ql \&;
 may have to be escaped from the shell.
+.Pp
+Using
+.Nm
+in combination with
+.Xr xargs 1
+requires some care:
+.Pp
+.Dl "$ find . -name \e*.jpg | xargs rm"
+.Pp
+would, given a file
+.Pa important\en.jpg
+(where \en is a newline), remove
+.Pa important .
+Use the
+.Ic -print0
+or
+.Ic -X
+flags, or use
+.Ic -exec
+instead.
 .Pp
 As there is no delimiter separating options and file names or file
 names and the



Re: RFC: changes to ports infrastructure

2010-08-19 Thread Joachim Schipper
On Thu, Aug 19, 2010 at 03:06:09PM +0200, Marc Espie wrote:
> On Thu, Aug 19, 2010 at 02:46:44PM +0200, Joachim Schipper wrote:
> > On Thu, Aug 19, 2010 at 11:55:22AM +0200, Marc Espie wrote:
> > > I want to tweak the directory structure for ports.

> > > Did I miss anything ?
> > 
> > [Y]ou can't really
> > install a partial ports tree. If all modules were kept in e.g.
> > ports/infrastructure/modules, it may be possible to install only a
> > hypothetical ports-infrastructure.tar.gz (like sys.tar.gz, but for
> > ports) and a couple of individual ports (e.g. under mystuff/).
> > 
> > This would make it very easy to upgrade just one package on a -stable
> > system.
>
> No-go. You also need proper depends to compute default pkgnames for the
> package, so you mostly need the full ports tree anyways.

Yeah, hadn't thought of that. Deriving that information only from
installed packages is probably not feasible.

> And modules was specifically designed to be modular, with no central place...
> I *could* have bsd.port.mk auto-checkout directories when needed with a 
> specific cvs script,
> 
> But considering the small size of the ports tree, it is probably one more
> useless feature.
> though.

I admit, that wouldn't be too useful.

Sorry for bothering you!

Joachim

-- 
TFMotD: motd (5) - message of the day



Re: RFC: changes to ports infrastructure

2010-08-19 Thread Joachim Schipper
On Thu, Aug 19, 2010 at 11:55:22AM +0200, Marc Espie wrote:
> I want to tweak the directory structure for ports.
> I'd like to go for a lot of stuff to 
> 
> ports/infrastructure/bin
> ports/infrastructure/lib
> ports/infrastructure/man
> 
> the current setup (build, fetch, install, package) is my mistake, and
> frankly it sucks. Some of the scripts are callable outside of bsd.port.mk,
> and you have to remember the path. There is no standard documentation out
> of the scripts, and there are a few perl modules that could use one
> single place.
> 
> I would keep db, plist, templates. I don't think they're that confusing.
> 
> The only drawback is that we're going to lose cvs history. doesn't seem
> like such a big problem to me. Also, third party may lose "compatibility"
> as scripts move around. That's easy to solve with symlinks, so I don't
> care too much.
> 
> Did I miss anything ?

Something you may also want to consider: currently, you can't really
install a partial ports tree. If all modules were kept in e.g.
ports/infrastructure/modules, it may be possible to install only a
hypothetical ports-infrastructure.tar.gz (like sys.tar.gz, but for
ports) and a couple of individual ports (e.g. under mystuff/).

This would make it very easy to upgrade just one package on a -stable
system (e.g. to fix a security issue or get an updated port from
-current). It would be even better if FETCH_PACKAGES could be made to
work in this scenario, but that's not required.

This is in no way a must-have feature, but if you're moving stuff
anyway...

Joachim

-- 
TFMotD: sigvec (3) - software signal facilities



tty(4) nit

2010-07-31 Thread Joachim Schipper
Remove a redundant comma in tty(4).

Joachim

Index: tty.4
===
RCS file: /usr/cvs/src/src/share/man/man4/tty.4,v
retrieving revision 1.36
diff -u -p -r1.36 tty.4
--- tty.4   13 Apr 2010 20:38:26 -  1.36
+++ tty.4   6 Jul 2010 19:23:35 -
@@ -60,7 +60,7 @@ These special terminal devices are calle
 .Em ptys
 and provide the mechanism necessary to give users the same interface to the
 system when logging in over a network (using
-.Xr ssh 1 ,
+.Xr ssh 1
 or
 .Xr telnet 1
 for example).



Re: Colemak layout?

2010-07-25 Thread Joachim Schipper
On Sun, Jul 25, 2010 at 03:11:17PM +0300, Timo Myyrd wrote:
> Why the colemak layout hasn't found its way to base?
> There has been a diff to add support but apparently it didn't made
> into base for some reason.

Because it uses precious kernel memory for very little gain. Search the
archives.

Joachim



Re: Patch for bogus pointer arithmetic in adw(4)

2010-06-23 Thread Joachim Schipper
On Tue, Jun 22, 2010 at 03:40:44PM -0300, Hudson Flavio V Mateus wrote:
> >> Is there any reson you use bcopy() not memcpy()?
> >> If not considder using memcpy() please. :)
> 
> > We couldn't care what you believe, unless you have diffs of your own
> > to submit.
> 
> I think the guy there asked if there is any difference, it was just that. I
> also don't know bcopy() and would like to know just out of curiosity (I'm
> really don't know, isn't not an irony): there's some difference between
> bcopy() and memcpy()?

In the hope of bringing *some* merit to this thread: there is, indeed,
an important difference between these functions: bcopy() is defined to
work properly for overlapping buffers (e.g. "bcopy(buf, buf + 1,
sizeof(buf) - 1)" will work as expected). memcpy() does have this
property on OpenBSD, but portable code should obviously not rely on
that. Of course, kernel code is not (meant to be) portable.

Joachim



Re: typo sys/conf/GENERIC

2010-05-12 Thread Joachim Schipper
On Wed, May 12, 2010 at 07:49:54AM -0700, J.C. Roberts wrote:
> On Wed, 12 May 2010 14:36:08 +0200 Joachim Schipper
>  wrote:
> > On Wed, May 12, 2010 at 03:01:50AM -0700, J.C. Roberts wrote:
> > > minor typo
> > 
> > It's meant to be "interface"; following ifconfig(8), one could say
> > 'if' makes more sense than 'i/f' - but it's not a typo.
> 
> All other entries use "if" rather than "i/f"

Ok, fair enough.

Joachim



Re: typo sys/conf/GENERIC

2010-05-12 Thread Joachim Schipper
On Wed, May 12, 2010 at 03:01:50AM -0700, J.C. Roberts wrote:
> minor typo

It's meant to be "interface"; following ifconfig(8), one could say 'if'
makes more sense than 'i/f' - but it's not a typo.

Joachim

> Index: GENERIC
> ===
> RCS file: /cvs/src/sys/conf/GENERIC,v
> retrieving revision 1.156
> diff -N -u -p GENERIC
> --- GENERIC   7 May 2010 13:16:18 -   1.156
> +++ GENERIC   12 May 2010 09:57:26 -
> @@ -112,7 +112,7 @@ pseudo-device vether  # Virtual ethernet
>  pseudo-devicevlan# IEEE 802.1Q VLAN 
>  
>  # for IPv6
> -#pseudo-device   faith   1   # IPv[46] tcp relay translation i/f
> +#pseudo-device   faith   1   # IPv[46] tcp relay translation if
>  
>  pseudo-devicebio 1   # ioctl multiplexing device



[patch] vis(3): NUL terminate -> NUL-terminate

2010-04-23 Thread Joachim Schipper
I noticed that vis(3) talks about "NUL terminated" strings, whereas
almost other sources (including e.g. strlcat(3), strtok(3), strpbrk(3))
talk about "NUL-terminated" strings (i.e. with a hyphen.)

The following patch fixes this.

Joachim

Index: vis.3
===
RCS file: /usr/cvs/src/src/lib/libc/gen/vis.3,v
retrieving revision 1.24
diff -u -p -r1.24 vis.3
--- vis.3   31 May 2007 19:19:29 -  1.24
+++ vis.3   22 Apr 2010 09:10:09 -
@@ -57,7 +57,7 @@ a string which represents the character
 If
 .Fa c
 needs no encoding, it is copied in unaltered.
-The string is NUL terminated and a pointer to the end of the string is
+The string is NUL-terminated and a pointer to the end of the string is
 returned.
 The maximum length of any encoding is four
 characters (not including the trailing NUL);
@@ -107,7 +107,7 @@ characters from
 .Fa src
 (this
 is useful for encoding a block of data that may contain NULs).
-All three forms NUL terminate
+All three forms NUL-terminate
 .Fa dst ,
 except for
 .Fn strnvis



Re: New intel X driver requires testing.

2010-04-22 Thread Joachim Schipper
On Sun, Apr 11, 2010 at 06:47:45PM +0100, Owain Ainsworth wrote:
> The tarball that may be found at http://xenocara.org/intel-current.tgz
> contains an update to the intel 2.9.1 driver (the last one that
> supported userland modesetting) with a load of backports for bugfixes
> and performance improvements from drivers up to 2.11.
> 
> In order to test this, you will need INTELDRM_GEM defined in your kernel
> option INTELDRM_GEM in your kernel config, or a patch to i915_drv.h in
> /sys/dev/pci/drm will suffice) since this driver does not support
> operation without kernel memory mamangement (it tries, but it seems
> broken in many places).

> Then reboot into a GEM kernel to test. Please test this if you have
> access to the hardware because soon GEM will be the default so anything
> that is broken needs to be fixed soon. If you don't test and your stuff
> breaks you can consider it to be your fault.
> 
> Please let myself and matthieu@ know of any testing results, positive or
> negative.

This doesn't seem to work for me. I'll provide what information I can
now; but I'm in a hurry and I'll *try* to be available, but I may not
have internet access until Saturday.

This machine, a Thinkpad SL510, worked fine with the Xenocara/the driver
from a snaphot; I compiled Xenocara (not just the driver - I got compile
errors) for the newer version. (So note that I've never tried this
Xenocara with the old driver; I'm sorry, I'm in a hurry...)

If I use this new driver with a non-INTELDRM_GEM kernel, the screen
(computer?) freezes as soon as X starts. Is this expected?

More seriously, if I do use a kernel with the simple configuration
provided below then the screen (computer?) freezes after a short while
(I *think* it's triggered by scrolling in Firefox but not by anything in
an xterm, but I'm not sure.)

Xorg.conf, dmesg and Xorg.0.log follow. I've added diffs to dmesg and
Xorg.0.log for the old and new kernel.

Thanks for your work!

Joachim

--- kernel configuration ---
include "arch/amd64/conf/GENERIC.MP"

option INTELDRM_GEM


--- Xorg.conf ---
# We only need to specify the FontPath, so we use defauls whereever possible.
Section "Device"
Identifier "Intel GM45 video driver"
Driver "intel"
EndSection
Section "Files"
FontPath   "/usr/X11R6/lib/X11/fonts/local/"
FontPath   "/usr/X11R6/lib/X11/fonts/misc/"
FontPath   "/usr/X11R6/lib/X11/fonts/75dpi/:unscaled"
FontPath   "/usr/X11R6/lib/X11/fonts/100dpi/:unscaled"
# package ghostscript-fonts
FontPath   "/usr/local/share/ghostscript/fonts/"
FontPath   "/usr/X11R6/lib/X11/fonts/Type1/"
FontPath   "/usr/X11R6/lib/X11/fonts/75dpi/"
FontPath   "/usr/X11R6/lib/X11/fonts/100dpi/"
# package terminus
FontPath   "/usr/local/lib/X11/fonts/terminus/"
EndSection
-

--- dmesg ---
OpenBSD 4.7-current (GENERIC.MP_WITH_INTELDRM_GEM) #0: Wed Apr 21 16:27:07 CEST 
2010

r...@polymnia.sshunet.nl:/usr/src/sys/arch/amd64/compile/GENERIC.MP_WITH_INTELDRM_GEM
real mem = 2003517440 (1910MB)
avail mem = 1940152320 (1850MB)
mainbus0 at root
bios0 at mainbus0: SMBIOS rev. 2.5 @ 0xe0010 (44 entries)
bios0: vendor LENOVO version "6JET65WW (1.23 )" date 10/19/2009
bios0: LENOVO 28477LG
acpi at bios0 not configured
mpbios0 at bios0: Intel MP Specification 1.4
cpu0 at mainbus0: apid 0 (boot processor)
cpu0: Intel(R) Core(TM)2 Duo CPU T6570 @ 2.10GHz, 2095.10 MHz
cpu0: 
FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,DS,ACPI,MMX,FXSR,SSE,SSE2,SS,HTT,TM,SBF,SSE3,MWAIT,DS-CPL,VMX,EST,TM2,SSSE3,CX16,xTPR,PDCM,SSE4.1,XSAVE,NXE,LONG
cpu0: 2MB 64b/line 8-way L2 cache
cpu0: apic clock running at 199MHz
cpu1 at mainbus0: apid 1 (application processor)
cpu1: Intel(R) Core(TM)2 Duo CPU T6570 @ 2.10GHz, 2094.75 MHz
cpu1: 
FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,DS,ACPI,MMX,FXSR,SSE,SSE2,SS,HTT,TM,SBF,SSE3,MWAIT,DS-CPL,VMX,EST,TM2,SSSE3,CX16,xTPR,PDCM,SSE4.1,XSAVE,NXE,LONG
cpu1: 2MB 64b/line 8-way L2 cache
mpbios0: bus 0 is type PCI   
mpbios0: bus 2 is type PCI   
mpbios0: bus 5 is type PCI   
mpbios0: bus 8 is type PCI   
mpbios0: bus 16 is type ISA   
ioapic0 at mainbus0: apid 2 pa 0xfec0, version 20, 24 pins
cpu0: unknown Enhanced SpeedStep CPU, msr 0x06114a2306004a23
cpu0: using only highest and lowest power states
cpu0: Enhanced SpeedStep 2094 MHz: speeds: 14800, 1200 MHz
pci0 at mainbus0 bus 0
pchb0 at pci0 dev 0 function 0 "Intel GM45 Host" rev 0x07
vga1 at pci0 dev 2 function 0 "Intel GM45 Video" rev 0x07
wsdisplay0 at vga1 mux 1: console (80x25, vt100 emulation)
wsdisplay0: screen 1-5 added (80x25, vt100 emulation)
intagp0 at vga1
agp0 at intagp0: aperture at 0xd000, size 0x1000
inteldrm0 at vga1: apic 2 int 16 (irq 10)
drm0 at inteldrm0
"Intel GM45 Video" rev 0x07 at pci0 dev 2 function 1 not configured
uhci0 at pci0 dev 26 function 0 "Intel 82801I USB"

Re: save some entropy

2009-12-15 Thread Joachim Schipper
On Mon, Dec 14, 2009 at 09:58:19AM +0100, Otto Moerbeek wrote:
> On Wed, Dec 02, 2009 at 10:38:10AM +0100, Otto Moerbeek wrote:
> > [A]part from the random page addresses obtained form mmap(2) malloc(3)
> > itself also randomizes cache en chunk operations. It uses a nibble of
> > randomness per call, so optimize that to not waste half the random
> > bits. 
> > 
> > Please test, should be a bit faster.
> 
> Anybody?

I just built a recent checkout with and without this patch included. It
seems to be stable. Somewhat to my surprise, it also seems to be
measurably faster on my system.

Without patch:
$ gcc -Wall -O2 -DNDEBUG -o malloc-test malloc-test.c && time ./malloc-test && 
time ./malloc-test && time ./malloc-test && time ./malloc-test && time 
./malloc-test && time ./malloc-test
0m32.11s real 0m28.88s user 0m2.60s system
0m32.05s real 0m28.22s user 0m3.17s system
0m44.20s real 0m40.11s user 0m3.36s system
0m32.25s real 0m28.52s user 0m3.40s system
0m31.57s real 0m28.41s user 0m3.02s system
0m31.74s real 0m28.36s user 0m2.98s system

With patch:
$ gcc -Wall -O2 -DNDEBUG -o malloc-test malloc-test.c && time ./malloc-test && 
time ./malloc-test && time ./malloc-test && time ./malloc-test && time 
./malloc-test && time ./malloc-test
0m32.11s real 0m28.88s user 0m2.60s system
0m29.64s real 0m26.39s user 0m3.21s system
0m29.32s real 0m26.21s user 0m3.05s system
0m29.31s real 0m25.89s user 0m3.35s system
0m29.57s real 0m26.51s user 0m3.01s system
0m29.56s real 0m26.44s user 0m3.10s system
0m29.50s real 0m26.06s user 0m3.36s system

The program I used to test this is very much ad-hoc, does not use memory
in a realistic fashion, etc, so this is not a proper (hah!) benchmark.
But having some numbers may be better than nothing.

malloc-test.c:
#include 
#include 
#include 

int
main(void)
{
void*p[8][16 * 1024];
int  i, j, k, l;

for (l = 0; l < 512; l++) {
for (i = 0; i < sizeof(p) / sizeof(*p); i++) {
for (j = 0; j < sizeof(p[i]) / sizeof(*p[i]); j++)
if ((p[i][j] = malloc(1 << i)) == NULL)
err(1, "malloc() failed");

for (k = i; k >= 0; k--) {
for (j = i; j < sizeof(p[i]) / sizeof(*p[i]); j 
+= sizeof(p) / sizeof(*p)) {
assert(p[k][j] != NULL);
free(p[k][j]);
p[k][j] = NULL;
}
}
}

for (i = 0; i < sizeof(p) / sizeof(*p); i++)
for (j = 0; j < sizeof(p[i]) / sizeof(*p[i]); j++)
free(p[i][j]);
}

return 0;
}

For completeness, a dmesg may be found at
http://www.joachimschipper.nl/posts/20091215/dmesg.

Should anyone care, the program above may be used for any purpose
whatsoever, with or without attribution ("public domain").

Joachim



Re: vfs cache diff, that needs some testing.

2009-06-13 Thread Joachim Schipper
On Sat, Jun 13, 2009 at 12:52:11PM +, Thordur I. Bjornsson wrote:
> Dorian B|ttner  wrote on Fri 12.Jun'09 at 19:25:05 
> +0200
> 
>  AFAIK the whole work was done to make the cache more sane. The current
>  version is just insane enough that Bob was crying, shouting and playing
>  with red wine bottles during c2k9.
>    
> > Hi Bob,
> >
> > tried your patch, got a kernel panic with it and took some screen shots  
> > with my digicam. Due to the panic message I wanted to ask first if I  
> > should upload the pix to some public space and link in here or rather  
> > send off list?
> 
> Anyone seen this panic yet ? If it all possible, post it somewhere so
> we can take a look.

I'm running it on amd64, and it works for me.

Joachim



Re: [patch] Make tcpbench server non-forking and non-blocking.

2009-06-09 Thread Joachim Schipper
On Tue, Jun 09, 2009 at 03:12:36PM +1000, Damien Miller wrote:
> On Mon, 8 Jun 2009, Christiano Farina Haesbaert wrote:
 
> > Another feature I was thinking would be to dump the output in one file
> > per host. What do you guys think ? Besides correcting fd limit, udp
> > support, any other ideas ?
> 
> Whatever the network hackers think is useful :)

I don't particularly know what I'm talking about, but it occurs to me
that stuff like latency (average/standard deviation) might be useful as
well.

Joachim



Re: [PATCH] bioctl(8): fix dd invocation

2009-05-09 Thread Joachim Schipper
On Sat, May 09, 2009 at 02:27:08PM +0200, Mark Kettenis wrote:
> > Date: Sat, 9 May 2009 12:20:25 +0200
> > From: Joachim Schipper 
> > 
> > The bioctl(8) man page helpfully suggests dd for zeroing out the
> > disklabel etc. on the new disk. However, the command doesn't work, since
> > OpenBSD's dd doesn't understand '1M'.
> 
> Uh, indeed it doesn't, but it does understand '1m'.

Yeah, sorry. Still, it should be fixed.

Joachim



[PATCH] bioctl(8): fix dd invocation

2009-05-09 Thread Joachim Schipper
The bioctl(8) man page helpfully suggests dd for zeroing out the
disklabel etc. on the new disk. However, the command doesn't work, since
OpenBSD's dd doesn't understand '1M'.

It may be slightly more readable to use 'bs=1024 count=1024' or some
variant.

Joachim

Index: bioctl.8
===
RCS file: /cvs/src/sbin/bioctl/bioctl.8,v
retrieving revision 1.66
diff -u -p -r1.66 bioctl.8
--- bioctl.819 Mar 2009 15:11:59 -  1.66
+++ bioctl.89 May 2009 10:07:23 -
@@ -232,7 +232,7 @@ or
 don't get confused by the random data that appears on the new disk.
 This can be done with the following command (assuming the new disk is sd3):
 .Bd -literal -offset 3n
-# dd if=/dev/zero of=/dev/rsd3c bs=1M count=1
+# dd if=/dev/zero of=/dev/rsd3c bs=1048576 count=1
 .Ed
 .Sh SEE ALSO
 .Xr ami 4 ,