Bug#714974: [PATCH] jfs: avoid misuse of cookie value of 2

2013-08-15 Thread Dave Kleikamp
For the sake of those not watching
https://bugzilla.kernel.org/show_bug.cgi?id=60737

It looks like the problem is that jfs was using a cookie value of 2 for
a real directory entry, where NFSv4 expect 2 to represent ... This
patch has so far only been lightly tested.

NFSv4 reserves cookie values 0, 1 and 2 for a rewind, and the . and ..
entries. jfs was using 0 and 1 for . and .., but 2 for a regular entry.
This patch makes jfs conform by using 1 and 2 for . and .. and fixes
any regular entry using the value 2.

Signed-off-by: Dave Kleikamp dave.kleik...@oracle.com

diff --git a/fs/jfs/jfs_dtree.c b/fs/jfs/jfs_dtree.c
index 8743ba9..93466e8 100644
--- a/fs/jfs/jfs_dtree.c
+++ b/fs/jfs/jfs_dtree.c
@@ -349,11 +349,8 @@ static u32 add_index(tid_t tid, struct inode *ip, s64 bn, 
int slot)
 
ASSERT(DO_INDEX(ip));
 
-   if (jfs_ip-next_index  2) {
-   jfs_warn(add_index: next_index = %d.  Resetting!,
-  jfs_ip-next_index);
-   jfs_ip-next_index = 2;
-   }
+   if (jfs_ip-next_index  3) {
+   jfs_ip-next_index = 3;
 
index = jfs_ip-next_index++;
 
@@ -2864,7 +2861,7 @@ void dtInitRoot(tid_t tid, struct inode *ip, u32 idotdot)
} else
ip-i_size = 1;
 
-   jfs_ip-next_index = 2;
+   jfs_ip-next_index = 3;
} else
ip-i_size = IDATASIZE;
 
