NetBSD Problem Report #44964

From campbell@mumble.net  Fri May 13 17:05:06 2011
Return-Path: <campbell@mumble.net>
Received: from mail.netbsd.org (mail.netbsd.org [204.152.190.11])
	by www.NetBSD.org (Postfix) with ESMTP id 84DB363C607
	for <gnats-bugs@gnats.NetBSD.org>; Fri, 13 May 2011 17:05:06 +0000 (UTC)
Message-Id: <20110513170503.9A877982B6@pluto.mumble.net>
Date: Fri, 13 May 2011 17:05:03 +0000 (UTC)
From: Taylor R Campbell <campbell+netbsd@mumble.net>
Reply-To: Taylor R Campbell <campbell+netbsd@mumble.net>
To: gnats-bugs@gnats.NetBSD.org
Subject: cgd seems to panic on unaligned writes instead of giving EINVAL
X-Send-Pr-Version: 3.95

>Number:         44964
>Category:       kern
>Synopsis:       cgd seems to panic on unaligned writes instead of giving EINVAL
>Confidential:   no
>Severity:       serious
>Priority:       medium
>Responsible:    kern-bug-people
>State:          closed
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Fri May 13 17:10:01 +0000 2011
>Closed-Date:    Sat Jun 04 16:41:44 +0000 2011
>Last-Modified:  Sat Jun 04 16:41:44 +0000 2011
>Originator:     Taylor R Campbell <campbell+netbsd@mumble.net>
>Release:        NetBSD 5.99.51
>Organization:
>Environment:
System: NetBSD oberon.local 5.99.51 NetBSD 5.99.51 (RIAMONODEBUG) #143: Sun May 8 19:13:30 UTC 2011 root@smalltalk.local:/home/riastradh/netbsd/current/obj/sys/arch/i386/compile/RIAMONODEBUG i386
Architecture: i386
Machine: i386
>Description:

	I was trying to take a snapshot of my old machine's disk and
	store it in a cgd partition on my new machine, transmitting the
	bits over the network.  On the new machine, I ran something
	like this:

		socat tcp4-listen:12345,bind=10.0.1.2,reuseaddr stdout \
		| dd if=/dev/stdin of=/dev/rcgd2d bs=1m

	On the old laptop, I ran something like this:

		dd if=/dev/rwd0d of=/dev/stdout bs=1m \
		| socat stdin tcp4:10.0.1.2:12345

	Whatever block size I specified on the new machine, it panicked
	-- very soon for large block sizes, and after a couple hours
	for bs=512 (the transfer was, uh, very slow with bs=512).  I
	suspect that this is because the blocks got cut up into
	randomly sized chunks in transit over the network, and dd
	happily relayed randomly sized chunks straight to cgd, which
	should have given EINVAL instead of panicking.

	The next morning I learned that `bs=N' is not, in fact, the
	same as `ibs=N obs=N'.  Thanks, POSIX.  WTF?

	Here's a gdb stack trace from a fault in rijndael_blockEncrypt.
	I belive the faulting address was 0xcc501000, which gdb
	helpfully reports is out of bounds.  Note the unusual value
	len=2896 in cgd_cipher.  I have the core dump, so if you want
	to see the values of any of the data structures (e.g., the
	struct buf in question), let me know.

#7  0xc010d13f in calltrap ()
#8  0xc06c9924 in rijndael_blockEncrypt (cipher=0xcf4226ac, key=0xc3465000,
    input=0xcc501000 <Address 0xcc501000 out of bounds>, inputLen=4096,
    outBuffer=<value optimized out>)
    at /home/riastradh/netbsd/current/src/sys/crypto/rijndael/rijndael-api-fst.c:136  
#9  0xc023f298 in aes_cbc_enc_int (privdata=0xcf422738, dst=0xc34ea000,
    src=0xcc501000, len=512)
    at /home/riastradh/netbsd/current/src/sys/dev/cgd_crypto.c:209
#10 0xc023ec90 in cgd_cipher_uio_cbc (privdata=0xcf422738,
    cipher=0xc023f260 <aes_cbc_enc_int>, dstuio=0xcf4227d0, srcuio=0xcf4227b4)
    at /home/riastradh/netbsd/current/src/sys/dev/cgd_crypto.c:115
