qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Eric Blake <eblake@redhat.com>
To: Laszlo Ersek <lersek@redhat.com>
Cc: aliguori@us.ibm.com, qemu-devel@nongnu.org
Subject: Re: [Qemu-devel] [PATCH 2/2] qga: try to unlink just created guest-file if fchmod() fails on it
Date: Tue, 07 May 2013 20:24:43 -0600	[thread overview]
Message-ID: <5189B76B.50509@redhat.com> (raw)
In-Reply-To: <51899DDC.3060101@redhat.com>

[-- Attachment #1: Type: text/plain, Size: 2714 bytes --]

On 05/07/2013 06:35 PM, Laszlo Ersek wrote:
>> This fixes the case of a mode 0000 file if fchmod fails, but doesn't fix
>> the case of a mode 0666 file if fchmod succeeds but fdopen fails.  It
>> also requires that unlink() while open works (true for most Unix
>> systems, but false for Windows systems and not required by POSIX
> 
> Please provide a reference for "not required by POSIX". The EBUSY
> condition I'm looking at says "The file named by the path argument
> cannot be unlinked because it is being used by the system or another
> process and the implementation considers this an error".

That's precisely what I'm looking at.  The Windows implementation
considers any open file as in use by the system, and gives an error when
trying to unlink an open file (it also gives an error when trying to
rmdir a directory that serves as the current working directory of any
process).

Another case is NFS, which has historically had not-so-nice behavior if
you attempt to unlink an open file (such as the file still being
resident in cache for several seconds longer and preventing removal of a
directory), whereas closing first and then unlinking suffers from no
delay or weird caching effects (admittedly, NFS as a file system breaks
several POSIX rules).

You are right that POSIX unlink() also says "If one or more processes
have the file open when the last link is removed, the link shall be
removed before unlink() returns, but the removal of the file contents
shall be postponed until all references to the file are closed."  But I
read that as permitting (but not requiring) an implementation to unlink
an open fd, if it further follows the rules about the file contents when
unlinking an open file.  Meanwhile, the EBUSY error permits (but not
requires) implementations to let unlink fail on an open file, and NFS
serves as a case study of this behavior even on Unix-y systems.

Even if you don't buy my argument about EBUSY, and even though we both
agree that Unix-y systems have historically allowed unlink of an open
file on a local file system (as well as rmdir of the current working
directory), it still doesn't change the fact that in reality, such
actions are not portable to Windows or NFS.  And whether or not Windows
is non-compliant with POSIX doesn't matter quite as much as whether our
code is portable to the systems we are porting to.

> 
> "Used by the system or another process" shouldn't really be the case here.

Used by another process - no.  Used by the system - yes, if the system
considers all open files to be in use.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 621 bytes --]

  reply	other threads:[~2013-05-08  2:24 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-05-07 11:47 [Qemu-devel] [PATCH] qga: set umask 0077 when daemonizing (CVE-2013-2007) Anthony Liguori
2013-05-07 15:55 ` Eric Blake
2013-05-07 16:56   ` [Qemu-devel] [PATCH 1/2] qga: distinguish binary modes in "guest_file_open_modes" map Laszlo Ersek
2013-05-07 17:19     ` Eric Blake
2013-05-07 17:27     ` Eric Blake
2013-05-07 18:54       ` Peter Maydell
2013-05-07 20:10       ` mdroth
2013-05-07 16:56   ` [Qemu-devel] [PATCH 2/2] qga: try to unlink just created guest-file if fchmod() fails on it Laszlo Ersek
2013-05-07 17:30     ` Eric Blake
2013-05-08  0:35       ` Laszlo Ersek
2013-05-08  2:24         ` Eric Blake [this message]
2013-05-07 20:28   ` [Qemu-devel] [PATCH] qga: set umask 0077 when daemonizing (CVE-2013-2007) mdroth
2013-05-07 20:54     ` Eric Blake
2013-05-07 18:49 ` Anthony Liguori
2013-05-08  2:03   ` Anthony Liguori
2013-05-09 14:39 ` Bruce Rogers

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=5189B76B.50509@redhat.com \
    --to=eblake@redhat.com \
    --cc=aliguori@us.ibm.com \
    --cc=lersek@redhat.com \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).