From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751378AbbIJUth (ORCPT ); Thu, 10 Sep 2015 16:49:37 -0400 Received: from mail.linuxfoundation.org ([140.211.169.12]:36357 "EHLO mail.linuxfoundation.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750994AbbIJUtf (ORCPT ); Thu, 10 Sep 2015 16:49:35 -0400 Date: Thu, 10 Sep 2015 13:49:34 -0700 From: Andrew Morton To: Dave Hansen Cc: Eric Paris , john@johnmccutchan.com, rlove@rlove.org, linux-kernel@vger.kernel.org, stable@vger.kernel.org Subject: Re: [PATCH] inotify: actually check for invalid bits in sys_inotify_add_watch() Message-Id: <20150910134934.c0105fc353726db1e35d73d3@linux-foundation.org> In-Reply-To: <55F0C195.4000003@sr71.net> References: <20150630173603.D986EDB7@viggo.jf.intel.com> <55F0ABDB.7040509@sr71.net> <1441840602.27892.99.camel@redhat.com> <55F0C195.4000003@sr71.net> X-Mailer: Sylpheed 3.4.1 (GTK+ 2.24.23; x86_64-pc-linux-gnu) Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, 9 Sep 2015 16:32:37 -0700 Dave Hansen 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 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 Cc: John McCutchan Cc: Robert Love Cc: Eric Paris Signed-off-by: Andrew Morton --- 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; _