On 27.03.2018 14:30, Nikolay Borisov wrote:
>
>
> On 26.03.2018 11:27, Anand Jain wrote:
>> During the scan context, we aren't verifying the superblock-
>> checksum when read.
>> This patch fixes it by adding the checksum verification function
>> btrfs_check_super_csum() in the function btrfs_read_disk_super().
>> And makes device scan to error fail if the primary superblock csum
>> is wrong, whereas if the copy-superblock csum is wrong it will just
>> just report missmatch and continue mount/scan as usual. When the
>> mount is successful We anyway overwrite all superblocks upon unmount.
>>
>> The context in which this will be called is - device scan, device ready,
>> and mount -o device option.
>>
>> Test script:
>>
>> Corrupt primary superblock and check if device scan and mount
>> fails:
>> mkfs.btrfs -fq /dev/sdc
>> dd if=/dev/urandom of=/dev/sdc ibs=1 obs=1 count=1 seek=64K
>> btrfs dev scan
>> mount /dev/sdb /btrfs
>>
>> Corrupt secondary superblock and check if device scan and mount
>> is succcessful, check for the dmesg for errors.
>> mkfs.btrfs -fq /dev/sdc
>> dd if=/dev/urandom of=/dev/sdc ibs=1 obs=1 count=1 seek=64K
>> btrfs dev scan
>> mount /dev/sdb /btrfs
>
> Have you considered adding fstests, it will be very easy to test for
> this behavior?
>
>>
>> Signed-off-by: Anand Jain <anand.jain@xxxxxxxxxx>
>> ---
>> fs/btrfs/volumes.c | 15 +++++++++++++++
>> 1 file changed, 15 insertions(+)
>>
>> diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
>> index 45dd0674571b..384e503c83ff 100644
>> --- a/fs/btrfs/volumes.c
>> +++ b/fs/btrfs/volumes.c
>> @@ -1149,6 +1149,7 @@ static int btrfs_read_disk_super(struct block_device *bdev, u64 bytenr,
>> struct page **page,
>> struct btrfs_super_block **disk_super)
>> {
>> + int err;
>> void *p;
>> pgoff_t index;
>>
>> @@ -1177,6 +1178,20 @@ static int btrfs_read_disk_super(struct block_device *bdev, u64 bytenr,
>> /* align our pointer to the offset of the super block */
>> *disk_super = p + (bytenr & ~PAGE_MASK);
>>
>> + err = btrfs_check_super_csum((char *) *disk_super);
>> + if (err) {
> if (err < 0)
>> + if (err == -EINVAL)
>> + pr_err("BTRFS error (device %pg): "\
>> + "unsupported checksum type, bytenr=%llu",
>> + bdev, bytenr);
>
> Same comment as before regarding string literals splitting across lines.
>
>> + else
>> + pr_err("BTRFS error (device %pg): "\
>> + "superblock checksum failed, bytenr=%llu",
>> + bdev, bytenr);
>> + btrfs_release_disk_super(*page);
>> + return err;
>> + }
Also it will be better to have the checksum check after we have verified
some basic invariants - that bytenr and magic have sane values.
>> +
>> if (btrfs_super_bytenr(*disk_super) != bytenr ||
>> btrfs_super_magic(*disk_super) != BTRFS_MAGIC) {
>> btrfs_release_disk_super(*page);
>>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
