qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2] qga: Fix ubsan warning
@ 2025-07-30  7:27 Thomas Huth
  2025-07-30  8:28 ` Kostiantyn Kostiuk
                   ` (2 more replies)
  0 siblings, 3 replies; 5+ messages in thread
From: Thomas Huth @ 2025-07-30  7:27 UTC (permalink / raw)
  To: qemu-devel, Michael Roth, Kostiantyn Kostiuk
  Cc: qemu-trivial, Daniel P . Berrangé

From: Thomas Huth <thuth@redhat.com>

When compiling QEMU with --enable-ubsan there is a undefined behavior
warning when running "make check":

 .../qga/commands-linux.c:452:15: runtime error: applying non-zero offset 5 to null pointer
 #0 0x55ea7b89450c in build_guest_fsinfo_for_pci_dev ..../qga/commands-linux.c:452:15

Fix it by avoiding the additional pointer variable here and use an
"offset" integer variable instead.

Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 v2: Use an integer offset variable instead for checking for a NULL pointer

 qga/commands-linux.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/qga/commands-linux.c b/qga/commands-linux.c
index 9e8a934b9a6..0c41eb97190 100644
--- a/qga/commands-linux.c
+++ b/qga/commands-linux.c
@@ -400,10 +400,10 @@ static bool build_guest_fsinfo_for_pci_dev(char const *syspath,
                                            Error **errp)
 {
     unsigned int pci[4], host, hosts[8], tgt[3];
-    int i, nhosts = 0, pcilen;
+    int i, offset, nhosts = 0, pcilen;
     GuestPCIAddress *pciaddr = disk->pci_controller;
     bool has_ata = false, has_host = false, has_tgt = false;
-    char *p, *q, *driver = NULL;
+    char *p, *driver = NULL;
     bool ret = false;
 
     p = strstr(syspath, "/devices/pci");
@@ -445,13 +445,13 @@ static bool build_guest_fsinfo_for_pci_dev(char const *syspath,
 
     p = strstr(syspath, "/ata");
     if (p) {
-        q = p + 4;
+        offset = 4;
         has_ata = true;
     } else {
         p = strstr(syspath, "/host");
-        q = p + 5;
+        offset = 5;
     }
-    if (p && sscanf(q, "%u", &host) == 1) {
+    if (p && sscanf(p + offset, "%u", &host) == 1) {
         has_host = true;
         nhosts = build_hosts(syspath, p, has_ata, hosts,
                              ARRAY_SIZE(hosts), errp);
-- 
2.50.1



^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH v2] qga: Fix ubsan warning
  2025-07-30  7:27 [PATCH v2] qga: Fix ubsan warning Thomas Huth
@ 2025-07-30  8:28 ` Kostiantyn Kostiuk
  2025-07-30  8:58   ` Thomas Huth
  2025-07-30 11:46 ` Daniel P. Berrangé
  2025-08-05 18:26 ` Michael Tokarev
  2 siblings, 1 reply; 5+ messages in thread
From: Kostiantyn Kostiuk @ 2025-07-30  8:28 UTC (permalink / raw)
  To: Thomas Huth
  Cc: qemu-devel, Michael Roth, qemu-trivial, Daniel P . Berrangé

[-- Attachment #1: Type: text/plain, Size: 2247 bytes --]

Reviewed-by: Kostiantyn Kostiuk <kkostiuk@redhat.com>

@Thomas Huth <thuth@redhat.com> Is this fix critical to merge during code
freeze?

On Wed, Jul 30, 2025 at 10:27 AM Thomas Huth <thuth@redhat.com> wrote:

> From: Thomas Huth <thuth@redhat.com>
>
> When compiling QEMU with --enable-ubsan there is a undefined behavior
> warning when running "make check":
>
>  .../qga/commands-linux.c:452:15: runtime error: applying non-zero offset
> 5 to null pointer
>  #0 0x55ea7b89450c in build_guest_fsinfo_for_pci_dev
> ..../qga/commands-linux.c:452:15
>
> Fix it by avoiding the additional pointer variable here and use an
> "offset" integer variable instead.
>
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>  v2: Use an integer offset variable instead for checking for a NULL pointer
>
>  qga/commands-linux.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/qga/commands-linux.c b/qga/commands-linux.c
> index 9e8a934b9a6..0c41eb97190 100644
> --- a/qga/commands-linux.c
> +++ b/qga/commands-linux.c
> @@ -400,10 +400,10 @@ static bool build_guest_fsinfo_for_pci_dev(char
> const *syspath,
>                                             Error **errp)
>  {
>      unsigned int pci[4], host, hosts[8], tgt[3];
> -    int i, nhosts = 0, pcilen;
> +    int i, offset, nhosts = 0, pcilen;
>      GuestPCIAddress *pciaddr = disk->pci_controller;
>      bool has_ata = false, has_host = false, has_tgt = false;
> -    char *p, *q, *driver = NULL;
> +    char *p, *driver = NULL;
>      bool ret = false;
>
>      p = strstr(syspath, "/devices/pci");
> @@ -445,13 +445,13 @@ static bool build_guest_fsinfo_for_pci_dev(char
> const *syspath,
>
>      p = strstr(syspath, "/ata");
>      if (p) {
> -        q = p + 4;
> +        offset = 4;
>          has_ata = true;
>      } else {
>          p = strstr(syspath, "/host");
> -        q = p + 5;
> +        offset = 5;
>      }
> -    if (p && sscanf(q, "%u", &host) == 1) {
> +    if (p && sscanf(p + offset, "%u", &host) == 1) {
>          has_host = true;
>          nhosts = build_hosts(syspath, p, has_ata, hosts,
>                               ARRAY_SIZE(hosts), errp);
> --
> 2.50.1
>
>

[-- Attachment #2: Type: text/html, Size: 3159 bytes --]

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v2] qga: Fix ubsan warning
  2025-07-30  8:28 ` Kostiantyn Kostiuk
@ 2025-07-30  8:58   ` Thomas Huth
  0 siblings, 0 replies; 5+ messages in thread
From: Thomas Huth @ 2025-07-30  8:58 UTC (permalink / raw)
  To: Kostiantyn Kostiuk
  Cc: qemu-devel, Michael Roth, qemu-trivial, Daniel P . Berrangé

On 30/07/2025 10.28, Kostiantyn Kostiuk wrote:
> Reviewed-by: Kostiantyn Kostiuk <kkostiuk@redhat.com 
> <mailto:kkostiuk@redhat.com>>
> 
> @Thomas Huth Is this fix critical to merge during 
> code freeze?

I don't think so, the problem only occurs when compiling with 
--enable-ubsan, so normal users should not be affected by this.

  Thomas


> On Wed, Jul 30, 2025 at 10:27 AM Thomas Huth <thuth@redhat.com 
> <mailto:thuth@redhat.com>> wrote:
> 
>     From: Thomas Huth <thuth@redhat.com <mailto:thuth@redhat.com>>
> 
>     When compiling QEMU with --enable-ubsan there is a undefined behavior
>     warning when running "make check":
> 
>       .../qga/commands-linux.c:452:15: runtime error: applying non-zero
>     offset 5 to null pointer
>       #0 0x55ea7b89450c in build_guest_fsinfo_for_pci_dev ..../qga/commands-
>     linux.c:452:15
> 
>     Fix it by avoiding the additional pointer variable here and use an
>     "offset" integer variable instead.
> 
>     Signed-off-by: Thomas Huth <thuth@redhat.com <mailto:thuth@redhat.com>>
>     ---
>       v2: Use an integer offset variable instead for checking for a NULL pointer
> 
>       qga/commands-linux.c | 10 +++++-----
>       1 file changed, 5 insertions(+), 5 deletions(-)
> 
>     diff --git a/qga/commands-linux.c b/qga/commands-linux.c
>     index 9e8a934b9a6..0c41eb97190 100644
>     --- a/qga/commands-linux.c
>     +++ b/qga/commands-linux.c
>     @@ -400,10 +400,10 @@ static bool build_guest_fsinfo_for_pci_dev(char
>     const *syspath,
>                                                  Error **errp)
>       {
>           unsigned int pci[4], host, hosts[8], tgt[3];
>     -    int i, nhosts = 0, pcilen;
>     +    int i, offset, nhosts = 0, pcilen;
>           GuestPCIAddress *pciaddr = disk->pci_controller;
>           bool has_ata = false, has_host = false, has_tgt = false;
>     -    char *p, *q, *driver = NULL;
>     +    char *p, *driver = NULL;
>           bool ret = false;
> 
>           p = strstr(syspath, "/devices/pci");
>     @@ -445,13 +445,13 @@ static bool build_guest_fsinfo_for_pci_dev(char
>     const *syspath,
> 
>           p = strstr(syspath, "/ata");
>           if (p) {
>     -        q = p + 4;
>     +        offset = 4;
>               has_ata = true;
>           } else {
>               p = strstr(syspath, "/host");
>     -        q = p + 5;
>     +        offset = 5;
>           }
>     -    if (p && sscanf(q, "%u", &host) == 1) {
>     +    if (p && sscanf(p + offset, "%u", &host) == 1) {
>               has_host = true;
>               nhosts = build_hosts(syspath, p, has_ata, hosts,
>                                    ARRAY_SIZE(hosts), errp);
>     -- 
>     2.50.1
> 



^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v2] qga: Fix ubsan warning
  2025-07-30  7:27 [PATCH v2] qga: Fix ubsan warning Thomas Huth
  2025-07-30  8:28 ` Kostiantyn Kostiuk
@ 2025-07-30 11:46 ` Daniel P. Berrangé
  2025-08-05 18:26 ` Michael Tokarev
  2 siblings, 0 replies; 5+ messages in thread
From: Daniel P. Berrangé @ 2025-07-30 11:46 UTC (permalink / raw)
  To: Thomas Huth; +Cc: qemu-devel, Michael Roth, Kostiantyn Kostiuk, qemu-trivial

On Wed, Jul 30, 2025 at 09:27:09AM +0200, Thomas Huth wrote:
> From: Thomas Huth <thuth@redhat.com>
> 
> When compiling QEMU with --enable-ubsan there is a undefined behavior
> warning when running "make check":
> 
>  .../qga/commands-linux.c:452:15: runtime error: applying non-zero offset 5 to null pointer
>  #0 0x55ea7b89450c in build_guest_fsinfo_for_pci_dev ..../qga/commands-linux.c:452:15
> 
> Fix it by avoiding the additional pointer variable here and use an
> "offset" integer variable instead.
> 
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>  v2: Use an integer offset variable instead for checking for a NULL pointer
> 
>  qga/commands-linux.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|



^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v2] qga: Fix ubsan warning
  2025-07-30  7:27 [PATCH v2] qga: Fix ubsan warning Thomas Huth
  2025-07-30  8:28 ` Kostiantyn Kostiuk
  2025-07-30 11:46 ` Daniel P. Berrangé
@ 2025-08-05 18:26 ` Michael Tokarev
  2 siblings, 0 replies; 5+ messages in thread
From: Michael Tokarev @ 2025-08-05 18:26 UTC (permalink / raw)
  To: Thomas Huth, qemu-devel, Michael Roth, Kostiantyn Kostiuk
  Cc: qemu-trivial, Daniel P . Berrangé

On 30.07.2025 10:27, Thomas Huth wrote:
> From: Thomas Huth <thuth@redhat.com>
> 
> When compiling QEMU with --enable-ubsan there is a undefined behavior
> warning when running "make check":
> 
>   .../qga/commands-linux.c:452:15: runtime error: applying non-zero offset 5 to null pointer
>   #0 0x55ea7b89450c in build_guest_fsinfo_for_pci_dev ..../qga/commands-linux.c:452:15
> 
> Fix it by avoiding the additional pointer variable here and use an
> "offset" integer variable instead.
> 
> Signed-off-by: Thomas Huth <thuth@redhat.com>

It's a nice cleanup too.

Thanks,  queued.

/mjt



^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2025-08-05 18:26 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-07-30  7:27 [PATCH v2] qga: Fix ubsan warning Thomas Huth
2025-07-30  8:28 ` Kostiantyn Kostiuk
2025-07-30  8:58   ` Thomas Huth
2025-07-30 11:46 ` Daniel P. Berrangé
2025-08-05 18:26 ` Michael Tokarev

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).