[dpdk-stable] [PATCH v1] examples/vm_power_manager: fix buffer overrun

[dpdk-stable] [PATCH v1] examples/vm_power_manager: fix buffer overrun

[David Hunt]

The freqs array in freq_info struct has RTE_MAX_LCORE_FREQS elements,
yet the code can attemtp to look at the index at POWER_MANAGER_MAX_CPUS,
which may be greater than RTE_MAX_LCORE_FREQS. Fix to limit index to
RTE_MAX_LCORE_FREQS.

Fixes: d26c18c93260 ("examples/vm_power: cpu frequency in host")
     Coverity issue: 337660

Signed-off-by: David Hunt <david.hunt@intel.com>
---
 examples/vm_power_manager/power_manager.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/examples/vm_power_manager/power_manager.c b/examples/vm_power_manager/power_manager.c
index aef832644..35be30d2e 100644
--- a/examples/vm_power_manager/power_manager.c
+++ b/examples/vm_power_manager/power_manager.c
@@ -143,7 +143,7 @@ power_manager_get_current_frequency(unsigned core_num)
 	rte_spinlock_lock(&global_core_freq_info[core_num].power_sl);
 	index = rte_power_get_freq(core_num);
 	rte_spinlock_unlock(&global_core_freq_info[core_num].power_sl);
-	if (index >= POWER_MGR_MAX_CPUS)
+	if (index >= RTE_MAX_LCORE_FREQS)
 		freq = 0;
 	else
 		freq = global_core_freq_info[core_num].freqs[index];
-- 
2.17.1

Re: [dpdk-stable] [dpdk-dev] [PATCH v1] examples/vm_power_manager: fix buffer overrun

[Pattan, Reshma]

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of David Hunt
> Sent: Wednesday, April 10, 2019 1:49 PM
> To: dev@dpdk.org
> Cc: Hunt, David <david.hunt@intel.com>; stable@dpdk.org
> Subject: [dpdk-dev] [PATCH v1] examples/vm_power_manager: fix buffer
> overrun
> 
> The freqs array in freq_info struct has RTE_MAX_LCORE_FREQS elements, yet
> the code can attemtp to look at the index at POWER_MANAGER_MAX_CPUS,

Couple of nit-picks,

:%s/ attempt/ attempt

> which may be greater than RTE_MAX_LCORE_FREQS. Fix to limit index to
> RTE_MAX_LCORE_FREQS.
> 
> Fixes: d26c18c93260 ("examples/vm_power: cpu frequency in host")
>      Coverity issue: 337660

Candidate for stable@dpdk.org?

Acked-by: Reshma Pattan <reshma.pattan@intel.com>

Thanks,
Reshma

Re: [dpdk-stable] [dpdk-dev] [PATCH v1] examples/vm_power_manager: fix buffer overrun

[Thomas Monjalon]

10/04/2019 14:49, David Hunt:
> The freqs array in freq_info struct has RTE_MAX_LCORE_FREQS elements,
> yet the code can attemtp to look at the index at POWER_MANAGER_MAX_CPUS,
> which may be greater than RTE_MAX_LCORE_FREQS. Fix to limit index to
> RTE_MAX_LCORE_FREQS.
> 
> Fixes: d26c18c93260 ("examples/vm_power: cpu frequency in host")
>      Coverity issue: 337660
> 
> Signed-off-by: David Hunt <david.hunt@intel.com>

It seems to have been fixed in another patch, isn't it?

Re: [dpdk-stable] [dpdk-dev] [PATCH v1] examples/vm_power_manager: fix buffer overrun

[Hunt, David]

Hi Thomas,

On 22/4/2019 10:54 PM, Thomas Monjalon wrote:
> 10/04/2019 14:49, David Hunt:
>> The freqs array in freq_info struct has RTE_MAX_LCORE_FREQS elements,
>> yet the code can attemtp to look at the index at POWER_MANAGER_MAX_CPUS,
>> which may be greater than RTE_MAX_LCORE_FREQS. Fix to limit index to
>> RTE_MAX_LCORE_FREQS.
>>
>> Fixes: d26c18c93260 ("examples/vm_power: cpu frequency in host")
>>       Coverity issue: 337660
>>
>> Signed-off-by: David Hunt <david.hunt@intel.com>
> It seems to have been fixed in another patch, isn't it?
>

It was not fixed in another patch, although I can see the confusion.

A previous patch made the #defines more consistent, and 
POWER_MGR_MAX_CPUS was changed to RTE_MAX_LCORE on the affected line. 
However, this was later revealed as a coverity issue, and was fixed in 
this patch to be RTE_LCORE_MAX_FREQS, which is the size of the array 
it's trying to index into.

So looking at RC2, this patch is still needed.