@@ -2951,7 +2948,7 @@ static void add_missing_indices(struct inode *inode, s64 
bn)
for (i = 0; i  p-header.nextindex; i++) {
d = (struct ldtentry *) p-slot[stbl[i]];
index = le32_to_cpu(d-index);
-   if ((index  2) || (index = JFS_IP(inode)-next_index)) {
+   if ((index  3) || (index = JFS_IP(inode)-next_index)) {
d-index = cpu_to_le32(add_index(tid, inode, bn, i));
if (dtlck-index = dtlck-maxcnt)
dtlck = (struct dt_lock *) txLinelock(dtlck);
@@ -3031,7 +3028,7 @@ int jfs_readdir(struct file *file, struct dir_context 
*ctx)
struct jfs_dirent *jfs_dirent;
int jfs_dirents;
int overflow, fix_page, page_fixed = 0;
-   static int unique_pos = 2;  /* If we can't fix broken index */
+   static int unique_pos = 3;  /* If we can't fix broken index */
 
if (ctx-pos == DIREND)
return 0;
@@ -3039,15 +3036,16 @@ int jfs_readdir(struct file *file, struct dir_context 
*ctx)
if (DO_INDEX(ip)) {
/*
 * persistent index is stored in directory entries.
-* Special cases:0 = .
-*   1 = ..
+* Special cases:0 = rewind
+*   1 = .
+*   2 = ..
 *  -1 = End of directory
 */
do_index = 1;
 
dir_index = (u32) ctx-pos;
 
-   if (dir_index  1) {
+   if (dir_index  2) {
struct dir_table_slot dirtab_slot;
 
if (dtEmpty(ip) ||
@@ -3090,18 +3088,18 @@ int jfs_readdir(struct file *file, struct dir_context 
*ctx)
return 0;
}
} else {
-   if (dir_index == 0) {
+   if (dir_index  2) {
/*
 * self .
 */
-   ctx-pos = 0;
+   ctx-pos = 1;
if (!dir_emit(ctx, ., 1, ip-i_ino, DT_DIR))
return 0;
}
/*
 * parent ..
 */
-   ctx-pos = 1;
+   ctx-pos = 2;
if (!dir_emit(ctx, .., 2, PARENT(ip), DT_DIR))
return 0;
 
@@ -3122,22 +3120,24 @@ int jfs_readdir(struct file *file, struct dir_context 
*ctx)
/*
 * Legacy filesystem - OS/2  Linux JFS  0.3.6
 *
-* pn = index = 0:  First entry .
-* pn = 0; index = 1:   Second entry ..
+* pn = 0; index = 1:   First entry .
+* pn = 0; index = 2:   Second entry ..
 * pn  0:  Real entries, pn=1 - leftmost page
 * pn = index = -1: No more entries
 */
dtpos = ctx-pos;
-   if (dtpos == 0) {
+   if (dtpos  2) {
+   ctx-pos = 1;
/* build . entry */
if (!dir_emit(ctx, ., 1, ip-i_ino, DT_DIR))
return 0;
-   dtoffset-index = 1

Bug#714974: [PATCH] jfs: avoid misuse of cookie value of 2

2013-08-15 Thread Dave Kleikamp
On 08/15/2013 02:09 AM, Christian Kujau wrote:
 On Wed, 14 Aug 2013 at 21:29, Christian Kujau wrote:
 
 On Wed, 14 Aug 2013 at 22:54, Dave Kleikamp wrote:
 It looks like the problem is that jfs was using a cookie value of 2 for
 a real directory entry, where NFSv4 expect 2 to represent ... This
 patch has so far only been lightly tested.

 Hm, a first compile of 3.11-rc5 errors out with:

   CC [M]  fs/jfs/jfs_dtree.o
 /usr/local/src/linux-git/fs/jfs/jfs_dtree.c: In function ‘add_index’:
 /usr/local/src/linux-git/fs/jfs/jfs_dtree.c:493:13: error: invalid storage 
 class for function ‘free_index’
 [...]

 I'll run mrproper and try again...
 
 This did not help, but adding a closing bracket did, in fs/jfs/jfs_dtree.c:354
 
 if (jfs_ip-next_index  3) {
 jfs_ip-next_index = 3;
 }
-^
 
 This compiled and booted and now I can run find(1) over that whole NFS 
 share, without any readdir loop messages and with unique inode numbers, 
 yay!

My bad. That's what happens when you clean up the patch after you test
it. I intended to remove the opening bracket when I removed a warning.

   Tested-by: Christian Kujau li...@nerdbynature.de

Thanks. After sleeping on it, I'm contemplating a simpler patch. I'll
keep you up to date.

 
 Thanks!
 Christian.
 


-- 
To UNSUBSCRIBE, email to debian-kernel-requ...@lists.debian.org
with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
Archive: http://lists.debian.org/520cd9c8.8000...@oracle.com



Bug#714974: [PATCH] jfs: fix readdir cookie incompatibility with NFSv4

2013-08-15 Thread Dave Kleikamp
This patch replaces the one I posted yesterday. I like this better since
it doesn't require fixing existing on-disk cookies or skipping a
position in the in-inode index table.

NFSv4 reserves readdir cookie values 0-2 for special entries (. and ..),
but jfs allows a value of 2 for a non-special entry. This incompatibility
can result in the nfs client reporting a readdir loop.

This patch doesn't change the value stored internally, but adds one to
the value exposed to the iterate method.

Signed-off-by: Dave Kleikamp dave.kleik...@oracle.com
---
 fs/jfs/jfs_dtree.c | 31 +++
 1 file changed, 23 insertions(+), 8 deletions(-)

diff --git a/fs/jfs/jfs_dtree.c b/fs/jfs/jfs_dtree.c
index 8743ba9..0ec767e 100644
--- a/fs/jfs/jfs_dtree.c
+++ b/fs/jfs/jfs_dtree.c
@@ -3047,6 +3047,14 @@ int jfs_readdir(struct file *file, struct dir_context 
*ctx)
 
dir_index = (u32) ctx-pos;
 
+   /*
+* NFSv4 reserves cookies 1 and 2 for . and .. so we add
+* the value we return to the vfs is one greater than the
+* one we use internally.
+*/
+   if (dir_index)
+   dir_index--;
+
if (dir_index  1) {
struct dir_table_slot dirtab_slot;
 
@@ -3086,7 +3094,7 @@ int jfs_readdir(struct file *file, struct dir_context 
*ctx)
if (p-header.flag  BT_INTERNAL) {
jfs_err(jfs_readdir: bad index table);
DT_PUTPAGE(mp);
-   ctx-pos = -1;
+   ctx-pos = DIREND;
return 0;
}
} else {
@@ -3094,14 +3102,14 @@ int jfs_readdir(struct file *file, struct dir_context 
*ctx)
/*
 * self .
 */
-   ctx-pos = 0;
+   ctx-pos = 1;
if (!dir_emit(ctx, ., 1, ip-i_ino, DT_DIR))
return 0;
}
/*
 * parent ..
 */
-   ctx-pos = 1;
+   ctx-pos = 2;
if (!dir_emit(ctx, .., 2, PARENT(ip), DT_DIR))
return 0;
 
@@ -3122,22 +3130,23 @@ int jfs_readdir(struct file *file, struct dir_context 
*ctx)
/*
 * Legacy filesystem - OS/2  Linux JFS  0.3.6
 *
-* pn = index = 0:  First entry .
-* pn = 0; index = 1:   Second entry ..
+* pn = 0; index = 1:   First entry .
+* pn = 0; index = 2:   Second entry ..
 * pn  0:  Real entries, pn=1 - leftmost page
 * pn = index = -1: No more entries
 */
dtpos = ctx-pos;
-   if (dtpos == 0) {
+   if (dtpos  2) {
/* build . entry */
+   ctx-pos = 1;
if (!dir_emit(ctx, ., 1, ip-i_ino, DT_DIR))
return 0;
-   dtoffset-index = 1;
+   dtoffset-index = 2;
ctx-pos = dtpos;
}
 
if (dtoffset-pn == 0) {
-   if (dtoffset-index == 1) {
+   if (dtoffset-index == 2) {
/* build .. entry */
if (!dir_emit(ctx, .., 2, PARENT(ip), DT_DIR))
return 0;
@@ -3228,6 +3237,12 @@ int jfs_readdir(struct file *file, struct dir_context 
*ctx)
}
jfs_dirent-position = unique_pos++;
}
+   /*
+* We add 1 to the index because we may
+* use a value of 2 internally, and NFSv4
+* doesn't like that.
+*/
+   jfs_dirent-position++;
} else {
jfs_dirent-position = dtpos;
len = min(d_namleft, DTLHDRDATALEN_LEGACY);
-- 
1.8.3.4


-- 
To UNSUBSCRIBE, email to debian-kernel-requ...@lists.debian.org
with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
Archive: http://lists.debian.org/520d3ea7.1010...@oracle.com



Bug#714974: [PATCH] jfs: fix readdir cookie incompatibility with NFSv4

2013-08-15 Thread Dave Kleikamp
On 08/15/2013 04:26 PM, Christian Kujau wrote:
 On Thu, 15 Aug 2013 at 15:48, Dave Kleikamp wrote:
 This patch replaces the one I posted yesterday. I like this better since
 it doesn't require fixing existing on-disk cookies or skipping a
 position in the in-inode index table.
 
 Thanks. Applied to 3.11-rc5 and tested, no more readdir loop messages 
 and with unique inode numbers, great!
 
   Tested-by: Christian Kujau li...@nerdbynature.de
 
 Thanks for the fix!
 Christian.

Thanks for reporting, investigating and testing!

Dave


-- 
To UNSUBSCRIBE, email to debian-kernel-requ...@lists.debian.org
with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
Archive: http://lists.debian.org/520d519f.6080...@oracle.com



Bug#502383: Acknowledgement (linux-image-2.6.26-1-amd64: BUG at fs/jfs/jfs_metapage.c:742 assert(mp-count))

2008-10-17 Thread Dave Kleikamp
On Thu, 2008-10-16 at 07:24 -0700, Tom Epperly wrote:
 When I shutdown after reporting the bug, the machine didn't shutdown 
 smoothly.

This is common after seeing a BUG() in the kernel.  The trapping kernel
thread terminates without cleaning up after itself.  It may be holding
locks that cause other processes to hang.  The important thing is to
figure out and fix the initial BUG().

Shaggy
-- 
David Kleikamp
IBM Linux Technology Center




-- 
To UNSUBSCRIBE, email to [EMAIL PROTECTED]
with a subject of unsubscribe. Trouble? Contact [EMAIL PROTECTED]



Bug#502383: linux-image-2.6.26-1-amd64: BUG at fs/jfs/jfs_metapage.c:742 assert(mp-count)

2008-10-17 Thread Dave Kleikamp
On Wed, 2008-10-15 at 22:49 -0700, Tom Epperly wrote:

 I followed up in /var/log/syslog and found:
 
 Oct 15 21:09:12 faerun kernel: [11052.080364] BUG at 
 fs/jfs/jfs_metapage.c:742 assert(mp-count)
 Oct 15 21:09:13 faerun kernel: [11052.080424] [ cut here 
 ]
 Oct 15 21:09:13 faerun kernel: [11052.080427] kernel BUG at 
 fs/jfs/jfs_metapage.c:742!


 Oct 15 22:35:02 faerun kernel: [16209.686185] ERROR: (device sda6): 
 __get_metapage: mp-logical_size != size
 Oct 15 22:35:02 faerun kernel: [16209.705572] Pid: 11298, comm: gtk-gnash 
 Tainted: P  D   2.6.26-1-amd64 #1
 Oct 15 22:35:02 faerun kernel: [16209.705579] 
 Oct 15 22:35:02 faerun kernel: [16209.705580] Call Trace:
 Oct 15 22:35:02 faerun kernel: [16209.705619]  [a013a35b] 
 :jfs:__get_metapage+0x13e/0x39f

I suspect that these traps may be the result of memory corruption.
jfs's metapage structure is allocated from a dedicated mempool.  The
second is almost certainly a result of some kind of memory corruption,
as the logical_size is never set to anything but 4096, and the caller
always passes in the same value.  The first can also be explained by a
corrupted struct metapage.

The source of the memory corruption may be hard to determine.  It could
be a use-after-free of a memory structure, or some wild pointer in any
kernel code.

Have you seen any other traps?  I can't rule out a bug in jfs, but there
haven't been any recent changes in this part of the code.
-- 
David Kleikamp
IBM Linux Technology Center




-- 
To UNSUBSCRIBE, email to [EMAIL PROTECTED]
with a subject of unsubscribe. Trouble? Contact [EMAIL PROTECTED]