linux-kernel - Re: [PATCH 2/2] vfs: fix d_path() for unreachable paths

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20090921140220.GD14381@ZenIV.linux.org.uk>
Date:	Mon, 21 Sep 2009 15:02:20 +0100
From:	Al Viro <viro@...IV.linux.org.uk>
To:	Miklos Szeredi <miklos@...redi.hu>
Cc:	akpm@...ux-foundation.org, linux-fsdevel@...r.kernel.org,
	linux-kernel@...r.kernel.org, Valdis.Kletnieks@...edu,
	agruen@...e.de, hch@....de, hugh.dickins@...cali.co.uk,
	matthew@....cx
Subject: Re: [PATCH 2/2] vfs: fix d_path() for unreachable paths

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.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/