Rgds,
Dave.

Re: [dpdk-stable] [dpdk-dev] [PATCH v1] examples/vm_power_manager: fix buffer overrun

[Thomas Monjalon]

23/04/2019 10:21, Hunt, David:
> Hi Thomas,
> 
> On 22/4/2019 10:54 PM, Thomas Monjalon wrote:
> > 10/04/2019 14:49, David Hunt:
> >> The freqs array in freq_info struct has RTE_MAX_LCORE_FREQS elements,
> >> yet the code can attemtp to look at the index at POWER_MANAGER_MAX_CPUS,
> >> which may be greater than RTE_MAX_LCORE_FREQS. Fix to limit index to
> >> RTE_MAX_LCORE_FREQS.
> >>
> >> Fixes: d26c18c93260 ("examples/vm_power: cpu frequency in host")
> >>       Coverity issue: 337660
> >>
> >> Signed-off-by: David Hunt <david.hunt@intel.com>
> > It seems to have been fixed in another patch, isn't it?
> >
> 
> It was not fixed in another patch, although I can see the confusion.
> 
> A previous patch made the #defines more consistent, and 
> POWER_MGR_MAX_CPUS was changed to RTE_MAX_LCORE on the affected line. 
> However, this was later revealed as a coverity issue, and was fixed in 
> this patch to be RTE_LCORE_MAX_FREQS, which is the size of the array 
> it's trying to index into.
> 
> So looking at RC2, this patch is still needed.

I think it needs to be rebased in a v2 then.

Re: [dpdk-stable] [dpdk-dev] [PATCH v1] examples/vm_power_manager: fix buffer overrun

[Hunt, David]

On 23/4/2019 9:33 AM, Thomas Monjalon wrote:
> 23/04/2019 10:21, Hunt, David:
>> Hi Thomas,
>>
>> On 22/4/2019 10:54 PM, Thomas Monjalon wrote:
>>> 10/04/2019 14:49, David Hunt:
>>>> The freqs array in freq_info struct has RTE_MAX_LCORE_FREQS elements,
>>>> yet the code can attemtp to look at the index at POWER_MANAGER_MAX_CPUS,
>>>> which may be greater than RTE_MAX_LCORE_FREQS. Fix to limit index to
>>>> RTE_MAX_LCORE_FREQS.
>>>>
>>>> Fixes: d26c18c93260 ("examples/vm_power: cpu frequency in host")
>>>>        Coverity issue: 337660
>>>>
>>>> Signed-off-by: David Hunt <david.hunt@intel.com>
>>> It seems to have been fixed in another patch, isn't it?
>>>
>> It was not fixed in another patch, although I can see the confusion.
>>
>> A previous patch made the #defines more consistent, and
>> POWER_MGR_MAX_CPUS was changed to RTE_MAX_LCORE on the affected line.
>> However, this was later revealed as a coverity issue, and was fixed in
>> this patch to be RTE_LCORE_MAX_FREQS, which is the size of the array
>> it's trying to index into.
>>
>> So looking at RC2, this patch is still needed.
> I think it needs to be rebased in a v2 then.
>

Sure, will do.

[dpdk-stable] [PATCH v2] examples/vm_power_manager: fix buffer overrun

[David Hunt]

The freqs array in freq_info struct has RTE_MAX_LCORE_FREQS elements,
yet the code can attempt to look at the index at  RTE_MAX_LCORE,
which may be greater than RTE_MAX_LCORE_FREQS. Fix to limit index to
RTE_MAX_LCORE_FREQS.

Fixes: d26c18c93260 ("examples/vm_power: cpu frequency in host")
     Coverity issue: 337660

Signed-off-by: David Hunt <david.hunt@intel.com>
Acked-by: Reshma Pattan <reshma.pattan@intel.com>

---
v2 - Rebase to 19.05 RC2.
---
 examples/vm_power_manager/power_manager.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/examples/vm_power_manager/power_manager.c b/examples/vm_power_manager/power_manager.c
index 9553455be..9d4e587b0 100644
--- a/examples/vm_power_manager/power_manager.c
+++ b/examples/vm_power_manager/power_manager.c
@@ -143,7 +143,7 @@ power_manager_get_current_frequency(unsigned core_num)
 	rte_spinlock_lock(&global_core_freq_info[core_num].power_sl);
 	index = rte_power_get_freq(core_num);
 	rte_spinlock_unlock(&global_core_freq_info[core_num].power_sl);
-	if (index >= RTE_MAX_LCORE)
+	if (index >= RTE_MAX_LCORE_FREQS)
 		freq = 0;
 	else
 		freq = global_core_freq_info[core_num].freqs[index];
-- 
2.17.1

