Content-Transfer-Encoding: quoted-printable
From: Tvrtko Ursulin <tvrtko.ursulin@sophos.com>
To: Eric Paris <eparis@redhat.com>
CC: "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
 "linux-fsdevel@vger.kernel.org" <linux-fsdevel@vger.kernel.org>
Sender: "linux-kernel-owner@vger.kernel.org"
	<linux-kernel-owner@vger.kernel.org>
Date: Wed, 8 Sep 2010 09:24:04 +0100
Subject: Re: [BUG][PATCH][2.6.36-rc3] fanotify: Do not ignore result of
 permission decisions
Thread-Topic: [BUG][PATCH][2.6.36-rc3] fanotify: Do not ignore result of
 permission decisions
Thread-Index: ActPL1Ie8fAPdgdfSdCm31W+fVOnkA==
Message-ID: <201009080924.04841.tvrtko.ursulin@sophos.com>
References: <201009071608.02290.tvrtko.ursulin@sophos.com>
In-Reply-To: <201009071608.02290.tvrtko.ursulin@sophos.com>
Accept-Language: en-GB, en-US
user-agent: KMail/1.13.5 (Linux/2.6.34-12-desktop; KDE/4.4.4; x86_64; ; )
MIME-Version: 1.0
Content-Type: text/plain;
  charset="us-ascii"
Content-Language: en-US


Improved version of the fix which does not include the check
when permission events are not enabled in configuration and
stops processing if no interesting events remain.

Current code ignores access replies to permission decisions so
fix it in a way which will allow all listeners to still receive
non permission events.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@sophos.com>
---
 fs/notify/fsnotify.c             |   32 ++++++++++++++++++++++++--------
 include/linux/fsnotify_backend.h |    4 ++++
 2 files changed, 28 insertions(+), 8 deletions(-)

diff --git a/fs/notify/fsnotify.c b/fs/notify/fsnotify.c
index 3680242..2350a53 100644
--- a/fs/notify/fsnotify.c
+++ b/fs/notify/fsnotify.c
@@ -224,7 +224,7 @@ int fsnotify(struct inode *to_tell, __u32 mask, void *d=
ata, int data_is,
        struct fsnotify_group *inode_group, *vfsmount_group;
        struct fsnotify_event *event =3D NULL;
        struct vfsmount *mnt;
-       int idx, ret =3D 0;
+       int idx, ret =3D 0, res;
        /* global tests shouldn't care about events on child only the speci=
fic event */
        __u32 test_mask =3D (mask & ~FS_EVENT_ON_CHILD);

@@ -275,20 +275,36 @@ int fsnotify(struct inode *to_tell, __u32 mask, void =
*data, int data_is,

                if (inode_group > vfsmount_group) {
                        /* handle inode */
-                       send_to_group(to_tell, NULL, inode_mark, NULL, mask=
, data,
-                                     data_is, cookie, file_name, &event);
+                       res =3D send_to_group(to_tell, NULL, inode_mark, NU=
LL, mask, data,
+                                           data_is, cookie, file_name, &ev=
ent);
                        /* we didn't use the vfsmount_mark */
                        vfsmount_group =3D NULL;
                } else if (vfsmount_group > inode_group) {
-                       send_to_group(to_tell, mnt, NULL, vfsmount_mark, ma=
sk, data,
-                                     data_is, cookie, file_name, &event);
+                       res =3D send_to_group(to_tell, mnt, NULL, vfsmount_=
mark, mask, data,
+                                           data_is, cookie, file_name, &ev=
ent);
                        inode_group =3D NULL;
                } else {
-                       send_to_group(to_tell, mnt, inode_mark, vfsmount_ma=
rk,
-                                     mask, data, data_is, cookie, file_nam=
e,
-                                     &event);
+                       res =3D send_to_group(to_tell, mnt, inode_mark, vfs=
mount_mark,
+                                           mask, data, data_is, cookie, fi=
le_name,
+                                           &event);
                }

+#ifdef CONFIG_FANOTIFY_ACCESS_PERMISSIONS
+               /* If a listener denied on a permission event we will remem=
ber the reason
+                  and run the rest with a non-permission mask only. This a=
llows other
+                  listeners to receive non-permission notifications but we=
 do not care
+                  about further permission checks and want to deny this ev=
ent. */
+               if (unlikely((res =3D=3D -EPERM) && (mask & FS_ALL_PERM_EVE=
NTS))) {
+                       mask &=3D ~FS_ALL_PERM_EVENTS;
+                       ret =3D res;
+                       /* Stop processing if no interesting events remains=
. */
+                       if (!(mask & (ALL_FSNOTIFY_EVENTS & ~FS_EVENT_ON_CH=
ILD)))
+                               break;
+               }
+#else
+               (void)res;
+#endif
+
                if (inode_group)
                        inode_node =3D srcu_dereference(inode_node->next,
                                                      &fsnotify_mark_srcu);
diff --git a/include/linux/fsnotify_backend.h b/include/linux/fsnotify_back=
end.h
index e40190d..c8aea03 100644
--- a/include/linux/fsnotify_backend.h
+++ b/include/linux/fsnotify_backend.h
@@ -64,6 +64,10 @@

 #define FS_MOVE                        (FS_MOVED_FROM | FS_MOVED_TO)

+/* All events which require a permission response from userspace */
+#define FS_ALL_PERM_EVENTS (FS_OPEN_PERM |\
+                           FS_ACCESS_PERM)
+
 #define ALL_FSNOTIFY_EVENTS (FS_ACCESS | FS_MODIFY | FS_ATTRIB | \
                             FS_CLOSE_WRITE | FS_CLOSE_NOWRITE | FS_OPEN | =
\
                             FS_MOVED_FROM | FS_MOVED_TO | FS_CREATE | \

Sophos Plc, The Pentagon, Abingdon Science Park, Abingdon, OX14 3YP, United=
 Kingdom.
Company Reg No 2096520. VAT Reg No GB 348 3873 20.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/