NetBSD Problem Report #55815

From martin@aprisoft.de  Sun Nov 22 13:21:05 2020
Return-Path: <martin@aprisoft.de>
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 "mail.NetBSD.org CA" (not verified))
	by mollari.NetBSD.org (Postfix) with ESMTPS id 591A31A921F
	for <gnats-bugs@gnats.NetBSD.org>; Sun, 22 Nov 2020 13:21:05 +0000 (UTC)
Message-Id: <20201122132055.4E0B85CC848@emmas.aprisoft.de>
Date: Sun, 22 Nov 2020 14:20:55 +0100 (CET)
From: martin@NetBSd.org
Reply-To: martin@NetBSD.org
To: gnats-bugs@NetBSD.org
Subject: tar opens device files
X-Send-Pr-Version: 3.95

>Number:         55815
>Category:       bin
>Synopsis:       tar opens device files
>Confidential:   no
>Severity:       critical
>Priority:       high
>Responsible:    bin-bug-people
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Sun Nov 22 13:25:00 +0000 2020
>Last-Modified:  Wed Nov 25 19:10:01 +0000 2020
>Originator:     Martin Husemann
>Release:        NetBSD 9.99.75
>Organization:
The NetBSD Foundation, Inc.
>Environment:
System: NetBSD seven-days-to-the-wolves.aprisoft.de 9.99.75 NetBSD 9.99.75 (GENERIC) #425: Wed Nov 4 15:34:33 CET 2020 martin@seven-days-to-the-wolves.aprisoft.de:/work/src/sys/arch/amd64/compile/GENERIC amd64
Architecture: x86_64
Machine: amd64
>Description:

Tar does open device files, which is a *very* bad idea.

>How-To-Repeat:

ktrace tar cvf - /dev >/dev/null
(but carefull, might kill your machine)

>Fix:
Switch MKBSDTAR back to off.

>Audit-Trail:
From: Joerg Sonnenberger <joerg@bec.de>
To: gnats-bugs@netbsd.org
Cc: 
Subject: Re: bin/55815: tar opens device files
Date: Tue, 24 Nov 2020 21:12:19 +0100

 On Sun, Nov 22, 2020 at 01:25:00PM +0000, martin@NetBSd.org wrote:
 > Tar does open device files, which is a *very* bad idea.

 As discussed on IRC, this is a side effect of enabling ACL and extattr
 support OOB. It is avoided when using --no-acls --no-extattr. Using only
 the fd-based API variants is established practise, e.g. GNU tar has the
 same behavior. Other options would be racy, which is especially bad when
 dealing with file permissions.

 Most systems consider device open with side effect a bug. I can remember
 two cases in NetBSD off the top of my head. Tapes and IOPL device on
 x86. The former is harmless as side effect as far as archiving is
 concerned. The former has the rewind on open. IMO that should either be
 completely removed or deferred to the first actual IO operation.

 Joerg

From: Martin Husemann <martin@duskware.de>
To: gnats-bugs@netbsd.org
Cc: 
Subject: Re: bin/55815: tar opens device files
Date: Wed, 25 Nov 2020 06:54:12 +0100

 On Tue, Nov 24, 2020 at 08:15:02PM +0000, Joerg Sonnenberger wrote:
 >  As discussed on IRC, this is a side effect of enabling ACL and extattr
 >  support OOB. It is avoided when using --no-acls --no-extattr.

 Those should be the default then, or maybe check (is there a mount flag
 or something in statvfs?) whether any ACLs or ext attrs could be there at
 all.

 What happens if the device can not be opened at all, will it be left out of
 the tar?

 Martin

