On Thu, Jun 10, 2021 at 01:23:31PM +0200, Jan Kara wrote:
> > @@ -524,6 +561,34 @@ static ssize_t copy_event_to_user(struct fsnotify_group *group,
> > }
> > metadata.fd = fd;
> >
> > + /*
> > + * Currently, reporting a pidfd to an unprivileged listener is not
> > + * supported. The FANOTIFY_UNPRIV flag is to be kept here so that a
> > + * pidfd is not accidentally leaked to an unprivileged listener.
> > + */
> > + if (pidfd_mode && !FAN_GROUP_FLAG(group, FANOTIFY_UNPRIV)) {
>
> Hum, you've added FAN_REPORT_PIDFD to FANOTIFY_ADMIN_INIT_FLAGS so this
> condition should be always true? I don't think we need to be that much
> defensive and would just drop the check here.
Yes, that's right, so dropping this check is also fine with me.
> > @@ -558,6 +632,10 @@ static ssize_t copy_event_to_user(struct fsnotify_group *group,
> > put_unused_fd(fd);
> > fput(f);
> > }
> > +
> > + if (pidfd < 0)
> > + put_unused_fd(pidfd);
> > +
>
> put_unused_fd() is not enough to destroy the pidfd you have. That will just
> mark 'pidfd' as free in the fd table. You rather need to call close_fd()
> here to fully close open file.
Ah, I see, put_unused_fd() doesn't free up the file instance. I will swap
this out with close_fd() instead.
Thanks for the suggestions Jan!
/M
