hw/misc/npcm_clk.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)
Regression introduced by cf76c4
(hw/misc: Add nr_regs and cold_reset_values to NPCM CLK)
cold_reset_values has a different size, depending on device used
(NPCM7xx vs NPCM8xx). However, s->regs has a fixed size, which matches
NPCM8xx. Thus, when initializing a NPCM7xx, we go past cold_reset_values
ending.
Report by asan:
==2066==ERROR: AddressSanitizer: global-buffer-overflow on address 0x55d68a3e97f0 at pc 0x7fcaf2b2d14b bp 0x7ffff0cc3890 sp 0x7ffff0cc3040
READ of size 196 at 0x55d68a3e97f0 thread T0
#0 0x7fcaf2b2d14a in __interceptor_memcpy ../../../../src/libsanitizer/sanitizer_common/sanitizer_common_interceptors.inc:827
#1 0x55d688447e0d in memcpy /usr/include/x86_64-linux-gnu/bits/string_fortified.h:29
#2 0x55d688447e0d in npcm_clk_enter_reset ../hw/misc/npcm_clk.c:968
#3 0x55d6899b7213 in resettable_phase_enter ../hw/core/resettable.c:136
#4 0x55d6899a1ef7 in bus_reset_child_foreach ../hw/core/bus.c:97
#5 0x55d6899b717d in resettable_child_foreach ../hw/core/resettable.c:92
#6 0x55d6899b717d in resettable_phase_enter ../hw/core/resettable.c:129
#7 0x55d6899b4ead in resettable_container_child_foreach ../hw/core/resetcontainer.c:54
#8 0x55d6899b717d in resettable_child_foreach ../hw/core/resettable.c:92
#9 0x55d6899b717d in resettable_phase_enter ../hw/core/resettable.c:129
#10 0x55d6899b7bfa in resettable_assert_reset ../hw/core/resettable.c:55
#11 0x55d6899b8666 in resettable_reset ../hw/core/resettable.c:45
#12 0x55d688d15cd2 in qemu_system_reset ../system/runstate.c:527
#13 0x55d687fc5edd in qdev_machine_creation_done ../hw/core/machine.c:1738
#14 0x55d688d209bd in qemu_machine_creation_done ../system/vl.c:2779
#15 0x55d688d209bd in qmp_x_exit_preconfig ../system/vl.c:2807
#16 0x55d688d281fb in qemu_init ../system/vl.c:3838
#17 0x55d687ceab12 in main ../system/main.c:68
#18 0x7fcaef006249 (/lib/x86_64-linux-gnu/libc.so.6+0x27249)
#19 0x7fcaef006304 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x27304)
#20 0x55d687cf0010 in _start (/home/runner/work/qemu-ci/qemu-ci/build/qemu-system-arm+0x371c010)
0x55d68a3e97f0 is located 0 bytes to the right of global variable 'npcm7xx_cold_reset_values' defined in '../hw/misc/npcm_clk.c:134:23' (0x55d68a3e9780) of size 112
Impacted tests:
Summary of Failures:
check:
2/747 qemu:qtest+qtest-aarch64 / qtest-aarch64/qom-test ERROR 9.28s killed by signal 6 SIGABRT
4/747 qemu:qtest+qtest-arm / qtest-arm/qom-test ERROR 7.82s killed by signal 6 SIGABRT
32/747 qemu:qtest+qtest-aarch64 / qtest-aarch64/device-introspect-test ERROR 10.91s killed by signal 6 SIGABRT
35/747 qemu:qtest+qtest-arm / qtest-arm/device-introspect-test ERROR 11.33s killed by signal 6 SIGABRT
114/747 qemu:qtest+qtest-arm / qtest-arm/npcm7xx_pwm-test ERROR 0.98s killed by signal 6 SIGABRT
115/747 qemu:qtest+qtest-aarch64 / qtest-aarch64/test-hmp ERROR 2.95s killed by signal 6 SIGABRT
117/747 qemu:qtest+qtest-arm / qtest-arm/test-hmp ERROR 2.54s killed by signal 6 SIGABRT
151/747 qemu:qtest+qtest-arm / qtest-arm/npcm7xx_watchdog_timer-test ERROR 0.96s killed by signal 6 SIGABRT
247/747 qemu:qtest+qtest-arm / qtest-arm/npcm7xx_adc-test ERROR 0.96s killed by signal 6 SIGABRT
248/747 qemu:qtest+qtest-arm / qtest-arm/npcm7xx_gpio-test ERROR 1.05s killed by signal 6 SIGABRT
249/747 qemu:qtest+qtest-arm / qtest-arm/npcm7xx_rng-test ERROR 0.97s killed by signal 6 SIGABRT
250/747 qemu:qtest+qtest-arm / qtest-arm/npcm7xx_sdhci-test ERROR 0.97s killed by signal 6 SIGABRT
251/747 qemu:qtest+qtest-arm / qtest-arm/npcm7xx_smbus-test ERROR 0.89s killed by signal 6 SIGABRT
252/747 qemu:qtest+qtest-arm / qtest-arm/npcm7xx_timer-test ERROR 1.09s killed by signal 6 SIGABRT
253/747 qemu:qtest+qtest-arm / qtest-arm/npcm_gmac-test ERROR 1.12s killed by signal 6 SIGABRT
255/747 qemu:qtest+qtest-arm / qtest-arm/npcm7xx_emc-test ERROR 1.05s killed by signal 6 SIGABRT
check-functional:
22/203 qemu:func-thorough+func-arm-thorough+thorough / func-arm-arm_quanta_gsj ERROR 0.79s exit status 1
38/203 qemu:func-quick+func-aarch64 / func-aarch64-migration ERROR 1.97s exit status 1
45/203 qemu:func-quick+func-arm / func-arm-migration ERROR 1.90s exit status 1
Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
---
hw/misc/npcm_clk.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/hw/misc/npcm_clk.c b/hw/misc/npcm_clk.c
index d1f29759d59..0e85974cf96 100644
--- a/hw/misc/npcm_clk.c
+++ b/hw/misc/npcm_clk.c
@@ -964,8 +964,9 @@ static void npcm_clk_enter_reset(Object *obj, ResetType type)
NPCMCLKState *s = NPCM_CLK(obj);
NPCMCLKClass *c = NPCM_CLK_GET_CLASS(s);
- g_assert(sizeof(s->regs) >= c->nr_regs * sizeof(uint32_t));
- memcpy(s->regs, c->cold_reset_values, sizeof(s->regs));
+ size_t sizeof_regs = c->nr_regs * sizeof(uint32_t);
+ g_assert(sizeof(s->regs) >= sizeof_regs);
+ memcpy(s->regs, c->cold_reset_values, sizeof_regs);
s->ref_ns = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
npcm7xx_clk_update_all_clocks(s);
/*
--
2.39.5On Mon, 24 Feb 2025 at 20:51, Pierrick Bouvier <pierrick.bouvier@linaro.org> wrote: > > Regression introduced by cf76c4 > (hw/misc: Add nr_regs and cold_reset_values to NPCM CLK) > > cold_reset_values has a different size, depending on device used > (NPCM7xx vs NPCM8xx). However, s->regs has a fixed size, which matches > NPCM8xx. Thus, when initializing a NPCM7xx, we go past cold_reset_values > ending. > diff --git a/hw/misc/npcm_clk.c b/hw/misc/npcm_clk.c > index d1f29759d59..0e85974cf96 100644 > --- a/hw/misc/npcm_clk.c > +++ b/hw/misc/npcm_clk.c > @@ -964,8 +964,9 @@ static void npcm_clk_enter_reset(Object *obj, ResetType type) > NPCMCLKState *s = NPCM_CLK(obj); > NPCMCLKClass *c = NPCM_CLK_GET_CLASS(s); > > - g_assert(sizeof(s->regs) >= c->nr_regs * sizeof(uint32_t)); > - memcpy(s->regs, c->cold_reset_values, sizeof(s->regs)); > + size_t sizeof_regs = c->nr_regs * sizeof(uint32_t); > + g_assert(sizeof(s->regs) >= sizeof_regs); > + memcpy(s->regs, c->cold_reset_values, sizeof_regs); > s->ref_ns = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL); > npcm7xx_clk_update_all_clocks(s); > /* Whoops, thanks for catching this. Applied to target-arm.next, thanks. (Looking more closely at the cold_reset_values handling in npcm_gcr.c, that looks not quite right in a different way; I'll send a reply to that patch email about that.) -- PMM
On 2/25/25 05:41, Peter Maydell wrote: > On Mon, 24 Feb 2025 at 20:51, Pierrick Bouvier > <pierrick.bouvier@linaro.org> wrote: >> >> Regression introduced by cf76c4 >> (hw/misc: Add nr_regs and cold_reset_values to NPCM CLK) >> >> cold_reset_values has a different size, depending on device used >> (NPCM7xx vs NPCM8xx). However, s->regs has a fixed size, which matches >> NPCM8xx. Thus, when initializing a NPCM7xx, we go past cold_reset_values >> ending. > > >> diff --git a/hw/misc/npcm_clk.c b/hw/misc/npcm_clk.c >> index d1f29759d59..0e85974cf96 100644 >> --- a/hw/misc/npcm_clk.c >> +++ b/hw/misc/npcm_clk.c >> @@ -964,8 +964,9 @@ static void npcm_clk_enter_reset(Object *obj, ResetType type) >> NPCMCLKState *s = NPCM_CLK(obj); >> NPCMCLKClass *c = NPCM_CLK_GET_CLASS(s); >> >> - g_assert(sizeof(s->regs) >= c->nr_regs * sizeof(uint32_t)); >> - memcpy(s->regs, c->cold_reset_values, sizeof(s->regs)); >> + size_t sizeof_regs = c->nr_regs * sizeof(uint32_t); >> + g_assert(sizeof(s->regs) >= sizeof_regs); >> + memcpy(s->regs, c->cold_reset_values, sizeof_regs); >> s->ref_ns = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL); >> npcm7xx_clk_update_all_clocks(s); >> /* > > Whoops, thanks for catching this. Applied to target-arm.next, thanks. > > (Looking more closely at the cold_reset_values handling > in npcm_gcr.c, that looks not quite right in a different > way; I'll send a reply to that patch email about that.) > It may be a hole in our CI right now. Would that be interesting for CI to run all tests (check-functional + check w/o functional) with both ubsan and asan? > -- PMM
On Tue, 25 Feb 2025 at 20:57, Pierrick Bouvier <pierrick.bouvier@linaro.org> wrote: > > On 2/25/25 05:41, Peter Maydell wrote: > > (Looking more closely at the cold_reset_values handling > > in npcm_gcr.c, that looks not quite right in a different > > way; I'll send a reply to that patch email about that.) > > > > It may be a hole in our CI right now. > Would that be interesting for CI to run all tests (check-functional + > check w/o functional) with both ubsan and asan? We do have at least some ubsan tests in our CI right now (eg the "clang-system" job). The problem with ubsan coverage is the usual one that we already have too much CI going on, and it takes forever and we don't have that much headroom for adding more jobs. On the asan front, also, yes, coverage would be a good idea. Here I think we will probably have to gradually ratchet up the coverage because I'm pretty sure that at the moment we will find we don't get a clean pass (mostly for "uninteresting" memory leaks). (I do also usually run a local ubsan test build when doing my acculumation of patches in target-arm, but since that's a manual step it is fallible :-)) -- PMM
On 2/26/25 03:50, Peter Maydell wrote: > On Tue, 25 Feb 2025 at 20:57, Pierrick Bouvier > <pierrick.bouvier@linaro.org> wrote: >> >> On 2/25/25 05:41, Peter Maydell wrote: >>> (Looking more closely at the cold_reset_values handling >>> in npcm_gcr.c, that looks not quite right in a different >>> way; I'll send a reply to that patch email about that.) >>> >> >> It may be a hole in our CI right now. >> Would that be interesting for CI to run all tests (check-functional + >> check w/o functional) with both ubsan and asan? > > We do have at least some ubsan tests in our CI right now > (eg the "clang-system" job). The problem with ubsan coverage > is the usual one that we already have too much CI going on, > and it takes forever and we don't have that much headroom > for adding more jobs. I understand the problem behind spending more minutes on this. However, looking at our CI, we already duplicate functional testing a lot: buildtest.yml:functional-system-alpine: buildtest.yml:functional-system-ubuntu: buildtest.yml:functional-system-debian: buildtest.yml:functional-system-fedora: buildtest.yml:functional-system-centos: buildtest.yml:functional-system-opensuse: Would that hurt so much to have one configuration enabled with ubsan and asan, which catches *real* bugs, and potential security issues? Yes, it adds overhead, but it should not be x10. Around x2 to x3. On github running, running -j2, running all functional tests with sanitizers takes less than 1 hour, and the build takes the same amount in time (-j2 as well). Hopefully we have more cores available on our own runners. > > On the asan front, also, yes, coverage would be a good idea. > Here I think we will probably have to gradually ratchet > up the coverage because I'm pretty sure that at the moment > we will find we don't get a clean pass (mostly for "uninteresting" > memory leaks). > Yes, I run with ASAN_OPTIONS=detect_leaks=0, and I deactivate any test that is flaky. Two of them related to asan are tcg tests: - munmap-pthread - follow-fork-mode I didn't have time to investigate, so I just removed them in my tree. At this point, this whole list of tests concerned is: https://github.com/search?q=repo%3Apbo-linaro%2Fqemu-ci+%22ci+fix%22+author%3Apbo-linaro&type=commits > (I do also usually run a local > ubsan test build when doing my acculumation of patches in > target-arm, but since that's a manual step it is fallible :-)) > It's always said that "Maintainer time is precious", shouldn't that be CI job to catch this? I guess CI minutes are cheaper than engineer ones those days. > -- PMM
(edited cc list since it's moved away from a discussion of this particular patch and on to a testing/ci coverage issue) On Wed, 26 Feb 2025 at 19:03, Pierrick Bouvier <pierrick.bouvier@linaro.org> wrote: > > On 2/26/25 03:50, Peter Maydell wrote: > > On Tue, 25 Feb 2025 at 20:57, Pierrick Bouvier > > <pierrick.bouvier@linaro.org> wrote: > >> > >> On 2/25/25 05:41, Peter Maydell wrote: > >>> (Looking more closely at the cold_reset_values handling > >>> in npcm_gcr.c, that looks not quite right in a different > >>> way; I'll send a reply to that patch email about that.) > >>> > >> > >> It may be a hole in our CI right now. > >> Would that be interesting for CI to run all tests (check-functional + > >> check w/o functional) with both ubsan and asan? > > > > We do have at least some ubsan tests in our CI right now > > (eg the "clang-system" job). The problem with ubsan coverage > > is the usual one that we already have too much CI going on, > > and it takes forever and we don't have that much headroom > > for adding more jobs. > > I understand the problem behind spending more minutes on this. > > However, looking at our CI, we already duplicate functional testing a lot: > buildtest.yml:functional-system-alpine: > buildtest.yml:functional-system-ubuntu: > buildtest.yml:functional-system-debian: > buildtest.yml:functional-system-fedora: > buildtest.yml:functional-system-centos: > buildtest.yml:functional-system-opensuse: I think that these are mostly testing different target architectures, e.g. functional-system-alpine tests what build-system-alpine built, which covers avr, loongarch64, mips64 and mipsel targets; functional-system-ubuntu tests what build-system-ubuntu bulit, which is alpha, microblazeel, mips64el, and so on. So there is less overlap than it might appear. (Some of them complete pretty quickly because we have very few functional tests for some archs; some are slower where we're running more tests. e.g. https://gitlab.com/qemu-project/qemu/-/jobs/9213571833 is functional-system-fedora completing in 7 mins because we only have a few ppc tests, but this is functional-system-opensuse https://gitlab.com/qemu-project/qemu/-/jobs/9213571852 taking 27 mins because it's testing x86 and aarch64. The corresponding build jobs take about 30 mins each.) > Would that hurt so much to have one configuration enabled with ubsan and > asan, which catches *real* bugs, and potential security issues? > Yes, it adds overhead, but it should not be x10. Around x2 to x3. You'd need to have a duplicate of all of the above functional-system-* test jobs if you wanted to test all the guest architectures, I think. So it's 30 mins build * six configs plus 60 mins total for testing. Or we could convert (some of?) the existing jobs to use the sanitisers if we needed to economise on CI time. > I guess CI minutes are cheaper than engineer ones those days. You could make an argument that it's the other way around: from the project's point of view engineer minutes are cheap because we never pay engineers, whereas CI minutes are expensive because we must pay for them out of our project donations :-) -- PMM
On 26/02/2025 21.50, Peter Maydell wrote: > (edited cc list since it's moved away from a discussion of this > particular patch and on to a testing/ci coverage issue) > > On Wed, 26 Feb 2025 at 19:03, Pierrick Bouvier > <pierrick.bouvier@linaro.org> wrote: >> >> On 2/26/25 03:50, Peter Maydell wrote: >>> On Tue, 25 Feb 2025 at 20:57, Pierrick Bouvier >>> <pierrick.bouvier@linaro.org> wrote: >>>> >>>> On 2/25/25 05:41, Peter Maydell wrote: >>>>> (Looking more closely at the cold_reset_values handling >>>>> in npcm_gcr.c, that looks not quite right in a different >>>>> way; I'll send a reply to that patch email about that.) >>>>> >>>> >>>> It may be a hole in our CI right now. >>>> Would that be interesting for CI to run all tests (check-functional + >>>> check w/o functional) with both ubsan and asan? >>> >>> We do have at least some ubsan tests in our CI right now >>> (eg the "clang-system" job). The problem with ubsan coverage >>> is the usual one that we already have too much CI going on, >>> and it takes forever and we don't have that much headroom >>> for adding more jobs. ... >> Would that hurt so much to have one configuration enabled with ubsan and >> asan, which catches *real* bugs, and potential security issues? >> Yes, it adds overhead, but it should not be x10. Around x2 to x3. > > You'd need to have a duplicate of all of the above > functional-system-* test jobs if you wanted > to test all the guest architectures, I think. So it's > 30 mins build * six configs plus 60 mins total for testing. > Or we could convert (some of?) the existing jobs to use the > sanitisers if we needed to economise on CI time. I agree with Peter that having additional build jobs is currently rather a no-go ... but maybe we could enable ubsan and/or asan in some (more) of the existing pipelines? Thomas
Thanks! On Mon, Feb 24, 2025 at 12:51 PM Pierrick Bouvier < pierrick.bouvier@linaro.org> wrote: > Regression introduced by cf76c4 > (hw/misc: Add nr_regs and cold_reset_values to NPCM CLK) > > cold_reset_values has a different size, depending on device used > (NPCM7xx vs NPCM8xx). However, s->regs has a fixed size, which matches > NPCM8xx. Thus, when initializing a NPCM7xx, we go past cold_reset_values > ending. > > Report by asan: > ==2066==ERROR: AddressSanitizer: global-buffer-overflow on address > 0x55d68a3e97f0 at pc 0x7fcaf2b2d14b bp 0x7ffff0cc3890 sp 0x7ffff0cc3040 > READ of size 196 at 0x55d68a3e97f0 thread T0 > #0 0x7fcaf2b2d14a in __interceptor_memcpy > ../../../../src/libsanitizer/sanitizer_common/sanitizer_common_interceptors.inc:827 > #1 0x55d688447e0d in memcpy > /usr/include/x86_64-linux-gnu/bits/string_fortified.h:29 > #2 0x55d688447e0d in npcm_clk_enter_reset ../hw/misc/npcm_clk.c:968 > #3 0x55d6899b7213 in resettable_phase_enter ../hw/core/resettable.c:136 > #4 0x55d6899a1ef7 in bus_reset_child_foreach ../hw/core/bus.c:97 > #5 0x55d6899b717d in resettable_child_foreach > ../hw/core/resettable.c:92 > #6 0x55d6899b717d in resettable_phase_enter ../hw/core/resettable.c:129 > #7 0x55d6899b4ead in resettable_container_child_foreach > ../hw/core/resetcontainer.c:54 > #8 0x55d6899b717d in resettable_child_foreach > ../hw/core/resettable.c:92 > #9 0x55d6899b717d in resettable_phase_enter ../hw/core/resettable.c:129 > #10 0x55d6899b7bfa in resettable_assert_reset > ../hw/core/resettable.c:55 > #11 0x55d6899b8666 in resettable_reset ../hw/core/resettable.c:45 > #12 0x55d688d15cd2 in qemu_system_reset ../system/runstate.c:527 > #13 0x55d687fc5edd in qdev_machine_creation_done > ../hw/core/machine.c:1738 > #14 0x55d688d209bd in qemu_machine_creation_done ../system/vl.c:2779 > #15 0x55d688d209bd in qmp_x_exit_preconfig ../system/vl.c:2807 > #16 0x55d688d281fb in qemu_init ../system/vl.c:3838 > #17 0x55d687ceab12 in main ../system/main.c:68 > #18 0x7fcaef006249 (/lib/x86_64-linux-gnu/libc.so.6+0x27249) > #19 0x7fcaef006304 in __libc_start_main > (/lib/x86_64-linux-gnu/libc.so.6+0x27304) > #20 0x55d687cf0010 in _start > (/home/runner/work/qemu-ci/qemu-ci/build/qemu-system-arm+0x371c010) > > 0x55d68a3e97f0 is located 0 bytes to the right of global variable > 'npcm7xx_cold_reset_values' defined in '../hw/misc/npcm_clk.c:134:23' > (0x55d68a3e9780) of size 112 > > Impacted tests: > Summary of Failures: > > check: > 2/747 qemu:qtest+qtest-aarch64 / qtest-aarch64/qom-test > ERROR 9.28s killed by signal 6 SIGABRT > 4/747 qemu:qtest+qtest-arm / qtest-arm/qom-test > ERROR 7.82s killed by signal 6 SIGABRT > 32/747 qemu:qtest+qtest-aarch64 / qtest-aarch64/device-introspect-test > ERROR 10.91s killed by signal 6 SIGABRT > 35/747 qemu:qtest+qtest-arm / qtest-arm/device-introspect-test > ERROR 11.33s killed by signal 6 SIGABRT > 114/747 qemu:qtest+qtest-arm / qtest-arm/npcm7xx_pwm-test > ERROR 0.98s killed by signal 6 SIGABRT > 115/747 qemu:qtest+qtest-aarch64 / qtest-aarch64/test-hmp > ERROR 2.95s killed by signal 6 SIGABRT > 117/747 qemu:qtest+qtest-arm / qtest-arm/test-hmp > ERROR 2.54s killed by signal 6 SIGABRT > 151/747 qemu:qtest+qtest-arm / qtest-arm/npcm7xx_watchdog_timer-test > ERROR 0.96s killed by signal 6 SIGABRT > 247/747 qemu:qtest+qtest-arm / qtest-arm/npcm7xx_adc-test > ERROR 0.96s killed by signal 6 SIGABRT > 248/747 qemu:qtest+qtest-arm / qtest-arm/npcm7xx_gpio-test > ERROR 1.05s killed by signal 6 SIGABRT > 249/747 qemu:qtest+qtest-arm / qtest-arm/npcm7xx_rng-test > ERROR 0.97s killed by signal 6 SIGABRT > 250/747 qemu:qtest+qtest-arm / qtest-arm/npcm7xx_sdhci-test > ERROR 0.97s killed by signal 6 SIGABRT > 251/747 qemu:qtest+qtest-arm / qtest-arm/npcm7xx_smbus-test > ERROR 0.89s killed by signal 6 SIGABRT > 252/747 qemu:qtest+qtest-arm / qtest-arm/npcm7xx_timer-test > ERROR 1.09s killed by signal 6 SIGABRT > 253/747 qemu:qtest+qtest-arm / qtest-arm/npcm_gmac-test > ERROR 1.12s killed by signal 6 SIGABRT > 255/747 qemu:qtest+qtest-arm / qtest-arm/npcm7xx_emc-test > ERROR 1.05s killed by signal 6 SIGABRT > > check-functional: > 22/203 qemu:func-thorough+func-arm-thorough+thorough / > func-arm-arm_quanta_gsj ERROR 0.79s exit > status 1 > 38/203 qemu:func-quick+func-aarch64 / func-aarch64-migration > ERROR 1.97s exit status 1 > 45/203 qemu:func-quick+func-arm / func-arm-migration > ERROR 1.90s exit status 1 > > Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org> > Reviewed-by: Hao Wu <wuhaotsh@google.com> > --- > hw/misc/npcm_clk.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/hw/misc/npcm_clk.c b/hw/misc/npcm_clk.c > index d1f29759d59..0e85974cf96 100644 > --- a/hw/misc/npcm_clk.c > +++ b/hw/misc/npcm_clk.c > @@ -964,8 +964,9 @@ static void npcm_clk_enter_reset(Object *obj, > ResetType type) > NPCMCLKState *s = NPCM_CLK(obj); > NPCMCLKClass *c = NPCM_CLK_GET_CLASS(s); > > - g_assert(sizeof(s->regs) >= c->nr_regs * sizeof(uint32_t)); > - memcpy(s->regs, c->cold_reset_values, sizeof(s->regs)); > + size_t sizeof_regs = c->nr_regs * sizeof(uint32_t); > + g_assert(sizeof(s->regs) >= sizeof_regs); > + memcpy(s->regs, c->cold_reset_values, sizeof_regs); > s->ref_ns = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL); > npcm7xx_clk_update_all_clocks(s); > /* > -- > 2.39.5 > >
© 2016 - 2025 Red Hat, Inc.