Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751327AbbKNOxG (ORCPT ); Sat, 14 Nov 2015 09:53:06 -0500 Received: from mail.parknet.co.jp ([210.171.160.6]:52218 "EHLO mail.parknet.co.jp" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751155AbbKNOxB (ORCPT ); Sat, 14 Nov 2015 09:53:01 -0500 X-Greylist: delayed 1461 seconds by postgrey-1.27 at vger.kernel.org; Sat, 14 Nov 2015 09:53:01 EST From: OGAWA Hirofumi To: Richard Weinberger Cc: Vegard Nossum , LKML , Andrew Morton Subject: Re: Endless getdents() in vfat filesystem References: <56468C1E.2060401@oracle.com> <56472C2F.5090207@oracle.com> Date: Sat, 14 Nov 2015 23:28:36 +0900 In-Reply-To: <56472C2F.5090207@oracle.com> (Vegard Nossum's message of "Sat, 14 Nov 2015 13:42:23 +0100") Message-ID: <87r3js3baj.fsf@mail.parknet.co.jp> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/25.0.50 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3958 Lines: 136 Vegard Nossum writes: > On 11/14/2015 11:32 AM, Richard Weinberger wrote: >> On Sat, Nov 14, 2015 at 2:19 AM, Vegard Nossum wrote: >>> Hi, >>> >>> Using the attached disk image I observe that getdents() never returns >>> the end of the directory, i.e. mounting the disk image on a loopback >>> device and running 'ls' under strace shows an endless stream of: >>> >>> getdents(3, /* 2 entries */, 32768) = 48 >>> getdents(3, /* 2 entries */, 32768) = 48 >>> getdents(3, /* 2 entries */, 32768) = 48 >>> ... >> >> Please more details. Is this image hand crafted? >> If not, how has it been created? Is is supposed to work? > > It was created by fuzzing, it is not supposed to work per se. > >> From a quick look it seems as the root directory is bad but we report >> progress in ->iterate. >> ctx->pos is 2, we set it back to 0, because of the faked dot entries. >> but fat_get_entry() did not make any progress and we report 0 back to VFS. >> So, VFS sees progress and the game continues. >> >> Does the attached patch help? > > Yes, it does fixes the problem here, but I can't really comment on the > correctness of the patch. > > Thanks for the quick reponse, I made cleanup and made sure fake_offset is corrected. Richard, Signed-off-by was missed in your patch, so I added. Can you agree to Signed-off-by? Thanks. -- OGAWA Hirofumi [PATCH] fat: Fix fake_offset handling on error path [hirofumi@mail.parknet.co.jp: cleanup and make sure to correct fake_offset] Signed-off-by: Richard Weinberger Signed-off-by: OGAWA Hirofumi --- fs/fat/dir.c | 25 +++++++++++++++---------- 1 file changed, 15 insertions(+), 10 deletions(-) diff -puN fs/fat/dir.c~fat-fake_offset-fix fs/fat/dir.c --- linux-tux3/fs/fat/dir.c~fat-fake_offset-fix 2015-11-14 21:53:21.722269967 +0900 +++ linux-tux3-hirofumi/fs/fat/dir.c 2015-11-14 23:03:16.740608521 +0900 @@ -559,7 +559,7 @@ static int __fat_readdir(struct inode *i { struct super_block *sb = inode->i_sb; struct msdos_sb_info *sbi = MSDOS_SB(sb); - struct buffer_head *bh; + struct buffer_head *bh = NULL; struct msdos_dir_entry *de; unsigned char nr_slots; wchar_t *unicode = NULL; @@ -588,10 +588,9 @@ static int __fat_readdir(struct inode *i goto out; } - bh = NULL; get_new: if (fat_get_entry(inode, &cpos, &bh, &de) == -1) - goto end_of_dir; + goto out; parse_record: nr_slots = 0; /* @@ -614,7 +613,7 @@ parse_record: int status = fat_parse_long(inode, &cpos, &bh, &de, &unicode, &nr_slots); if (status < 0) { - ctx->pos = cpos; + bh = NULL; ret = status; goto out; } else if (status == PARSE_INVALID) @@ -622,7 +621,7 @@ parse_record: else if (status == PARSE_NOT_LONGNAME) goto parse_record; else if (status == PARSE_EOF) - goto end_of_dir; + goto out; if (nr_slots) { void *longname = unicode + FAT_MAX_UNI_CHARS; @@ -658,8 +657,9 @@ parse_record: fill_len = short_len; start_filldir: - if (!fake_offset) - ctx->pos = cpos - (nr_slots + 1) * sizeof(struct msdos_dir_entry); + ctx->pos = cpos - (nr_slots + 1) * sizeof(struct msdos_dir_entry); + if (fake_offset && ctx->pos < 2) + ctx->pos = 2; if (!memcmp(de->name, MSDOS_DOT, MSDOS_NAME)) { if (!dir_emit_dot(file, ctx)) @@ -685,14 +685,19 @@ record_end: fake_offset = 0; ctx->pos = cpos; goto get_new; -end_of_dir: - ctx->pos = cpos; + +out: + if (fake_offset && cpos < 2) + ctx->pos = 2; + else + ctx->pos = cpos; fill_failed: brelse(bh); if (unicode) __putname(unicode); -out: + mutex_unlock(&sbi->s_lock); + return ret; } _ -- 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/