lib/librte_power: fix using variables before validity check
Checks
Commit Message
From: HongBo Zheng <zhenghongbo3@huawei.com>
In function power_guest_channel_read_msg, 'lcore_id' is used before
validity check, which may cause buffer 'global_fds' accessed by index
'lcore_id' overflow.
This patch moves the validity check of 'lcore_id' before the 'lcore_id'
being used for the first time.
Fixes: 9dc843eb273b ("power: extend guest channel API for reading")
Cc: stable@dpdk.org
Signed-off-by: HongBo Zheng <zhenghongbo3@huawei.com>
Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
---
lib/power/guest_channel.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
Comments
> -----Original Message-----
> From: dev <dev-bounces@dpdk.org> On Behalf Of Min Hu (Connor)
>
> + if (lcore_id >= RTE_MAX_LCORE) {
> + RTE_LOG(ERR, GUEST_CHANNEL, "Channel(%u) is out of range
> 0...%d\n",
> + lcore_id, RTE_MAX_LCORE-1);
> + return -1;
> + }
> +
This looks good.
> fds.fd = global_fds[lcore_id];
> fds.events = POLLIN;
>
<snip>
> if (global_fds[lcore_id] < 0) {
> RTE_LOG(ERR, GUEST_CHANNEL, "Channel is not
> connected\n");
Another suggestion if you would like to improve the code, is below,
You can move "global_fds[lcore_id]" check to immediate after the line "fds.fd = global_fds[lcore_id]; "
Reviewed-by: Reshma Pattan <reshma.pattan@intel.com>
在 2021/4/28 23:13, Pattan, Reshma 写道:
>
>
>> -----Original Message-----
>> From: dev <dev-bounces@dpdk.org> On Behalf Of Min Hu (Connor)
>>
>> + if (lcore_id >= RTE_MAX_LCORE) {
>> + RTE_LOG(ERR, GUEST_CHANNEL, "Channel(%u) is out of range
>> 0...%d\n",
>> + lcore_id, RTE_MAX_LCORE-1);
>> + return -1;
>> + }
>> +
>
> This looks good.
>
>> fds.fd = global_fds[lcore_id];
>> fds.events = POLLIN;
>>
> <snip>
>
>> if (global_fds[lcore_id] < 0) {
>> RTE_LOG(ERR, GUEST_CHANNEL, "Channel is not
>> connected\n");
>
>
> Another suggestion if you would like to improve the code, is below,
> You can move "global_fds[lcore_id]" check to immediate after the line "fds.fd = global_fds[lcore_id]; "
Hi, thanks Pattan.
But I think "global_fds[lcore_id]" check may move before the line
"fds.fd = global_fds[lcore_id];
If it failed, "fds.fd = global_fds[lcore_id];" should not be performed.
What's your opinion?
>
> Reviewed-by: Reshma Pattan <reshma.pattan@intel.com>
>
> .
>
Any comment please?
29/04/2021 02:43, Min Hu (Connor):
>
> 在 2021/4/28 23:13, Pattan, Reshma 写道:
> >
> >
> >> -----Original Message-----
> >> From: dev <dev-bounces@dpdk.org> On Behalf Of Min Hu (Connor)
> >>
> >> + if (lcore_id >= RTE_MAX_LCORE) {
> >> + RTE_LOG(ERR, GUEST_CHANNEL, "Channel(%u) is out of range
> >> 0...%d\n",
> >> + lcore_id, RTE_MAX_LCORE-1);
> >> + return -1;
> >> + }
> >> +
> >
> > This looks good.
> >
> >> fds.fd = global_fds[lcore_id];
> >> fds.events = POLLIN;
> >>
> > <snip>
> >
> >> if (global_fds[lcore_id] < 0) {
> >> RTE_LOG(ERR, GUEST_CHANNEL, "Channel is not
> >> connected\n");
> >
> >
> > Another suggestion if you would like to improve the code, is below,
> > You can move "global_fds[lcore_id]" check to immediate after the line "fds.fd = global_fds[lcore_id]; "
> Hi, thanks Pattan.
> But I think "global_fds[lcore_id]" check may move before the line
> "fds.fd = global_fds[lcore_id];
> If it failed, "fds.fd = global_fds[lcore_id];" should not be performed.
> What's your opinion?
>
> >
> > Reviewed-by: Reshma Pattan <reshma.pattan@intel.com>
> -----Original Message-----
> From: Min Hu (Connor) <humin29@huawei.com>
> > Another suggestion if you would like to improve the code, is below,
> > You can move "global_fds[lcore_id]" check to immediate after the line
> "fds.fd = global_fds[lcore_id]; "
> Hi, thanks Pattan.
> But I think "global_fds[lcore_id]" check may move before the line
> "fds.fd = global_fds[lcore_id];
> If it failed, "fds.fd = global_fds[lcore_id];" should not be performed.
> What's your opinion?
>
That is correct. You can do that change.
@@ -166,6 +166,12 @@ int power_guest_channel_read_msg(void *pkt,
if (pkt_len == 0 || pkt == NULL)
return -1;
+ if (lcore_id >= RTE_MAX_LCORE) {
+ RTE_LOG(ERR, GUEST_CHANNEL, "Channel(%u) is out of range 0...%d\n",
+ lcore_id, RTE_MAX_LCORE-1);
+ return -1;
+ }
+
fds.fd = global_fds[lcore_id];
fds.events = POLLIN;
@@ -179,12 +185,6 @@ int power_guest_channel_read_msg(void *pkt,
return -1;
}
- if (lcore_id >= RTE_MAX_LCORE) {
- RTE_LOG(ERR, GUEST_CHANNEL, "Channel(%u) is out of range 0...%d\n",
- lcore_id, RTE_MAX_LCORE-1);
- return -1;
- }
-
if (global_fds[lcore_id] < 0) {
RTE_LOG(ERR, GUEST_CHANNEL, "Channel is not connected\n");
return -1;