NetBSD Problem Report #36656

From Wolfgang.Stukenbrock@nagler-company.com  Mon Jul 16 10:42:39 2007
Return-Path: <Wolfgang.Stukenbrock@nagler-company.com>
Received: from mail.netbsd.org (mail.netbsd.org [204.152.190.11])
	by narn.NetBSD.org (Postfix) with ESMTP id DD48D63B96E
	for <gnats-bugs@gnats.NetBSD.org>; Mon, 16 Jul 2007 10:42:39 +0000 (UTC)
Message-Id: <200707161023.l6GANDUt000586@test-s0.nagler-company.com>
Date: Mon, 16 Jul 2007 12:23:13 +0200 (CEST)
From: Wolfgang Stukenbrock <Wolfgang.Stukenbrock@nagler-company.com>
Reply-To: Wolfgang.Stukenbrock@nagler-company.com
To: gnats-bugs@NetBSD.org
Subject: amanda-client will fail to backup the requested filesystem
X-Send-Pr-Version: 3.95

>Number:         36656
>Category:       pkg
>Synopsis:       amanda-client will fail to backup the requested filesystem
>Confidential:   no
>Severity:       critical
>Priority:       high
>Responsible:    pkg-manager
>State:          closed
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Mon Jul 16 10:45:00 +0000 2007
>Closed-Date:    Sat Jun 11 21:11:22 +0000 2016
>Last-Modified:  Sat Jun 11 21:11:22 +0000 2016
>Originator:     Wolfgang Stukenbrock
>Release:        NetBSD 3.1
>Organization:
Dr. Nagler & Company GmbH
>Environment:


System: NetBSD test-s0 3.1 NetBSD 3.1 (test-s0) #0: Tue Apr 3 11:33:43 CEST 2007 root@test-s0:/usr/src/sys/arch/i386/compile/test-s0 i386
Architecture: i386
Machine: i386
>Description:
	The client code of amanda retries the information about the filesytem
	to be backed up from the amanda server in a UPD packet.
	The client code wil then check the retrieved path information agains
	the entries in /etc/fstab on all architectures other than pc-cygwin.
	(I don't get the point why amanda is dooing this, but thats not the
	problem.)
	The function "search_fstab()" in client-src/getfsent.c will to this
	check and if it assumes it has found a mount point it ignores the
	path in formation passed from the server and replaces it with the
	mountpoint. And here is a big bug in it!!!!!!
	The function "search_fstab()" setup three stat-buffers and set st_dev
	to -1 to mark then as invalid. The structures are allocated on the
	stack, so the contents of all other fileds is random.
	In most cases only two of the structures are used - so the last on
	will have st_dev == -1 in all cases.
	If you now pass try to backup a directory the search for a mountpoint
	wil fail as expected. But it there are some psuedo filesystem in
	/etc/fstab such as /proc, the call to stat on /proc will give st_dev
	with the value -1 and st_ino = 2 as a valid entry.
	Now the compare routine does not ignore any entry with st_dev == -1,
	so it depends on garbage on the stack if it will match or not.
	And if it will match, it will match all the time - as done on some of
	our machines.
	The result of the match is, that the amanda-client will tell the server
	it is backup up the requested filesystem (or directory inside of a
	filesystem), but in real the client starts backing up /proc.
	You can recognize this only if you look into the file lists - there
	is no other indication of this problem!

	The reason why it is nessesary to specify some directories too and not
	only mountpoints as starting point for a backup depends on the missing
	ability of the amanda software to backup a filesystem larger than a
	tape.
	This bug makes the amanda software unreliable so that you will not
	know if you have backuped up the requested things until you have
	checked the file list by hand!

	My amanda version is 2.4.4p4 from pkgsrc-1Q2007. I've not the time to
	switch to 2Q2007 but I'm shure this bug has not been fixed. It looks
	like it is in there since 2002 ...
	Not only NetBSD is affected, we have had the same problem on a Linux
	box too.
>How-To-Repeat:
	If you have the homedirectories located on a server in /home.stand and 
	you decide to backup each homedirectory as an own tape-file due to
	security and space reasons, you need to specify /home.stand/<user> for
	each user.
	If the server has /proc in the fstab, and you are out of luck with the
	garbage on the stack, /proc gets backup'd for each user again and again.
