NetBSD Problem Report #44210

From Wolfgang.Stukenbrock@nagler-company.com  Thu Dec  9 10:37:06 2010
Return-Path: <Wolfgang.Stukenbrock@nagler-company.com>
Received: from mail.netbsd.org (mail.netbsd.org [204.152.190.11])
	by www.NetBSD.org (Postfix) with ESMTP id EAB8D63B87A
	for <gnats-bugs@gnats.NetBSD.org>; Thu,  9 Dec 2010 10:37:05 +0000 (UTC)
Message-Id: <20101209103658.B41501E80CE@test-s0.nagler-company.com>
Date: Thu,  9 Dec 2010 11:36:58 +0100 (CET)
From: Wolfgang.Stukenbrock@nagler-company.com
Reply-To: Wolfgang.Stukenbrock@nagler-company.com
To: gnats-bugs@gnats.NetBSD.org
Subject: gzip support in /usr/src/sys/opencrypto/deflate.c is broken
X-Send-Pr-Version: 3.95

>Number:         44210
>Category:       kern
>Synopsis:       gzip support in /usr/src/sys/opencrypto/deflate.c is broken
>Confidential:   no
>Severity:       serious
>Priority:       medium
>Responsible:    kern-bug-people
>State:          closed
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Thu Dec 09 10:40:00 +0000 2010
>Closed-Date:    Mon Feb 21 17:10:22 +0000 2011
>Last-Modified:  Mon Feb 21 17:10:22 +0000 2011
>Originator:     Dr. W. Stukenbrock
>Release:        NetBSD 5.1
>Organization:
Dr. Nagler & Company GmbH
>Environment:


System: NetBSD test-s0 4.0 NetBSD 4.0 (NSW-WS) #0: Tue Aug 17 17:28:09 CEST 2010 wgstuken@test-s0:/usr/src/sys/arch/amd64/compile/NSW-WS amd64
Architecture: x86_64
Machine: amd64
>Description:
	The (initial) gzip support in /usr/src/sys/opencrypto/deflate.c does not honor the byte-order of
	the system it is running on.
	When injecting or extracting crc-data or the size information the byteorder of the host system is assumed.
	But this is wrong either on little- or big-endian systems depending on the correct order for the header.

	Next problem may be the alignment of the size and crc fields.
	I'm not shure if any compressed data output is always "a multiple of 4 plus 2" long, but I don't think so.
	If this is not the case, unaligned data access happend here, that may be a problem on some architectures.
	(remark: 10 bytes header plus compressed data is assumed to be aligned for u_int32_t ...)
>How-To-Repeat:
	Found by a look into the sources while applying the still open PR's named below again ....
>Fix:
	Correct the problems stated above - sorry I'm not 100% confirm with the gzip binary format, so
	I cannot tell you what macros to use.
	In order to avoid alignment problems memcpy() should be used to inject or extract the u_int32_t values.


	Now something different related to the deflate.c file:

	I've posted two PR's (for NetBSD 3.1 in 2007) that are still not integrated into the deflate module.
	(kern/36864 and kern/36865)
	Perhaps theese can be integrated by time, because I always need to apply at least 36864 again in order to get deflate working
	for our VPN-tunnel setup.
	Thanks in advance.

>Release-Note:

>Audit-Trail:

State-Changed-From-To: open->feedback
State-Changed-By: drochner@NetBSD.org
State-Changed-When: Fri, 18 Feb 2011 10:52:47 +0000
State-Changed-Why:
is deflate.c rev.1.17 OK?


From: "Matthias Drochner" <drochner@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/44210 CVS commit: src/sys/opencrypto
Date: Fri, 18 Feb 2011 10:50:57 +0000

 Module Name:	src
 Committed By:	drochner
 Date:		Fri Feb 18 10:50:57 UTC 2011

 Modified Files:
 	src/sys/opencrypto: deflate.c

 Log Message:
 address the obvious byteorder and alignment problems in gzip size/crc
 tail, should fix PR kern/44210 by Wolfgang Stukenbrock
 being here, fix a bug in crc calculation of decompressed data, and
 actually verify the crc


 To generate a diff of this commit:
 cvs rdiff -u -r1.16 -r1.17 src/sys/opencrypto/deflate.c

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

