This is a repost because I rebased the change. The first attempt was
done with the email "[BTRFS-PROGS][BUG][PATCH] Incorrect detection of a
removed device [was Re: “Bug”-report: inconsistency kernel <-> tools]"
dated 08/31/2012.
In the function btrfs_read_dev_super() it is possible to use the
variable fsid without initialisation.
In btrfs_read_dev_super(), during the scan of the superblock the
variable fsid is initialised with the value of fsid of the first
superblock. But if the first superblock contains an incorrect signature
this initialisation is skipped.
int btrfs_read_dev_super(int fd, struct btrfs_super_block *sb, u64
sb_bytenr)
{
u8 fsid[BTRFS_FSID_SIZE];
[...]
line 933:
for (i = 0; i < BTRFS_SUPER_MIRROR_MAX; i++) {
bytenr = btrfs_sb_offset(i);
ret = pread64(fd, &buf, sizeof(buf), bytenr);
if (ret < sizeof(buf))
break;
if (btrfs_super_bytenr(&buf) != bytenr ||
strncmp((char *)(&buf.magic), BTRFS_MAGIC,
sizeof(buf.magic)))
continue;
if (i == 0)
memcpy(fsid, buf.fsid, sizeof(fsid));
else if (memcmp(fsid, buf.fsid, sizeof(fsid)))
continue;
[...]
}
This bug could be triggered by the command "btrfs device delete", which
zeros the signature of the first superblock.
The enclosed patch corrects the initialisation of the fsid variable;
moreover if the fsid are different between the superblocks (the original
one and its backups) the function fails because the device cannot be
trusted. Finally it is handled the special case when the magic fields is
zeroed in the *first* superblock. In this case the device is skipped.
Please apply, thank.
You can pull from
http://cassiopea.homelinux.net/git/btrfs-progs-unstable.git
branch
btrfs_read_dev_super-bug
BR
G.Baroncelli
--
Signed-off-by: Goffredo Baroncelli <kreij...@inwind.it>
diff --git a/disk-io.c b/disk-io.c
index b21a87f..82fc3b8 100644
--- a/disk-io.c
+++ b/disk-io.c
@@ -910,6 +910,7 @@ struct btrfs_root *open_ctree_fd(int fp, const char
*path, u64 sb_bytenr,
int btrfs_read_dev_super(int fd, struct btrfs_super_block *sb, u64
sb_bytenr)
{
u8 fsid[BTRFS_FSID_SIZE];
+ int fsid_is_initialized = 0;
struct btrfs_super_block buf;
int i;
int ret;
@@ -936,15 +937,26 @@ int btrfs_read_dev_super(int fd, struct
btrfs_super_block *sb, u64 sb_bytenr)
if (ret < sizeof(buf))
break;
- if (btrfs_super_bytenr(&buf) != bytenr ||
- strncmp((char *)(&buf.magic), BTRFS_MAGIC,
+ if (btrfs_super_bytenr(&buf) != bytenr )
+ continue;
+ /* if magic is NULL, the device was removed */
+ if (buf.magic == 0 && i==0)
+ return -1;
+ if (strncmp((char *)(&buf.magic), BTRFS_MAGIC,
sizeof(buf.magic)))
continue;
- if (i == 0)
+ if (!fsid_is_initialized){
memcpy(fsid, buf.fsid, sizeof(fsid));
- else if (memcmp(fsid, buf.fsid, sizeof(fsid)))
- continue;
+ fsid_is_initialized = 1;
+ } else if (memcmp(fsid, buf.fsid, sizeof(fsid))) {
+ /*
+ * the superblocks (the original one and
+ * its backups) contain data of different
+ * filesystems -> the disk cannot be trusted
+ */
+ return -1;
+ }
if (btrfs_super_generation(&buf) > transid) {
memcpy(sb, &buf, sizeof(*sb));
diff --git a/disk-io.c b/disk-io.c
index b21a87f..82fc3b8 100644
--- a/disk-io.c
+++ b/disk-io.c
@@ -910,6 +910,7 @@ struct btrfs_root *open_ctree_fd(int fp, const char *path, u64 sb_bytenr,
int btrfs_read_dev_super(int fd, struct btrfs_super_block *sb, u64 sb_bytenr)
{
u8 fsid[BTRFS_FSID_SIZE];
+ int fsid_is_initialized = 0;
struct btrfs_super_block buf;
int i;
int ret;
@@ -936,15 +937,26 @@ int btrfs_read_dev_super(int fd, struct btrfs_super_block *sb, u64 sb_bytenr)
if (ret < sizeof(buf))
break;
- if (btrfs_super_bytenr(&buf) != bytenr ||
- strncmp((char *)(&buf.magic), BTRFS_MAGIC,
+ if (btrfs_super_bytenr(&buf) != bytenr )
+ continue;
+ /* if magic is NULL, the device was removed */
+ if (buf.magic == 0 && i==0)
+ return -1;
+ if (strncmp((char *)(&buf.magic), BTRFS_MAGIC,
sizeof(buf.magic)))
continue;
- if (i == 0)
+ if (!fsid_is_initialized){
memcpy(fsid, buf.fsid, sizeof(fsid));
- else if (memcmp(fsid, buf.fsid, sizeof(fsid)))
- continue;
+ fsid_is_initialized = 1;
+ } else if (memcmp(fsid, buf.fsid, sizeof(fsid))) {
+ /*
+ * the superblocks (the original one and
+ * its backups) contain data of different
+ * filesystems -> the disk cannot be trusted
+ */
+ return -1;
+ }
if (btrfs_super_generation(&buf) > transid) {
memcpy(sb, &buf, sizeof(*sb));
