On Fri, Aug 21, 2026 at 12:51:37PM +0200, Ján Tomko via Devel wrote:
> From: Ján Tomko <[email protected]>
> 
> If parsed number of extents is 0, the calculated buffer size
> won't hold the first regex_unit.
> 
> Closes: https://gitlab.com/libvirt/libvirt/-/work_items/913
> Signed-off-by: Ján Tomko <[email protected]>
> ---
>  src/storage/storage_backend_logical.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/src/storage/storage_backend_logical.c 
> b/src/storage/storage_backend_logical.c
> index 51e9337820..81f9d62d8b 100644
> --- a/src/storage/storage_backend_logical.c
> +++ b/src/storage/storage_backend_logical.c
> @@ -161,12 +161,12 @@ 
> virStorageBackendLogicalParseVolExtents(virStorageVolDef *vol,
>  
>      /* Allocate space for 'nextents' regex_unit strings plus a comma for 
> each */
>      regex = g_new0(char, nextents * (strlen(regex_unit) + 1) + 1);
> -    strcat(regex, regex_unit);
> -    for (i = 1; i < nextents; i++) {
> +    for (i = 0; i < nextents; i++) {
>          /* "," is the separator of "devices" field */
> -        strcat(regex, ",");
>          strcat(regex, regex_unit);
> +        strcat(regex, ",");
>      }
> +    regex[strlen(regex) - 1] = '\0';

We allocated 'regex' with 'g_new0', with 1 more byte than we need,
so it is always going to be NUL terminated.

This writes to regex[-1]  when nextents == 0, so is both redundant
and causing a bug. Drop the line and we're fine.


As a more general point, IMHO any code which involves using g_new
with a string length calculation ought to be killed and replaced
with GString so we eliminate the entire class of errors with string
bounds access.

With regards,
Daniel
-- 
|: https://berrange.com       ~~        https://hachyderm.io/@berrange :|
|: https://libvirt.org          ~~          https://entangle-photo.org :|
|: https://pixelfed.art/berrange   ~~    https://fstop138.berrange.com :|

Reply via email to