Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1242881

[PATCH 3.2 069/107] hfs,hfsplus: cache pages correctly between bnode_create and bnode_free

From Ben Hutchings <ben@decadent.org.uk>
Newsgroups linux.kernel
Subject [PATCH 3.2 069/107] hfs,hfsplus: cache pages correctly between bnode_create and bnode_free
Date 2015-10-09 02:30 +0200
Message-ID <qhtJq-2EJ-61@gated-at.bofh.it> (permalink)
References <qhtzH-2tg-3@gated-at.bofh.it>
Organization linux.* mail to news gateway

Show all headers | View raw


3.2.72-rc1 review patch.  If anyone has any objections, please let me know.

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

From: Hin-Tak Leung <htl10@users.sourceforge.net>

commit 7cb74be6fd827e314f81df3c5889b87e4c87c569 upstream.

Pages looked up by __hfs_bnode_create() (called by hfs_bnode_create() and
hfs_bnode_find() for finding or creating pages corresponding to an inode)
are immediately kmap()'ed and used (both read and write) and kunmap()'ed,
and should not be page_cache_release()'ed until hfs_bnode_free().

This patch fixes a problem I first saw in July 2012: merely running "du"
on a large hfsplus-mounted directory a few times on a reasonably loaded
system would get the hfsplus driver all confused and complaining about
B-tree inconsistencies, and generates a "BUG: Bad page state".  Most
recently, I can generate this problem on up-to-date Fedora 22 with shipped
kernel 4.0.5, by running "du /" (="/" + "/home" + "/mnt" + other smaller
mounts) and "du /mnt" simultaneously on two windows, where /mnt is a
lightly-used QEMU VM image of the full Mac OS X 10.9:

$ df -i / /home /mnt
Filesystem                  Inodes   IUsed      IFree IUse% Mounted on
/dev/mapper/fedora-root    3276800  551665    2725135   17% /
/dev/mapper/fedora-home   52879360  716221   52163139    2% /home
/dev/nbd0p2             4294967295 1387818 4293579477    1% /mnt

After applying the patch, I was able to run "du /" (60+ times) and "du
/mnt" (150+ times) continuously and simultaneously for 6+ hours.

There are many reports of the hfsplus driver getting confused under load
and generating "BUG: Bad page state" or other similar issues over the
years.  [1]

The unpatched code [2] has always been wrong since it entered the kernel
tree.  The only reason why it gets away with it is that the
kmap/memcpy/kunmap follow very quickly after the page_cache_release() so
the kernel has not had a chance to reuse the memory for something else,
most of the time.

The current RW driver appears to have followed the design and development
of the earlier read-only hfsplus driver [3], where-by version 0.1 (Dec
2001) had a B-tree node-centric approach to
read_cache_page()/page_cache_release() per bnode_get()/bnode_put(),
migrating towards version 0.2 (June 2002) of caching and releasing pages
per inode extents.  When the current RW code first entered the kernel [2]
in 2005, there was an REF_PAGES conditional (and "//" commented out code)
to switch between B-node centric paging to inode-centric paging.  There
was a mistake with the direction of one of the REF_PAGES conditionals in
__hfs_bnode_create().  In a subsequent "remove debug code" commit [4], the
read_cache_page()/page_cache_release() per bnode_get()/bnode_put() were
removed, but a page_cache_release() was mistakenly left in (propagating
the "REF_PAGES <-> !REF_PAGE" mistake), and the commented-out
page_cache_release() in bnode_release() (which should be spanned by
!REF_PAGES) was never enabled.

References:
[1]:
Michael Fox, Apr 2013
http://www.spinics.net/lists/linux-fsdevel/msg63807.html
("hfsplus volume suddenly inaccessable after 'hfs: recoff %d too large'")

Sasha Levin, Feb 2015
http://lkml.org/lkml/2015/2/20/85 ("use after free")

