* [Qemu-devel] [PATCH] kvm-all: Use 'tmpcpu' instead of 'cpu' in sub-looping to avoid 'cpu' be NULL
@ 2014-07-19 1:21 Chen Gang
2014-07-20 7:29 ` Jan Kiszka
2014-07-21 9:53 ` Paolo Bonzini
0 siblings, 2 replies; 4+ messages in thread
From: Chen Gang @ 2014-07-19 1:21 UTC (permalink / raw)
To: Michael Tokarev, pbonzini; +Cc: qemu-trivial, qemu-devel, kvm
If kvm_arch_remove_sw_breakpoint() in CPU_FOREACH() always be fail, it
will let 'cpu' NULL. And the next kvm_arch_remove_sw_breakpoint() in
QTAILQ_FOREACH_SAFE() will get NULL parameter for 'cpu'.
And kvm_arch_remove_sw_breakpoint() can assumes 'cpu' must never be NULL,
so need define additional temporary variable for 'cpu' to avoid the case.
Signed-off-by: Chen Gang <gang.chen.5i5j@gmail.com>
---
kvm-all.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/kvm-all.c b/kvm-all.c
index 3ae30ee..1402f4f 100644
--- a/kvm-all.c
+++ b/kvm-all.c
@@ -2077,12 +2077,13 @@ void kvm_remove_all_breakpoints(CPUState *cpu)
{
struct kvm_sw_breakpoint *bp, *next;
KVMState *s = cpu->kvm_state;
+ CPUState *tmpcpu;
QTAILQ_FOREACH_SAFE(bp, &s->kvm_sw_breakpoints, entry, next) {
if (kvm_arch_remove_sw_breakpoint(cpu, bp) != 0) {
/* Try harder to find a CPU that currently sees the breakpoint. */
- CPU_FOREACH(cpu) {
- if (kvm_arch_remove_sw_breakpoint(cpu, bp) == 0) {
+ CPU_FOREACH(tmpcpu) {
+ if (kvm_arch_remove_sw_breakpoint(tmpcpu, bp) == 0) {
break;
}
}
--
1.7.11.7
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [Qemu-devel] [PATCH] kvm-all: Use 'tmpcpu' instead of 'cpu' in sub-looping to avoid 'cpu' be NULL
2014-07-19 1:21 [Qemu-devel] [PATCH] kvm-all: Use 'tmpcpu' instead of 'cpu' in sub-looping to avoid 'cpu' be NULL Chen Gang
@ 2014-07-20 7:29 ` Jan Kiszka
2014-07-20 8:29 ` Chen Gang
2014-07-21 9:53 ` Paolo Bonzini
1 sibling, 1 reply; 4+ messages in thread
From: Jan Kiszka @ 2014-07-20 7:29 UTC (permalink / raw)
To: Chen Gang, Michael Tokarev, pbonzini; +Cc: qemu-trivial, qemu-devel, kvm
[-- Attachment #1: Type: text/plain, Size: 1595 bytes --]
On 2014-07-19 03:21, Chen Gang wrote:
> If kvm_arch_remove_sw_breakpoint() in CPU_FOREACH() always be fail, it
> will let 'cpu' NULL. And the next kvm_arch_remove_sw_breakpoint() in
> QTAILQ_FOREACH_SAFE() will get NULL parameter for 'cpu'.
>
> And kvm_arch_remove_sw_breakpoint() can assumes 'cpu' must never be NULL,
> so need define additional temporary variable for 'cpu' to avoid the case.
>
>
> Signed-off-by: Chen Gang <gang.chen.5i5j@gmail.com>
> ---
> kvm-all.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/kvm-all.c b/kvm-all.c
> index 3ae30ee..1402f4f 100644
> --- a/kvm-all.c
> +++ b/kvm-all.c
> @@ -2077,12 +2077,13 @@ void kvm_remove_all_breakpoints(CPUState *cpu)
> {
> struct kvm_sw_breakpoint *bp, *next;
> KVMState *s = cpu->kvm_state;
> + CPUState *tmpcpu;
>
> QTAILQ_FOREACH_SAFE(bp, &s->kvm_sw_breakpoints, entry, next) {
> if (kvm_arch_remove_sw_breakpoint(cpu, bp) != 0) {
> /* Try harder to find a CPU that currently sees the breakpoint. */
> - CPU_FOREACH(cpu) {
> - if (kvm_arch_remove_sw_breakpoint(cpu, bp) == 0) {
> + CPU_FOREACH(tmpcpu) {
> + if (kvm_arch_remove_sw_breakpoint(tmpcpu, bp) == 0) {
> break;
> }
> }
>
Good catch. To make it clear in the changelog: The actual issue is that
we misuse "cpu" as an iteration variable while its original value is
still in use. That cpu can eventually become NULL this way is one result.
Jan
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 263 bytes --]
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [Qemu-devel] [PATCH] kvm-all: Use 'tmpcpu' instead of 'cpu' in sub-looping to avoid 'cpu' be NULL
2014-07-20 7:29 ` Jan Kiszka
@ 2014-07-20 8:29 ` Chen Gang
0 siblings, 0 replies; 4+ messages in thread
From: Chen Gang @ 2014-07-20 8:29 UTC (permalink / raw)
To: Jan Kiszka; +Cc: qemu-trivial, pbonzini, Michael Tokarev, qemu-devel, kvm
On 07/20/2014 03:29 PM, Jan Kiszka wrote:
> On 2014-07-19 03:21, Chen Gang wrote:
>> If kvm_arch_remove_sw_breakpoint() in CPU_FOREACH() always be fail, it
>> will let 'cpu' NULL. And the next kvm_arch_remove_sw_breakpoint() in
>> QTAILQ_FOREACH_SAFE() will get NULL parameter for 'cpu'.
>>
>> And kvm_arch_remove_sw_breakpoint() can assumes 'cpu' must never be NULL,
>> so need define additional temporary variable for 'cpu' to avoid the case.
>>
>>
>> Signed-off-by: Chen Gang <gang.chen.5i5j@gmail.com>
>> ---
>> kvm-all.c | 5 +++--
>> 1 file changed, 3 insertions(+), 2 deletions(-)
>>
>> diff --git a/kvm-all.c b/kvm-all.c
>> index 3ae30ee..1402f4f 100644
>> --- a/kvm-all.c
>> +++ b/kvm-all.c
>> @@ -2077,12 +2077,13 @@ void kvm_remove_all_breakpoints(CPUState *cpu)
>> {
>> struct kvm_sw_breakpoint *bp, *next;
>> KVMState *s = cpu->kvm_state;
>> + CPUState *tmpcpu;
>>
>> QTAILQ_FOREACH_SAFE(bp, &s->kvm_sw_breakpoints, entry, next) {
>> if (kvm_arch_remove_sw_breakpoint(cpu, bp) != 0) {
>> /* Try harder to find a CPU that currently sees the breakpoint. */
>> - CPU_FOREACH(cpu) {
>> - if (kvm_arch_remove_sw_breakpoint(cpu, bp) == 0) {
>> + CPU_FOREACH(tmpcpu) {
>> + if (kvm_arch_remove_sw_breakpoint(tmpcpu, bp) == 0) {
>> break;
>> }
>> }
>>
>
> Good catch. To make it clear in the changelog: The actual issue is that
> we misuse "cpu" as an iteration variable while its original value is
> still in use. That cpu can eventually become NULL this way is one result.
>
OK, thanks. If necessary, I shall send patch v2 for additional comments.
(if really necessary to send, please let me know)
Thanks.
--
Chen Gang
Open share and attitude like air water and life which God blessed
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [Qemu-devel] [PATCH] kvm-all: Use 'tmpcpu' instead of 'cpu' in sub-looping to avoid 'cpu' be NULL
2014-07-19 1:21 [Qemu-devel] [PATCH] kvm-all: Use 'tmpcpu' instead of 'cpu' in sub-looping to avoid 'cpu' be NULL Chen Gang
2014-07-20 7:29 ` Jan Kiszka
@ 2014-07-21 9:53 ` Paolo Bonzini
1 sibling, 0 replies; 4+ messages in thread
From: Paolo Bonzini @ 2014-07-21 9:53 UTC (permalink / raw)
To: Chen Gang, Michael Tokarev; +Cc: qemu-trivial, qemu-devel, kvm
Il 19/07/2014 03:21, Chen Gang ha scritto:
> If kvm_arch_remove_sw_breakpoint() in CPU_FOREACH() always be fail, it
> will let 'cpu' NULL. And the next kvm_arch_remove_sw_breakpoint() in
> QTAILQ_FOREACH_SAFE() will get NULL parameter for 'cpu'.
>
> And kvm_arch_remove_sw_breakpoint() can assumes 'cpu' must never be NULL,
> so need define additional temporary variable for 'cpu' to avoid the case.
>
>
> Signed-off-by: Chen Gang <gang.chen.5i5j@gmail.com>
> ---
> kvm-all.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/kvm-all.c b/kvm-all.c
> index 3ae30ee..1402f4f 100644
> --- a/kvm-all.c
> +++ b/kvm-all.c
> @@ -2077,12 +2077,13 @@ void kvm_remove_all_breakpoints(CPUState *cpu)
> {
> struct kvm_sw_breakpoint *bp, *next;
> KVMState *s = cpu->kvm_state;
> + CPUState *tmpcpu;
>
> QTAILQ_FOREACH_SAFE(bp, &s->kvm_sw_breakpoints, entry, next) {
> if (kvm_arch_remove_sw_breakpoint(cpu, bp) != 0) {
> /* Try harder to find a CPU that currently sees the breakpoint. */
> - CPU_FOREACH(cpu) {
> - if (kvm_arch_remove_sw_breakpoint(cpu, bp) == 0) {
> + CPU_FOREACH(tmpcpu) {
> + if (kvm_arch_remove_sw_breakpoint(tmpcpu, bp) == 0) {
> break;
> }
> }
>
Applying to uq/master, thanks.
Paolo
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2014-07-21 9:53 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-07-19 1:21 [Qemu-devel] [PATCH] kvm-all: Use 'tmpcpu' instead of 'cpu' in sub-looping to avoid 'cpu' be NULL Chen Gang
2014-07-20 7:29 ` Jan Kiszka
2014-07-20 8:29 ` Chen Gang
2014-07-21 9:53 ` Paolo Bonzini
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).