NetBSD Problem Report #38051

From kre@munnari.OZ.AU  Mon Feb 18 12:18:07 2008
Return-Path: <kre@munnari.OZ.AU>
Received: from mail.netbsd.org (mail.netbsd.org [204.152.190.11])
	by narn.NetBSD.org (Postfix) with ESMTP id 0A08663B938
	for <gnats-bugs@gnats.NetBSD.org>; Mon, 18 Feb 2008 12:18:07 +0000 (UTC)
Message-Id: <200802181216.m1ICGGAr001105@jade.coe.psu.ac.th>
Date: Mon, 18 Feb 2008 19:16:16 +0700 (ICT)
From: kre@munnari.OZ.AU
To: gnats-bugs@gnats.NetBSD.org
Subject: security/sfs recent patch uses __NetBSD_Prereq__ incorrectly
X-Send-Pr-Version: 3.95

>Number:         38051
>Category:       pkg
>Synopsis:       security/sfs recent patch uses __NetBSD_Prereq__ incorrectly
>Confidential:   no
>Severity:       serious
>Priority:       high
>Responsible:    dholland
>State:          closed
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Mon Feb 18 12:20:02 +0000 2008
>Closed-Date:    Mon Mar 12 00:32:13 +0000 2018
>Last-Modified:  Mon Mar 12 00:32:13 +0000 2018
>Originator:     Robert Elz
>Release:        NetBSD 3.99.15  (pkgsrc current within past 4 hours)
>Organization:
	Prince of Songkla University
>Environment:
System: NetBSD jade.coe.psu.ac.th 3.99.15 NetBSD 3.99.15 (GENERIC-1.696-20060125) #8: Wed Jan 25 04:59:39 ICT 2006 kre@jade.coe.psu.ac.th:/usr/obj/current/kernels/JADE_ASUS i386
Architecture: i386
Machine: i386
>Description:
	I compile packages using pkg_comp and libkver for a NetBSD 3.0
	environment (then they work on everything I care abuot).

	Until Saturday, security/sfs compiled file.   Then

Modified Files:
	pkgsrc/security/sfs: distinfo
Added Files:
	pkgsrc/security/sfs/patches: patch-bv

Log Message:
Deal with fourth arg to mount(2) in NetBSD.  It appeared between
4.99.23 and 4.99.24.

	That's clearly something that needed doing, but it needs
	doing correctly.

	The test added was ...
		#elif defined(__NetBSD__) && __NetBSD_Prereq__(4,99,24)

	which (as any sand person would read it) tests for 4.99.24 or
	later, as a prerequsisite.

	But what it really tests is if 4.99.24 is >= __NetBSD_Version__
	which is clearly true (for me), as __NetBSD_Version__ is 3.0.0

	I (still) suspect that the "sense" of __NetBSD_Prereq__() is
	more intended to be used in LKMs, where the appropriate think
	to know is that the kernel hasn't been made newer than the
	LKM author knows about (which might mean that any internal
	kernel interface has altered, so the LKM needs to be checked
	for continued correct design).

>How-To-Repeat:
	Attempt to build security/sfs (on presumably just about
	anything except 4.99.24 where it will presumably work).

