[PATCH 1/1] block: partition: aix: bound LV name formatting

[PATCH 1/1] block: partition: aix: bound LV name formatting

[Ren Wei]

From: Zhiling Zou <roxy520tt@gmail.com>

AIX logical volume names are stored on disk as fixed-size fields.
The partition parser reads them into struct lvname, but later formats
the fields with %s. If an on-disk name is not NUL-terminated, the string
formatting code can keep reading past the end of the 64-byte name field.

Limit the formatted string length to the size of the on-disk name field
when printing AIX logical volume names.

Fixes: 6ceea22bbbc8 ("partitions: add aix lvm partition support files")
Cc: stable@vger.kernel.org
Reported-by: Yuan Tan <yuantan098@gmail.com>
Reported-by: Xin Liu <bird@lzu.edu.cn>
Assisted-by: Codex:gpt-5.4
Signed-off-by: Zhiling Zou <roxy520tt@gmail.com>
Signed-off-by: Ren Wei <n05ec@lzu.edu.cn>
---
 block/partitions/aix.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/block/partitions/aix.c b/block/partitions/aix.c
index f3c4174e003e..de19c19c85b2 100644
--- a/block/partitions/aix.c
+++ b/block/partitions/aix.c
@@ -261,7 +261,8 @@ int aix_partition(struct parsed_partitions *state)
 				put_partition(state, lv_ix + 1,
 				  (i + 1 - lp_ix) * pp_blocks_size + psn_part1,
 				  lvip[lv_ix].pps_per_lv * pp_blocks_size);
-				seq_buf_printf(&state->pp_buf, " <%s>\n",
+				seq_buf_printf(&state->pp_buf, " <%.*s>\n",
+					       (int)sizeof(n[lv_ix].name),
 					       n[lv_ix].name);
 				lvip[lv_ix].lv_is_contiguous = 1;
 				ret = 1;
@@ -273,7 +274,8 @@ int aix_partition(struct parsed_partitions *state)
 			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);
+				snprintf(tmp, sizeof(tmp), "%.*s",
+					 (int)sizeof(n[i].name), n[i].name);
 				pr_warn("partition %s (%u pp's found) is "
 					"not contiguous\n",
 					tmp, lvip[i].pps_found);
-- 
2.43.0

Re: [PATCH 1/1] block: partition: aix: bound LV name formatting

[Philippe De Muyter]

Hello Ren Wei,

On Fri, Jun 26, 2026 at 03:21:22PM +0800, Ren Wei wrote:
> From: Zhiling Zou <roxy520tt@gmail.com>
> 
> AIX logical volume names are stored on disk as fixed-size fields.
> The partition parser reads them into struct lvname, but later formats
> the fields with %s. If an on-disk name is not NUL-terminated, the string
> formatting code can keep reading past the end of the 64-byte name field.
> 
> Limit the formatted string length to the size of the on-disk name field
> when printing AIX logical volume names.
> 
> Fixes: 6ceea22bbbc8 ("partitions: add aix lvm partition support files")
> Cc: stable@vger.kernel.org
> Reported-by: Yuan Tan <yuantan098@gmail.com>
> Reported-by: Xin Liu <bird@lzu.edu.cn>
> Assisted-by: Codex:gpt-5.4
> Signed-off-by: Zhiling Zou <roxy520tt@gmail.com>
> Signed-off-by: Ren Wei <n05ec@lzu.edu.cn>
> ---
>  block/partitions/aix.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/block/partitions/aix.c b/block/partitions/aix.c
> index f3c4174e003e..de19c19c85b2 100644
> --- a/block/partitions/aix.c
> +++ b/block/partitions/aix.c
> @@ -261,7 +261,8 @@ int aix_partition(struct parsed_partitions *state)
>  				put_partition(state, lv_ix + 1,
>  				  (i + 1 - lp_ix) * pp_blocks_size + psn_part1,
>  				  lvip[lv_ix].pps_per_lv * pp_blocks_size);
> -				seq_buf_printf(&state->pp_buf, " <%s>\n",
> +				seq_buf_printf(&state->pp_buf, " <%.*s>\n",
> +					       (int)sizeof(n[lv_ix].name),
>  					       n[lv_ix].name);
>  				lvip[lv_ix].lv_is_contiguous = 1;
>  				ret = 1;
> @@ -273,7 +274,8 @@ int aix_partition(struct parsed_partitions *state)
>  			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);
> +				snprintf(tmp, sizeof(tmp), "%.*s",
> +					 (int)sizeof(n[i].name), n[i].name);

Is this change necessary ? snprintf always adds a NULL terminator and
truncates the input if needed, isn't it ?

>  				pr_warn("partition %s (%u pp's found) is "
>  					"not contiguous\n",
>  					tmp, lvip[i].pps_found);
> -- 
> 2.43.0

Best Regards

Philippe De Muyter

Re: [PATCH 1/1] block: partition: aix: bound LV name formatting

[tt roxy]

