vxge: potential NULL dereference
Subsystems (3)
PATCH 1/1 vxge: potential NULL dereference
At the start of the function we test whether the "vpath" is NULL but we need another test here as well.
---
This is a static checker bug, I'm not sure if we ever pass a NULL pointer for "vpath".
drivers/net/vxge/vxge-traffic.c +2 -1
@@ -2157,7 +2157,8 @@ out2: (alarm_event = VXGE_HW_EVENT_UNKNOWN))return VXGE_HW_OK ; -__vxge_hw_device_handle_error (hldev, vpath->vp_id, alarm_event); +if (vpath) +__vxge_hw_device_handle_error (hldev, vpath->vp_id, alarm_event);if (alarm_event =VXGE_HW_EVENT_SERR )return VXGE_HW_ERR_CRITICAL ;
David Miller
maintainerFrom: Dan Carpenter <[email protected]>
Date: Fri, 10 Sep 2010 13:54:23 +0200
Quoted 7 lines
At the start of the function we test whether the "vpath" is NULL but we need another test here as well.
Signed-off-by Dan Carpenter <[email protected]>---
This is a static checker bug, I'm not sure if we ever pass a NULL pointer for "vpath".
I cannot see any case where this can happen. There are two cases:
1) __vxge_hw_vpath_alarm_process() is invoked via vxge_hw_device_begin_irq(), which looks like:
ret = __vxge_hw_vpath_alarm_process( &hldev->virtual_paths[i], skip_alarms);
that vpath pointer first argument will never be NULL.
2) __vxge_hw_vpath_alarm_process() is invoked via vxge_hw_vpath_alarm_process() which uses:
status = __vxge_hw_vpath_alarm_process(vp->vpath, skip_alarms);
All vpath valid active vpath handles always have a non-NULL vp->vpath virtual path back pointer, as setup by vxge_hw_vpath_open():
...
vp->vpath = vpath; ... *vpath_handle = vp; attr->fifo_attr.userdata = vpath->fifoh; attr->ring_attr.userdata = vpath->ringh; return VXGE_HW_OK;
So we can simply remove the first NULL check as this can never actually be NULL.
Quoted 2 lines · David Miller
So we can simply remove the first NULL check as this can never actually be NULL.
Ok. Thanks, Dave. I should have looked more into this myself. I'll send a patch to remove the check tomorrow.
regards,
dan carpenter
Quoted 14 lines · David Miller
From: Dan Carpenter <[email protected]>
Date: Fri, 10 Sep 2010 13:54:23 +0200Quoted 7 lines
At the start of the function we test whether the "vpath" is NULL but we need another test here as well.
Signed-off-by Dan Carpenter <[email protected]>---
This is a static checker bug, I'm not sure if we ever pass a NULL pointer for "vpath".I cannot see any case where this can happen. There are two cases:
Quoted 7 lines
1)
__vxge_hw_vpath_alarm_process()is invoked viavxge_hw_device_begin_irq(), which looks like:ret = __vxge_hw_vpath_alarm_process( &hldev->virtual_paths[i], skip_alarms);that vpath pointer first argument will never be NULL.
It is possible to the vpath to be NULL in this array if it is not populated in __vxge_hw_vp_initialize due to the vpath being masked off my the adapter. vxge_hw_device_begin_irq calls __vxge_hw_vpath_alarm_process on all possible vpaths regardless of their allocation. This is the case we need to worry about.
It is not an issue because in the first instance of the vpath being NULL, its sets the alarm_event to be VXGE_HW_EVENT_UNKNOWN. The first check in the out2 error path checks for VXGE_HW_EVENT_UNKNOWN and returns. So its not possible to hit this...though it is ugly code. I welcome a reworking of the code to something mroe elegant. :)
Thanks,
Jon
Quoted 21 lines
2)
__vxge_hw_vpath_alarm_process()is invoked viavxge_hw_vpath_alarm_process()which uses:status = __vxge_hw_vpath_alarm_process(vp->vpath, skip_alarms);All vpath valid active vpath handles always have a non-NULL
vp->vpathvirtual path back pointer, as setup byvxge_hw_vpath_open():...
vp->vpath = vpath; ... *vpath_handle = vp; attr->fifo_attr.userdata = vpath->fifoh; attr->ring_attr.userdata = vpath->ringh; return VXGE_HW_OK;So we can simply remove the first NULL check as this can never actually be NULL.
David Miller
maintainerFrom: Jon Mason <[email protected]>
Date: Fri, 10 Sep 2010 17:32:15 -0400
Quoted 11 lines
It is possible to the vpath to be NULL in this array if it is not populated in
__vxge_hw_vp_initializedue to the vpath being masked off my the adapter.vxge_hw_device_begin_irqcalls__vxge_hw_vpath_alarm_processon all possible vpaths regardless of their allocation. This is the case we need to worry about.It is not an issue because in the first instance of the vpath being NULL, its sets the
alarm_eventto beVXGE_HW_EVENT_UNKNOWN. The first check in the out2 error path checks forVXGE_HW_EVENT_UNKNOWNand returns. So its not possible to hit this...though it is ugly code. I welcome a reworking of the code to something mroe elegant. :)
Aha, I see, thanks for explaining this.