https://bugs.launchpad.net/ubuntu/+source/linux/+bug/740814
https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1027887
https://bugzilla.kernel.org/show_bug.cgi?id=42342
https://bugzilla.kernel.org/show_bug.cgi?id=63841
https://bugzilla.kernel.org/show_bug.cgi?id=78761

[2]:
http://git.kernel.org/cgit/linux/kernel/git/tglx/history.git/commit/\
fs/hfs/bnode.c?id=d1081202f1d0ee35ab0beb490da4b65d4bc763db
commit d1081202f1d0ee35ab0beb490da4b65d4bc763db
Author: Andrew Morton <akpm@osdl.org>
Date:   Wed Feb 25 16:17:36 2004 -0800

    [PATCH] HFS rewrite

http://git.kernel.org/cgit/linux/kernel/git/tglx/history.git/commit/\
fs/hfsplus/bnode.c?id=91556682e0bf004d98a529bf829d339abb98bbbd

commit 91556682e0bf004d98a529bf829d339abb98bbbd
Author: Andrew Morton <akpm@osdl.org>
Date:   Wed Feb 25 16:17:48 2004 -0800

    [PATCH] HFS+ support

[3]:
http://sourceforge.net/projects/linux-hfsplus/

http://sourceforge.net/projects/linux-hfsplus/files/Linux%202.4.x%20patch/hfsplus%200.1/
http://sourceforge.net/projects/linux-hfsplus/files/Linux%202.4.x%20patch/hfsplus%200.2/

http://linux-hfsplus.cvs.sourceforge.net/viewvc/linux-hfsplus/linux/\
fs/hfsplus/bnode.c?r1=1.4&r2=1.5

Date:   Thu Jun 6 09:45:14 2002 +0000
Use buffer cache instead of page cache in bnode.c. Cache inode extents.

[4]:
http://git.kernel.org/cgit/linux/kernel/git/\
stable/linux-stable.git/commit/?id=a5e3985fa014029eb6795664c704953720cc7f7d

commit a5e3985fa014029eb6795664c704953720cc7f7d
Author: Roman Zippel <zippel@linux-m68k.org>
Date:   Tue Sep 6 15:18:47 2005 -0700

[PATCH] hfs: remove debug code

Signed-off-by: Hin-Tak Leung <htl10@users.sourceforge.net>
Signed-off-by: Sergei Antonov <saproj@gmail.com>
Reviewed-by: Anton Altaparmakov <anton@tuxera.com>
Reported-by: Sasha Levin <sasha.levin@oracle.com>
Cc: Al Viro <viro@zeniv.linux.org.uk>
Cc: Christoph Hellwig <hch@infradead.org>
Cc: Vyacheslav Dubeyko <slava@dubeyko.com>
Cc: Sougata Santra <sougata@tuxera.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
---
 fs/hfs/bnode.c     | 9 ++++-----
 fs/hfsplus/bnode.c | 3 ---
 2 files changed, 4 insertions(+), 8 deletions(-)

--- a/fs/hfs/bnode.c
+++ b/fs/hfs/bnode.c
@@ -287,7 +287,6 @@ static struct hfs_bnode *__hfs_bnode_cre
 			page_cache_release(page);
 			goto fail;
 		}
-		page_cache_release(page);
 		node->page[i] = page;
 	}
 
@@ -397,11 +396,11 @@ node_error:
 
 void hfs_bnode_free(struct hfs_bnode *node)
 {
-	//int i;
+	int i;
 
-	//for (i = 0; i < node->tree->pages_per_bnode; i++)
-	//	if (node->page[i])
-	//		page_cache_release(node->page[i]);
+	for (i = 0; i < node->tree->pages_per_bnode; i++)
+		if (node->page[i])
+			page_cache_release(node->page[i]);
 	kfree(node);
 }
 
--- a/fs/hfsplus/bnode.c
+++ b/fs/hfsplus/bnode.c
@@ -454,7 +454,6 @@ static struct hfs_bnode *__hfs_bnode_cre
 			page_cache_release(page);
 			goto fail;
 		}
