linux-ext4 - Re: [Bug 32972] New: EXT4 causes corrupt BitTorrent downloads

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <BANLkTikQa3UaBVVFdrvzwUcdR_qvOPcmFw@mail.gmail.com>
Date:	Sun, 10 Apr 2011 22:30:13 +0800
From:	Yongqiang Yang <xiaoqiangnk@...il.com>
To:	bugzilla-daemon@...zilla.kernel.org, damien@...ssart.com,
	feng.tang@...el.com
Cc:	linux-ext4@...r.kernel.org
Subject: Re: [Bug 32972] New: EXT4 causes corrupt BitTorrent downloads

Ah, it is a wrong fix.

original code:
	map_bh(bh, inode->i_sb, map.m_pblk);
	bh->b_state = (bh->b_state & ~EXT4_MAP_FLAGS) | map.m_flags;

	if (buffer_unwritten(bh)) {
		/* A delayed write to unwritten bh should be marked
		 * new and mapped.  Mapped ensures that we don't do
		 * get_block multiple times when we write to the same
		 * offset and new ensures that we do proper zero out
		 * for partial write.
		 */
		set_buffer_new(bh);
		set_buffer_mapped(bh);
	}

after the patch:
	map_bh(bh, inode->i_sb, map.m_pblk);
	bh->b_state = (bh->b_state & ~EXT4_MAP_FLAGS) | map.m_flags;

	if (buffer_unwritten(bh)) {
		/* A delayed write to unwritten bh should be marked
		 * new and mapped.  Mapped ensures that we don't do
		 * get_block multiple times when we write to the same
		 * offset and new ensures that we do proper zero out
		 * for partial write.
		 */
		set_buffer_new(bh);
	}

Actually, mapped flag is cleared by statement:
	bh->b_state = (bh->b_state & ~EXT4_MAP_FLAGS) | map.m_flags;

So the right code should be：

	bh->b_state = (bh->b_state & ~EXT4_MAP_FLAGS) | map.m_flags;
	map_bh(bh, inode->i_sb, map.m_pblk);
	if (buffer_unwritten(bh)) {
		/* A delayed write to unwritten bh should be marked
		 * new and mapped.  Mapped ensures that we don't do
		 * get_block multiple times when we write to the same
		 * offset and new ensures that we do proper zero out
		 * for partial write.
		 */
		set_buffer_new(bh);
	}



On Sun, Apr 10, 2011 at 9:44 PM,  <bugzilla-daemon@...zilla.kernel.org> wrote:
> https://bugzilla.kernel.org/show_bug.cgi?id=32972
>
>           Summary: EXT4 causes corrupt BitTorrent downloads
>           Product: File System
>           Version: 2.5
>    Kernel Version: 2.6.39-rc2+
>          Platform: All
>        OS/Version: Linux
>              Tree: Mainline
>            Status: NEW
>          Severity: high
>          Priority: P1
>         Component: ext4
>        AssignedTo: fs_ext4@...nel-bugs.osdl.org
>        ReportedBy: damien@...ssart.com
>                CC: feng.tang@...el.com
>        Regression: Yes
>
>
> Using the Transmission BitTorrent client (version 2.11) with the most recent
> kernel (2.6.39-rc2+), any torrent I try to download is corrupt. By using
> Transmission's "Verify Local Data" functionality, I am able to reproduce this
> consistently. During verification, the percent complete will reverse itself as
> the verification process invalidates pieces that were just downloaded.
>
> I was able to bisect the issue and track it down to this commit:
>
> commit 6de9843dab3f2a1d4d66d80aa9e5782f80977d20
> Author: Feng Tang <feng.tang@...el.com>
> Date:   Wed Mar 23 14:05:03 2011 -0400
>
>    ext4: remove redundant set_buffer_mapped() in ext4_da_get_block_prep()
>
>    The map_bh() call will have already set the buffer_head to mapped.
>
>    Signed-off-by: Feng Tang <feng.tang@...el.com>
>    Signed-off-by: "Theodore Ts'o" <tytso@....edu>
>
> diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
> index f44307a..dec10e2 100644
> --- a/fs/ext4/inode.c
> +++ b/fs/ext4/inode.c
> @@ -2502,7 +2502,6 @@ static int ext4_da_get_block_prep(struct inode *inode,
> sector_t iblock,
>                 * for partial write.
>                 */
>                set_buffer_new(bh);
> -               set_buffer_mapped(bh);
>        }
>        return 0;
>  }
>
>
> I confirmed that reverting this commit makes the problem go away.
>
> Thanks,
> -Damien
>
> --
> Configure bugmail: https://bugzilla.kernel.org/userprefs.cgi?tab=email
> ------- You are receiving this mail because: -------
> You are watching the assignee of the bug.
> --
> To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
> the body of a message to majordomo@...r.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>



-- 
Best Wishes
Yongqiang Yang
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html