From: Eric Paris <eparis@redhat.com>
To: linux-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org
Cc: agruen@suse.de, tvrtko.ursulin@sophos.com
Subject: [PATCH 08/20] fanotify: ignore fanotify ignore marks if open writers
Date: Thu, 28 Oct 2010 17:32:20 -0400 [thread overview]
Message-ID: <20101028213220.24810.72673.stgit@paris.rdu.redhat.com> (raw)
In-Reply-To: <20101028213139.24810.34058.stgit@paris.rdu.redhat.com>
fanotify will clear ignore marks if a task changes the contents of an
inode. The problem is with the races around when userspace finishes
checking a file and when that result is actually attached to the inode.
This race was described as such:
Consider the following scenario with hostile processes A and B, and
victim process C:
1. Process A opens new file for writing. File check request is generated.
2. File check is performed in userspace. Check result is "file has no malware".
3. The "permit" response is delivered to kernel space.
4. File ignored mark set.
5. Process A writes dummy bytes to the file. File ignored flags are cleared.
6. Process B opens the same file for reading. File check request is generated.
7. File check is performed in userspace. Check result is "file has no malware".
8. Process A writes malware bytes to the file. There is no cached response yet.
9. The "permit" response is delivered to kernel space and is cached in fanotify.
10. File ignored mark set.
11. Now any process C will be permitted to open the malware file.
There is a race between steps 8 and 10
While fanotify makes no strong guarantees about systems with hostile
processes there is no reason we cannot harden against this race. We do
that by simply ignoring any ignore marks if the inode has open writers (aka
i_writecount > 0). (We actually do not ignore ignore marks if the
FAN_MARK_SURV_MODIFY flag is set)
Reported-by: Vasily Novikov <vasily.novikov@kaspersky.com>
Signed-off-by: Eric Paris <eparis@redhat.com>
---
fs/notify/fanotify/fanotify_user.c | 10 ++++++++++
1 files changed, 10 insertions(+), 0 deletions(-)
diff --git a/fs/notify/fanotify/fanotify_user.c b/fs/notify/fanotify/fanotify_user.c
index 1c09e63..b265936 100644
--- a/fs/notify/fanotify/fanotify_user.c
+++ b/fs/notify/fanotify/fanotify_user.c
@@ -610,6 +610,16 @@ static int fanotify_add_inode_mark(struct fsnotify_group *group,
pr_debug("%s: group=%p inode=%p\n", __func__, group, inode);
+ /*
+ * If some other task has this inode open for write we should not add
+ * an ignored mark, unless that ignored mark is supposed to survive
+ * modification changes anyway.
+ */
+ if ((flags & FAN_MARK_IGNORED_MASK) &&
+ !(flags & FAN_MARK_IGNORED_SURV_MODIFY) &&
+ (atomic_read(&inode->i_writecount) > 0))
+ return 0;
+
fsn_mark = fsnotify_find_inode_mark(group, inode);
if (!fsn_mark) {
int ret;
next prev parent reply other threads:[~2010-10-28 21:32 UTC|newest]
Thread overview: 25+ messages / expand[flat|nested] mbox.gz Atom feed top
2010-10-28 21:31 [PATCH 01/20] fanotify: allow fanotify to be built Eric Paris
2010-10-28 21:31 ` [PATCH 02/20] fsnotify: implement ordering between notifiers Eric Paris
2010-10-28 21:31 ` [PATCH 03/20] fanotify: implement fanotify listener ordering Eric Paris
2010-10-29 15:01 ` John Stoffel
2010-10-28 21:31 ` [PATCH 04/20] fanotify: use __aligned_u64 in fanotify userspace metadata Eric Paris
2010-10-28 21:32 ` [PATCH 05/20] fsnotify: correctly handle return codes from listeners Eric Paris
2010-10-28 21:32 ` [PATCH 06/20] fsnotify: call fsnotify_parent in perm events Eric Paris
2010-10-28 21:32 ` [PATCH 07/20] fanotify: allow userspace to flush all marks Eric Paris
2010-10-28 21:32 ` Eric Paris [this message]
2010-10-28 21:32 ` [PATCH 09/20] fsnotify: implement a default maximum queue depth Eric Paris
2010-10-28 21:32 ` [PATCH 10/20] fanotify: allow userspace to override max " Eric Paris
2010-11-01 17:09 ` Tvrtko Ursulin
2010-11-01 17:23 ` Eric Paris
2010-11-01 17:34 ` Tvrtko Ursulin
2010-10-28 21:32 ` [PATCH 11/20] fanotify: limit the number of marks in a single fanotify group Eric Paris
2010-10-28 21:32 ` [PATCH 12/20] fanotify: allow userspace to override max marks Eric Paris
2010-11-01 17:16 ` Tvrtko Ursulin
2010-10-28 21:32 ` [PATCH 13/20] fanotify: limit number of listeners per user Eric Paris
2010-10-28 21:32 ` [PATCH 14/20] fanotify: do not send events for irregular files Eric Paris
2010-10-28 21:33 ` [PATCH 15/20] fsnotify: rename FS_IN_ISDIR to FS_ISDIR Eric Paris
2010-10-28 21:33 ` [PATCH 16/20] fanotify: ignore events on directories unless specifically requested Eric Paris
2010-10-28 21:33 ` [PATCH 17/20] fanotify: do not recalculate the mask if the ignored mask changed Eric Paris
2010-10-28 21:33 ` [PATCH 18/20] fanotify: Fix FAN_CLOSE comments Eric Paris
2010-10-28 21:33 ` [PATCH 19/20] fs/notify/fanotify/fanotify_user.c: fix warnings Eric Paris
2010-10-28 21:33 ` [PATCH 20/20] fsnotify: remove alignment padding from fsnotify_mark on 64 bit builds Eric Paris
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20101028213220.24810.72673.stgit@paris.rdu.redhat.com \
--to=eparis@redhat.com \
--cc=agruen@suse.de \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=tvrtko.ursulin@sophos.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).