linux-kernel - Re: [PATCH] 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 for Android: free password hash cracker in your pocket

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

Message-Id: <20150910134934.c0105fc353726db1e35d73d3@linux-foundation.org>
Date:	Thu, 10 Sep 2015 13:49:34 -0700
From:	Andrew Morton <akpm@...ux-foundation.org>
To:	Dave Hansen <dave@...1.net>
Cc:	Eric Paris <eparis@...hat.com>, john@...nmccutchan.com,
	rlove@...ve.org, linux-kernel@...r.kernel.org,
	stable@...r.kernel.org
Subject: Re: [PATCH] inotify: actually check for invalid bits in
 sys_inotify_add_watch()

On Wed, 9 Sep 2015 16:32:37 -0700 Dave Hansen <dave@...1.net> wrote:

> On 09/09/2015 04:16 PM, Eric Paris wrote:
> > Looks fine to me. And usually akpm picks them up these days.
> 
> Is that an Acked-by? :)

I grabbed it for 4.3.

I removed your cc:stable.  I don't see anything in here which warrants
a backport.  If there *is* a reason for backporting then your
changelogological skills are sorely wanting!

The changelog is pretty sucky really.  What are the reasons for this
change, apart from "do what the comment said"?  What's the benefit?

And the code comment sucks.  "don't allow invalid bits": well duh.  And
"we don't want flags set" is also useless: it doesn't explain *why* we
don't want those flags set.

And given that there is potential to break existing userspace, we need
some decent reasons for making this change.

From: Dave Hansen <dave.hansen@...ux.intel.com>
Subject: inotify: actually check for invalid bits in sys_inotify_add_watch()

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.

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>
Cc: Robert Love <rlove@...ve.org>
Cc: Eric Paris <eparis@...isplace.org>
Signed-off-by: Andrew Morton <akpm@...ux-foundation.org>
---

 fs/notify/inotify/inotify_user.c |    3 +++
 1 file changed, 3 insertions(+)

diff -puN fs/notify/inotify/inotify_user.c~inotify-actually-check-for-invalid-bits-in-sys_inotify_add_watch fs/notify/inotify/inotify_user.c
--- a/fs/notify/inotify/inotify_user.c~inotify-actually-check-for-invalid-bits-in-sys_inotify_add_watch
+++ a/fs/notify/inotify/inotify_user.c
@@ -707,6 +707,9 @@ SYSCALL_DEFINE3(inotify_add_watch, int,
 	unsigned flags = 0;

 	/* don't allow invalid bits: we don't want flags set */
+	if (unlikely(mask & ~ALL_INOTIFY_BITS))
+		return -EINVAL;
+	/* require at least one valid bit set in the mask */
 	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/