>Fix:
	I suspect the correct line should be

		#elif defined(__NetBSD__) && !__NetBSD_Prereq__(4,99,23)

	which tests that 4.99.23 < __NetBSD_Version__
		(ie: !(4.99.23 >= __NetBSD_Version))

	That is, the system being compiled against is 4.99.24
	or later, which is (apparently) where the change is required.

	Probably better is just to forget __NetBSD_Prereq__() and simply
	test
		#elif defined(__NetBSD__) && __NetBSD_Version__ >= 499002400

	I can't test either of those is exactly what is needed, as I don't
	have 4.99.23 and 4.99.24 systems around to validate it (and
	don't really feel inclined to build them just for this).

	I have submitted this PS as high/serious, not because it is
	really urgent that security/sfs get fixed, but because it is
	urgent that there be some documentation for __NetBSD_Prereq__()
	so people actually know how to use the thing (which all relates
	back to a pending PR of mine that ought to eb closed now, since
	4.0 was released, and it was only ever relevant to what was
	released as 4.0) and a discussion on tech-kern that (to me)
	only ended up demonstrating that __NetBSD_Prereq__() is either
	misunderstood or simply useless and irrelevant (and so should
	either be documented, or deleted).

	ie: After processing this PR against security/sfs, please
	change its category to kern and leave it open instead of
	closing it, if that's possible...

	ps: with the changed patch, security/sfs does build on 3.0 at least.

>Release-Note:

>Audit-Trail:

Responsible-Changed-From-To: pkg-manager->apb
Responsible-Changed-By: wiz@narn.netbsd.org
Responsible-Changed-When: Mon, 18 Feb 2008 18:09:30 +0000
Responsible-Changed-Why:
apb committed the change in question.


From: Robert Elz <kre@munnari.OZ.AU>
To: gnats-bugs@NetBSD.org
Cc: 
Subject: Re: pkg/38051: security/sfs recent patch uses __NetBSD_Prereq__ incorrectly 
Date: Tue, 19 Feb 2008 19:41:28 +0700

     Date:        Mon, 18 Feb 2008 12:20:02 +0000 (UTC)
     From:        kre@munnari.oz.au
     Message-ID:  <20080218122002.D5CCC63B938@narn.NetBSD.org>

   | 	Probably better is just to forget __NetBSD_Prereq__() and simply
   | 	test
   | 		#elif defined(__NetBSD__) && __NetBSD_Version__ >= 499002400

 I see what happened now, I'm just not running a recent enough -current (or 4.0)
 where 
 I make packages to have noticed this before.   The test must be written
 this way, and __NetBSD_Prereq__() ignored, because of the change made to
 <sys/param.h> in version 1.242 of that file, that altered the sense of the
 test (it used to be >= and is now <=)

 That means the macro is useless for testing the system version, as depending
 upon the version (the very thing you don't know) it returns different answers.

 I suspect that the original >= test was probably what was originally
 intended when __NetBSD_Prereq__() was invented - but that might just because
 of the way I rationalised its design when I saw it.   For many uses the
 current (<=) test is certainly more intuitive.

 In any case, there's no way to safely use that macro without testing the
 system version first to see how the macro result needs to be interpreted,
 which would be flat out insane...

 As for __NetBSD_Prereq__() ... it is way too late now to revert the
 change to param.h (it's been changed almost 18 months, and is that way
 in 4.0 I presume), so the only rational thing to do is simply to delete it.
 Any code using it now is broken (unless it is very very very unusual)
 and the best thing to do is to cause compilation/linker errors upon it.

 If a macro or macros are needed for testing __NetBSD_Version__ then new
 ones are needed.

 kre

From: "David A. Holland" <dholland@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/38051 CVS commit: pkgsrc/security/sfs
Date: Mon, 12 Mar 2018 00:29:24 +0000

 Module Name:	pkgsrc
 Committed By:	dholland
 Date:		Mon Mar 12 00:29:24 UTC 2018

 Modified Files:
 	pkgsrc/security/sfs: distinfo
 	pkgsrc/security/sfs/patches: patch-bv

 Log Message:
 Fix (mis)use of __NetBSD_Prereq__ per PR 38051.


 To generate a diff of this commit:
 cvs rdiff -u -r1.8 -r1.9 pkgsrc/security/sfs/distinfo
 cvs rdiff -u -r1.1 -r1.2 pkgsrc/security/sfs/patches/patch-bv

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

Responsible-Changed-From-To: apb->dholland
Responsible-Changed-By: dholland@NetBSD.org
Responsible-Changed-When: Mon, 12 Mar 2018 00:30:21 +0000
Responsible-Changed-Why:
i'll fix it


State-Changed-From-To: open->closed
State-Changed-By: dholland@NetBSD.org
State-Changed-When: Mon, 12 Mar 2018 00:32:13 +0000
State-Changed-Why:
The patch has been fixed (this won't make sfs build, it's got deeper problems)
as for __NetBSD_Prereq__ itself being wrong, I think the only sane thing we
can do at this point is remove it. Please do so...


>Unformatted:

NetBSD Home
NetBSD PR Database Search

(Contact us) $NetBSD: query-full-pr,v 1.43 2018/01/16 07:36:43 maya Exp $
$NetBSD: gnats_config.sh,v 1.9 2014/08/02 14:16:04 spz Exp $
Copyright © 1994-2017 The NetBSD Foundation, Inc. ALL RIGHTS RESERVED.