Re: fanotify read returns with errno == EOPENSTALE

[Amir Goldstein <amir73il-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>]

On Thu, Apr 20, 2017 at 5:20 PM, Jan Kara <jack-AlSwsSmVLrQ@public.gmane.org> wrote:
> On Thu 20-04-17 14:33:04, Amir Goldstein wrote:
>>
>> Sorry I messed up the previous patch. please try this one:
>>
>> diff --git a/fs/notify/fanotify/fanotify_user.c
>> b/fs/notify/fanotify/fanotify_user.c
>> index 2b37f27..7864354 100644
>> --- a/fs/notify/fanotify/fanotify_user.c
>> +++ b/fs/notify/fanotify/fanotify_user.c
>> @@ -295,6 +295,16 @@ static ssize_t fanotify_read(struct file *file,
>> char __user *buf,
>>                 }
>>
>>                 ret = copy_event_to_user(group, kevent, buf);
>> +               if (unlikely(ret == -EOPENSTALE)) {
>> +                       /*
>> +                        * We cannot report events with stale fd so drop it.
>> +                        * Setting ret to 0 will continue the event loop and
>> +                        * do the right thing if there are no more events to
>> +                        * read (i.e. return bytes read, -EAGAIN or wait).
>> +                        */
>> +                       ret = 0;
>> +               }
>> +
>>                 /*
>>                  * Permission events get queued to wait for response.  Other
>>                  * events can be destroyed now.
>> @@ -305,7 +315,7 @@ static ssize_t fanotify_read(struct file *file,
>> char __user *buf,
>>                                 break;
>>                 } else {
>>  #ifdef CONFIG_FANOTIFY_ACCESS_PERMISSIONS
>> -                       if (ret < 0) {
>> +                       if (ret <= 0) {
>>                                 FANOTIFY_PE(kevent)->response = FAN_DENY;
>>                                 wake_up(&group->fanotify_data.access_waitq);
>>                                 break;
>
> I don't think you want to break out of the reading loop when ret == 0 and
> the code might be more readable as:
>
>                if (!(kevent->mask & FAN_ALL_PERM_EVENTS)) {
>                         fsnotify_destroy_event(group, kevent);
>                } else {
> #ifdef CONFIG_FANOTIFY_ACCESS_PERMISSIONS
>                         if (ret <= 0) {
>                                 FANOTIFY_PE(kevent)->response = FAN_DENY;
>                                 wake_up(&group->fanotify_data.access_waitq);
>                         } else {
>                                 spin_lock(&group->notification_lock);
>                                 list_add_tail(&kevent->list,
>                                               &group->fanotify_data.access_list);
>                                 spin_unlock(&group->notification_lock);
>                         }
> #endif
>                 }
>                 if (ret < 0)
>                         break;
>
> Hmm?
>

Right, I missed that.
Thanks.