RE: [PATCH] xfs: Fix possible truncation of log data in xlog_bread_noalign()

Tony Lu
2013-02-26T07:28:57Z

[-- Attachment #1: Type: text/plain, Size: 4131 bytes --]

I get a reliable way to reproduce this bug. The logprint and metadump are attached.

Kernel version: 2.6.38.8
Mkfs.xfs version: xfsprogs-3.1.1
mkfs.xfs -s size=4096 /dev/sda1

Run the following mount-cp-umount script to reproduce:
#!/bin/sh
device=/dev/sda1
mount_point=/mnt
times=10

for ((num=1;num<=$times;num++))
do
echo "$num mount $device $mount_point"
mount $device $mount_point

echo "cp -rf /bin $mount_point/$num"
cp -rf /bin $mount_point/$num

echo "$num umount $device $mount_point"
umount $mount_point

#num=$(($num + 1))
done

After several times of mount/cp/umount, this xfs crashes, and the xfs partition can not be mounted any more. Here is the output of console.
-sh-4.1# ./umount-test
1 mount /dev/sda1 /mnt
XFS mounting filesystem sda1
cp -rf /bin /mnt/1
1 umount /dev/sda1 /mnt
2 mount /dev/sda1 /mnt
XFS mounting filesystem sda1
cp -rf /bin /mnt/2
2 umount /dev/sda1 /mnt
3 mount /dev/sda1 /mnt
XFS mounting filesystem sda1
cp -rf /bin /mnt/3
3 umount /dev/sda1 /mnt
4 mount /dev/sda1 /mnt
XFS mounting filesystem sda1
cp -rf /bin /mnt/4
4 umount /dev/sda1 /mnt
5 mount /dev/sda1 /mnt
XFS mounting filesystem sda1
Starting XFS recovery on filesystem: sda1 (logdev: internal)
Ending XFS recovery on filesystem: sda1 (logdev: internal)cp -rf /bin /mnt/5
5 umount /dev/sda1 /mnt
6 mount /dev/sda1 /mnt

XFS mounting filesystem sda1
Starting XFS recovery on filesystem: sda1 (logdev: internal)
Ending XFS recovery on filesystem: sda1 (logdev: internal)Interrupt
cp -rf /bin /mnt/6
6 umount /dev/sda1 /mnt
7 mount /dev/sda1 /mnt

XFS mounting filesystem sda1
cp -rf /bin /mnt/7
7 umount /dev/sda1 /mnt
Interrupt
8 mount /dev/sda1 /mnt
XFS mounting filesystem sda1
Starting XFS recovery on filesystem: sda1 (logdev: internal)
XFS: xlog_recover_process_data: bad clientid
XFS: log mount/recovery failed: error 5
XFS: log mount failed

Thanks
-Tony
>-----Original Message-----
>From: Dave Chinner [mailto:david@fromorbit.com]
>Sent: Sunday, February 24, 2013 10:10 PM
>To: Tony Lu
>Cc: Ben Myers; xfs@oss.sgi.com; Alex Elder; Dave Chinner;
>linux-fsdevel@vger.kernel.org; linux-kernel@vger.kernel.org; Chris Metcalf
>Subject: Re: [PATCH] xfs: Fix possible truncation of log data in
>xlog_bread_noalign()
>
>On Sun, Feb 24, 2013 at 04:46:30AM +0000, Tony Lu wrote:
>> >> For example, if xlog_bread_noalign() wants to read blocks from #1
>> >> to # 9, in which case the passed parameter blk_no is 1, and nbblks
>> >> is 8, sectBBsize is 8, after the round down and round up
>> >> operations, we get blk_no as 0, and nbblks as still 8. We
>> >> definitely lose the last block of the log data.
>> >
>> >Yes, I fully understand that. But I also understand how the log
>> >works and that this behaviour *should not happen*. That's why
>> >I'm asking questions about what the problem you are trying to fix.
>>
>> I am not sure about this, since I saw many reads on
>> non-sector-align blocks even when successfully mounting good XFS
>> partitions.
>
>I didn't say that non-sector align reads should not be attempted by
>log recovery - it's obvious from the on disk format of the log that
>we have to parse it in chunks of 512 bytes to make sense of it's
>contents, and that leads to the 512 byte reads and other subsequent
>unaligned reads.
>
>*However*
>
>Seeing that there are unaligned reads occurring does not mean that
>the structures in the log should be unaligned. Your test output
>indicated a log record header at an unaligned block address, and
>that's incorrect. It doesn't matter what the rest of the log
>recovery code does with non-aligned IO - the fact is that your debug
>implies that the contents of the log is corrupt and that implies a
>deeper problem....
>
>> And also there is code in xlog_write_log_records() which handles
>> non-sector-align reads and writes.
>
>Yes, it does handle it, but that doesn't mean that it is correct to
>pass unaligned block ranges to it.
>
>Cheers,
>
>Dave.
>
>--
>Dave Chinner
>david@fromorbit.com

[-- Attachment #2: metadump.tar.gz --]
[-- Type: application/x-gzip, Size: 539952 bytes --]

[-- Attachment #3: Type: text/plain, Size: 121 bytes --]