On Tue, Jun 24, 2025 at 3:43 PM Christian Brauner <brauner@xxxxxxxxxx> wrote:
>
> On Tue, Jun 24, 2025 at 03:15:18PM +0200, Amir Goldstein wrote:
> > On Tue, Jun 24, 2025 at 10:29 AM Christian Brauner <brauner@xxxxxxxxxx> wrote:
> > >
> > > Introduce new pidfs file handle values.
> > >
> > > Signed-off-by: Christian Brauner <brauner@xxxxxxxxxx>
> > > ---
> > > include/linux/exportfs.h | 5 +++++
> > > 1 file changed, 5 insertions(+)
> > >
> > > diff --git a/include/linux/exportfs.h b/include/linux/exportfs.h
> > > index 25c4a5afbd44..5bb757b51f5c 100644
> > > --- a/include/linux/exportfs.h
> > > +++ b/include/linux/exportfs.h
> > > @@ -131,6 +131,11 @@ enum fid_type {
> > > * Filesystems must not use 0xff file ID.
> > > */
> > > FILEID_INVALID = 0xff,
> > > +
> > > + /* Internal kernel fid types */
> > > +
> > > + /* pidfs fid type */
> > > + FILEID_PIDFS = 0x100,
> > > };
> > >
> >
> > Jan,
> >
> > I just noticed that we have a fh_type <= 0xff assumption
> > built into fanotify:
> >
> > /* Fixed size struct for file handle */
> > struct fanotify_fh {
> > u8 type;
> > u8 len;
> >
> > and we do not enforce it.
> > there is only check of type range 1..0xffff
> > in exportfs_encode_inode_fh()
> >
> > We should probably do either:
> >
> > --- a/fs/notify/fanotify/fanotify.c
> > +++ b/fs/notify/fanotify/fanotify.c
> > @@ -454,7 +454,7 @@ static int fanotify_encode_fh(struct fanotify_fh
> > *fh, struct inode *inode,
> > dwords = fh_len >> 2;
> > type = exportfs_encode_fid(inode, buf, &dwords);
> > err = -EINVAL;
> > - if (type <= 0 || type == FILEID_INVALID || fh_len != dwords << 2)
> > + if (type <= 0 || type >= FILEID_INVALID || fh_len != dwords << 2)
> > goto out_err;
> >
> > fh->type = type;
> >
> > OR
> >
> > --- a/fs/notify/fanotify/fanotify.h
> > +++ b/fs/notify/fanotify/fanotify.h
> > @@ -29,11 +29,10 @@ enum {
> >
> > /* Fixed size struct for file handle */
> > struct fanotify_fh {
> > - u8 type;
> > + u16 type;
> > u8 len;
> > #define FANOTIFY_FH_FLAG_EXT_BUF 1
> > u8 flags;
> > - u8 pad;
> > } __aligned(4);
> >
> >
> > Christian,
> >
> > Do you know if pidfs supports (or should support) fanotify with FAN_REPORT_FID?
>
> I think it's at least supported by fanotify in that FAN_REPORT_FID and
> FAN_REPORT_PIDFD aren't mutually exclusive options afaict. I don't know
> if it's used though.
>
> > If it does not need to be supported we can block it in fanotify_test_fid(),
> > but if it does need fanotify support, we need to think about this.
>
> Sure, block it.
>
hmm, we are already denying sb/mount marks from SB_NOUSER,
but inode marks are allowed.
I don't want to special case pidfs.
I guess we can do:
--- a/fs/notify/fanotify/fanotify_user.c
+++ b/fs/notify/fanotify/fanotify_user.c
@@ -1736,6 +1736,10 @@ static int fanotify_test_fid(struct dentry
*dentry, unsigned int flags)
if (mark_type != FAN_MARK_INODE && !exportfs_can_decode_fh(nop))
return -EOPNOTSUPP;
+ /* We do not support reporting autonomous file handles. */
+ if (nop && nop->flags & EXPORT_OP_AUTONOMOUS_HANDLES)
+ return -EOPNOTSUPP;
+
return 0;
}
if needed
> >
> > Especially, if we want fanotify to report autonomous file handles.
> > In that case, the design that adds the flag in the open_by_handle_at()
> > syscall won't do.
>
> Sorry, the design that adds the flag? You mean the FD_INVALID?
> Why is that?
I mean the design that adds the flag FILEID_IS_AUTONOMOUS
in do_sys_name_to_handle(), because fanotify encodes the file handle
with the vfs helper exportfs_encode_fid(), so the resulting fid it reports
is not autonomous.
But I think I have a suggestion to untangle all this mess.
I will explain in another email.
Thanks,
Amir.
