Re: [PATCH 00/17] VFS: Filesystem information and notifications [ver #17]
From: Al Viro <viro@zeniv.linux.org.uk>
Date: 2020-03-06 20:56:32
Also in:
linux-fsdevel, lkml
On Fri, Mar 06, 2020 at 08:49:26PM +0000, Al Viro wrote:
On Fri, Mar 06, 2020 at 08:45:23PM +0000, Al Viro wrote:quoted
On Fri, Mar 06, 2020 at 08:38:44PM +0000, Al Viro wrote:quoted
On Fri, Mar 06, 2020 at 08:37:05PM +0000, Al Viro wrote:quoted
You are misreading mntput_no_expire(), BTW - your get_mount() can bloody well race with umount(2), hitting the moment when we are done figuring out whether it's busy but hadn't cleaned ->mnt_ns (let alone set MNT_DOOMED) yet. If somebody calls umount(2) on a filesystem that is not mounted anywhere else, they are not supposed to see the sucker return 0 until the filesystem is shut down. You break that.While we are at it, d_alloc_parallel() requires i_rwsem on parent held at least shared.Egads... Let me see if I got it right - you are providing procfs symlinks to objects on the internal mount of that thing. And those objects happen to be directories, so one can get to their parent that way. Or am I misreading that thing?IDGI. You have (in your lookup) kstrtoul, followed by snprintf, followed by strcmp and WARN_ON() in case of mismatch? Is there any point in having stat(2) on "00" spew into syslog? Confused...
AFAICS, refcounting in there cannot be right:
+static struct dentry *mountfs_lookup(struct inode *dir, struct dentry *dentry,
+ unsigned int flags)
+{
+ struct mountfs_entry *entry = dir->i_private;
+ int i = 0;
+
+ if (entry) {
+ for (i = 0; i < ARRAY_SIZE(mountfs_attrs); i++)
+ if (strcmp(mountfs_attrs[i], dentry->d_name.name) == 0)
+ break;
+ if (i == ARRAY_SIZE(mountfs_attrs))
+ return ERR_PTR(-ENOMEM);
+ i++;
+ } else {
+ entry = mountfs_get_entry(dentry->d_name.name);
+ if (!entry)
+ return ERR_PTR(-ENOENT);
+ }
+
+ return mountfs_lookup_entry(dentry, entry, i);
+}
ends up consuming a reference in mountfs_lookup_entry() (at the very least,
drops it in case of inode allocation hitting OOM) and nothing in the
that loop in mountfs_lookup() appears to do a matching reference grab.