NetBSD Problem Report #50849

From www@NetBSD.org  Thu Feb 25 08:00:07 2016
Return-Path: <www@NetBSD.org>
Received: from mail.netbsd.org (mail.NetBSD.org [199.233.217.200])
	(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits))
	(Client CN "mail.netbsd.org", Issuer "Postmaster NetBSD.org" (verified OK))
	by mollari.NetBSD.org (Postfix) with ESMTPS id 4CE427ACC2
	for <gnats-bugs@gnats.NetBSD.org>; Thu, 25 Feb 2016 08:00:07 +0000 (UTC)
Message-Id: <20160225080006.3D98D7ACD7@mollari.NetBSD.org>
Date: Thu, 25 Feb 2016 08:00:06 +0000 (UTC)
From: m4j0rd0m0@gmail.com
Reply-To: m4j0rd0m0@gmail.com
To: gnats-bugs@NetBSD.org
Subject: Relax TC option ROM header check for ROM widths < 4
X-Send-Pr-Version: www-1.0

>Number:         50849
>Category:       kern
>Synopsis:       Relax TC option ROM header check for ROM widths < 4
>Confidential:   no
>Severity:       non-critical
>Priority:       low
>Responsible:    kern-bug-people
>State:          closed
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Thu Feb 25 08:05:00 +0000 2016
>Closed-Date:    Mon Dec 12 16:32:31 +0000 2016
>Last-Modified:  Mon Dec 12 16:32:31 +0000 2016
>Originator:     Felix Deichmann
>Release:        all
>Organization:
>Environment:
n/a
>Description:
Check of the test pattern in a TURBOchannel option ROM header implies a fixed ROM width of 4 bytes (32 bit) at the moment. While this may be OK for all presently supported TURBOchannel devices (which have a ROM width of 4), devices with an allowed, real ROM header width of 1 or 2 could fail (as the values of upper bytes are unknown/don't care).
The ROM header width is given in the ROM header, so the actual value can be used to relax the check.
>How-To-Repeat:
Boot with a (theoretical) TURBOchannel option installed which has an option ROM header width < 4.
>Fix:
--- src/sys/dev/tc/tc.c_1_51	Thu Feb 25 09:00:00 2016
+++ src/sys/dev/tc/tc.c	Thu Feb 25 09:00:00 2016
@@ -229,7 +229,7 @@
 		if (romp->tcr_stride.v != 4)
 			continue;

-		for (j = 0; j < 4; j++)
+		for (j = 0; j < romp->tcr_width.v; j++)
 			if (romp->tcr_test[j+0*romp->tcr_stride.v] != 0x55 ||
 			    romp->tcr_test[j+1*romp->tcr_stride.v] != 0x00 ||
 			    romp->tcr_test[j+2*romp->tcr_stride.v] != 0xaa ||

>Release-Note:

>Audit-Trail:
From: Felix Deichmann <m4j0rd0m0@gmail.com>
To: gnats-bugs@NetBSD.org
Cc: 
Subject: Re: kern/50849: Relax TC option ROM header check for ROM widths < 4
Date: Tue, 19 Jul 2016 18:08:35 +0200

 There is another issue in tc_checkslot() where a continue statement 
 seems to be intended for an outer for-loop, but in fact acts on the 
 inner for-loop.

 Updated patch follows:

 Index: tc.c
 ===================================================================
 RCS file: /cvsroot/src/sys/dev/tc/tc.c,v
 retrieving revision 1.51
 diff -u -p -r1.51 tc.c
 --- tc.c	4 Jun 2011 01:57:34 -0000	1.51
 +++ tc.c	19 Jul 2016 16:01:06 -0000
 @@ -223,23 +223,25 @@ tc_checkslot(tc_addr_t slotbase, char *n
   			break;

   		default:
 -			continue;
 +			goto next;
   		}

   		if (romp->tcr_stride.v != 4)
 -			continue;
 +			goto next;

 -		for (j = 0; j < 4; j++)
 +		for (j = 0; j < romp->tcr_width.v; j++)
   			if (romp->tcr_test[j+0*romp->tcr_stride.v] != 0x55 ||
   			    romp->tcr_test[j+1*romp->tcr_stride.v] != 0x00 ||
   			    romp->tcr_test[j+2*romp->tcr_stride.v] != 0xaa ||
   			    romp->tcr_test[j+3*romp->tcr_stride.v] != 0xff)
 -				continue;
 +				goto next;

   		for (j = 0; j < TC_ROM_LLEN; j++)
   			namep[j] = romp->tcr_modname[j].v;
   		namep[j] = '\0';
   		return (1);
 +next:
 +		/* null statement */;
   	}
   	return (0);
   }