From: Wolfgang Stukenbrock <Wolfgang.Stukenbrock@nagler-company.com>
To: gnats-bugs@NetBSD.org
Cc: 
Subject: Re: PR/44210 CVS commit: src/sys/opencrypto
Date: Fri, 18 Feb 2011 12:59:42 +0100

 Hallo - nur vorab ein Lebenszeichen - daher auch nur an Dich.

 Ich werde erst nächste Woche dazu kommen es anzusehen.

 Gruß

 W. Stukenbrock

 Matthias Drochner wrote:

 > The following reply was made to PR kern/44210; it has been noted by GNATS.
 > 
 > From: "Matthias Drochner" <drochner@netbsd.org>
 > To: gnats-bugs@gnats.NetBSD.org
 > Cc: 
 > Subject: PR/44210 CVS commit: src/sys/opencrypto
 > Date: Fri, 18 Feb 2011 10:50:57 +0000
 > 
 >  Module Name:	src
 >  Committed By:	drochner
 >  Date:		Fri Feb 18 10:50:57 UTC 2011
 >  
 >  Modified Files:
 >  	src/sys/opencrypto: deflate.c
 >  
 >  Log Message:
 >  address the obvious byteorder and alignment problems in gzip size/crc
 >  tail, should fix PR kern/44210 by Wolfgang Stukenbrock
 >  being here, fix a bug in crc calculation of decompressed data, and
 >  actually verify the crc
 >  
 >  
 >  To generate a diff of this commit:
 >  cvs rdiff -u -r1.16 -r1.17 src/sys/opencrypto/deflate.c
 >  
 >  Please note that diffs are not public domain; they are subject to the
 >  copyright notices on the relevant files.
 >  
 > 


From: Wolfgang Stukenbrock <Wolfgang.Stukenbrock@nagler-company.com>
To: gnats-bugs@NetBSD.org
Cc: kern-bug-people@NetBSD.org, gnats-admin@NetBSD.org, netbsd-bugs@NetBSD.org,
        Wolfgang.Stukenbrock@nagler-company.com
Subject: Re: PR/44210 CVS commit: src/sys/opencrypto
Date: Mon, 21 Feb 2011 10:47:32 +0100

 Hi, now I've had a look at it.

 The patch looks good, it seems to address the problems.
 I've no way to validate it by running it at the moment, but I think you 
 have done it.
 The "look into the sources" doesn't show any wrong things to me.

 Perhaps a DPRINTF() should be added around line 437 if the packet is 
 dropped due to bad size and/or crc-data. There are DPRINTF() in all 
 other error cases ...

 I think you can close this report.

 best regards

 W. Stukenbrock

 Wolfgang Stukenbrock wrote:

 > The following reply was made to PR kern/44210; it has been noted by GNATS.
 > 
 > From: Wolfgang Stukenbrock <Wolfgang.Stukenbrock@nagler-company.com>
 > To: gnats-bugs@NetBSD.org
 > Cc: 
 > Subject: Re: PR/44210 CVS commit: src/sys/opencrypto
 > Date: Fri, 18 Feb 2011 12:59:42 +0100
 > 
 >  Hallo - nur vorab ein Lebenszeichen - daher auch nur an Dich.
 >  
 >  Ich werde erst nächste Woche dazu kommen es anzusehen.
 >  
 >  Gruß
 >  
 >  W. Stukenbrock
 >  
 >  Matthias Drochner wrote:
 >  
 >  > The following reply was made to PR kern/44210; it has been noted by GNATS.
 >  > 
 >  > From: "Matthias Drochner" <drochner@netbsd.org>
 >  > To: gnats-bugs@gnats.NetBSD.org
 >  > Cc: 
 >  > Subject: PR/44210 CVS commit: src/sys/opencrypto
 >  > Date: Fri, 18 Feb 2011 10:50:57 +0000
 >  > 
 >  >  Module Name:	src
 >  >  Committed By:	drochner
 >  >  Date:		Fri Feb 18 10:50:57 UTC 2011
 >  >  
 >  >  Modified Files:
 >  >  	src/sys/opencrypto: deflate.c
 >  >  
 >  >  Log Message:
 >  >  address the obvious byteorder and alignment problems in gzip size/crc
 >  >  tail, should fix PR kern/44210 by Wolfgang Stukenbrock
 >  >  being here, fix a bug in crc calculation of decompressed data, and
 >  >  actually verify the crc
 >  >  
 >  >  
 >  >  To generate a diff of this commit:
 >  >  cvs rdiff -u -r1.16 -r1.17 src/sys/opencrypto/deflate.c
 >  >  
 >  >  Please note that diffs are not public domain; they are subject to the
 >  >  copyright notices on the relevant files.
 >  >  
 >  > 
 >  
 >  
 > 


 -- 


 Dr. Nagler & Company GmbH
 Hauptstraße 9
 92253 Schnaittenbach

 Tel. +49 9622/71 97-42
 Fax +49 9622/71 97-50

 Wolfgang.Stukenbrock@nagler-company.com
 http://www.nagler-company.com


 Hauptsitz: Schnaittenbach
 Handelregister: Amberg HRB
 Gerichtsstand: Amberg
 Steuernummer: 201/118/51825
 USt.-ID-Nummer: DE 273143997
 Geschäftsführer: Dr. Martin Nagler, Dr. Dr. Karl-Kuno Kunze


State-Changed-From-To: feedback->closed
State-Changed-By: drochner@NetBSD.org
State-Changed-When: Mon, 21 Feb 2011 17:10:22 +0000
State-Changed-Why:
submitter is satisfied
(I've tested on i386 only, so neither big-endian nor strict alignment,
but at least checksum generation and verification works (tested
against userland gzip). I plan to touch this file again anyway to
sync GZIP buffer handling with the DEFLATE case -- I'll try to think
of adding some DPRINTF then.)


>Unformatted:

Home	PR Database Search