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