#11 0xc023eee6 in cgd_cipher_aes_cbc (privdata=0xc3465000, dstuio=0xcf4227d0,
    srcuio=0xcf4227b4, iv=0xcf42281c, dir=-867168256)
    at /home/riastradh/netbsd/current/src/sys/dev/cgd_crypto.c:235
#12 0xc023db25 in cgd_cipher (cs=0xcf39ef04, dstv=0xc34e9000, srcv=0xcc500000,
    len=2896, blkno=<value optimized out>, secsize=512, dir=2)
    at /home/riastradh/netbsd/current/src/sys/dev/cgd.c:904
#13 0xc023de9a in cgdstart (dksc=0xcf39ef08, bp=0xc322e790)
    at /home/riastradh/netbsd/current/src/sys/dev/cgd.c:392
#14 0xc028882d in dk_start (di=0xc0bd8a58, dksc=0xcf39ef08)
    at /home/riastradh/netbsd/current/src/sys/dev/dksubr.c:239
#15 0xc0288ab6 in dk_strategy (di=0xc0bd8a58, dksc=0xcf39ef08, bp=0xc322e790)
    at /home/riastradh/netbsd/current/src/sys/dev/dksubr.c:225
#16 0xc023e1d1 in cgdstrategy (bp=0xc322e790)
    at /home/riastradh/netbsd/current/src/sys/dev/cgd.c:297
#17 0xc051315c in physio (strategy=0xc023e190 <cgdstrategy>, obp=0x0,
    dev=23827, flags=0, min_phys=0xc0512e60 <minphys>, uio=0xcf422c7c)
    at /home/riastradh/netbsd/current/src/sys/kern/kern_physio.c:352
#18 0xc023d7b7 in cgdwrite (dev=23827, uio=0xcf422c7c, flags=592)
    at /home/riastradh/netbsd/current/src/sys/dev/cgd.c:496
#19 0xc0753627 in cdev_write (dev=23827, uio=0xcf422c7c, flag=592)
    at /home/riastradh/netbsd/current/src/sys/kern/subr_devsw.c:860
#20 0xc073ed07 in spec_write (v=0xcf422bf4)
    at /home/riastradh/netbsd/current/src/sys/miscfs/specfs/spec_vnops.c:666
#21 0xc089285d in VOP_WRITE (vp=0xcf0388c4, uio=0xcf422c7c, ioflag=592,
    cred=0xcf372480)
    at /home/riastradh/netbsd/current/src/sys/kern/vnode_if.c:431
#22 0xc0877f7f in vn_write (fp=0xcf371ac0, offset=0xcf371ac0, uio=0xcf422c7c,
    cred=0xcf372480, flags=1)
    at /home/riastradh/netbsd/current/src/sys/kern/vfs_vnops.c:566
#23 0xc0777413 in dofilewrite (fd=4, fp=0x0, buf=0xbb600000, nbyte=2896,
    offset=0xcf371ac0, flags=1, retval=0xcf422d28)
    at /home/riastradh/netbsd/current/src/sys/kern/sys_generic.c:357
#24 0xc0777540 in sys_write (l=0xcf3c02a0, uap=0xcf422d00, retval=0xcf422d28)
    at /home/riastradh/netbsd/current/src/sys/kern/sys_generic.c:325
#25 0xc0783079 in syscall (frame=0xcf422d48)
    at /home/riastradh/netbsd/current/src/sys/sys/syscallvar.h:61
#26 0xc01005d6 in syscall1 ()

	gdb stack trace from another panic.

#9  0xc07622e5 in panic (fmt=0xc0b19830 "cgd_cipher: len %% blocksize != 0")
    at /home/riastradh/netbsd/current/src/sys/kern/subr_prf.c:298
#10 0xc023dc75 in cgd_cipher (cs=0xcf2d8e0c, dstv=0xc36b0000, srcv=0xcc500000, 
    len=9608, blkno=<value optimized out>, secsize=512, dir=2)
    at /home/riastradh/netbsd/current/src/sys/dev/cgd.c:862
#11 0xc023de9a in cgdstart (dksc=0xcf2d8e10, bp=0xc33c3794)
    at /home/riastradh/netbsd/current/src/sys/dev/cgd.c:392
#12 0xc028882d in dk_start (di=0xc0bd8a58, dksc=0xcf2d8e10)
    at /home/riastradh/netbsd/current/src/sys/dev/dksubr.c:239
