[Bug report] partitions/aix: unbounded lvname string read panics the boot partition scan (regression in v7.1)

[Bug report] partitions/aix: unbounded lvname string read panics the boot partition scan (regression in v7.1)

[Guikun Yang]

Hi Jens,

Booting a kernel with CONFIG_AIX_PARTITION=y on a disk whose AIX LVM
logical volume name area contains no NUL byte panics during the
automatic partition scan. The name is read from disk into a
kmalloc(16384) buffer and then handed to seq_buf_printf() as a C
string, so vsnprintf() walks past the end of the allocation:

  BUG: KASAN: use-after-free in string+0x4a9/0x4f0
  Read of size 1 at addr ffff888004ab4000 by task swapper/0/1
   string
   vsnprintf
   seq_buf_printf
   aix_partition+0xd42
   msdos_partition
   bdev_disk_changed
   disk_scan_partitions
   virtblk_probe
  unable to handle page fault for address: ffff888004ab4000
  Oops: 0000 [#1] SMP DEBUG_PAGEALLOC KASAN NOPTI
  Kernel panic - not syncing: Fatal exception

## How the bound was lost

The lack of NUL termination in on-disk lvname strings is a known
property. d43fdae7ba ("partitions/aix: append null character to print
data from disk", 2018) says so explicitly and bounded the pr_warn()
consumer with a local buffer:

  char tmp[sizeof(n[i].name) + 1]; // null char
  snprintf(tmp, sizeof(tmp), "%s", n[i].name);

At that time the other consumer was equally bounded:

  char tmp[70];
  snprintf(tmp, sizeof(tmp), " <%s>\n", n[lv_ix].name);
  strlcat(state->pp_buf, tmp, PAGE_SIZE);

c2d466b9fe ("block: partitions: Replace pp_buf with struct seq_buf")
removed that pair in favour of a direct call, as part of eliminating
local tmp buffers:

-				char tmp[70];
-				snprintf(tmp, sizeof(tmp), " <%s>\n",
-					 n[lv_ix].name);
-				strlcat(state->pp_buf, tmp, PAGE_SIZE);
+				seq_buf_printf(&state->pp_buf, " <%s>\n",
+					       n[lv_ix].name);

seq_buf_printf() bounds the destination, but the source is now the raw
on-disk string with no length limit, so the read is unbounded. The
pr_warn() path kept its 2018 buffer and is still safe; only this
consumer regressed.

alloc_lvn() fills the whole allocation from disk with no terminator
handling (block/partitions/aix.c):

  size_t count = sizeof(struct lvname) * LVM_MAXLVS;   /* 64 * 256 */
  p = kmalloc(count, GFP_KERNEL);
  read_lba(state, lba, (u8 *)p, count);

struct lvname is just char name[64], and lv_ix is bounded by
state->limit (DISK_MAX_PARTS = 256, equal to LVM_MAXLVS), so the array
index itself is in range - it is the string read that runs off the end
of the 16 KiB object.

## Affected versions

Checked by fetching block/partitions/aix.c at each tag:

  v7.0      char tmp[70] + snprintf   bounded
  v7.1      seq_buf_printf            unbounded
  v7.2      seq_buf_printf            unbounded
  v7.3      seq_buf_printf            unbounded
  v7.4-rc2  seq_buf_printf            unbounded

So this is a regression introduced in v7.1 and still present in
mainline.

## Reproduction

The trigger is the boot-time partition scan; no userspace action, no
race, no special mount options. CONFIG_AIX_PARTITION=y is already
enabled in distribution kernels.

An image whose lvname area is filled with non-NUL bytes panics 3/3.
The negative control differs only in having a NUL-terminated short
name: the same code path parses it normally and prints
"vda1 <lv000000>" with no KASAN report.

## Suggested fix

Restore the read bound. Either give the conversion a precision:

-	seq_buf_printf(&state->pp_buf, " <%s>\n", n[lv_ix].name);
+	seq_buf_printf(&state->pp_buf, " <%.*s>\n",
+		       (int)sizeof(n[lv_ix].name), n[lv_ix].name);

or terminate every name once, right after the read in alloc_lvn(),
which also covers any future consumer:

  if (read_lba(state, lba, (u8 *)p, count) < count) {
          kfree(p);
          return NULL;
  }
+ for (i = 0; i < LVM_MAXLVS; i++)
+         p[i].name[sizeof(p[i].name) - 1] = '\0';
  return p;

I prefer the second: the 2018 fix bounded one consumer at a time, and
this regression happened because the other one lost its buffer.
Terminating at the source removes that class of mistake.

Reproducer (image builder for both the positive and negative case,
serial logs) available on request. Happy to send this as a patch if
you prefer.

Fixes: c2d466b9fe ("block: partitions: Replace pp_buf with struct seq_buf")

Thanks,
Guikun Yang