linux-kernel - [PATCH] inotify: Remove broken mask checks causing unmount to be EINVAL

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 for Android: free password hash cracker in your pocket

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

Message-Id: <1359144088-23869-1-git-send-email-paul.gortmaker@windriver.com>
Date:	Fri, 25 Jan 2013 15:01:28 -0500
From:	Paul Gortmaker <paul.gortmaker@...driver.com>
To:	linux-kernel@...r.kernel.org
Cc:	Jim Somerville <Jim.Somerville@...driver.com>,
	Jerome Marchand <jmarchan@...hat.com>,
	John McCutchan <john@...nmccutchan.com>,
	Robert Love <rlove@...ve.org>,
	Eric Paris <eparis@...isplace.org>,
	stable <stable@...r.kernel.org>,
	Paul Gortmaker <paul.gortmaker@...driver.com>
Subject: [PATCH] inotify: Remove broken mask checks causing unmount to be EINVAL

From: Jim Somerville <Jim.Somerville@...driver.com>

Running the command:

	inotifywait -e unmount /mnt/disk

immediately aborts with a -EINVAL return code.  This is however
a valid parameter.  This abort occurs only if unmount is the
sole event parameter.  If other event parameters are supplied,
then the unmount event wait will work.

The problem was introduced by commit 44b350fc23e36e95c8e042b7ded
("inotify: Fix mask checks").  In that commit, it states:

	The mask checks in inotify_update_existing_watch() and
	inotify_new_watch() are useless because inotify_arg_to_mask()
	sets FS_IN_IGNORED and FS_EVENT_ON_CHILD bits anyway.

But instead of removing the useless checks, it did this:

	        mask = inotify_arg_to_mask(arg);
	-       if (unlikely(!mask))
	+       if (unlikely(!(mask & IN_ALL_EVENTS)))
	                return -EINVAL;

The problem is that IN_ALL_EVENTS doesn't include IN_UNMOUNT, and
other parts of the code keep IN_UNMOUNT separate from IN_ALL_EVENTS.
So the check should be:

	if (unlikely(!(mask & (IN_ALL_EVENTS | IN_UNMOUNT))))

But inotify_arg_to_mask(arg) always sets the IN_UNMOUNT bit in the mask
anyway, so the check is always going to pass and thus should simply
be removed.  Also note that inotify_arg_to_mask completely controls
what mask bits get set from arg, there's no way for invalid bits to
get enabled there.

Lets fix it by simply removing the useless broken checks.

Cc: Jerome Marchand <jmarchan@...hat.com>
Cc: John McCutchan <john@...nmccutchan.com>
Cc: Robert Love <rlove@...ve.org>
Cc: Eric Paris <eparis@...isplace.org>
Cc: stable <stable@...r.kernel.org>  # 2.6.37+
Signed-off-by: Jim Somerville <Jim.Somerville@...driver.com>
Signed-off-by: Paul Gortmaker <paul.gortmaker@...driver.com>
---
 fs/notify/inotify/inotify_user.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/fs/notify/inotify/inotify_user.c b/fs/notify/inotify/inotify_user.c
index 228a2c2..07f7a92 100644
--- a/fs/notify/inotify/inotify_user.c
+++ b/fs/notify/inotify/inotify_user.c
@@ -576,8 +576,6 @@ static int inotify_update_existing_watch(struct fsnotify_group *group,
 
 	/* don't allow invalid bits: we don't want flags set */
 	mask = inotify_arg_to_mask(arg);
-	if (unlikely(!(mask & IN_ALL_EVENTS)))
-		return -EINVAL;
 
 	fsn_mark = fsnotify_find_inode_mark(group, inode);
 	if (!fsn_mark)
@@ -629,8 +627,6 @@ static int inotify_new_watch(struct fsnotify_group *group,
 
 	/* don't allow invalid bits: we don't want flags set */
 	mask = inotify_arg_to_mask(arg);
-	if (unlikely(!(mask & IN_ALL_EVENTS)))
-		return -EINVAL;
 
 	tmp_i_mark = kmem_cache_alloc(inotify_inode_mark_cachep, GFP_KERNEL);
 	if (unlikely(!tmp_i_mark))
-- 
1.8.1

--
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/