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
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
> -----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
> -----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
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?
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?
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.
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.
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.
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.
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.
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.
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
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
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 :-)
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 :-)
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.
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.
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. >
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. >
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
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
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
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