>Fix:
	There are multiple ways to fix this problem:
	1. change configure.in and set IGNORE_FSTAB in all cases.
	   This will comment out the whole function - as done for cygwin only
	   up to now. I cannot see any use in looking up /etc/fstab for the
	   filesystem to be backed up and change the path. (Perhaps if you
	   Jspecify a path to a symlink it would be nessesary to replace it,
	   but I treat this as a config error on the server.)
	2. change the "samefile()" routine that does the check and check st_dev
	   for -1 - never return true if it is -1.
	3. filter out any special filesystem in all "get_fstab_nextenty()" 
	   implementations.
	4. remove the whole /etc/fstab lookup stuff from the client code if
	   running gtar as dump-program. (just do not call the fstab lookup
	   stuff in such a case)
	- I'm shure there are even more ways to fix it ...
	I would prefere version 1, because I cannot see any advantage in looking
	into /etc/fstab if I want to backup everything below given directory on
	a filesystem. But version 2 is also a good aproach if the semantic with
	/etc/fstab shouldn't been thrown out.

	remark: I've didn't send fix bug-report to the amanda developpers,
	        because I don't know a way to do it and there are lots of fixed
		in the pkgsrc-tree for amanda. I think that one shoul be an
		addirional fix to original distribution.

>Release-Note:

>Audit-Trail:
From: Klaus Heinz <k.heinz@oktsieben.kh-22.de>
To: gnats-bugs@NetBSD.org
Cc: 
Subject: Re: pkg/36656: amanda-client will fail to backup the requested filesystem
Date: Sat, 6 Oct 2007 02:19:48 +0200

 Wolfgang Stukenbrock wrote:

 > 	remark: I've didn't send fix bug-report to the amanda developpers,
 > 	        because I don't know a way to do it and there are lots of fixed
 > 		in the pkgsrc-tree for amanda. I think that one shoul be an
 > 		addirional fix to original distribution.

 You can file bugs at Sourceforge, see

   http://sourceforge.net/tracker/?group_id=120&atid=100120


 A comparison between 2.4.4p4, 2.4.5p1 and 2.5.2p1 (latest stable version)
 shows that the bug is probably fixed in the latter by using

   memset(stats, 0, SIZEOF(stats));

 on the structure array "stats".

 Personally, I would tend to do the following: Prepare a patch based on
 the differences between 2.4.4p4 and 2.5.2p1 and ask on the amanda
 mailing list for comments about your analysis of the problem and that patch.
 If the resulting patch fixes the problem, pkgsrc could probably incorporate
 this.

 ciao
      Klaus


State-Changed-From-To: open->feedback
State-Changed-By: wiz@NetBSD.org
State-Changed-When: Sun, 21 Feb 2010 14:43:59 +0000
State-Changed-Why:
pkgsrc is now at 2.5.2p1 -- is the problem still there?


From: Wolfgang Stukenbrock <Wolfgang.Stukenbrock@nagler-company.com>
To: gnats-bugs@NetBSD.org
Cc: pkg-manager@NetBSD.org, gnats-admin@NetBSD.org, pkgsrc-bugs@NetBSD.org,
        Wolfgang.Stukenbrock@nagler-company.com
Subject: Re: pkg/36656: amanda-client will fail to backup the requested filesystem
Date: Fri, 09 Jul 2010 13:34:06 +0200

 Hi - sorry for the delay ..

 Setting the stat-buffers to zero will not solve the problem.
 It just reduces the possibility that the problem is triggered!
 It may happen still for all entries in /etc/fstab where stat() returns 
 -1 as st_dev.

 I've just checked that with 2.5.2p1.

 The code in amanda is nonsence and broken - from my opinion.
 There cannot be any reason to consult /etc/fstab and replace the path 
 send from the server to be backed up!
 So the best sollution is to eleminate this lookup.

 The following patch disables the lookup.

 *** client-src/getfsent.h.orig  Tue Jul 10 15:30:03 2007
 --- client-src/getfsent.h       Tue Jul 10 15:31:22 2007
 ***************
 *** 62,65 ****
 --- 62,73 ----

    char *amname_to_fstype(char *str);

 + /*
 +  * never look into fstab for filesystem directories - this is 
 nonesence and will fail sometimes
 +  * and results in backing up e.g. /proc instead ...
 +  */
 + #ifndef IGNORE_FSTAB
 + #define IGNORE_FSTAB 1
 + #endif
 +
    #endif /* ! GETFSENT_H */

 W. Stukenbrock

 Klaus Heinz wrote:

 > The following reply was made to PR pkg/36656; it has been noted by GNATS.
 > 
 > From: Klaus Heinz <k.heinz@oktsieben.kh-22.de>
 > To: gnats-bugs@NetBSD.org
 > Cc: 
 > Subject: Re: pkg/36656: amanda-client will fail to backup the requested filesystem
 > Date: Sat, 6 Oct 2007 02:19:48 +0200
 > 
 >  Wolfgang Stukenbrock wrote:
 >  
 >  > 	remark: I've didn't send fix bug-report to the amanda developpers,
 >  > 	        because I don't know a way to do it and there are lots of fixed
 >  > 		in the pkgsrc-tree for amanda. I think that one shoul be an
 >  > 		addirional fix to original distribution.
 >  
 >  You can file bugs at Sourceforge, see
 >  
 >    http://sourceforge.net/tracker/?group_id=120&atid=100120
 >  
 >  
 >  A comparison between 2.4.4p4, 2.4.5p1 and 2.5.2p1 (latest stable version)
 >  shows that the bug is probably fixed in the latter by using
 >  
 >    memset(stats, 0, SIZEOF(stats));
 >  
 >  on the structure array "stats".
 >  
 >  Personally, I would tend to do the following: Prepare a patch based on
 >  the differences between 2.4.4p4 and 2.5.2p1 and ask on the amanda
 >  mailing list for comments about your analysis of the problem and that patch.
 >  If the resulting patch fixes the problem, pkgsrc could probably incorporate
 >  this.
 >  
 >  ciao
 >       Klaus
 >    
 >  
 > 