On Sat, Jun 27, 2026 at 4:50 AM Philippe De Muyter <phdm@macqel.be> wrote:
>
> Hello Ren Wei,
>
> On Fri, Jun 26, 2026 at 03:21:22PM +0800, Ren Wei wrote:
> > From: Zhiling Zou <roxy520tt@gmail.com>
> >
> > AIX logical volume names are stored on disk as fixed-size fields.
> > The partition parser reads them into struct lvname, but later formats
> > the fields with %s. If an on-disk name is not NUL-terminated, the string
> > formatting code can keep reading past the end of the 64-byte name field.
> >
> > Limit the formatted string length to the size of the on-disk name field
> > when printing AIX logical volume names.
> >
> > Fixes: 6ceea22bbbc8 ("partitions: add aix lvm partition support files")
> > Cc: stable@vger.kernel.org
> > Reported-by: Yuan Tan <yuantan098@gmail.com>
> > Reported-by: Xin Liu <bird@lzu.edu.cn>
> > Assisted-by: Codex:gpt-5.4
> > Signed-off-by: Zhiling Zou <roxy520tt@gmail.com>
> > Signed-off-by: Ren Wei <n05ec@lzu.edu.cn>
> > ---
> >  block/partitions/aix.c | 6 ++++--
> >  1 file changed, 4 insertions(+), 2 deletions(-)
> >
> > diff --git a/block/partitions/aix.c b/block/partitions/aix.c
> > index f3c4174e003e..de19c19c85b2 100644
> > --- a/block/partitions/aix.c
> > +++ b/block/partitions/aix.c
> > @@ -261,7 +261,8 @@ int aix_partition(struct parsed_partitions *state)
> >                               put_partition(state, lv_ix + 1,
> >                                 (i + 1 - lp_ix) * pp_blocks_size + psn_part1,
> >                                 lvip[lv_ix].pps_per_lv * pp_blocks_size);
> > -                             seq_buf_printf(&state->pp_buf, " <%s>\n",
> > +                             seq_buf_printf(&state->pp_buf, " <%.*s>\n",
> > +                                            (int)sizeof(n[lv_ix].name),
> >                                              n[lv_ix].name);
> >                               lvip[lv_ix].lv_is_contiguous = 1;
> >                               ret = 1;
> > @@ -273,7 +274,8 @@ int aix_partition(struct parsed_partitions *state)
> >                       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);
> > +                             snprintf(tmp, sizeof(tmp), "%.*s",
> > +                                      (int)sizeof(n[i].name), n[i].name);
>
> Is this change necessary ? snprintf always adds a NULL terminator and
> truncates the input if needed, isn't it ?
>

Yes, snprintf() guarantees that the destination buffer is
NUL-terminated and truncates the output.

However, with %s and no precision, snprintf() still treats n[i].name as
a NUL-terminated source string. If the on-disk fixed-size name field is
not NUL-terminated, snprintf() can keep reading past the end of
n[i].name while looking for the terminator, before truncating the output
into tmp.

So the issue is not an overflow of tmp, but an out-of-bounds read from
the fixed-width source field. The precision is needed to bound that
source read to sizeof(n[i].name).

> >                               pr_warn("partition %s (%u pp's found) is "
> >                                       "not contiguous\n",
> >                                       tmp, lvip[i].pps_found);
> > --
> > 2.43.0
>
> Best Regards
>
> Philippe De Muyter

Re: [PATCH 1/1] block: partition: aix: bound LV name formatting

[Philippe De Muyter]

Hello tt roxy,

On Sat, Jun 27, 2026 at 12:37:50PM +0800, tt roxy wrote:
> On Sat, Jun 27, 2026 at 4:50 AM Philippe De Muyter <phdm@macqel.be> wrote:
> >
> > Hello Ren Wei,
> >
> > On Fri, Jun 26, 2026 at 03:21:22PM +0800, Ren Wei wrote:
> > > From: Zhiling Zou <roxy520tt@gmail.com>
> > > @@ -273,7 +274,8 @@ int aix_partition(struct parsed_partitions *state)
> > >                       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);
> > > +                             snprintf(tmp, sizeof(tmp), "%.*s",
> > > +                                      (int)sizeof(n[i].name), n[i].name);
> >
> > Is this change necessary ? snprintf always adds a NULL terminator and
> > truncates the input if needed, isn't it ?
> >
> 
> Yes, snprintf() guarantees that the destination buffer is
> NUL-terminated and truncates the output.
> 
> However, with %s and no precision, snprintf() still treats n[i].name as
> a NUL-terminated source string. If the on-disk fixed-size name field is
> not NUL-terminated, snprintf() can keep reading past the end of
> n[i].name while looking for the terminator, before truncating the output
> into tmp.
> 
> So the issue is not an overflow of tmp, but an out-of-bounds read from
> the fixed-width source field. The precision is needed to bound that
> source read to sizeof(n[i].name).
> 
> > >                               pr_warn("partition %s (%u pp's found) is "
> > >                                       "not contiguous\n",
> > >                                       tmp, lvip[i].pps_found);
> > > --

Actually the 'tmp' buffer is not needed.  Printing directky n[i].name
with '%.*s' in pr_warn would be enough to fix the problem IMHO.

Philippe