#13 0xc0288ab6 in dk_strategy (di=0xc0bd8a58, dksc=0xcf2d8e10, bp=0xc33c3794)
    at /home/riastradh/netbsd/current/src/sys/dev/dksubr.c:225
#14 0xc023e1d1 in cgdstrategy (bp=0xc33c3794)
    at /home/riastradh/netbsd/current/src/sys/dev/cgd.c:297
#15 0xc051315c in physio (strategy=0xc023e190 <cgdstrategy>, obp=0x0, 
    dev=23827, flags=0, min_phys=0xc0512e60 <minphys>, uio=0xcf86bc7c)
    at /home/riastradh/netbsd/current/src/sys/kern/kern_physio.c:352
#16 0xc023d7b7 in cgdwrite (dev=23827, uio=0xcf86bc7c, flags=592)
    at /home/riastradh/netbsd/current/src/sys/dev/cgd.c:496
#17 0xc0753627 in cdev_write (dev=23827, uio=0xcf86bc7c, flag=592)
    at /home/riastradh/netbsd/current/src/sys/kern/subr_devsw.c:860
#18 0xc073ed07 in spec_write (v=0xcf86bbf4)
    at /home/riastradh/netbsd/current/src/sys/miscfs/specfs/spec_vnops.c:666
#19 0xc089285d in VOP_WRITE (vp=0xd0016638, uio=0xcf86bc7c, ioflag=592, 
    cred=0xcf360300)
    at /home/riastradh/netbsd/current/src/sys/kern/vnode_if.c:431
#20 0xc0877f7f in vn_write (fp=0xcf361c00, offset=0xcf361c00, uio=0xcf86bc7c,
    cred=0xcf360300, flags=1)
    at /home/riastradh/netbsd/current/src/sys/kern/vfs_vnops.c:566
#21 0xc0777413 in dofilewrite (fd=4, fp=0x6, buf=0xbb800000, nbyte=9608,
    offset=0xcf361c00, flags=1, retval=0xcf86bd28)
    at /home/riastradh/netbsd/current/src/sys/kern/sys_generic.c:357
#22 0xc0777540 in sys_write (l=0xd8325800, uap=0xcf86bd00, retval=0xcf86bd28)
    at /home/riastradh/netbsd/current/src/sys/kern/sys_generic.c:325
#23 0xc0783079 in syscall (frame=0xcf86bd48)
    at /home/riastradh/netbsd/current/src/sys/sys/syscallvar.h:61
#24 0xc01005d6 in syscall1 ()

>How-To-Repeat:

	Write unaligned blocks to a cgd.  Hope you didn't have any
	important session state on your machine.

>Fix:

	Yes, please!

>Release-Note:

>Audit-Trail:
From: christos@zoulas.com (Christos Zoulas)
To: gnats-bugs@NetBSD.org
Cc: 
Subject: Re: kern/44964: cgd seems to panic on unaligned writes instead of giving EINVAL
Date: Fri, 13 May 2011 22:00:10 -0400 (EDT)

 Should be fixed on revision rijndael-api-fst.c rev 1.21

 christos

From: Taylor R Campbell <campbell+netbsd@mumble.net>
To: gnats-bugs@NetBSD.org
Cc: 
Subject: Re: kern/44964: cgd seems to panic on unaligned writes instead of giving EINVAL
Date: Sat, 14 May 2011 05:07:32 +0000

    Date: Sat, 14 May 2011 02:05:03 +0000 (UTC)
    From: christos@zoulas.com (Christos Zoulas)

    Should be fixed on revision rijndael-api-fst.c rev 1.21

 (1.22, presumably.)  I don't think this fully fixes the problem: other
 parts of cgd are not prepared to handle unaligned writes.  For
 example, cgd_cipher requires that len be an integral multiple of
 blocksize (by assertion) and an integral multiple of secsize (by
 subtracting secsize from the size_t len until len is zero).

 Also, I'm worried that using 8-bit operations rather than 32-bit
 operations will hurt performance; have you compared the performance
 before and after?

 How about the following patch to reject unaligned writes?  Untested,
 but I'll test it in the next couple days if I get a chance.

 Index: cgd.c
 ===================================================================
 RCS file: /cvsroot/src/sys/dev/cgd.c,v
 retrieving revision 1.71
 diff -p -u -r1.71 cgd.c
 --- cgd.c	19 Nov 2010 06:44:39 -0000	1.71
 +++ cgd.c	14 May 2011 05:06:22 -0000
 @@ -293,6 +293,21 @@ cgdstrategy(struct buf *bp)

  	DPRINTF_FOLLOW(("cgdstrategy(%p): b_bcount = %ld\n", bp,
  	    (long)bp->b_bcount));
 +
 +	/*
 +	 * Reject unaligned writes.  We can encrypt and decrypt only
 +	 * complete disk sectors, and we let the ciphers require their
 +	 * buffers to be aligned to 32-bit boundaries.
 +	 */
 +	if (bp->b_blkno < 0 ||
 +	    (bp->b_bcount % DEV_BSIZE) != 0 ||
 +	    ((uintptr_t)bp->b_data & 3) != 0) {
 +		bp->b_error = EINVAL;
 +		bp->b_resid = bp->b_bcount;
 +		biodone(bp);
 +		return;
 +	}
 +
  	/* XXXrcd: Should we test for (cs != NULL)? */
  	dk_strategy(di, &cs->sc_dksc, bp);
  	return;