Re: [dpdk-stable] [dpdk-dev] [PATCH v1] examples/vm_power_manager: fix buffer overrun

[Kevin Traynor]

On 18/04/2019 16:14, Pattan, Reshma wrote:
>> Fixes: d26c18c93260 ("examples/vm_power: cpu frequency in host")
>>      Coverity issue: 337660
> Candidate for stable@dpdk.org?

There is no reply to this comment - re-asking as I will probably have
the same question in a few weeks :-)

Re: [dpdk-stable] [dpdk-dev] [PATCH v1] examples/vm_power_manager: fix buffer overrun

[Hunt, David]

Hi Kevin,

On 23/4/2019 11:26 AM, Kevin Traynor wrote:
> On 18/04/2019 16:14, Pattan, Reshma wrote:
>>> Fixes: d26c18c93260 ("examples/vm_power: cpu frequency in host")
>>>       Coverity issue: 337660
>> Candidate for stable@dpdk.org?
> There is no reply to this comment - re-asking as I will probably have
> the same question in a few weeks :-)


There was a v2 this morning and it had a --cc stable on it. :)

Rgds,
Dave.

Re: [dpdk-stable] [dpdk-dev] [PATCH v1] examples/vm_power_manager: fix buffer overrun

[Kevin Traynor]

On 23/04/2019 11:31, Hunt, David wrote:
> Hi Kevin,
> 
> On 23/4/2019 11:26 AM, Kevin Traynor wrote:
>> On 18/04/2019 16:14, Pattan, Reshma wrote:
>>>> Fixes: d26c18c93260 ("examples/vm_power: cpu frequency in host")
>>>>       Coverity issue: 337660
>>> Candidate for stable@dpdk.org?
>> There is no reply to this comment - re-asking as I will probably have
>> the same question in a few weeks :-)
> 
> 
> There was a v2 this morning and it had a --cc stable on it. :)
> 

Yes, you're right, but that just puts it in the email and it gets lost
from the commit message. The 'Cc: stable@dpdk.org' tag is needed in the
commit message so it can be picked up by scripts later when finding
which patches should be backported.

https://doc.dpdk.org/guides/contributing/stable.html#what-changes-should-be-backported

As it's discussed, maybe Thomas can add it for this patch when applying.

thanks,
Kevin.

> Rgds,
> Dave.
> 

[dpdk-stable] [PATCH v3] examples/vm_power_manager: fix buffer overrun

[David Hunt]

The freqs array in freq_info struct has RTE_MAX_LCORE_FREQS elements,
yet the code can attempt to look at the index at  RTE_MAX_LCORE,
which may be greater than RTE_MAX_LCORE_FREQS. Fix to limit index to
RTE_MAX_LCORE_FREQS.

Fixes: d26c18c93260 ("examples/vm_power: cpu frequency in host")
Coverity issue: 337660
CC: stable@dpdk.org
Signed-off-by: David Hunt <david.hunt@intel.com>
Acked-by: Reshma Pattan <reshma.pattan@intel.com>

---
v2 - Rebase to 19.05 RC2.
v3 - add CC stable
---
 examples/vm_power_manager/power_manager.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/examples/vm_power_manager/power_manager.c b/examples/vm_power_manager/power_manager.c
index 9553455be..9d4e587b0 100644
--- a/examples/vm_power_manager/power_manager.c
+++ b/examples/vm_power_manager/power_manager.c
@@ -143,7 +143,7 @@ power_manager_get_current_frequency(unsigned core_num)
 	rte_spinlock_lock(&global_core_freq_info[core_num].power_sl);
 	index = rte_power_get_freq(core_num);
 	rte_spinlock_unlock(&global_core_freq_info[core_num].power_sl);
-	if (index >= RTE_MAX_LCORE)
+	if (index >= RTE_MAX_LCORE_FREQS)
 		freq = 0;
 	else
 		freq = global_core_freq_info[core_num].freqs[index];
-- 
2.17.1

Re: [dpdk-stable] [PATCH v3] examples/vm_power_manager: fix buffer overrun

[Thomas Monjalon]

26/04/2019 10:42, David Hunt:
> The freqs array in freq_info struct has RTE_MAX_LCORE_FREQS elements,
> yet the code can attempt to look at the index at  RTE_MAX_LCORE,
> which may be greater than RTE_MAX_LCORE_FREQS. Fix to limit index to
> RTE_MAX_LCORE_FREQS.
> 
> Fixes: d26c18c93260 ("examples/vm_power: cpu frequency in host")
> Coverity issue: 337660
> CC: stable@dpdk.org
> Signed-off-by: David Hunt <david.hunt@intel.com>
> Acked-by: Reshma Pattan <reshma.pattan@intel.com>

Applied, thanks