From: "Christos Zoulas" <christos@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/50849 CVS commit: src/sys/dev/tc
Date: Tue, 19 Jul 2016 12:58:06 -0400

 Module Name:	src
 Committed By:	christos
 Date:		Tue Jul 19 16:58:06 UTC 2016

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

 Log Message:
 PR/50849: Felix Deichmann: Relax TC option ROM header check for ROM widths < 4


 To generate a diff of this commit:
 cvs rdiff -u -r1.51 -r1.52 src/sys/dev/tc/tc.c

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

From: Felix Deichmann <m4j0rd0m0@gmail.com>
To: gnats-bugs@NetBSD.org, kern-bug-people@netbsd.org,
 gnats-admin@netbsd.org, netbsd-bugs@netbsd.org
Cc: 
Subject: Re: PR/50849 CVS commit: src/sys/dev/tc
Date: Tue, 19 Jul 2016 19:13:49 +0200

 Thanks Christos, but the initial intention of the patch (making it work 
 with variable ROM bus widths of TC adapters) is now lost. Patch once 
 more please:

 Index: tc.c
 ===================================================================
 RCS file: /cvsroot/src/sys/dev/tc/tc.c,v
 retrieving revision 1.52
 diff -u -p -r1.52 tc.c
 --- tc.c	19 Jul 2016 16:58:05 -0000	1.52
 +++ tc.c	19 Jul 2016 17:10:34 -0000
 @@ -212,7 +212,7 @@ tc_check_romp(const struct tc_rommap *ro
   	if (romp->tcr_stride.v != 4)
   		return 0;

 -	for (size_t j = 0; j < 4; j++) {
 +	for (size_t j = 0; j < romp->tcr_width.v; j++) {
   		if (romp->tcr_test[j + 0 * romp->tcr_stride.v] != 0x55 ||
   		    romp->tcr_test[j + 1 * romp->tcr_stride.v] != 0x00 ||
   		    romp->tcr_test[j + 2 * romp->tcr_stride.v] != 0xaa ||

From: christos@zoulas.com (Christos Zoulas)
To: Felix Deichmann <m4j0rd0m0@gmail.com>, gnats-bugs@NetBSD.org, 
	kern-bug-people@netbsd.org, gnats-admin@netbsd.org, 
	netbsd-bugs@netbsd.org
Cc: 
Subject: Re: PR/50849 CVS commit: src/sys/dev/tc
Date: Tue, 19 Jul 2016 14:27:37 -0400

 On Jul 19,  7:13pm, m4j0rd0m0@gmail.com (Felix Deichmann) wrote:
 -- Subject: Re: PR/50849 CVS commit: src/sys/dev/tc

 | Thanks Christos, but the initial intention of the patch (making it work 
 | with variable ROM bus widths of TC adapters) is now lost. Patch once 
 | more please:

 Got it!

 christos

State-Changed-From-To: open->closed
State-Changed-By: flxd@NetBSD.org
State-Changed-When: Mon, 12 Dec 2016 16:32:31 +0000
State-Changed-Why:
Fix tested/works. Pull-ups not desired (at the moment).


>Unformatted:

Home	PR Database Search