archive mirror
 help / color / mirror / Atom feed
* [RFC PATCH 0/4] vfs: map unique ino/dev pairs for user space
@ 2018-07-31 21:10 Mark Fasheh
  2018-07-31 21:10 ` [PATCH 1/4] vfs: introduce function to map unique ino/dev pairs Mark Fasheh
                   ` (4 more replies)
  0 siblings, 5 replies; 17+ messages in thread
From: Mark Fasheh @ 2018-07-31 21:10 UTC (permalink / raw)
  To: Al Viro, linux-fsdevel
  Cc: linux-btrfs, Andrew Morton, Amir Goldstein, linux-unionfs,
	Jeff Mahoney, linux-nfs


These patches are follow-up to this conversation on fsdevel which provides a
good break-down of the core problem:

That e-mail was in turn a follow-up to the following patch series:

which tried to solve the dev portion of this problem with a bit of structure
re-routing but was never accepted.

We have an inconsistency in how the kernel is exporting inode number /
device pairs for user space.  I'm not talking about stat(2) or statx(2) here
but everywhere else where an ino/dev is exported into a buffer for user

We typically do this by simply dumping the potentially 32 bit inode->i_ino
and single device from super->s_dev.  Sometimes these values differ from
what is returned via stat(2) or statx(2).  Some filesystems might even show
duplicate (but internally different!) pairs when the raw i_ino/s_dev is

Aside from breaking software which expects these pairs to be unique, this
can put the user in a situation where they might not be able to find an
inode referenced from some kernel log (be it /proc/, printk buffer, etc). 
What's even worse - depending on how ino is exported, they might even find
an existing but /wrong/ file.

The following patches fix these inconsistencies by introducing a VFS helper
function which calls the underlying filesystem ->getattr to get our real
inode number / device pair.  The returned values can then be used at those
places in the kernel where we are incorrectly reporting our ino/dev pair. 
We then update fs/proc/ and fs/locks.c to use this helper when writing to
/proc/PID/maps and /proc/locks respectively.

For anyone who is watching the evolution of these patches, this would be one
implementation of the 'callback' method which I discussed in my last e-mail. 
This approach is very straight forward and easy to understand but has some

- Not all of the call sites can tolerate errors.  Instead, we fallback to
  inode->i_ino and super->s_dev in case of getattr error.  That behavior is
  no worse than what we have today but what we have today is pretty bad so I
  don't particularly feel good about that. It's better than nothing though.

- There are places in the kernel where we could never call into ->getattr. 
  There are tons of tracepoints for example that are printing the the wrong
  ino/dev pair.

- The helper function has to fake a path struct because getting a vfsmount
  from inside these call sites is impossible.  This isn't actually a big
  deal as nobody except NFS cares and the NFS patch is very trivial.  It's
  ugly though, so if we had consensus on this approach I would happily
  rework our ->getattr function signature, perhaps pulling the vfsmount and
  dentry arguments back out.

- The dentry argument to ->getattr is potentially problematic as we have
  some places which _only_ have an inode. Even if we had a dentry I'm not
  sure what would keep it from going negative while in the middle of our
  ->getattr call.

Comments and review would be greatly appreciated. Thanks,

^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2018-08-03  3:58 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-07-31 21:10 [RFC PATCH 0/4] vfs: map unique ino/dev pairs for user space Mark Fasheh
2018-07-31 21:10 ` [PATCH 1/4] vfs: introduce function to map unique ino/dev pairs Mark Fasheh
2018-07-31 23:21   ` Mark Fasheh
2018-08-01  5:41     ` Amir Goldstein
2018-08-01 15:59       ` Mark Fasheh
2018-07-31 21:10 ` [PATCH 2/4] nfs: check for NULL vfsmount in nfs_getattr Mark Fasheh
2018-07-31 22:16   ` Al Viro
2018-07-31 22:51     ` Mark Fasheh
2018-08-02  0:43       ` Al Viro
2018-08-03  2:04         ` Mark Fasheh
2018-07-31 21:10 ` [PATCH 3/4] proc: use vfs helper to get ino/dev pairs for maps file Mark Fasheh
2018-07-31 21:10 ` [PATCH 4/4] locks: map correct ino/dev pairs when exporting to userspace Mark Fasheh
2018-08-01  5:37   ` Amir Goldstein
2018-08-01 17:45     ` Mark Fasheh
2018-08-01 11:03   ` Jeff Layton
2018-08-01 20:38     ` Mark Fasheh
2018-08-02  0:24 ` [RFC PATCH 0/4] vfs: map unique ino/dev pairs for user space J. R. Okajima

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).