On Thu, Nov 14, 2024 at 02:13:06PM +0100, Erin Shepherd wrote: > On 14/11/2024 13:52, Christian Brauner wrote: > > > I think you need at least something like the following completely > > untested draft on top: > > > > - the pidfs_finish_open_by_handle_at() is somewhat of a clutch to handle > > thread vs thread-group pidfds but it works. > > > > - In contrast to pidfd_open() that uses dentry_open() to create a pidfd > > open_by_handle_at() uses file_open_root(). That's overall fine but > > makes pidfds subject to security hooks which they aren't via > > pidfd_open(). It also necessitats pidfs_finish_open_by_handle_at(). > > There's probably other solutions I'm not currently seeing. > > These two concerns combined with the special flag make me wonder if pidfs > is so much of a special snowflake we should just special case it up front > and skip all of the shared handle decode logic? Care to try a patch and see what it looks like? > > > - The exportfs_decode_fh_raw() call that's used to decode the pidfd is > > passed vfs_dentry_acceptable() as acceptability callback. For pidfds > > we don't need any of that functionality and we don't need any of the > > disconnected dentry handling logic. So the easiest way to fix that is > > to rely on EXPORT_OP_UNRESTRICTED_OPEN to skip everything. That in > > turns means the only acceptability we have is the nop->fh_to_dentry() > > callback for pidfs. > > With the current logic we go exportfs_decode_fh_raw(...) -> > find_acceptable_alias(result) -> vfs_dentry_acceptable(context, result). > vfs_dentry_acceptable immediately returns 1 if ctx->flags is 0, which will > always be the case if EXPORT_OP_UNRESTRICTED_OPEN was set, so we immediately > fall back out of the call tree and return result. > > So I'm not 100% sure we actually need this special case but I'm not opposed. Oh right, I completely forgot that find_acceptable_alias() is a no-op for pidfs.
