[PATCH 1/3] util: Fix uninitialized variable in virSocketAddrFormatWithPrefix

Julio Faracco posted 3 patches 1 month ago
[PATCH 1/3] util: Fix uninitialized variable in virSocketAddrFormatWithPrefix
Posted by Julio Faracco 1 month ago
The virSocketAddrFormatWithPrefix() function has a bug where the
'network' variable is left uninitialized when masked=false. This
occurs because the function only assigns to 'network' inside the
masked=true conditional branch.

When masked=false, the caller wants to format the original address
with a prefix notation (e.g., "1.2.3.4/24") without applying the
network mask. However, the code was only initializing 'network'
when masking was requested, causing the subsequent
virSocketAddrFormat(&network) call to operate on uninitialized data.

Fix this by adding an else branch that copies the original address
to 'network' when masking is not requested. This ensures 'network'
is properly initialized in both code paths.

Signed-off-by: Julio Faracco <jcfaracco@gmail.com>
---
 src/util/virsocketaddr.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/src/util/virsocketaddr.c b/src/util/virsocketaddr.c
index f53768878e..80ee3b4c51 100644
--- a/src/util/virsocketaddr.c
+++ b/src/util/virsocketaddr.c
@@ -549,10 +549,14 @@ virSocketAddrFormatWithPrefix(virSocketAddr *addr,
         return NULL;
     }
 
-    if (masked && virSocketAddrMaskByPrefix(addr, prefix, &network) < 0) {
-        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
-                       _("Failure to mask address"));
-        return NULL;
+    if (masked) {
+	if (virSocketAddrMaskByPrefix(addr, prefix, &network) < 0) {
+            virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+                           _("Failure to mask address"));
+            return NULL;
+	}
+    } else {
+        network = *addr;
     }
 
     netstr = virSocketAddrFormat(&network);
-- 
2.52.0
Re: [PATCH 1/3] util: Fix uninitialized variable in virSocketAddrFormatWithPrefix
Posted by Ján Tomko via Devel 3 weeks, 2 days ago
On a Monday in 2026, Julio Faracco wrote:
>The virSocketAddrFormatWithPrefix() function has a bug where the
>'network' variable is left uninitialized when masked=false. This
>occurs because the function only assigns to 'network' inside the
>masked=true conditional branch.
>
>When masked=false, the caller wants to format the original address

There is no such caller, ever since its introduction in:
commit 426afc0082f1d28449380a5c9260d64ed7183e38
     util: rename/move iptablesFormatNetwork to virSocketAddrFormatWithPrefix
we always passed masked=true.

I think dropping the "masked" argument is easier here.

Also, calling it "unitialized" evokes some kind of omission that made
the function work by accident. Here, the "addr" is never used
so the function would not even work.

Jano

>with a prefix notation (e.g., "1.2.3.4/24") without applying the
>network mask. However, the code was only initializing 'network'
>when masking was requested, causing the subsequent
>virSocketAddrFormat(&network) call to operate on uninitialized data.
>
>Fix this by adding an else branch that copies the original address
>to 'network' when masking is not requested. This ensures 'network'
>is properly initialized in both code paths.
>
>Signed-off-by: Julio Faracco <jcfaracco@gmail.com>
>---
> src/util/virsocketaddr.c | 12 ++++++++----
> 1 file changed, 8 insertions(+), 4 deletions(-)
Re: [PATCH 1/3] util: Fix uninitialized variable in virSocketAddrFormatWithPrefix
Posted by Julio Faracco 3 weeks, 2 days ago
Em sex., 16 de jan. de 2026 às 13:55, Ján Tomko <jtomko@redhat.com> escreveu:
>
> On a Monday in 2026, Julio Faracco wrote:
> >The virSocketAddrFormatWithPrefix() function has a bug where the
> >'network' variable is left uninitialized when masked=false. This
> >occurs because the function only assigns to 'network' inside the
> >masked=true conditional branch.
> >
> >When masked=false, the caller wants to format the original address
>
> There is no such caller, ever since its introduction in:
> commit 426afc0082f1d28449380a5c9260d64ed7183e38
>      util: rename/move iptablesFormatNetwork to virSocketAddrFormatWithPrefix
> we always passed masked=true.
>
> I think dropping the "masked" argument is easier here.

I was thinking of renaming the function name to something like:
virSocketAddrFormatWithMask and drop the argument, but I would like to
see opinions first.
Seems the right way (drop the argument) based on your comments and context.

>
> Also, calling it "unitialized" evokes some kind of omission that made
> the function work by accident. Here, the "addr" is never used
> so the function would not even work.
>
> Jano
>
> >with a prefix notation (e.g., "1.2.3.4/24") without applying the
> >network mask. However, the code was only initializing 'network'
> >when masking was requested, causing the subsequent
> >virSocketAddrFormat(&network) call to operate on uninitialized data.
> >
> >Fix this by adding an else branch that copies the original address
> >to 'network' when masking is not requested. This ensures 'network'
> >is properly initialized in both code paths.

ACK. Submitting a V2 with masked code only makes more sense.

> >
> >Signed-off-by: Julio Faracco <jcfaracco@gmail.com>
> >---
> > src/util/virsocketaddr.c | 12 ++++++++----
> > 1 file changed, 8 insertions(+), 4 deletions(-)
Re: [PATCH 1/3] util: Fix uninitialized variable in virSocketAddrFormatWithPrefix
Posted by Julio Faracco 1 month ago
Waiting for feedback, but this is interesting because I don't see any usage of:

virSocketAddrFormatWithPrefix(..., ..., false)

Perhaps, we need to change the scope of this function...

Em seg., 5 de jan. de 2026 às 10:29, Julio Faracco
<jcfaracco@gmail.com> escreveu:

>
> The virSocketAddrFormatWithPrefix() function has a bug where the
> 'network' variable is left uninitialized when masked=false. This
> occurs because the function only assigns to 'network' inside the
> masked=true conditional branch.
>
> When masked=false, the caller wants to format the original address
> with a prefix notation (e.g., "1.2.3.4/24") without applying the
> network mask. However, the code was only initializing 'network'
> when masking was requested, causing the subsequent
> virSocketAddrFormat(&network) call to operate on uninitialized data.
>
> Fix this by adding an else branch that copies the original address
> to 'network' when masking is not requested. This ensures 'network'
> is properly initialized in both code paths.
>
> Signed-off-by: Julio Faracco <jcfaracco@gmail.com>
> ---
>  src/util/virsocketaddr.c | 12 ++++++++----
>  1 file changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/src/util/virsocketaddr.c b/src/util/virsocketaddr.c
> index f53768878e..80ee3b4c51 100644
> --- a/src/util/virsocketaddr.c
> +++ b/src/util/virsocketaddr.c
> @@ -549,10 +549,14 @@ virSocketAddrFormatWithPrefix(virSocketAddr *addr,
>          return NULL;
>      }
>
> -    if (masked && virSocketAddrMaskByPrefix(addr, prefix, &network) < 0) {
> -        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> -                       _("Failure to mask address"));
> -        return NULL;
> +    if (masked) {
> +       if (virSocketAddrMaskByPrefix(addr, prefix, &network) < 0) {
> +            virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +                           _("Failure to mask address"));
> +            return NULL;
> +       }
> +    } else {
> +        network = *addr;
>      }
>
>      netstr = virSocketAddrFormat(&network);
> --
> 2.52.0
>

--
Julio Faracco