| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <CAE8kqZJYQoffVPgWCXLxcWY9a4ruz-n8vVBnrqryPKY+wcH29Q@mail.gmail.com>
Date: Tue, 6 Dec 2011 09:24:45 +0800
From: boyd yang <boyd.yang@...il.com>
To: Jan Kara <jack@...e.cz>
Cc: Josef Bacik <josef@...hat.com>, linux-fsdevel@...r.kernel.org,
linux-kernel@...r.kernel.org, eparis@...hat.com,
xiyou.wangcong@...il.com
Subject: Re: [PATCH] fanotify: to differ file access event from different threads
Thanks Jan Karan!
I once thought it was buried.
Now I have resent the patch.
On Thu, Nov 24, 2011 at 10:58 PM, Jan Kara <jack@...e.cz> wrote:
> On Thu 17-11-11 18:21:26, boyd yang wrote:
>> How can this patch get merged?
>> Who is responsible for this?
> Eric Paris is responsible for merging this patch. I see you have CCed him
> so that should be OK. Can you maybe resend the patch in a separate email
> again? I see your last submission was included in a reply to email together
> with other text etc. which makes merging it using our standard tools
> harder...
>
> Honza
>
>> We are developing some real-time antivirus software, the fanotify can make
>> it work without dirver/kenel-modues( like RedirFS).
>> But the bug costs a lot trouble.
>> If we used the patch, our software works great!
>>
>> I think other antivirus or file-motoring softwares are influnced by the bug
>> too.
>>
>>
>> On Fri, Oct 14, 2011 at 2:56 PM, boyd yang <boyd.yang@...il.com> wrote:
>> >
>> > Thanks, I updated the patch.
>> > Now it passed checkpatch.pl.
>> >
>> > I used the flag you mentioned.
>> >
>> > fanotify: to differ file access event from different threads
>> > When fanotify is monitoring the whole mount point "/", and multiple
>> > threads iterate the same direcotry, some thread will hang.
>> > This patch let fanotify to differ access events from different
>> > threads, prevent fanotify from merging access events from different
>> > threads.
>> > It also hide overflow events to reach user space.
>> > Signed-off-by: Boyd Yang <boyd.yang@...il.com>
>> >
>> > diff -r -u linux-3.1-rc4_orig/fs/notify/fanotify/fanotify.c
>> > linux-3.1-rc4/fs/notify/fanotify/fanotify.c
>> > --- linux-3.1-rc4_orig/fs/notify/fanotify/fanotify.c 2011-08-29
>> > 12:16:01.000000000 +0800
>> > +++ linux-3.1-rc4/fs/notify/fanotify/fanotify.c 2011-10-14
>> > 14:17:53.055958000 +0800
>> > @@ -15,7 +15,8 @@
>> >
>> > if (old->to_tell == new->to_tell &&
>> > old->data_type == new->data_type &&
>> > - old->tgid == new->tgid) {
>> > + old->tgid == new->tgid &&
>> > + old->pid == new->pid) {
>> > switch (old->data_type) {
>> > case (FSNOTIFY_EVENT_PATH):
>> > if ((old->path.mnt == new->path.mnt) &&
>> > @@ -144,11 +145,16 @@
>> > return PTR_ERR(notify_event);
>> >
>> > #ifdef CONFIG_FANOTIFY_ACCESS_PERMISSIONS
>> > - if (event->mask & FAN_ALL_PERM_EVENTS) {
>> > - /* if we merged we need to wait on the new event */
>> > - if (notify_event)
>> > - event = notify_event;
>> > - ret = fanotify_get_response_from_access(group, event);
>> > + /*if overflow, do not wait for response*/
>> > + if (event->mask&FS_Q_OVERFLOW) {
>> > + pr_debug("fanotify overflow!\n");
>> > + } else {
>> > + if (event->mask & FAN_ALL_PERM_EVENTS) {
>> > + /* if we merged we need to wait on the new event */
>> > + if (notify_event)
>> > + event = notify_event;
>> > + ret = fanotify_get_response_from_access(group, event);
>> > + }
>> > }
>> > #endif
>> >
>> > diff -r -u linux-3.1-rc4_orig/fs/notify/notification.c
>> > linux-3.1-rc4/fs/notify/notification.c
>> > --- linux-3.1-rc4_orig/fs/notify/notification.c 2011-08-29
>> > 12:16:01.000000000 +0800
>> > +++ linux-3.1-rc4/fs/notify/notification.c 2011-10-14 13:52:36.946608000 +0800
>> > @@ -95,6 +95,7 @@
>> > BUG_ON(!list_empty(&event->private_data_list));
>> >
>> > kfree(event->file_name);
>> > + put_pid(event->pid);
>> > put_pid(event->tgid);
>> > kmem_cache_free(fsnotify_event_cachep, event);
>> > }
>> > @@ -374,6 +375,7 @@
>> > return NULL;
>> > }
>> > }
>> > + event->pid = get_pid(old_event->pid);
>> > event->tgid = get_pid(old_event->tgid);
>> > if (event->data_type == FSNOTIFY_EVENT_PATH)
>> > path_get(&event->path);
>> > @@ -417,6 +419,7 @@
>> > event->name_len = strlen(event->file_name);
>> > }
>> >
>> > + event->pid = get_pid(task_pid(current));
>> > event->tgid = get_pid(task_tgid(current));
>> > event->sync_cookie = cookie;
>> > event->to_tell = to_tell;
>> > diff -r -u linux-3.1-rc4_orig/include/linux/fsnotify_backend.h
>> > linux-3.1-rc4/include/linux/fsnotify_backend.h
>> > --- linux-3.1-rc4_orig/include/linux/fsnotify_backend.h 2011-08-29
>> > 12:16:01.000000000 +0800
>> > +++ linux-3.1-rc4/include/linux/fsnotify_backend.h 2011-10-14
>> > 13:51:50.380168000 +0800
>> > @@ -238,6 +238,7 @@
>> > u32 sync_cookie; /* used to corrolate events, namely inotify mv events */
>> > const unsigned char *file_name;
>> > size_t name_len;
>> > + struct pid *pid;
>> > struct pid *tgid;
>> >
>> > #ifdef CONFIG_FANOTIFY_ACCESS_PERMISSIONS
>> >
>> >
>> >
>> >
>> >
>> > On Thu, Oct 13, 2011 at 9:38 PM, Josef Bacik <josef@...hat.com> wrote:
>> > > On Thu, Oct 13, 2011 at 04:56:43PM +0800, boyd yang wrote:
>> > >> This patch fixes a hang problem of Eric Paris's fs Notification/fanotify.
>> > >>
>> > >> Fanotify brings a way to intercept file access events.
>> > >> When multiple threadsiterate the same direcotry, some thread will hang.
>> > >> This patch let fanotify to differ access events from different
>> > >> threads, prevent fanotify from merging access events from different
>> > >> threads.
>> > >>
>> > >
>> > > You need to run this through checkpatch.pl, you have a ton of formatting
>> > > problems. Also your email client seems to have word-wrapped parts of this, so
>> > > use a email client that doesn't word wrap.
>> > >
>> > >> =============================================================
>> > >>
>> > >> diff -r -u linux-3.1-rc4_orig/fs/notify/fanotify/fanotify.c
>> > >> linux-3.1-rc4/fs/notify/fanotify/fanotify.c
>> > >> --- linux-3.1-rc4_orig/fs/notify/fanotify/fanotify.c 2011-08-29
>> > >> 12:16:01.000000000 +0800
>> > >> +++ linux-3.1-rc4/fs/notify/fanotify/fanotify.c 2011-10-10
>> > >> 12:28:23.276847000 +0800
>> > >> @@ -15,7 +15,8 @@
>> > >>
>> > >> if (old->to_tell == new->to_tell &&
>> > >> old->data_type == new->data_type &&
>> > >> - old->tgid == new->tgid) {
>> > >> + old->tgid == new->tgid &&
>> > >> + old->pid == new->pid) {
>> > >> switch (old->data_type) {
>> > >> case (FSNOTIFY_EVENT_PATH):
>> > >> if ((old->path.mnt == new->path.mnt) &&
>> > >> @@ -144,11 +145,19 @@
>> > >> return PTR_ERR(notify_event);
>> > >>
>> > >> #ifdef CONFIG_FANOTIFY_ACCESS_PERMISSIONS
>> > >> - if (event->mask & FAN_ALL_PERM_EVENTS) {
>> > >> - /* if we merged we need to wait on the new event */
>> > >> - if (notify_event)
>> > >> - event = notify_event;
>> > >> - ret = fanotify_get_response_from_access(group, event);
>> > >> + //if overflow, do not wait for response
>> > >> + if(fsnotify_isoverflow(event))
>> > >> + {
>> > >> + pr_debug("fanotify overflow!\n");
>> > >> + }
>> > >> + else
>> > >> + {
>> > >> + if (event->mask & FAN_ALL_PERM_EVENTS) {
>> > >> + /* if we merged we need to wait on the new event */
>> > >> + if (notify_event)
>> > >> + event = notify_event;
>> > >> + ret = fanotify_get_response_from_access(group, event);
>> > >> + }
>> > >> }
>> > >
>> > > The overflow event should only have FS_Q_OVERFLOW set in it's mask right? So
>> > > why is this test needed at all? Thanks,
>> > >
>> > > Josef
>> > >
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
>> the body of a message to majordomo@...r.kernel.org
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
> --
> Jan Kara <jack@...e.cz>
> SUSE Labs, CR
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists