Re: [PATCH] partitions: aix: bound the lvd scan to one sector

[Hannes Reinecke <hare@suse.de>]

On 6/6/26 7:07 PM, Michael Bommarito wrote:
> aix_partition() reads the logical-volume descriptor array as a single
> sector and then scans it:
> 
> 	if (numlvs && (d = read_part_sector(state, vgda_sector + 1, &sect))) {
> 		struct lvd *p = (struct lvd *)d;
> 		...
> 		for (i = 0; foundlvs < numlvs && i < state->limit; i += 1) {
> 			lvip[i].pps_per_lv = be16_to_cpu(p[i].num_lps);
> 
> p points at a single 512-byte sector, which holds 512 / sizeof(struct
> lvd) = 16 entries, but the loop runs until foundlvs reaches the on-disk
> numlvs or i reaches state->limit (DISK_MAX_PARTS, 256). numlvs is an
> on-disk __be16 read straight from the volume group descriptor and is not
> validated, so a crafted AIX image with numlvs larger than 16 and lvd
> entries whose num_lps fields are zero (so foundlvs never advances) drives
> the loop to read p[i] well past the end of the read sector buffer.
> 
> The 2014 off-by-one fix d97a86c170b4 hardened the matching write of
> lvip[lv_ix] but left this read loop unbounded.
> 
> Bound the scan to the number of struct lvd entries that fit in the
> sector that was actually read.
> 
> Fixes: 6ceea22bbbc8 ("partitions: add aix lvm partition support files")
> Assisted-by: Claude:claude-opus-4-8
> Signed-off-by: Michael Bommarito <michael.bommarito@gmail.com>
> ---
>   block/partitions/aix.c | 9 ++++++++-
>   1 file changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/block/partitions/aix.c b/block/partitions/aix.c
> index 29b8f4cebb63d..6679e825ba329 100644
> --- a/block/partitions/aix.c
> +++ b/block/partitions/aix.c
> @@ -208,7 +208,14 @@ int aix_partition(struct parsed_partitions *state)
>   		if (n) {
>   			int foundlvs = 0;
>   
> -			for (i = 0; foundlvs < numlvs && i < state->limit; i += 1) {
> +			/*
> +			 * The lvd array was read as a single sector; only the
> +			 * struct lvd entries that fit in it are valid.  Bound the
> +			 * scan so an on-disk numlvs larger than that cannot walk
> +			 * the read buffer out of bounds.
> +			 */
> +			for (i = 0; foundlvs < numlvs && i < state->limit &&
> +				    i < 512 / (int)sizeof(struct lvd); i += 1) {

I would use SECTOR_SIZE here. And maybe 'i++' instead of 'i += 1'.
>   				lvip[i].pps_per_lv = be16_to_cpu(p[i].num_lps);
>   				if (lvip[i].pps_per_lv)
>   					foundlvs += 1;
> 
> base-commit: 5200f5f493f79f14bbdc349e402a40dfb32f23c8

Cheers,

Hannes
-- 
Dr. Hannes Reinecke                  Kernel Storage Architect
hare@suse.de                                +49 911 74053 688
SUSE Software Solutions GmbH, Frankenstr. 146, 90461 Nürnberg
HRB 36809 (AG Nürnberg), GF: I. Totev, A. McDonald, W. Knoblich