From: Joerg Sonnenberger <joerg@bec.de>
To: gnats-bugs@netbsd.org
Cc: gnats-admin@netbsd.org, netbsd-bugs@netbsd.org, martin@NetBSD.org
Subject: Re: bin/55815: tar opens device files
Date: Wed, 25 Nov 2020 10:10:43 +0100

 On Wed, Nov 25, 2020 at 05:55:01AM +0000, Martin Husemann wrote:
 > The following reply was made to PR bin/55815; it has been noted by GNATS.
 > 
 > From: Martin Husemann <martin@duskware.de>
 > To: gnats-bugs@netbsd.org
 > Cc: 
 > Subject: Re: bin/55815: tar opens device files
 > Date: Wed, 25 Nov 2020 06:54:12 +0100
 > 
 >  On Tue, Nov 24, 2020 at 08:15:02PM +0000, Joerg Sonnenberger wrote:
 >  >  As discussed on IRC, this is a side effect of enabling ACL and extattr
 >  >  support OOB. It is avoided when using --no-acls --no-extattr.
 >  
 >  Those should be the default then, or maybe check (is there a mount flag
 >  or something in statvfs?) whether any ACLs or ext attrs could be there at
 >  all.

 That would be the ostrich approach of ignoring that open with side
 effects is fundamentally a bad idea. It also doesn't match what other
 tar implementations do and violates POLA just as well.

 >  
 >  What happens if the device can not be opened at all, will it be left out of
 >  the tar?

 No ACLs or extattrs will be saved in that case.

 Joerg

From: Martin Husemann <martin@duskware.de>
To: gnats-bugs@netbsd.org
Cc: 
Subject: Re: bin/55815: tar opens device files
Date: Wed, 25 Nov 2020 10:19:16 +0100

 On Wed, Nov 25, 2020 at 10:10:43AM +0100, Joerg Sonnenberger wrote:
 > >  Those should be the default then, or maybe check (is there a mount flag
 > >  or something in statvfs?) whether any ACLs or ext attrs could be there at
 > >  all.
 > 
 > That would be the ostrich approach of ignoring that open with side
 > effects is fundamentally a bad idea. It also doesn't match what other
 > tar implementations do and violates POLA just as well.

 I agree that device open having an effect on the state of the device is
 a bad thing in general, but it has been the case in Unix since the beginning,
 or am I misremembering something? At least it was that way when dealing with
 all tapes I ever dealt with.

 But besides that: if there *could* be no extattr or ACL on the file system
 (like I *knew* for sure for the case at hand, it was an anciend FFSv1
 file system way too old for any of that) and making that information
 available (like with a mount flag or a fs superblock flag or whatever) -
 why is skipping the unneccessary open in that case an ostrich aproach?

 Or why would it violate POLA? I would call it a smart optimization instead,
 and would be very suprised to see thousands of totally unneeded open()
 calls.

 Martin

From: Christos Zoulas <christos@zoulas.com>
To: gnats-bugs@netbsd.org
Cc: gnats-admin@netbsd.org,
 netbsd-bugs@netbsd.org,
 "martin@netbsd.org" <martin@NetBSD.org>
Subject: Re: bin/55815: tar opens device files
Date: Wed, 25 Nov 2020 08:34:16 -0500

 Unfortunately it looks like implementing a no-side-effects open for some =
 devices is=20
 difficult because those devices (tapes in particular) rely on the =
 different semantics=20
 for open depending on different minor numbers. As joerg mentions, =
 deferring this=20
 to the first I/O would be a good thing but it is not trivial to do and =
 we have a bunch=20
 of them. What we seem to want is an open() flavor that gives us access =
 just to the
 extended attributes and stat info, so we could just do that (open =
 O_EXTATTR)?

 christos=

From: Robert Elz <kre@munnari.OZ.AU>
To: gnats-bugs@netbsd.org
Cc: 
Subject: Re: bin/55815: tar opens device files
Date: Wed, 25 Nov 2020 22:51:59 +0700

     Date:        Wed, 25 Nov 2020 09:20:01 +0000 (UTC)
     From:        Martin Husemann <martin@duskware.de>
     Message-ID:  <20201125092001.974C81A921F@mollari.NetBSD.org>

   |  I agree that device open having an effect on the state of the device is
   |  a bad thing in general,

 I'm not sue I really agree, though "deferred to the first I/O" would be
 semantically equivalent I think (except that with tapes, just open/close
 has always been a method to accomplish rewind), so that would be possible.
 But not "doing things" at all would be wrong (would break a long time
 interface expectation, whatever anyone believe it ought to be).

   |  but it has been the case in Unix since the beginning,
   |  or am I misremembering something?

 You're not, that is the way it has always been - or at least as far back
 as I can remember (I never saw any of 1st-4th edition systems).

 I am not sure I understand the point here though, why is tar opening
 files at all if it doesn't plan on reading from them?   Is this some
 kind of optimisation attempt, where since most files will be regular
 filesystem files, and will need to be opened, open() then fstat() has
 one less pathname lookup (namei) than stat() followed when needed by open()
 (the normal case)?    If so, we should just stop doing that, as it is broken,
 and revert to the stat()+open() model (where the open only happens if we
 want the data contained in the file, rather than just its name and attributes).
 We need that for symnlinks too (ie: the stat() would actually be lstat()),
 surely.

 I don't know what's available for fetching extattrs, but if that requires
 an open, we should fix that, and make them available without opening.
 But surely there is already a way, I can't believe that ls attempts to
 open files, yet it shows (when asked) extattrs, right?

 kre


