[PATCH 0/2] partitions/aix: fix problems due to disk corruption

[PATCH 0/2] partitions/aix: fix problems due to disk corruption

[Mauricio Faria de Oliveira]

We've recently received a disk image from an AIX LUN that when
attached on Linux displayed errors on console, then eventually
hung the system.

Apparently the LUN was originally installed with AIX and later
exercised with some I/O stress/overwrites which caused certain
bits to be wrong in just the right way for Linux to get a NULL
pointer and invalid data.

This is the test-case used ('--partscan' is the important bit).
  $ sudo losetup --show --find --partscan aix-lun.img

Patch 1 resolves the particular problem the disk image has.
Patch 2 improves the code a bit further (tested synthetically).

Mauricio Faria de Oliveira (2):
  partitions/aix: fix usage of uninitialized lv_info and lvname
    structures
  partitions/aix: append null character to print data from disk

 block/partitions/aix.c | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

-- 
2.17.1

[PATCH 1/2] partitions/aix: fix usage of uninitialized lv_info and lvname structures

[Mauricio Faria de Oliveira]

The if-block that sets a successful return value in aix_partition()
uses 'lvip[].pps_per_lv' and 'n[].name' potentially uninitialized.

For example, if 'numlvs' is zero or alloc_lvn() fails, neither is
initialized, but are used anyway if alloc_pvd() succeeds after it.

So, make the alloc_pvd() call conditional on their initialization.

This has been hit when attaching an apparently corrupted/stressed
AIX LUN, misleading the kernel to pr_warn() invalid data and hang.

    [...] partition (null) (11 pp's found) is not contiguous
    [...] partition (null) (2 pp's found) is not contiguous
    [...] partition (null) (3 pp's found) is not contiguous
    [...] partition (null) (64 pp's found) is not contiguous

Fixes: 6ceea22bbbc8 ("partitions: add aix lvm partition support files")
Signed-off-by: Mauricio Faria de Oliveira <mfo@canonical.com>
---
 block/partitions/aix.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/block/partitions/aix.c b/block/partitions/aix.c
index 007f95eea0e1..850cbd1860d4 100644
--- a/block/partitions/aix.c
+++ b/block/partitions/aix.c
@@ -178,7 +178,7 @@ int aix_partition(struct parsed_partitions *state)
 	u32 vgda_sector = 0;
 	u32 vgda_len = 0;
 	int numlvs = 0;
-	struct pvd *pvd;
+	struct pvd *pvd = NULL;
 	struct lv_info {
 		unsigned short pps_per_lv;
 		unsigned short pps_found;
@@ -232,10 +232,11 @@ int aix_partition(struct parsed_partitions *state)
 				if (lvip[i].pps_per_lv)
 					foundlvs += 1;
 			}
+			/* pvd loops depend on n[].name and lvip[].pps_per_lv */
+			pvd = alloc_pvd(state, vgda_sector + 17);
 		}
 		put_dev_sector(sect);
 	}
-	pvd = alloc_pvd(state, vgda_sector + 17);
 	if (pvd) {
 		int numpps = be16_to_cpu(pvd->pp_count);
 		int psn_part1 = be32_to_cpu(pvd->psn_part1);
-- 
2.17.1

[PATCH 2/2] partitions/aix: append null character to print data from disk

[Mauricio Faria de Oliveira]

Even if properly initialized, the lvname array (i.e., strings)
is read from disk, and might contain corrupt data (e.g., lack
the null terminating character for strings).

So, make sure the partition name string used in pr_warn() has
the null terminating character.

Fixes: 6ceea22bbbc8 ("partitions: add aix lvm partition support files")
Suggested-by: Daniel J. Axtens <daniel.axtens@canonical.com>
Signed-off-by: Mauricio Faria de Oliveira <mfo@canonical.com>
---
 block/partitions/aix.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/block/partitions/aix.c b/block/partitions/aix.c
index 850cbd1860d4..903f3ed175d0 100644
--- a/block/partitions/aix.c
+++ b/block/partitions/aix.c
@@ -283,10 +283,14 @@ int aix_partition(struct parsed_partitions *state)
 				next_lp_ix += 1;
 		}
 		for (i = 0; i < state->limit; i += 1)
-			if (lvip[i].pps_found && !lvip[i].lv_is_contiguous)
+			if (lvip[i].pps_found && !lvip[i].lv_is_contiguous) {
+				char tmp[sizeof(n[i].name) + 1]; // null char
+
+				snprintf(tmp, sizeof(tmp), "%s", n[i].name);
 				pr_warn("partition %s (%u pp's found) is "
 					"not contiguous\n",
-					n[i].name, lvip[i].pps_found);
+					tmp, lvip[i].pps_found);
+			}
 		kfree(pvd);
 	}
 	kfree(n);
-- 
2.17.1

Re: [PATCH 0/2] partitions/aix: fix problems due to disk corruption

[Jens Axboe]

On 7/25/18 7:46 PM, Mauricio Faria de Oliveira wrote:
> We've recently received a disk image from an AIX LUN that when
> attached on Linux displayed errors on console, then eventually
> hung the system.
> 
> Apparently the LUN was originally installed with AIX and later
> exercised with some I/O stress/overwrites which caused certain
> bits to be wrong in just the right way for Linux to get a NULL
> pointer and invalid data.
> 
> This is the test-case used ('--partscan' is the important bit).
>   $ sudo losetup --show --find --partscan aix-lun.img
> 
> Patch 1 resolves the particular problem the disk image has.
> Patch 2 improves the code a bit further (tested synthetically).
> 
> Mauricio Faria de Oliveira (2):
>   partitions/aix: fix usage of uninitialized lv_info and lvname
>     structures
>   partitions/aix: append null character to print data from disk
> 
>  block/partitions/aix.c | 13 +++++++++----
>  1 file changed, 9 insertions(+), 4 deletions(-)

Looks good to me, I have applied for 4.19.

-- 
Jens Axboe