On a Thursday in 2026, Daniel P. Berrangé wrote:
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.
The intention was to erase the trailing comma, similar to virBufferTrim(buf, ",');
This writes to regex[-1] when nextents == 0, so is both redundant and causing a bug. Drop the line and we're fine.
Sorry for not self-NACKing earlier, it would have spared you from writing this reply. Jano
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 :|
signature.asc
Description: PGP signature