From: christos@zoulas.com (Christos Zoulas)
To: gnats-bugs@NetBSD.org, kern-bug-people@netbsd.org, 
	gnats-admin@netbsd.org, netbsd-bugs@netbsd.org, 
	Taylor R Campbell <campbell+netbsd@mumble.net>
Cc: 
Subject: Re: kern/44964: cgd seems to panic on unaligned writes instead of giving EINVAL
Date: Sat, 14 May 2011 11:10:24 -0400

 On May 14,  5:15am, campbell+netbsd@mumble.net (Taylor R Campbell) wrote:
 -- Subject: Re: kern/44964: cgd seems to panic on unaligned writes instead of

 |  (1.22, presumably.)  I don't think this fully fixes the problem: other
 |  parts of cgd are not prepared to handle unaligned writes.  For
 |  example, cgd_cipher requires that len be an integral multiple of
 |  blocksize (by assertion) and an integral multiple of secsize (by
 |  subtracting secsize from the size_t len until len is zero).
 |  
 |  Also, I'm worried that using 8-bit operations rather than 32-bit
 |  operations will hurt performance; have you compared the performance
 |  before and after?

 I don't think that it is going to be that bad, because the code is
 using GETU32() PUTU32() in other parts to handle unaligned reads
 and writes which is even more expensive.
 |
 |  How about the following patch to reject unaligned writes?  Untested,
 |  but I'll test it in the next couple days if I get a chance.

 I agree, commit that patch. cgd will not work properly anyway. But I would
 also leave my patch in, because this was the only part of the code that
 did not handle unaligned buffers.

 christos

From: "Taylor R Campbell" <riastradh@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/44964 CVS commit: src/sys/dev
Date: Thu, 19 May 2011 20:34:13 +0000

 Module Name:	src
 Committed By:	riastradh
 Date:		Thu May 19 20:34:13 UTC 2011

 Modified Files:
 	src/sys/dev: cgd.c

 Log Message:
 Reject unaligned writes to cgd.

 Fixes the following PRs:

 PR kern/44515 (cgd dies on non-aligned writes to the raw device)
 PR kern/44964 (cgd seems to panic on unaligned writes instead of giving EINVAL)

 ok christos


 To generate a diff of this commit:
 cvs rdiff -u -r1.71 -r1.72 src/sys/dev/cgd.c

 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.

From: "Taylor R Campbell" <riastradh@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/44964 CVS commit: src/tests/dev/cgd
Date: Thu, 19 May 2011 20:37:50 +0000

 Module Name:	src
 Committed By:	riastradh
 Date:		Thu May 19 20:37:50 UTC 2011

 Modified Files:
 	src/tests/dev/cgd: t_cgd.sh

 Log Message:
 Expand tests for unaligned writes to cgd.  No more xfail.

 PR kern/44515
 PR kern/44964


 To generate a diff of this commit:
 cvs rdiff -u -r1.9 -r1.10 src/tests/dev/cgd/t_cgd.sh

 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.

State-Changed-From-To: open->closed
State-Changed-By: jruoho@NetBSD.org
State-Changed-When: Sat, 04 Jun 2011 16:41:44 +0000
State-Changed-Why:

Thanks for fixing this.



>Unformatted:

Home	PR Database Search