* [PATCH] qga: Fix ubsan warning @ 2025-07-28 17:30 Thomas Huth 2025-07-28 17:53 ` Daniel P. Berrangé 0 siblings, 1 reply; 5+ messages in thread From: Thomas Huth @ 2025-07-28 17:30 UTC (permalink / raw) To: qemu-devel, Michael Roth, Kostiantyn Kostiuk; +Cc: qemu-trivial 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 Add a check to avoid incrementing the NULL pointer here. Signed-off-by: Thomas Huth <thuth@redhat.com> --- qga/commands-linux.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/qga/commands-linux.c b/qga/commands-linux.c index 9e8a934b9a6..caf7c3ca22b 100644 --- a/qga/commands-linux.c +++ b/qga/commands-linux.c @@ -449,7 +449,9 @@ static bool build_guest_fsinfo_for_pci_dev(char const *syspath, has_ata = true; } else { p = strstr(syspath, "/host"); - q = p + 5; + if (p) { + q = p + 5; + } } if (p && sscanf(q, "%u", &host) == 1) { has_host = true; -- 2.50.1 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] qga: Fix ubsan warning 2025-07-28 17:30 [PATCH] qga: Fix ubsan warning Thomas Huth @ 2025-07-28 17:53 ` Daniel P. Berrangé 2025-07-29 6:02 ` Thomas Huth 0 siblings, 1 reply; 5+ messages in thread From: Daniel P. Berrangé @ 2025-07-28 17:53 UTC (permalink / raw) To: Thomas Huth; +Cc: qemu-devel, Michael Roth, Kostiantyn Kostiuk, qemu-trivial On Mon, Jul 28, 2025 at 07:30:16PM +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 > > Add a check to avoid incrementing the NULL pointer here. > > Signed-off-by: Thomas Huth <thuth@redhat.com> > --- > qga/commands-linux.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/qga/commands-linux.c b/qga/commands-linux.c > index 9e8a934b9a6..caf7c3ca22b 100644 > --- a/qga/commands-linux.c > +++ b/qga/commands-linux.c > @@ -449,7 +449,9 @@ static bool build_guest_fsinfo_for_pci_dev(char const *syspath, > has_ata = true; > } else { > p = strstr(syspath, "/host"); > - q = p + 5; > + if (p) { > + q = p + 5; > + } > } > if (p && sscanf(q, "%u", &host) == 1) { q is always non-NULL if p is non-NULL, so this is safe, but I would be more happy with this changing to 'q && sscanf' to eliminate the indirection. 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] qga: Fix ubsan warning 2025-07-28 17:53 ` Daniel P. Berrangé @ 2025-07-29 6:02 ` Thomas Huth 2025-07-29 6:23 ` Daniel P. Berrangé 2025-07-29 6:58 ` Kostiantyn Kostiuk 0 siblings, 2 replies; 5+ messages in thread From: Thomas Huth @ 2025-07-29 6:02 UTC (permalink / raw) To: Daniel P. Berrangé Cc: qemu-devel, Michael Roth, Kostiantyn Kostiuk, qemu-trivial On 28/07/2025 19.53, Daniel P. Berrangé wrote: > On Mon, Jul 28, 2025 at 07:30:16PM +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 >> >> Add a check to avoid incrementing the NULL pointer here. >> >> Signed-off-by: Thomas Huth <thuth@redhat.com> >> --- >> qga/commands-linux.c | 4 +++- >> 1 file changed, 3 insertions(+), 1 deletion(-) >> >> diff --git a/qga/commands-linux.c b/qga/commands-linux.c >> index 9e8a934b9a6..caf7c3ca22b 100644 >> --- a/qga/commands-linux.c >> +++ b/qga/commands-linux.c >> @@ -449,7 +449,9 @@ static bool build_guest_fsinfo_for_pci_dev(char const *syspath, >> has_ata = true; >> } else { >> p = strstr(syspath, "/host"); >> - q = p + 5; >> + if (p) { >> + q = p + 5; >> + } >> } >> if (p && sscanf(q, "%u", &host) == 1) { > > q is always non-NULL if p is non-NULL, so this is safe, but I would be more > happy with this changing to 'q && sscanf' to eliminate the indirection. If we agree to do a bigger change here, I'd rather drop the "q" pointer completely and use a new integer variable instead, something like: int offset; ... p = strstr(syspath, "/ata"); if (p) { offset = 4; has_ata = true; } else { offset = 5; p = strstr(syspath, "/host"); } if (p && sscanf(p + offset, "%u", &host) == 1) { ... } WDYT? Thomas ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] qga: Fix ubsan warning 2025-07-29 6:02 ` Thomas Huth @ 2025-07-29 6:23 ` Daniel P. Berrangé 2025-07-29 6:58 ` Kostiantyn Kostiuk 1 sibling, 0 replies; 5+ messages in thread From: Daniel P. Berrangé @ 2025-07-29 6:23 UTC (permalink / raw) To: Thomas Huth; +Cc: qemu-devel, Michael Roth, Kostiantyn Kostiuk, qemu-trivial On Tue, Jul 29, 2025 at 08:02:25AM +0200, Thomas Huth wrote: > On 28/07/2025 19.53, Daniel P. Berrangé wrote: > > On Mon, Jul 28, 2025 at 07:30:16PM +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 > > > > > > Add a check to avoid incrementing the NULL pointer here. > > > > > > Signed-off-by: Thomas Huth <thuth@redhat.com> > > > --- > > > qga/commands-linux.c | 4 +++- > > > 1 file changed, 3 insertions(+), 1 deletion(-) > > > > > > diff --git a/qga/commands-linux.c b/qga/commands-linux.c > > > index 9e8a934b9a6..caf7c3ca22b 100644 > > > --- a/qga/commands-linux.c > > > +++ b/qga/commands-linux.c > > > @@ -449,7 +449,9 @@ static bool build_guest_fsinfo_for_pci_dev(char const *syspath, > > > has_ata = true; > > > } else { > > > p = strstr(syspath, "/host"); > > > - q = p + 5; > > > + if (p) { > > > + q = p + 5; > > > + } > > > } > > > if (p && sscanf(q, "%u", &host) == 1) { > > > > q is always non-NULL if p is non-NULL, so this is safe, but I would be more > > happy with this changing to 'q && sscanf' to eliminate the indirection. > > If we agree to do a bigger change here, I'd rather drop the "q" pointer > completely and use a new integer variable instead, something like: > > int offset; > ... > p = strstr(syspath, "/ata"); > if (p) { > offset = 4; > has_ata = true; > } else { > offset = 5; > p = strstr(syspath, "/host"); > } > if (p && sscanf(p + offset, "%u", &host) == 1) { > ... > } > > WDYT? Works for me. 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] qga: Fix ubsan warning 2025-07-29 6:02 ` Thomas Huth 2025-07-29 6:23 ` Daniel P. Berrangé @ 2025-07-29 6:58 ` Kostiantyn Kostiuk 1 sibling, 0 replies; 5+ messages in thread From: Kostiantyn Kostiuk @ 2025-07-29 6:58 UTC (permalink / raw) To: Thomas Huth Cc: Daniel P. Berrangé, qemu-devel, Michael Roth, qemu-trivial [-- Attachment #1: Type: text/plain, Size: 1999 bytes --] LGTM On Tue, Jul 29, 2025 at 9:02 AM Thomas Huth <thuth@redhat.com> wrote: > On 28/07/2025 19.53, Daniel P. Berrangé wrote: > > On Mon, Jul 28, 2025 at 07:30:16PM +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 > >> > >> Add a check to avoid incrementing the NULL pointer here. > >> > >> Signed-off-by: Thomas Huth <thuth@redhat.com> > >> --- > >> qga/commands-linux.c | 4 +++- > >> 1 file changed, 3 insertions(+), 1 deletion(-) > >> > >> diff --git a/qga/commands-linux.c b/qga/commands-linux.c > >> index 9e8a934b9a6..caf7c3ca22b 100644 > >> --- a/qga/commands-linux.c > >> +++ b/qga/commands-linux.c > >> @@ -449,7 +449,9 @@ static bool build_guest_fsinfo_for_pci_dev(char > const *syspath, > >> has_ata = true; > >> } else { > >> p = strstr(syspath, "/host"); > >> - q = p + 5; > >> + if (p) { > >> + q = p + 5; > >> + } > >> } > >> if (p && sscanf(q, "%u", &host) == 1) { > > > > q is always non-NULL if p is non-NULL, so this is safe, but I would be > more > > happy with this changing to 'q && sscanf' to eliminate the indirection. > > If we agree to do a bigger change here, I'd rather drop the "q" pointer > completely and use a new integer variable instead, something like: > > int offset; > ... > p = strstr(syspath, "/ata"); > if (p) { > offset = 4; > has_ata = true; > } else { > offset = 5; > p = strstr(syspath, "/host"); > } > if (p && sscanf(p + offset, "%u", &host) == 1) { > ... > } > > WDYT? > > Thomas > > [-- Attachment #2: Type: text/html, Size: 2950 bytes --] ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2025-07-29 6:58 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2025-07-28 17:30 [PATCH] qga: Fix ubsan warning Thomas Huth 2025-07-28 17:53 ` Daniel P. Berrangé 2025-07-29 6:02 ` Thomas Huth 2025-07-29 6:23 ` Daniel P. Berrangé 2025-07-29 6:58 ` Kostiantyn Kostiuk
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.