linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Al Viro <viro@ZenIV.linux.org.uk>
To: Miklos Szeredi <miklos@szeredi.hu>
Cc: akpm@linux-foundation.org, linux-fsdevel@vger.kernel.org,
	linux-kernel@vger.kernel.org, Valdis.Kletnieks@vt.edu,
	agruen@suse.de, hch@lst.de, hugh.dickins@tiscali.co.uk,
	matthew@wil.cx
Subject: Re: [PATCH 2/2] vfs: fix d_path() for unreachable paths
Date: Mon, 21 Sep 2009 15:02:20 +0100	[thread overview]
Message-ID: <20090921140220.GD14381@ZenIV.linux.org.uk> (raw)
In-Reply-To: <E1MpiMo-0005h9-DG@pomaz-ex.szeredi.hu>

On Mon, Sep 21, 2009 at 02:51:42PM +0200, Miklos Szeredi wrote:
> Hugh Dickins reported that an old version of gnome-vfs-daemon crashes
> because it finds an entry in /proc/mounts where the mountpoint is
> unreachable.  So revert /proc/mounts to the old behavior (or rather a
> less crazy version of the old behavior).
> 
> Also revert the effect on /proc/${PID}/maps for memory maps set up
> with shmem_file_setup() or hugetlb_file_setup().  These functions set
> up unlinked files under a kernel-private vfsmount.  Since this
> vfsmount is unreachable from userspace, these maps will be reported
> with the "(unreachable)" prefix, which is undesirable, because it
> changes the kernel ABI and might break applications for no good
> reason.

Ho-hum...
	a) d_op you've got there look like a candidate for libfs, if
we go for that at all
	b) I *really* don't like changing the behaviour of d_path() instead
of doing new behaviour in a new function and deciding where to use it on
case-by-case basis
	c) having just grepped for d_path()... oh, man
* blackfin cplbinfo: utter crap; it's used to decide which procfs file
is being opened - by dumping full pathname into a (on-stack) buffer
and then parsing it.  Stupid *and* broken.
* blackfin traps.c:decode_address(): for one thing, pathnames has been
known to be longer than 256 bytes.  For another... locking in that loop
over processes and VMAs of each looks very suspicios, while we are at it.
* pohmelfs_construct_path_string(): just what will happen if we are called
from process chrooted into that bugger?  d_path() is badly abused there.
* ext4_file_open(): d_path() per se is probably OK, but initializing
path.mnt/path.dentry isn't.  mount --move racing with that thing => loads
of fun.
* sysfs_open_file(): racy in an obvious way, but probably won't do anything
worse than garbage in oopsen.

I'm very uncomfortable with the silent change of behaviour, especially since
existing callers seem to be split between "part of user-visible ABI" and
"generally bogus, waiting for a gnat to fart".  At the very least it needs
a serious audit of callers.

  reply	other threads:[~2009-09-21 14:02 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-09-21 12:48 [PATCH 1/2] vfs: seq_file: add helpers for data filling Miklos Szeredi
2009-09-21 12:51 ` [PATCH 2/2] vfs: fix d_path() for unreachable paths Miklos Szeredi
2009-09-21 14:02   ` Al Viro [this message]
2009-09-21 14:10     ` Mike Frysinger
2009-09-21 14:38       ` Al Viro
2009-09-21 14:43         ` Al Viro
2009-09-21 15:31           ` Mike Frysinger
2009-09-21 15:30         ` Mike Frysinger
2009-09-21 15:03     ` Miklos Szeredi

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=20090921140220.GD14381@ZenIV.linux.org.uk \
    --to=viro@zeniv.linux.org.uk \
    --cc=Valdis.Kletnieks@vt.edu \
    --cc=agruen@suse.de \
    --cc=akpm@linux-foundation.org \
    --cc=hch@lst.de \
    --cc=hugh.dickins@tiscali.co.uk \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=matthew@wil.cx \
    --cc=miklos@szeredi.hu \
    /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).