[PATCH] powerpc/nvram: fix segmentation fault issue in print-config

[Likhitha Korrapati]

print-config option in nvram results in segmentation fault when the
user provides a very large value.

without the patch:
[root@xxx powerpc-utils]# nvram --print-config=real-mode?
true
[root@xxx powerpc-utils]# nvram --print-config=$(perl -e 'p
rint "A"x1000000')
Segmentation fault (core dumped)

The Segmentation fault occurs because the code tries to access memory
beyond the bounds of the data at index varlen. varlen is the length of
the string provided by the user.

This patch adds a condition to check whether the length of the data is
greater than varlen to prevent accessing out of bounds.

with the patch:
[root@xxx powerpc-utils]# ./src/nvram --print-config=real-m
ode?
true
[root@xxx powerpc-utils]# ./src/nvram --print-config=$(perl
-e 'print "A"x1000000')

Reported-by: Shirisha Ganta <shir...@linux.ibm.com>
Signed-off-by: Likhitha Korrapati <likh...@linux.ibm.com>
---
src/nvram.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/nvram.c b/src/nvram.c
index f051e9c..095e747 100644
--- a/src/nvram.c
+++ b/src/nvram.c
@@ -1280,7 +1280,7 @@ print_of_config(struct nvram *nvram, char *config_var, char *pname,

data = (char *)phead + sizeof(*phead);
while (*data != '\0') {
- if ((data[varlen] == '=') &&
+ if (strlen(data) > varlen && (data[varlen] == '=') &&
strncmp(config_var, data, varlen) == 0) {
printf("%s%c", data + varlen + 1, terminator);
rc = 0;
--
2.39.3

[Shirisha ganta]

On Thu, 2024-01-25 at 15:44 +0530, Likhitha Korrapati wrote:
> print-config option in nvram results in segmentation fault when the
> user provides a very large value.
>
> without the patch:
> [root@xxx powerpc-utils]# nvram --print-config=real-mode?
> true
> [root@xxx powerpc-utils]# nvram --print-config=$(perl -e 'p
> rint "A"x1000000')
> Segmentation fault (core dumped)
>
> The Segmentation fault occurs because the code tries to access memory
> beyond the bounds of the data at index varlen. varlen is the length
> of
> the string provided by the user.
>
> This patch adds a condition to check whether the length of the data
> is
> greater than varlen to prevent accessing out of bounds.
>
> with the patch:
> [root@xxx powerpc-utils]# ./src/nvram --print-config=real-m
> ode?
> true
> [root@xxx powerpc-utils]# ./src/nvram --print-config=$(perl
> -e 'print "A"x1000000')
>
> Reported-by: Shirisha Ganta <shir...@linux.ibm.com>
> Signed-off-by: Likhitha Korrapati <likh...@linux.ibm.com>

Thanks for the fix patch.
I have tested on Power10 machine, the segmentation fault is resolved
with the patch applied.

Output without patch
# a=$(perl -e 'print "A"x1000000'); nvram --print-config=$a
Segmentation fault (core dumped)

Output with patch
# a=$(perl -e 'print "A"x1000000'); ./src/nvram --print-config=$a
#

Tested-by: Shirisha Ganta <shir...@linux.ibm.com>

[Tyrel Datwyler]

The indentation of the code above was already wrong before this change. We
follow kernel coding style, or at least adopted it over time. Hence, the
inconsistencies in spots of the code base. As such indentations are 8
characters. I'll go ahead and fix that up on my end.

-Tyrel

[Tyrel Datwyler]

Nevermind, after looking at the code the entirety of nvram.c is indented wrong.
So, best to keep it consistent for now.

[Tyrel Datwyler]

On 1/25/24 02:14, Likhitha Korrapati wrote:

> print-config option in nvram results in segmentation fault when the
> user provides a very large value.
>
> without the patch:
> [root@xxx powerpc-utils]# nvram --print-config=real-mode?
> true
> [root@xxx powerpc-utils]# nvram --print-config=$(perl -e 'p
> rint "A"x1000000')
> Segmentation fault (core dumped)
>
> The Segmentation fault occurs because the code tries to access memory
> beyond the bounds of the data at index varlen. varlen is the length of
> the string provided by the user.
>
> This patch adds a condition to check whether the length of the data is
> greater than varlen to prevent accessing out of bounds.
>
> with the patch:
> [root@xxx powerpc-utils]# ./src/nvram --print-config=real-m
> ode?
> true
> [root@xxx powerpc-utils]# ./src/nvram --print-config=$(perl
> -e 'print "A"x1000000')
>
> Reported-by: Shirisha Ganta <shir...@linux.ibm.com>
> Signed-off-by: Likhitha Korrapati <likh...@linux.ibm.com>
> ---

Patch applied to powerpc-utils/next branch.

https://github.com/ibm-power-utilities/powerpc-utils/commit/3f72b8326a2fc9a9dffb4b31d0ce3abf12e24751

Thanks,
-Tyrel