mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Greg KH <gregkh@suse.de>
To: linux-kernel@vger.kernel.org, stable@kernel.org
Cc: stable-review@kernel.org, torvalds@linux-foundation.org,
	akpm@linux-foundation.org, alan@lxorguk.ukuu.org.uk,
	Eric Paris <eparis@redhat.com>
Subject: [01/30] inotify: do not reuse watch descriptors
Date: Wed, 20 Jan 2010 20:15:22 -0800	[thread overview]
Message-ID: <20100121041832.593597059@mini.kroah.org> (raw)
In-Reply-To: <20100121041852.GA9656@kroah.com>

2.6.32-stable review patch.  If anyone has any objections, please let us know.

------------------

From: Eric Paris <eparis@redhat.com>

commit 9e572cc9877ee6c43af60778f6b8d5ba0692d935 upstream.

Since commit 7e790dd5fc937bc8d2400c30a05e32a9e9eef276 ("inotify: fix
error paths in inotify_update_watch") inotify changed the manor in which
it gave watch descriptors back to userspace.  Previous to this commit
inotify acted like the following:

  inotify_add_watch(X, Y, Z) = 1
  inotify_rm_watch(X, 1);
  inotify_add_watch(X, Y, Z) = 2

but after this patch inotify would return watch descriptors like so:

  inotify_add_watch(X, Y, Z) = 1
  inotify_rm_watch(X, 1);
  inotify_add_watch(X, Y, Z) = 1

which I saw as equivalent to opening an fd where

  open(file) = 1;
  close(1);
  open(file) = 1;

seemed perfectly reasonable.  The issue is that quite a bit of userspace
apparently relies on the behavior in which watch descriptors will not be
quickly reused.  KDE relies on it, I know some selinux packages rely on
it, and I have heard complaints from other random sources such as debian
bug 558981.

Although the man page implies what we do is ok, we broke userspace so
this patch almost reverts us to the old behavior.  It is still slightly
racey and I have patches that would fix that, but they are rather large
and this will fix it for all real world cases.  The race is as follows:

 - task1 creates a watch and blocks in idr_new_watch() before it updates
   the hint.
 - task2 creates a watch and updates the hint.
 - task1 updates the hint with it's older wd
 - task removes the watch created by task2
 - task adds a new watch and will reuse the wd originally given to task2

it requires moving some locking around the hint (last_wd) but this should
solve it for the real world and be -stable safe.

As a side effect this patch papers over a bug in the lib/idr code which
is causing a large number WARN's to pop on people's system and many
reports in kerneloops.org.  I'm working on the root cause of that idr
bug seperately but this should make inotify immune to that issue.

Signed-off-by: Eric Paris <eparis@redhat.com>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>

---
 fs/notify/inotify/inotify_user.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

--- a/fs/notify/inotify/inotify_user.c
+++ b/fs/notify/inotify/inotify_user.c
@@ -558,7 +558,7 @@ retry:
 
 	spin_lock(&group->inotify_data.idr_lock);
 	ret = idr_get_new_above(&group->inotify_data.idr, &tmp_ientry->fsn_entry,
-				group->inotify_data.last_wd,
+				group->inotify_data.last_wd+1,
 				&tmp_ientry->wd);
 	spin_unlock(&group->inotify_data.idr_lock);
 	if (ret) {
@@ -638,7 +638,7 @@ static struct fsnotify_group *inotify_ne
 
 	spin_lock_init(&group->inotify_data.idr_lock);
 	idr_init(&group->inotify_data.idr);
-	group->inotify_data.last_wd = 1;
+	group->inotify_data.last_wd = 0;
 	group->inotify_data.user = user;
 	group->inotify_data.fa = NULL;
 



  reply	other threads:[~2010-01-21  4:23 UTC|newest]

Thread overview: 31+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-01-21  4:18 [00/30] 2.6.32.5 review Greg KH
2010-01-21  4:15 ` Greg KH [this message]
2010-01-21  4:15 ` [02/30] inotify: only warn once for inotify problems Greg KH
2010-01-21  4:15 ` [03/30] revert "drivers/video/s3c-fb.c: fix clock setting for Samsung SoC Framebuffer" Greg KH
2010-01-21  4:15 ` [04/30] memcg: ensure list is empty at rmdir Greg KH
2010-01-21  4:15 ` [05/30] drm/i915: remove loop in Ironlake interrupt handler Greg KH
2010-01-21  4:15 ` [06/30] block: Fix incorrect reporting of partition alignment Greg KH
2010-01-21  4:15 ` [07/30] x86, mce: Thermal monitoring depends on APIC being enabled Greg KH
2010-01-21  4:15 ` [08/30] futexes: Remove rw parameter from get_futex_key() Greg KH
2010-01-21  4:15 ` [09/30] page allocator: update NR_FREE_PAGES only when necessary Greg KH
2010-01-21  4:15 ` [10/30] x86, apic: use physical mode for IBM summit platforms Greg KH
2010-01-21  4:15 ` [11/30] edac: i5000_edac critical fix panic out of bounds Greg KH
2010-01-21  4:15 ` [12/30] x86: SGI UV: Fix mapping of MMIO registers Greg KH
2010-01-21  4:15 ` [13/30] mfd: WM835x GPIO direction register is not locked Greg KH
2010-01-21  4:15 ` [14/30] mfd: Correct WM835x ISINK ramp time defines Greg KH
2010-01-21  4:15 ` [15/30] ALSA: hda - Fix missing capture mixer for ALC861/660 codecs Greg KH
2010-01-21  4:15 ` [16/30] V4L/DVB (13868): gspca - sn9c20x: Fix test of unsigned Greg KH
2010-01-21  4:15 ` [17/30] reiserfs: truncate blocks not used by a write Greg KH
2010-01-21  4:15 ` [18/30] HID: add device IDs for new model of Apple Wireless Keyboard Greg KH
2010-01-21  4:15 ` [19/30] PCI/cardbus: Add a fixup hook and fix powerpc Greg KH
2010-01-21  4:15 ` [20/30] [SCSI] megaraid_sas: remove sysfs poll_mode_io world writeable permissions Greg KH
2010-01-21  4:15 ` [21/30] Input: pmouse - move Sentelic probe down the list Greg KH
2010-01-21  4:15 ` [22/30] asus-laptop: add Lenovo SL hotkey support Greg KH
2010-01-21  4:15 ` [23/30] sched: Fix cpu_clock() in NMIs, on !CONFIG_HAVE_UNSTABLE_SCHED_CLOCK Greg KH
2010-01-21  4:15 ` [24/30] sparc64: Fix NMI programming when perf events are active Greg KH
2010-01-21  4:15 ` [25/30] sparc64: Fix Niagara2 perf event handling Greg KH
2010-01-21  4:15 ` [26/30] i2c: Do not use device name after device_unregister Greg KH
2010-01-21  4:15 ` [27/30] i2c/pca: Dont use *_interruptible Greg KH
2010-01-21  4:15 ` [28/30] serial/8250_pnp: add a new Fujitsu Wacom Tablet PC device Greg KH
2010-01-21  4:15 ` [29/30] sched: Fix task priority bug Greg KH
2010-01-21  4:15 ` [30/30] vfs: Fix vmtruncate() regression Greg KH

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20100121041832.593597059@mini.kroah.org \
    --to=gregkh@suse.de \
    --cc=akpm@linux-foundation.org \
    --cc=alan@lxorguk.ukuu.org.uk \
    --cc=eparis@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=stable-review@kernel.org \
    --cc=stable@kernel.org \
    --cc=torvalds@linux-foundation.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

Powered by JetHome