[v2,1/2] power: fix power management env detection
Checks
Commit Message
Anything coming from sysfs has a newline at the end. Cut it off before
comparing the strings.
Fixes: 20ab67608a39 ("power: add environment capability probing")
Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
Acked-by: David Hunt <david.hunt@intel.com>
---
lib/librte_power/power_common.c | 8 ++++++++
1 file changed, 8 insertions(+)
Comments
On Mon, Jul 13, 2020 at 03:54:27PM +0100, Anatoly Burakov wrote:
> Anything coming from sysfs has a newline at the end. Cut it off before
> comparing the strings.
>
> Fixes: 20ab67608a39 ("power: add environment capability probing")
>
> Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
> Acked-by: David Hunt <david.hunt@intel.com>
> ---
> lib/librte_power/power_common.c | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
> diff --git a/lib/librte_power/power_common.c b/lib/librte_power/power_common.c
> index 59023d986b..22b016ca9d 100644
> --- a/lib/librte_power/power_common.c
> +++ b/lib/librte_power/power_common.c
> @@ -15,6 +15,7 @@ int
> cpufreq_check_scaling_driver(const char *driver_name)
> {
> unsigned int lcore_id = 0; /* always check core 0 */
> + size_t end_idx;
> char fullpath[PATH_MAX];
> char readbuf[PATH_MAX];
> char *s;
> @@ -39,6 +40,13 @@ cpufreq_check_scaling_driver(const char *driver_name)
> if (s == NULL)
> return 0;
>
> + /* when read from sysfs, driver name has an extra newline at the end */
> + end_idx = strnlen(readbuf, sizeof(readbuf));
> + /* prevent underflow if len is zero */
> + if (end_idx > 0)
> + end_idx--;
> + readbuf[end_idx] = '\0';
> +
Would it not be safer to add " && readbuf[end_idx - 1] == '\n'" to the
condition, to check that it's terminated as expected? Theoretically if we
had a long string returned which was truncated, or only just fit, there
would not be a '\n' at the end.
/Bruce
On 13-Jul-20 4:33 PM, Bruce Richardson wrote:
> On Mon, Jul 13, 2020 at 03:54:27PM +0100, Anatoly Burakov wrote:
>> Anything coming from sysfs has a newline at the end. Cut it off before
>> comparing the strings.
>>
>> Fixes: 20ab67608a39 ("power: add environment capability probing")
>>
>> Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
>> Acked-by: David Hunt <david.hunt@intel.com>
>> ---
>> lib/librte_power/power_common.c | 8 ++++++++
>> 1 file changed, 8 insertions(+)
>>
>> diff --git a/lib/librte_power/power_common.c b/lib/librte_power/power_common.c
>> index 59023d986b..22b016ca9d 100644
>> --- a/lib/librte_power/power_common.c
>> +++ b/lib/librte_power/power_common.c
>> @@ -15,6 +15,7 @@ int
>> cpufreq_check_scaling_driver(const char *driver_name)
>> {
>> unsigned int lcore_id = 0; /* always check core 0 */
>> + size_t end_idx;
>> char fullpath[PATH_MAX];
>> char readbuf[PATH_MAX];
>> char *s;
>> @@ -39,6 +40,13 @@ cpufreq_check_scaling_driver(const char *driver_name)
>> if (s == NULL)
>> return 0;
>>
>> + /* when read from sysfs, driver name has an extra newline at the end */
>> + end_idx = strnlen(readbuf, sizeof(readbuf));
>> + /* prevent underflow if len is zero */
>> + if (end_idx > 0)
>> + end_idx--;
>> + readbuf[end_idx] = '\0';
>> +
> Would it not be safer to add " && readbuf[end_idx - 1] == '\n'" to the
> condition, to check that it's terminated as expected? Theoretically if we
> had a long string returned which was truncated, or only just fit, there
> would not be a '\n' at the end.
>
> /Bruce
>
Yep, true, however unlikely :) I'll submit a v3.
@@ -15,6 +15,7 @@ int
cpufreq_check_scaling_driver(const char *driver_name)
{
unsigned int lcore_id = 0; /* always check core 0 */
+ size_t end_idx;
char fullpath[PATH_MAX];
char readbuf[PATH_MAX];
char *s;
@@ -39,6 +40,13 @@ cpufreq_check_scaling_driver(const char *driver_name)
if (s == NULL)
return 0;
+ /* when read from sysfs, driver name has an extra newline at the end */
+ end_idx = strnlen(readbuf, sizeof(readbuf));
+ /* prevent underflow if len is zero */
+ if (end_idx > 0)
+ end_idx--;
+ readbuf[end_idx] = '\0';
+
/* does the driver name match? */
if (strncmp(readbuf, driver_name, sizeof(readbuf)) != 0)
return 0;