From: Joerg Sonnenberger <joerg@bec.de>
To: gnats-bugs@netbsd.org
Cc: gnats-admin@netbsd.org, netbsd-bugs@netbsd.org, martin@NetBSD.org
Subject: Re: bin/55815: tar opens device files
Date: Wed, 25 Nov 2020 19:06:26 +0100

 On Wed, Nov 25, 2020 at 03:55:01PM +0000, Robert Elz wrote:
 >  I am not sure I understand the point here though, why is tar opening
 >  files at all if it doesn't plan on reading from them?   Is this some
 >  kind of optimisation attempt, where since most files will be regular
 >  filesystem files, and will need to be opened, open() then fstat() has
 >  one less pathname lookup (namei) than stat() followed when needed by open()
 >  (the normal case)?    If so, we should just stop doing that, as it is broken,
 >  and revert to the stat()+open() model (where the open only happens if we
 >  want the data contained in the file, rather than just its name and attributes).
 >  We need that for symnlinks too (ie: the stat() would actually be lstat()),
 >  surely.

 No, it is about ensuring that the extattr and ACL actually belong to the
 same object as the rest of the permissions.

 Joerg

From: Christos Zoulas <christos@zoulas.com>
To: gnats-bugs@netbsd.org
Cc: gnats-admin@netbsd.org,
 netbsd-bugs@netbsd.org,
 "martin@netbsd.org" <martin@NetBSD.org>
Subject: Re: bin/55815: tar opens device files
Date: Wed, 25 Nov 2020 13:20:28 -0500

 > I don't know what's available for fetching extattrs, but if that requires
 > an open, we should fix that, and make them available without opening.
 > But surely there is already a way, I can't believe that ls attempts to
 > open files, yet it shows (when asked) extattrs, right?

 There are methods by pathname and by fd. By pathname suffer from
 TOCTOU, so we need to do by fd. To get an fd we need open. So the
 only reasonable solution seems to be to provide an open flag that does
 not have side effects on devices.

 christos

From: Robert Elz <kre@munnari.OZ.AU>
To: Joerg Sonnenberger <joerg@bec.de>
Cc: gnats-bugs@netbsd.org, martin@NetBSD.org
Subject: Re: bin/55815: tar opens device files
Date: Thu, 26 Nov 2020 02:06:34 +0700

     Date:        Wed, 25 Nov 2020 19:06:26 +0100
     From:        Joerg Sonnenberger <joerg@bec.de>
     Message-ID:  <20201125180626.GA111551@bec.de>

   | No, it is about ensuring that the extattr and ACL actually belong to the
   | same object as the rest of the permissions.

 Ah, OK.

 Is this for creating the archive, or extracting it?

 In the creating case, does it really matter?   If the filesystem is
 changing while attempting to create an archive, then there's got to
 be race conditions, and while it would be nice to correctly add either
 the before, or the after to the archive, I'm not sure that's important
 enough to really matter.

 In the extracting case, at least when extracting a non-openable (or
 potentially non-openable) file, couldn't tar extract to a file in a
 randomnly named 700 mode sub-dir, set the attributes/acl on the file in
 that sub-dir, and then rename it to where it belongs?

 Wouldn't any attempt to open the file (almost) necessarily hang when
 the file is a fifo ?   But those deserve to be archived/extracted
 just as accurately as regular files, directories, and special files.
 (I'm not sure the same is true of sockets, but perhaps).

 kre

NetBSD Home
NetBSD PR Database Search

(Contact us) $NetBSD: query-full-pr,v 1.46 2020/01/03 16:35:01 leot Exp $
$NetBSD: gnats_config.sh,v 1.9 2014/08/02 14:16:04 spz Exp $
Copyright © 1994-2020 The NetBSD Foundation, Inc. ALL RIGHTS RESERVED.