Skip to contentSkip to conversation

KVM: dereference of NULL pointer in set_pal_result()

PATCH In review Roel Kluin 3 participants 5 messages Jan 7, 2010 – Jan 8, 2010

lore
Share & tools
lore

Roel Kluin

author
#0 lore raw

PATCH 1/1 KVM: dereference of NULL pointer in set_pal_result()

Do not dereference a NULL pointer

Signed-off-by Roel Kluin <[email protected]>
---
 arch/ia64/kvm/kvm_fw.c |    6 ++++--
 1 files changed, 4 insertions(+), 2 deletions(-)
arch/ia64/kvm/kvm_fw.c +4 -2
index e4b8231..d28494f 100644
@@ -75,9 +75,11 @@ static void set_pal_result(struct kvm_vcpu *vcpu,
 	struct exit_ctl_data *p;
 
 	p = kvm_get_exit_data(vcpu);
-	if (p && p->exit_reason == EXIT_REASON_PAL_CALL) {
+	if (!p)
+		return;
+	if (p->exit_reason == EXIT_REASON_PAL_CALL) {
 		p->u.pal_data.ret = result;
-		return ;
+		return;
 	}
 	INIT_PAL_STATUS_UNIMPLEMENTED(p->u.pal_data.ret);
 }
#1 lore raw

For the check "(p && p->exit_reason = EXIT_REASON_PAL_CALL", if p is NULL, the reference about "p->exit_reason = EXIT_REASON_PAL_CALL" won't be checked any more, so no issue here. Xiantao

Quoted 25 lines · Roel Kluin

Do not dereference a NULL pointer

Signed-off-by Roel Kluin <[email protected]>
---
 arch/ia64/kvm/kvm_fw.c |    6 ++++--
 1 files changed, 4 insertions(+), 2 deletions(-)
arch/ia64/kvm/kvm_fw.c +4 -2
index e4b8231..d28494f 100644
@@ -75,9 +75,11 @@ static void set_pal_result(struct kvm_vcpu *vcpu,
 	struct exit_ctl_data *p;
 
 	p = kvm_get_exit_data(vcpu);
-	if (p && p->exit_reason = EXIT_REASON_PAL_CALL) {
+	if (!p)
+		return;
+	if (p->exit_reason = EXIT_REASON_PAL_CALL) {
 		p->u.pal_data.ret = result;
-		return ;
+		return;
 	}
 	INIT_PAL_STATUS_UNIMPLEMENTED(p->u.pal_data.ret);
 }
#2 lore raw
Quoted 9 lines · Xiantao

For the check "(p && p->exit_reason = EXIT_REASON_PAL_CALL", if p is NULL, the reference about "p->exit_reason = EXIT_REASON_PAL_CALL" won't be checked any more, so no issue here.

Quoted 7 lines
arch/ia64/kvm/kvm_fw.c +0 -0
index e4b8231..d28494f 100644
@@ -75,9 +75,11 @@ static void set_pal_result(struct kvm_vcpu *vcpu,
 	struct exit_ctl_data *p;
Quoted 10 lines

arch/ia64/kvm/kvm_fw.c static void set_pal_result(struct kvm_vcpu *vcpu, · line 84

Quoted 10 lines
arch/ia64/kvm/kvm_fw.c +4 -2
 	p = kvm_get_exit_data(vcpu);
-	if (p && p->exit_reason = EXIT_REASON_PAL_CALL) {
+	if (!p)
+		return;
+	if (p->exit_reason = EXIT_REASON_PAL_CALL) {
 		p->u.pal_data.ret = result;
-		return ;
+		return;
 	}
 	INIT_PAL_STATUS_UNIMPLEMENTED(p->u.pal_data.ret);

IMHO it's not the test but the INIT_PAL_STATUS_UNIMPLEMENTED() that does the unwanted dereferencing, and that's fixed by the patch.

Gabor

Signature · 4 lines
     ---------------------------------------------------------
     MTA SZTAKI Computer and Automation Research Institute
                Hungarian Academy of Sciences
     ---------------------------------------------------------
#3 lore raw
Quoted 13 lines · Gabor Gombas
Quoted 11 lines · Xiantao

For the check "(p && p->exit_reason = EXIT_REASON_PAL_CALL", if p is NULL, the reference about "p->exit_reason = EXIT_REASON_PAL_CALL" won't be checked any more, so no issue here.

Quoted 7 lines
arch/ia64/kvm/kvm_fw.c +0 -0
index e4b8231..d28494f 100644
@@ -75,9 +75,11 @@ static void set_pal_result(struct kvm_vcpu
 *vcpu,  	struct exit_ctl_data *p;
Quoted 12 lines
Quoted 9 lines

arch/ia64/kvm/kvm_fw.c static void set_pal_result(struct kvm_vcpu *vcpu, · line 84

Quoted 9 lines
arch/ia64/kvm/kvm_fw.c +3 -2
 	p = kvm_get_exit_data(vcpu);
-	if (p && p->exit_reason = EXIT_REASON_PAL_CALL) { +	if (!p)
+		return;
+	if (p->exit_reason = EXIT_REASON_PAL_CALL) {
 		p->u.pal_data.ret = result;
-		return ;
+		return;
 	}
 	INIT_PAL_STATUS_UNIMPLEMENTED(p->u.pal_data.ret);

IMHO it's not the test but the INIT_PAL_STATUS_UNIMPLEMENTED() that does the unwanted dereferencing, and that's fixed by the patch.

Make sense.
Xiantao

author
#4 lore raw
In reply to Gabor Gombas
Quoted 13 lines
Quoted 12 lines

For the check "(p && p->exit_reason = EXIT_REASON_PAL_CALL", if p is NULL, the reference about "p->exit_reason = EXIT_REASON_PAL_CALL" won't be checked any more, so no issue here.

Quoted 10 lines

    p = kvm_get_exit_data(vcpu);
-   if (p && p->exit_reason = EXIT_REASON_PAL_CALL) {
+   if (!p)
+           return;
+   if (p->exit_reason = EXIT_REASON_PAL_CALL) { p->u.pal_data.ret = result;
-           return ;
+           return;
    } INIT_PAL_STATUS_UNIMPLEMENTED(p->u.pal_data.ret);

Quoted 4 lines

IMHO it's not the test but the INIT_PAL_STATUS_UNIMPLEMENTED() that does the unwanted dereferencing, and that's fixed by the patch.

Gabor

Correct. Thanks,

Roel