On Thu, Apr 20, 2017 at 5:20 PM, Jan Kara <jack@xxxxxxx> 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.
