[PATCH] hw/core/qdev-clock: Fix potential null pointer dereference

hemanshu.khilari.foss posted 1 patch 1 week, 1 day ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20260521172032.85047-2-hemanshu.khilari.foss@gmail.com
Maintainers: Paolo Bonzini <pbonzini@redhat.com>, "Daniel P. Berrangé" <berrange@redhat.com>, Luc Michel <luc@lmichel.fr>
hw/core/qdev-clock.c | 5 +++++
1 file changed, 5 insertions(+)
[PATCH] hw/core/qdev-clock: Fix potential null pointer dereference
Posted by hemanshu.khilari.foss 1 week, 1 day ago
qdev_get_clocklist function returns a pointer to the NamedClockList
struct. This function is called in qdev_alias_clock and the returned
pointer is immidiately dereferenced without a null check.

A null check is added after qdev_get_clocklist is called which prints an
error message and the function is aborted.

Cc: luc@lmichel.fr
Cc: hemanshu_dev@proton.me
Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/2342
Signed-off-by: hemanshu.khilari.foss <hemanshu.khilari.foss@gmail.com>
---
 hw/core/qdev-clock.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/hw/core/qdev-clock.c b/hw/core/qdev-clock.c
index 6e2967e433..cf09f3d09e 100644
--- a/hw/core/qdev-clock.c
+++ b/hw/core/qdev-clock.c
@@ -157,6 +157,11 @@ Clock *qdev_alias_clock(DeviceState *dev, const char *name,
                         DeviceState *alias_dev, const char *alias_name)
 {
     NamedClockList *ncl = qdev_get_clocklist(dev, name);
+    if (!ncl) {
+        error_report("Can not find clock-in '%s' for device type '%s'",
+                     name, object_get_typename(OBJECT(dev)));
+        abort();
+    }
     Clock *clk = ncl->clock;
 
     ncl = qdev_init_clocklist(alias_dev, alias_name, true, ncl->output, clk);
-- 
2.42.0
Re: [PATCH] hw/core/qdev-clock: Fix potential null pointer dereference
Posted by Peter Maydell 4 days, 7 hours ago
On Thu, 21 May 2026 at 19:13, hemanshu.khilari.foss
<hemanshu.khilari.foss@gmail.com> wrote:

Hi; thanks for this patch. It looks good, except for a couple
of minor things that I've noted below.

> qdev_get_clocklist function returns a pointer to the NamedClockList
> struct. This function is called in qdev_alias_clock and the returned
> pointer is immidiately dereferenced without a null check.

"immediately".

You should mention in the commit message that passing the function
a clock name that doesn't exist is a programming error, and so
this change is not fixing a bug, only making the reporting of
that programming error a bit more helpful and bringing it in to
line with qdev_get_clock_in() and qdev_get_clock_out().

> A null check is added after qdev_get_clocklist is called which prints an
> error message and the function is aborted.
>
> Cc: luc@lmichel.fr
> Cc: hemanshu_dev@proton.me
> Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/2342
> Signed-off-by: hemanshu.khilari.foss <hemanshu.khilari.foss@gmail.com>
> ---
>  hw/core/qdev-clock.c | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/hw/core/qdev-clock.c b/hw/core/qdev-clock.c
> index 6e2967e433..cf09f3d09e 100644
> --- a/hw/core/qdev-clock.c
> +++ b/hw/core/qdev-clock.c
> @@ -157,6 +157,11 @@ Clock *qdev_alias_clock(DeviceState *dev, const char *name,
>                          DeviceState *alias_dev, const char *alias_name)
>  {
>      NamedClockList *ncl = qdev_get_clocklist(dev, name);
> +    if (!ncl) {
> +        error_report("Can not find clock-in '%s' for device type '%s'",
> +                     name, object_get_typename(OBJECT(dev)));

The message should say "clock", not "clock-in", since this is
for clock aliasing and it works for both input and output clocks.

> +        abort();
> +    }
>      Clock *clk = ncl->clock;

Our coding style (docs/devel/style.rst) requires that all
variable declarations go at the top of a function or code block,
so you can't put this if() above the "Clock *clk" declaration.
Instead you can put "Clock *clk;" above, and then the
"clk = ncl->clock;" initializer below.

thanks
-- PMM