-		page_cache_release(page);
 		node->page[i] = page;
 	}
 
@@ -566,13 +565,11 @@ node_error:
 
 void hfs_bnode_free(struct hfs_bnode *node)
 {
-#if 0
 	int i;
 
 	for (i = 0; i < node->tree->pages_per_bnode; i++)
 		if (node->page[i])
 			page_cache_release(node->page[i]);
-#endif
 	kfree(node);
 }
 

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Back to linux.kernel | Previous | NextPrevious in thread | Next in thread | Find similar | Unroll thread


Thread

[PATCH 3.2 000/107] 3.2.72-rc1 review Ben Hutchings <ben@decadent.org.uk> - 2015-10-09 02:30 +0200
  [PATCH 3.2 072/107] USB: option: add ZTE PIDs Ben Hutchings <ben@decadent.org.uk> - 2015-10-09 02:30 +0200
  [PATCH 3.2 069/107] hfs,hfsplus: cache pages correctly between  bnode_create and bnode_free Ben Hutchings <ben@decadent.org.uk> - 2015-10-09 02:30 +0200
  [PATCH 3.2 015/107] ocfs2: fix BUG in ocfs2_downconvert_thread_do_work() Ben Hutchings <ben@decadent.org.uk> - 2015-10-09 02:30 +0200
  [PATCH 3.2 049/107] DRM - radeon: Don't link train DisplayPort on  HPD until we get the dpcd Ben Hutchings <ben@decadent.org.uk> - 2015-10-09 02:30 +0200
  [PATCH 3.2 098/107] ipv6: lock socket in ip6_datagram_connect() Ben Hutchings <ben@decadent.org.uk> - 2015-10-09 02:30 +0200
  [PATCH 3.2 024/107] libfc: Fix fc_fcp_cleanup_each_cmd() Ben Hutchings <ben@decadent.org.uk> - 2015-10-09 02:30 +0200
  [PATCH 3.2 094/107] Initialize msg/shm IPC objects before doing  ipc_addid() Ben Hutchings <ben@decadent.org.uk> - 2015-10-09 02:40 +0200
  [PATCH 3.2 060/107] drm/i915: Always mark the object as dirty  when used by the GPU Ben Hutchings <ben@decadent.org.uk> - 2015-10-09 02:40 +0200
  [PATCH 3.2 047/107] eCryptfs: Invalidate dcache entries when  lower i_nlink is zero Ben Hutchings <ben@decadent.org.uk> - 2015-10-09 02:40 +0200
  [PATCH 3.2 009/107] target: REPORT LUNS should return LUN 0 even  for dynamic ACLs Ben Hutchings <ben@decadent.org.uk> - 2015-10-09 02:40 +0200
  [PATCH 3.2 095/107] net/tipc: initialize security state for new  connection  socket Ben Hutchings <ben@decadent.org.uk> - 2015-10-09 02:40 +0200
  [PATCH 3.2 018/107] x86/ldt: Make modify_ldt synchronous Ben Hutchings <ben@decadent.org.uk> - 2015-10-09 02:40 +0200
  [PATCH 3.2 036/107] PCI: Add VPD function 0 quirk for Intel  Ethernet devices Ben Hutchings <ben@decadent.org.uk> - 2015-10-09 02:40 +0200
  [PATCH 3.2 016/107] net: Clone skb before setting peeked flag Ben Hutchings <ben@decadent.org.uk> - 2015-10-09 02:40 +0200
  [PATCH 3.2 026/107] x86/ldt: Further fix FPU emulation Ben Hutchings <ben@decadent.org.uk> - 2015-10-09 02:40 +0200
  [PATCH 3.2 029/107] sparc64: Fix userspace FPU register corruptions. Ben Hutchings <ben@decadent.org.uk> - 2015-10-09 02:40 +0200
  [PATCH 3.2 023/107] libiscsi: Fix host busy blocking during  connection teardown Ben Hutchings <ben@decadent.org.uk> - 2015-10-09 02:40 +0200
  [PATCH 3.2 019/107] x86/ldt: Correct LDT access in single  stepping logic Ben Hutchings <ben@decadent.org.uk> - 2015-10-09 02:40 +0200
  [PATCH 3.2 017/107] net: Fix skb_set_peeked use-after-free bug Ben Hutchings <ben@decadent.org.uk> - 2015-10-09 02:40 +0200
  [PATCH 3.2 005/107] crypto: ixp4xx - Remove bogus BUG_ON on  scattered dst buffer Ben Hutchings <ben@decadent.org.uk> - 2015-10-09 02:40 +0200
  [PATCH 3.2 033/107] PCI: Fix TI816X class code quirk Ben Hutchings <ben@decadent.org.uk> - 2015-10-09 02:40 +0200
  [PATCH 3.2 051/107] rtlwifi: rtl8192cu: Add new device ID Ben Hutchings <ben@decadent.org.uk> - 2015-10-09 02:40 +0200
  [PATCH 3.2 032/107] [media] rc-core: fix remove uevent generation Ben Hutchings <ben@decadent.org.uk> - 2015-10-09 02:40 +0200
  [PATCH 3.2 068/107] powerpc/MSI: Fix race condition in tearing  down MSI interrupts Ben Hutchings <ben@decadent.org.uk> - 2015-10-09 02:40 +0200
  [PATCH 3.2 037/107] usb: gadget: m66592-udc: forever loop in  set_feature() Ben Hutchings <ben@decadent.org.uk> - 2015-10-09 02:40 +0200
  [PATCH 3.2 054/107] xfs: return errors from partial I/O failures  to files Ben Hutchings <ben@decadent.org.uk> - 2015-10-09 02:40 +0200
  [PATCH 3.2 103/107] ipv6: prevent fib6_run_gc() contention Ben Hutchings <ben@decadent.org.uk> - 2015-10-09 02:40 +0200
  [PATCH 3.2 044/107] serial: 8250: bind to ALi Fast Infrared  Controller (ALI5123) Ben Hutchings <ben@decadent.org.uk> - 2015-10-09 02:40 +0200
  [PATCH 3.2 070/107] hfs: fix B-tree corruption after insertion at  position 0 Ben Hutchings <ben@decadent.org.uk> - 2015-10-09 02:40 +0200
  [PATCH 3.2 050/107] rtlwifi: rtl8192cu: Add new device ID Ben Hutchings <ben@decadent.org.uk> - 2015-10-09 02:40 +0200
  [PATCH 3.2 048/107] xfs: Fix xfs_attr_leafblock definition Ben Hutchings <ben@decadent.org.uk> - 2015-10-09 02:40 +0200
  [PATCH 3.2 025/107] ipc,sem: fix use after free on IPC_RMID after  a task using same semaphore set exits Ben Hutchings <ben@decadent.org.uk> - 2015-10-09 02:40 +0200
  [PATCH 3.2 007/107] target/iscsi: Fix double free of a TUR  followed by a solicited NOPOUT Ben Hutchings <ben@decadent.org.uk> - 2015-10-09 02:40 +0200
  [PATCH 3.2 081/107] usb: Use the USB_SS_MULT() macro to get the  burst multiplier. Ben Hutchings <ben@decadent.org.uk> - 2015-10-09 02:50 +0200
  [PATCH 3.2 079/107] s390/compat: correct uc_sigmask of the compat  signal frame Ben Hutchings <ben@decadent.org.uk> - 2015-10-09 02:50 +0200
  [PATCH 3.2 091/107] virtio-net: drop NETIF_F_FRAGLIST Ben Hutchings <ben@decadent.org.uk> - 2015-10-09 02:50 +0200
  [PATCH 3.2 053/107] drivercore: Fix unregistration path of  platform devices Ben Hutchings <ben@decadent.org.uk> - 2015-10-09 02:50 +0200
  [PATCH 3.2 006/107] USB: sierra: add 1199:68AB device ID Ben Hutchings <ben@decadent.org.uk> - 2015-10-09 02:50 +0200
  [PATCH 3.2 059/107] spi: spi-pxa2xx: Check status register to  determine if SSSR_TINT is disabled Ben Hutchings <ben@decadent.org.uk> - 2015-10-09 02:50 +0200
  [PATCH 3.2 002/107] pktgen: Require CONFIG_INET due to use of  IPv4 checksum function Ben Hutchings <ben@decadent.org.uk> - 2015-10-09 02:50 +0200
  [PATCH 3.2 085/107] cifs: use server timestamp for ntlmv2  authentication Ben Hutchings <ben@decadent.org.uk> - 2015-10-09 02:50 +0200
  [PATCH 3.2 066/107] ARM: 8429/1: disable GCC SRA optimization Ben Hutchings <ben@decadent.org.uk> - 2015-10-09 02:50 +0200
  [PATCH 3.2 061/107] Add radeon suspend/resume quirk for HP Compaq  dc5750. Ben Hutchings <ben@decadent.org.uk> - 2015-10-09 02:50 +0200
  [PATCH 3.2 046/107] USB: ftdi_sio: Added custom PID for  CustomWare products Ben Hutchings <ben@decadent.org.uk> - 2015-10-09 02:50 +0200
  [PATCH 3.2 013/107] perf: Fix fasync handling on inherited events Ben Hutchings <ben@decadent.org.uk> - 2015-10-09 02:50 +0200
  [PATCH 3.2 074/107] btrfs: skip waiting on ordered range for  special files Ben Hutchings <ben@decadent.org.uk> - 2015-10-09 02:50 +0200
  [PATCH 3.2 052/107] of/address: Don't loop forever in  of_find_matching_node_by_address(). Ben Hutchings <ben@decadent.org.uk> - 2015-10-09 02:50 +0200
  [PATCH 3.2 042/107] NFSv4: don't set SETATTR for O_RDONLY|O_EXCL Ben Hutchings <ben@decadent.org.uk> - 2015-10-09 02:50 +0200
  [PATCH 3.2 093/107] ipc/sem.c: fully initialize sem_array before  making it visible Ben Hutchings <ben@decadent.org.uk> - 2015-10-09 02:50 +0200
  [PATCH 3.2 039/107] auxdisplay: ks0108: fix refcount Ben Hutchings <ben@decadent.org.uk> - 2015-10-09 02:50 +0200
  [PATCH 3.2 083/107] usb: xhci: Clear XHCI_STATE_DYING on start Ben Hutchings <ben@decadent.org.uk> - 2015-10-09 02:50 +0200
  [PATCH 3.2 062/107] IB/uverbs: reject invalid or unknown opcodes Ben Hutchings <ben@decadent.org.uk> - 2015-10-09 02:50 +0200
  [PATCH 3.2 058/107] IB/uverbs: Fix race between ib_uverbs_open  and remove_one Ben Hutchings <ben@decadent.org.uk> - 2015-10-09 02:50 +0200
  [PATCH 3.2 102/107] perf tools: Fix build with perl 5.18 Ben Hutchings <ben@decadent.org.uk> - 2015-10-09 02:50 +0200
  [PATCH 3.2 075/107] ARM: 7880/1: Clear the IT state independent  of the Thumb-2 mode Ben Hutchings <ben@decadent.org.uk> - 2015-10-09 02:50 +0200
  [PATCH 3.2 105/107] parisc: Filter out spurious interrupts in  PA-RISC irq handler Ben Hutchings <ben@decadent.org.uk> - 2015-10-09 02:50 +0200
  [PATCH 3.2 100/107] net/ipv6: Correct PIM6 mrt_lock handling Ben Hutchings <ben@decadent.org.uk> - 2015-10-09 02:50 +0200
  [PATCH 3.2 030/107] dcache: Handle escaped paths in prepend_path Ben Hutchings <ben@decadent.org.uk> - 2015-10-09 02:50 +0200
  [PATCH 3.2 041/107] windfarm: decrement client count when  unregistering Ben Hutchings <ben@decadent.org.uk> - 2015-10-09 02:50 +0200
  [PATCH 3.2 076/107] ARM: fix Thumb2 signal handling when ARMv6 is  enabled Ben Hutchings <ben@decadent.org.uk> - 2015-10-09 02:50 +0200
  [PATCH 3.2 057/107] IB/mlx4: Use correct SL on AH query under RoCE Ben Hutchings <ben@decadent.org.uk> - 2015-10-09 02:50 +0200
  [PATCH 3.2 090/107] ipv6: addrconf: validate new MTU before  applying it Ben Hutchings <ben@decadent.org.uk> - 2015-10-09 02:50 +0200
  [PATCH 3.2 010/107] MIPS: Fix sched_getaffinity with MT FPAFF enabled Ben Hutchings <ben@decadent.org.uk> - 2015-10-09 02:50 +0200
  [PATCH 3.2 063/107] Input: evdev - do not report errors form flush() Ben Hutchings <ben@decadent.org.uk> - 2015-10-09 03:00 +0200
  [PATCH 3.2 078/107] ASoC: fix broken pxa SoC support Ben Hutchings <ben@decadent.org.uk> - 2015-10-09 03:00 +0200
  [PATCH 3.2 064/107] crypto: ghash-clmulni: specify context size  for ghash async algorithm Ben Hutchings <ben@decadent.org.uk> - 2015-10-09 03:00 +0200
  [PATCH 3.2 065/107] fs: create and use seq_show_option for escaping Ben Hutchings <ben@decadent.org.uk> - 2015-10-09 03:00 +0200
  [PATCH 3.2 084/107] xhci: change xhci 1.0 only restrictions to  support xhci 1.1 Ben Hutchings <ben@decadent.org.uk> - 2015-10-09 03:00 +0200
  [PATCH 3.2 071/107] perf header: Fixup reading of HEADER_NRCPUS  feature Ben Hutchings <ben@decadent.org.uk> - 2015-10-09 03:00 +0200
  [PATCH 3.2 104/107] ipv6: update ip6_rt_last_gc every time GC is run Ben Hutchings <ben@decadent.org.uk> - 2015-10-09 03:00 +0200
  [PATCH 3.2 055/107] IB/qib: Change lkey table allocation to  support more MRs Ben Hutchings <ben@decadent.org.uk> - 2015-10-09 03:00 +0200
  [PATCH 3.2 073/107] Btrfs: fix read corruption of compressed and  shared extents Ben Hutchings <ben@decadent.org.uk> - 2015-10-09 03:00 +0200
  Re: [PATCH 3.2 000/107] 3.2.72-rc1 review Guenter Roeck <linux@roeck-us.net> - 2015-10-09 03:00 +0200
    Re: [PATCH 3.2 000/107] 3.2.72-rc1 review Ben Hutchings <ben@decadent.org.uk> - 2015-10-09 03:20 +0200
  [PATCH 3.2 077/107] x86/platform: Fix Geode LX timekeeping in the  generic x86 build Ben Hutchings <ben@decadent.org.uk> - 2015-10-09 03:00 +0200
  [PATCH 3.2 087/107] ocfs2/dlm: fix deadlock when dispatch assert  master Ben Hutchings <ben@decadent.org.uk> - 2015-10-09 03:00 +0200
  [PATCH 3.2 056/107] SUNRPC: xs_reset_transport must mark the  connection as disconnected Ben Hutchings <ben@decadent.org.uk> - 2015-10-09 03:00 +0200
  [PATCH 3.2 082/107] xhci: give command abortion one more chance  before killing xhci Ben Hutchings <ben@decadent.org.uk> - 2015-10-09 03:00 +0200

csiph-web