* Out-of-bound read suspected in libbpf.c
@ 2025-02-19 15:13 Nandakumar Edamana
2025-02-20 21:20 ` Eduard Zingerman
0 siblings, 1 reply; 6+ messages in thread
From: Nandakumar Edamana @ 2025-02-19 15:13 UTC (permalink / raw)
To: bpf
I think I've come across an out-of-bound read in libbpf.c that could
result in
potential information leak (or crash). I wanted to test it myself, but
unfortunately it will take a while for me to set up things. So, let me just
describe it and submit a candidate patch.
Codebase at: commit 6537cfb395f352782918d8ee7b7f10ba2cc3cbf2 (HEAD ->
master, origin/master, origin/HEAD)
File: ./tools/lib/bpf/libbpf.c
Function: set_kcfg_value_str
The first subscripted read of `value` (which is untrusted, IIUC) at line
2108
(`value[len - 1]`) is safe because `set_kcfg_value_str` is called only after
making sure `value` is at least one character long (although not a good
practice
in long-term). The problem is at the strip-quotes part. Here, `len -= 2` is
performed, possibly thinking `value` is at least two characters long,
since the
caller makes sure `value` has an opening quote and this function
(`set_kcfg_value_str) has a check `if (value[len - 1] != '"')` for the
closing
quote. But the problem is, both the opening and closing quotes could be the
same. More specifically, `value` could be a string that consists of a single
quote alone, and it could pass all these tests. Then `len` underflows,
resulting
in a wrap-around since it is of type `size_t`. The next branch
`if (len >= ext->kcfg.sz)` will possibly reduce the value of `len`, but will
still be greater than `strlen(value)`, making `memcpy()` at 2122 read
out-of-bound.
Suggested change:
diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index 194809da5172..1cc87dbd015d 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -2106,7 +2106,7 @@ static int set_kcfg_value_str(struct extern_desc
*ext, char *ext_val,
}
len = strlen(value);
- if (value[len - 1] != '"') {
+ if (len < 2 || value[len - 1] != '"') {
pr_warn("extern (kcfg) '%s': invalid string config '%s'\n",
ext->name, value);
return -EINVAL;
--
Nandakumar Edamana
https://nandakumar.org/
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: Out-of-bound read suspected in libbpf.c
2025-02-19 15:13 Out-of-bound read suspected in libbpf.c Nandakumar Edamana
@ 2025-02-20 21:20 ` Eduard Zingerman
2025-02-21 0:41 ` [PATCH] fix: out-of-bound read " Nandakumar Edamana
0 siblings, 1 reply; 6+ messages in thread
From: Eduard Zingerman @ 2025-02-20 21:20 UTC (permalink / raw)
To: Nandakumar Edamana, bpf
On Wed, 2025-02-19 at 20:43 +0530, Nandakumar Edamana wrote:
[...]
> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index 194809da5172..1cc87dbd015d 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -2106,7 +2106,7 @@ static int set_kcfg_value_str(struct extern_desc
> *ext, char *ext_val,
> }
>
> len = strlen(value);
> - if (value[len - 1] != '"') {
> + if (len < 2 || value[len - 1] != '"') {
Makes sense to me, could you please send a formal patch?
> pr_warn("extern (kcfg) '%s': invalid string config '%s'\n",
> ext->name, value);
> return -EINVAL;
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH] fix: out-of-bound read in libbpf.c
2025-02-20 21:20 ` Eduard Zingerman
@ 2025-02-21 0:41 ` Nandakumar Edamana
2025-02-21 18:31 ` Andrii Nakryiko
0 siblings, 1 reply; 6+ messages in thread
From: Nandakumar Edamana @ 2025-02-21 0:41 UTC (permalink / raw)
To: bpf; +Cc: Eduard Zingerman, Nandakumar Edamana
Signed-off-by: Nandakumar Edamana <nandakumar@nandakumar.co.in>
---
tools/lib/bpf/libbpf.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index 194809da5172..1cc87dbd015d 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -2106,7 +2106,7 @@ static int set_kcfg_value_str(struct extern_desc *ext, char *ext_val,
}
len = strlen(value);
- if (value[len - 1] != '"') {
+ if (len < 2 || value[len - 1] != '"') {
pr_warn("extern (kcfg) '%s': invalid string config '%s'\n",
ext->name, value);
return -EINVAL;
--
2.30.2
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] fix: out-of-bound read in libbpf.c
2025-02-21 0:41 ` [PATCH] fix: out-of-bound read " Nandakumar Edamana
@ 2025-02-21 18:31 ` Andrii Nakryiko
2025-02-21 21:01 ` [PATCH bpf-next] libbpf: Fix out-of-bound read Nandakumar Edamana
0 siblings, 1 reply; 6+ messages in thread
From: Andrii Nakryiko @ 2025-02-21 18:31 UTC (permalink / raw)
To: Nandakumar Edamana; +Cc: bpf, Eduard Zingerman
On Thu, Feb 20, 2025 at 4:42 PM Nandakumar Edamana
<nandakumar@nandakumar.co.in> wrote:
>
You need to provide commit message here explaining (briefly) what you
are trying to do
Also, please use "[PATCH bpf-next] libbpf: " as subject prefix
pw-bot: cr
> Signed-off-by: Nandakumar Edamana <nandakumar@nandakumar.co.in>
> ---
> tools/lib/bpf/libbpf.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index 194809da5172..1cc87dbd015d 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -2106,7 +2106,7 @@ static int set_kcfg_value_str(struct extern_desc *ext, char *ext_val,
> }
>
> len = strlen(value);
> - if (value[len - 1] != '"') {
> + if (len < 2 || value[len - 1] != '"') {
> pr_warn("extern (kcfg) '%s': invalid string config '%s'\n",
> ext->name, value);
> return -EINVAL;
> --
> 2.30.2
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH bpf-next] libbpf: Fix out-of-bound read
2025-02-21 18:31 ` Andrii Nakryiko
@ 2025-02-21 21:01 ` Nandakumar Edamana
2025-02-24 22:10 ` patchwork-bot+netdevbpf
0 siblings, 1 reply; 6+ messages in thread
From: Nandakumar Edamana @ 2025-02-21 21:01 UTC (permalink / raw)
To: bpf; +Cc: Andrii Nakryiko, Eduard Zingerman, Nandakumar Edamana
In `set_kcfg_value_str`, an untrusted string is accessed with the assumption
that it will be at least two characters long due to the presence of checks for
opening and closing quotes. But the check for the closing quote
(value[len - 1] != '"') misses the fact that it could be checking the opening
quote itself in case of an invalid input that consists of just the opening
quote.
This commit adds an explicit check to make sure the string is at least two
characters long.
Signed-off-by: Nandakumar Edamana <nandakumar@nandakumar.co.in>
---
tools/lib/bpf/libbpf.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index 194809da5172..1cc87dbd015d 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -2106,7 +2106,7 @@ static int set_kcfg_value_str(struct extern_desc *ext, char *ext_val,
}
len = strlen(value);
- if (value[len - 1] != '"') {
+ if (len < 2 || value[len - 1] != '"') {
pr_warn("extern (kcfg) '%s': invalid string config '%s'\n",
ext->name, value);
return -EINVAL;
--
2.30.2
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH bpf-next] libbpf: Fix out-of-bound read
2025-02-21 21:01 ` [PATCH bpf-next] libbpf: Fix out-of-bound read Nandakumar Edamana
@ 2025-02-24 22:10 ` patchwork-bot+netdevbpf
0 siblings, 0 replies; 6+ messages in thread
From: patchwork-bot+netdevbpf @ 2025-02-24 22:10 UTC (permalink / raw)
To: Nandakumar Edamana; +Cc: bpf, andrii.nakryiko, eddyz87
Hello:
This patch was applied to bpf/bpf-next.git (master)
by Andrii Nakryiko <andrii@kernel.org>:
On Sat, 22 Feb 2025 02:31:11 +0530 you wrote:
> In `set_kcfg_value_str`, an untrusted string is accessed with the assumption
> that it will be at least two characters long due to the presence of checks for
> opening and closing quotes. But the check for the closing quote
> (value[len - 1] != '"') misses the fact that it could be checking the opening
> quote itself in case of an invalid input that consists of just the opening
> quote.
>
> [...]
Here is the summary with links:
- [bpf-next] libbpf: Fix out-of-bound read
https://git.kernel.org/bpf/bpf-next/c/236d3910117e
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2025-02-24 22:10 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-02-19 15:13 Out-of-bound read suspected in libbpf.c Nandakumar Edamana
2025-02-20 21:20 ` Eduard Zingerman
2025-02-21 0:41 ` [PATCH] fix: out-of-bound read " Nandakumar Edamana
2025-02-21 18:31 ` Andrii Nakryiko
2025-02-21 21:01 ` [PATCH bpf-next] libbpf: Fix out-of-bound read Nandakumar Edamana
2025-02-24 22:10 ` patchwork-bot+netdevbpf
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.