linux-kernel - [PATCH] [v2] inotify: actually check for invalid bits in sys_inotify_add

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [day] [month] [year] [list]

Message-Id: <20150910214914.50512584@viggo.jf.intel.com>
Date:	Thu, 10 Sep 2015 14:49:14 -0700
From:	Dave Hansen <dave@...1.net>
To:	dave@...1.net
Cc:	dave.hansen@...ux.intel.com, john@...nmccutchan.com,
	rlove@...ve.org, eparis@...isplace.org,
	linux-kernel@...r.kernel.org, akpm@...ux-foundation.org
Subject: [PATCH] [v2] inotify: actually check for invalid bits in sys_inotify_add_watch()


Changes from V1:
 * beef up changelog and comment text.

---

From: Dave Hansen <dave.hansen@...ux.intel.com>

The comment here says that it is checking for invalid bits.  But,
the mask is *actually* checking to ensure that _any_ valid bit
is set, which is quite different.

Without this check, an unexpected bit could get set on an inotify
object.  Since these bits are also interpreted by the
fsnotify/dnotify code, there is the potential for an object to be
mishandled inside the kernel.  For instance, can we be sure that
setting the dnotify flag FS_DN_RENAME on an inotify watch is
harmless?

Add the actual check which was intended.  Retain the existing
check because it actually does something useful: ensure that some
inotify bits are being added to the watch.  Plus, this is
existing behavior which would be nice to preserve.

I did a quick sniff test that inotify functions and that my
'inotify-tools' package passes 'make check'.

Signed-off-by: Dave Hansen <dave.hansen@...ux.intel.com>
Cc: John McCutchan <john@...nmccutchan.com> (maintainer:INOTIFY)
Cc: Robert Love <rlove@...ve.org> (maintainer:INOTIFY)
Cc: Eric Paris <eparis@...isplace.org> (maintainer:INOTIFY)
Cc: linux-kernel@...r.kernel.org (open list)
Cc: akpm@...ux-foundation.org
---

 b/fs/notify/inotify/inotify_user.c |   14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff -puN fs/notify/inotify/inotify_user.c~inotify-EINVAL-on-invalid-bit fs/notify/inotify/inotify_user.c
--- a/fs/notify/inotify/inotify_user.c~inotify-EINVAL-on-invalid-bit	2015-09-10 14:38:43.478932202 -0700
+++ b/fs/notify/inotify/inotify_user.c	2015-09-10 14:38:43.490932756 -0700
@@ -706,7 +706,19 @@ SYSCALL_DEFINE3(inotify_add_watch, int,
 	int ret;
 	unsigned flags = 0;
 
-	/* don't allow invalid bits: we don't want flags set */
+	/*
+	 * We share a lot of code with fs/dnotify.  We also share
+	 * the bit layout between inotify's IN_* and the fsnotify
+	 * FS_*.  This check ensures that only the inotify IN_*
+	 * bits get passed in and set in watches/events.
+	 */
+	if (unlikely(mask & ~ALL_INOTIFY_BITS))
+		return -EINVAL;
+	/*
+	 * Require at least one valid bit set in the mask.
+	 * Without _something_ set, we would have no events to
+	 * watch for.
+	 */
 	if (unlikely(!(mask & ALL_INOTIFY_BITS)))
 		return -EINVAL;
 
_
--
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/