Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

hv: fix possible buffer overflow issues #2061

Merged
merged 1 commit into from Dec 14, 2018
Merged

hv: fix possible buffer overflow issues #2061

merged 1 commit into from Dec 14, 2018

Conversation

yonghuah
Copy link
Contributor

  • cpu_secondary_init() @cpu.c
  • ptirq_intx_pin_remap() @ assign.c
    etc.

Tracked-On: #1252
Signed-off-by: Yonghua Huang yonghua.huang@intel.com
Acked-by: Eddie Dong eddie.dong@intel.com

@ghost
Copy link

ghost commented Dec 13, 2018

2061
6 new potential violations to the coding guideline detected. Please help check. Thanks!

1.filename:/hypervisor/arch/x86/assign.c  function:ptirq_intx_pin_remap  offset:632:
       reason:Procedure contains UR data flow anomalies. : entry

2.filename:/hypervisor/arch/x86/assign.c  function:ptirq_intx_pin_remap  offset:724:
       reason:Attempt to use uninitialised pointer. : entry

3.filename:/hypervisor/common/hypercall.c  function:hcall_vm_intr_monitor  offset:1066:
       reason:Value is not of appropriate type. : (signed char and unsigned char): intr_hdr -> buf_cnt <= ( 24 * 2U

4.filename:/hypervisor/common/hypercall.c  function:hcall_vm_intr_monitor  offset:1066:
       reason:Literal value requires a U suffix. : 24

5.filename:/hypervisor/common/hypercall.c  function:hcall_vm_intr_monitor  offset:1066:
       reason:Use of mixed mode arithmetic. : (unsigned int and signed char): intr_hdr -> buf_cnt <= ( 24 * 2U

6.filename:/hypervisor/common/hypercall.c  function:hcall_vm_intr_monitor  offset:1066:
       reason:Value is not of appropriate type. : (unsigned int and signed char): intr_hdr -> buf_cnt <= ( 24 *

dongyaozu
dongyaozu requested changes Dec 13, 2018
View reviewed changes
hypervisor/arch/x86/assign.c Outdated
if (pic_pin && (virt_pin > NR_VPIC_PINS_TOTAL)) {
status = -EINVAL;
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you can have "else" here, which is easier.

hypervisor/arch/x86/assign.c Outdated
pr_err("%s, add intx remapping failed",
__func__);
status = -ENODEV;
spinlock_release(&ptdev_lock);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the spinlock can be moved out of the {}. We can merge this now, without goto END

@ghost
Copy link

ghost commented Dec 14, 2018

2061
6 new potential violations to the coding guideline detected. Please help check. Thanks!

1.filename:/hypervisor/arch/x86/assign.c  function:ptirq_intx_pin_remap  offset:632:
       reason:Procedure contains UR data flow anomalies. : entry

2.filename:/hypervisor/arch/x86/assign.c  function:ptirq_intx_pin_remap  offset:724:
       reason:Attempt to use uninitialised pointer. : entry

3.filename:/hypervisor/common/hypercall.c  function:hcall_vm_intr_monitor  offset:1066:
       reason:Value is not of appropriate type. : (signed char and unsigned char): intr_hdr -> buf_cnt <= ( 24 * 2U

4.filename:/hypervisor/common/hypercall.c  function:hcall_vm_intr_monitor  offset:1066:
       reason:Literal value requires a U suffix. : 24

5.filename:/hypervisor/common/hypercall.c  function:hcall_vm_intr_monitor  offset:1066:
       reason:Use of mixed mode arithmetic. : (unsigned int and signed char): intr_hdr -> buf_cnt <= ( 24 * 2U

6.filename:/hypervisor/common/hypercall.c  function:hcall_vm_intr_monitor  offset:1066:
       reason:Value is not of appropriate type. : (unsigned int and signed char): intr_hdr -> buf_cnt <= ( 24 *

@yonghuah
Copy link
Contributor Author

@HuiHuangShi @dongyaozu thanks for comments, have updated, please review, thanks.

@ghost
Copy link

ghost commented Dec 14, 2018

2061
No new violations to the coding guideline detected.

1 similar comment
@ghost
Copy link

ghost commented Dec 14, 2018

2061
No new violations to the coding guideline detected.

@ghost ghost added the Coding Guidelines: PASS Coding Guidelines Manual Check label Dec 14, 2018
@yonghuah yonghuah force-pushed the hv_fix_KW_issues_ww50 branch 2 times, most recently from cb94e36 to 3c8d8e5 Compare December 14, 2018 05:54
@yonghuah
Copy link
Contributor Author

@dongyaozu Eddie, have update for validating pCPU ID statement, pls review. thanks.

dongyaozu
dongyaozu approved these changes Dec 14, 2018
View reviewed changes
hypervisor/arch/x86/cpu.c Outdated
@@ -412,6 +412,7 @@ void init_cpu_pre(uint16_t pcpu_id)
bitmap_set_nolock(pcpu_id, &pcpu_active_bitmap);

/* Set state for this CPU to initializing */
ASSERT(pcpu_id < CONFIG_MAX_PCPU_NUM, "");
cpu_set_current_state(pcpu_id, PCPU_STATE_INITIALIZING);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why we need this one? The caller side is using fixed #

@acrnsi
Copy link

acrnsi commented Dec 14, 2018

Ok to verify

@ghost
Copy link

ghost commented Dec 14, 2018

2061
No new violations to the coding guideline detected.

@ghost
Copy link

ghost commented Dec 14, 2018

2061
No new violations to the coding guideline detected.

@wenlingz wenlingz merged commit 57bf26d into projectacrn:master Dec 14, 2018
@yonghuah
hv: fix possible buffer overflow issues
ce5f57c 
 - cpu_secondary_init() @cpu.c
 - ptirq_intx_pin_remap() @ assign.c
   etc.

Tracked-On: projectacrn#1252
Signed-off-by: Yonghua Huang <yonghua.huang@intel.com>
Acked-by: Eddie Dong <eddie.dong@intel.com>
@yonghuah yonghuah deleted the hv_fix_KW_issues_ww50 branch June 20, 2019 14:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dongyaozu dongyaozu dongyaozu approved these changes

@anthonyzxu anthonyzxu Awaiting requested review from anthonyzxu

Assignees
No one assigned
Labels
Coding Guidelines: PASS Coding Guidelines Manual Check Tracked On Pass
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@yonghuah @acrnsi @dongyaozu @wenlingz @Mingyuan18