NetworkPkg:Resolved Coverity Issues in MnpDxe #10680
Conversation
Resolved INTEGER_OVERFLOW Coverity Issues in MNP Dxe 1.MnpStop,MnpInstanceDeliverPacket Expression "MnpDeviceData->ConfiguredChildrenNumber--" and Instance->RcvdPacketQueueSize-- where Both variables are known to be equal to 0,underflows the type that receives it. Signed-off-by: santhosh kumar V <santhoshkumarv@ami.com>
tianocore-assign-reviewers
bot
requested review from
ajfish,
leiflindholm,
mdkinney,
SaloniKasbekar and
Zclarkwilliams
| @@ -1305,6 +1305,10 @@ MnpStop ( | |||
| MnpDeviceData = MnpServiceData->MnpDeviceData; | |||
| ASSERT (MnpDeviceData->ConfiguredChildrenNumber > 0); | |||
|
|
|||
| if (MnpDeviceData->ConfiguredChildrenNumber <= 0) { | |||
There was a problem hiding this comment.
ConfiguredChildrenNumber is UINTN - does this change not generate a new Coverity warning?
| @@ -1305,6 +1305,10 @@ MnpStop ( | |||
| MnpDeviceData = MnpServiceData->MnpDeviceData; | |||
| ASSERT (MnpDeviceData->ConfiguredChildrenNumber > 0); | |||
|
|
|||
| if (MnpDeviceData->ConfiguredChildrenNumber <= 0) { | |||
| return EFI_OUT_OF_RESOURCES; | |||
There was a problem hiding this comment.
This does not feel like an appropriate return code.
Also, if we look at the call path, if we return an error here, we will fall back all the way to MnpConfigure which will itself pass this return value on.
But the documentation for MnpConfigure says that for "Others" error codes (as well as for EFI_OUT_OF_RESOURCES), "The MNP child driver instance has been reset to startup defaults.".
But I don't see anything like that happening as a result of this patch.
And given this error could only ever be triggered if the driver state ended up inconcistent, that would feel like an important thing to do.
| @@ -325,6 +325,9 @@ MnpInstanceDeliverPacket ( | |||
| } | |||
|
|
|||
| ASSERT (Instance->RcvdPacketQueueSize != 0); | |||
| if (Instance->RcvdPacketQueueSize <= 0) { | |||
| @@ -325,6 +325,9 @@ MnpInstanceDeliverPacket ( | |||
| } | |||
|
|
|||
| ASSERT (Instance->RcvdPacketQueueSize != 0); | |||
| if (Instance->RcvdPacketQueueSize <= 0) { | |||
| return EFI_OUT_OF_RESOURCES; | |||
There was a problem hiding this comment.
This does not feel like an appropriate return value on finding that the packet queue is empty and there is nothing to deliver.
Resolved INTEGER_OVERFLOW Coverity Issues in MNP Dxe 1.MnpStop,MnpInstanceDeliverPacket
Expression "MnpDeviceData->ConfiguredChildrenNumber--" and Instance->RcvdPacketQueueSize-- where Both variables are known to be equal to 0,underflows the type that receives it.
Description