From: Wolfgang Stukenbrock <Wolfgang.Stukenbrock@nagler-company.com>
To: gnats-bugs@NetBSD.org
Cc: pkg-manager@NetBSD.org, gnats-admin@NetBSD.org, pkgsrc-bugs@NetBSD.org,
        Wolfgang.Stukenbrock@nagler-company.com
Subject: Re: pkg/36656: amanda-client will fail to backup the requested filesystem
Date: Mon, 06 Sep 2010 15:07:51 +0200

 Hi, again.

 I'm still getting a reminder to send some feedback.
 I thought I've already done this a long time ago.


 Neverless, I've checked the sources from 2.5.2p1 again: (last check done 
 at 2.5.2p1)

 Result: the problem is still in there!

 If a special mountpoint will return dev==-1 and ino==0 the wrong
 filesystem will be backed up!
 Only the access to uninitialized data has been removed so that the
 fault is no longer triggered by random. It is now fixed to ino==0.

 The main problem here is, that amanda assumes that dev==-1 can never be 
 a valid return of the stat() call - stat() returns 0.
 But in NetBSD tdev==-1 is a valid return value!
 So this code can never be secure for a NetBSD system.

 W. Stukenbrock




State-Changed-From-To: feedback->open
State-Changed-By: wiz@NetBSD.org
State-Changed-When: Mon, 06 Sep 2010 13:11:16 +0000
State-Changed-Why:
Feedback supplied, problem still exists.


From: Tim Zingelman <tez@netbsd.org>
To: gnats-bugs@netbsd.org, 
	Wolfgang Stukenbrock <Wolfgang.Stukenbrock@nagler-company.com>, solaris-pkg-people@netbsd.org, 
	gnats-admin@netbsd.org, pkgsrc-bugs@netbsd.org
Cc: 
Subject: Re: pkg/36656: amanda-client will fail to backup the requested filesystem
Date: Tue, 29 Mar 2011 11:50:00 -0500

 Does this patch solve the problem?

 cat patches/patch-client-src_getfsent.c
 $NetBSD$

 --- client-src/getfsent.c.orig  2011-03-29 16:29:42.605775000 +0000
 +++ client-src/getfsent.c
 @@ -476,6 +476,7 @@ samefile(
  {
    int i;
    for(i = 0; i < 3; ++i) {
 +    if (stats[i].st_dev == (dev_t)-1) continue;
      if (stats[i].st_dev == estat->st_dev &&
         stats[i].st_ino == estat->st_ino)
        return 1;

 or is there a case when we would want a match with st_dev=-1?

From: Wolfgang Stukenbrock <Wolfgang.Stukenbrock@nagler-company.com>
To: gnats-bugs@NetBSD.org
Cc: pkg-manager@NetBSD.org, gnats-admin@NetBSD.org, pkgsrc-bugs@NetBSD.org,
        Wolfgang.Stukenbrock@nagler-company.com
Subject: Re: pkg/36656: amanda-client will fail to backup the requested filesystem
Date: Wed, 30 Mar 2011 09:53:06 +0200

 Hi,

 yes this patch will fix the problem.
 It corresponds to the version 2 in my original posting.

 If this will be in future releases of pkgsrc, the PR can be closed.

 thanks

 W. Stukenbrock

 Tim Zingelman wrote:

 > The following reply was made to PR pkg/36656; it has been noted by GNATS.
 > 
 > From: Tim Zingelman <tez@netbsd.org>
 > To: gnats-bugs@netbsd.org, 
 > 	Wolfgang Stukenbrock <Wolfgang.Stukenbrock@nagler-company.com>, solaris-pkg-people@netbsd.org, 
 > 	gnats-admin@netbsd.org, pkgsrc-bugs@netbsd.org
 > Cc: 
 > Subject: Re: pkg/36656: amanda-client will fail to backup the requested filesystem
 > Date: Tue, 29 Mar 2011 11:50:00 -0500
 > 
 >  Does this patch solve the problem?
 >  
 >  cat patches/patch-client-src_getfsent.c
 >  $NetBSD$
 >  
 >  --- client-src/getfsent.c.orig  2011-03-29 16:29:42.605775000 +0000
 >  +++ client-src/getfsent.c
 >  @@ -476,6 +476,7 @@ samefile(
 >   {
 >     int i;
 >     for(i = 0; i < 3; ++i) {
 >  +    if (stats[i].st_dev == (dev_t)-1) continue;
 >       if (stats[i].st_dev == estat->st_dev &&
 >          stats[i].st_ino == estat->st_ino)
 >         return 1;
 >  
 >  or is there a case when we would want a match with st_dev=-1?
 >  
 > 


State-Changed-From-To: open->feedback
State-Changed-By: shattered@NetBSD.org
State-Changed-When: Sat, 12 Dec 2015 14:28:34 +0000
State-Changed-Why:
the patch hasn't been committed and the code is still the same.  however, has this been reported upstream at any point?


From: Wolfgang Stukenbrock <wgstuken@grimme-reitenspezial.de>
To: <gnats-bugs@NetBSD.org>
Cc: <pkg-manager@netbsd.org>,
  <pkgsrc-bugs@netbsd.org>,
  <gnats-admin@netbsd.org>,
  <shattered@NetBSD.org>,
  <Wolfgang.Stukenbrock@nagler-company.com>
Subject: Re: pkg/36656 (amanda-client will fail to backup the requested filesystem)
Date: Mon, 21 Dec 2015 12:14:06 +0100

 Hi to all,

 not from my side.
 We have switched away from amanda to bacula a long time ago, so I  
 haven't tracked this anymore.
 A short look at the history of this report show me, that "someone"  
 has developed a small patch that filters out st_dev==-1.
 I thought that this has been added to pkgsrc patches.

 I still think it should be fixed in pkgsrc.
 Of cause it can (and perhaps even should) be reported to the  
 maintainer. But in the past there are lot of other fixes done in  
 pkgsrc for amanda, so why not another one.

 mary chrismas and a happy new year.

 W. Stukenbrock

 On 12.12.2015, at 15:28, <shattered@NetBSD.org>  
 <shattered@NetBSD.org> wrote:

 > Synopsis: amanda-client will fail to backup the requested filesystem
 >
 > State-Changed-From-To: open->feedback
 > State-Changed-By: shattered@NetBSD.org
 > State-Changed-When: Sat, 12 Dec 2015 14:28:34 +0000
 > State-Changed-Why:
 > the patch hasn't been committed and the code is still the same.   
 > however, has this been reported upstream at any point?
 >
 >
 >

State-Changed-From-To: feedback->open
State-Changed-By: dholland@NetBSD.org
State-Changed-When: Sat, 11 Jun 2016 20:57:00 +0000
State-Changed-Why:
Feedback was received in december. It's a one-line patch, let's just do it...


From: "David A. Holland" <dholland@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/36656 CVS commit: pkgsrc/sysutils/amanda-common
Date: Sat, 11 Jun 2016 21:07:28 +0000

 Module Name:	pkgsrc
 Committed By:	dholland
 Date:		Sat Jun 11 21:07:28 UTC 2016

 Modified Files:
 	pkgsrc/sysutils/amanda-common: distinfo
 	pkgsrc/sysutils/amanda-common/patches: patch-client-src_getfsent.c

 Log Message:
 Add small patch from PR 36656 to prevent false matches when looking in
 fstab.

 (It can also potentially prevent true matches and the whole chunk of
 code involved should ideally be rewritten sanely, but it's better than
 randomly doing entirely the wrong thing.)


 To generate a diff of this commit:
 cvs rdiff -u -r1.20 -r1.21 pkgsrc/sysutils/amanda-common/distinfo
 cvs rdiff -u -r1.1 -r1.2 \
     pkgsrc/sysutils/amanda-common/patches/patch-client-src_getfsent.c

 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: dholland@NetBSD.org
State-Changed-When: Sat, 11 Jun 2016 21:11:22 +0000
State-Changed-Why:
I committed tez@'s patch.


>Unformatted:

Home	PR Database Search