diff mbox series

[v2] Fixes the null pointer deferences in nsim_bpf

Message ID 20231110111823.2775-1-kdipendra88@gmail.com (mailing list archive)
State Changes Requested
Delegated to: Netdev Maintainers
Headers show
Series [v2] Fixes the null pointer deferences in nsim_bpf | expand

Checks

Context Check Description
netdev/series_format warning Single patches do not need cover letters; Target tree name not specified in the subject
netdev/tree_selection success Guessed tree name to be net-next
netdev/fixes_present success Fixes tag not required for -next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 1134 this patch: 1134
netdev/cc_maintainers fail 3 blamed authors not CCed: daniel@iogearbox.net quentin@isovalent.com horms@kernel.org; 4 maintainers not CCed: daniel@iogearbox.net quentin@isovalent.com horms@kernel.org bpf@vger.kernel.org
netdev/build_clang success Errors and warnings before: 1161 this patch: 1161
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success Fixes tag looks correct
netdev/build_allmodconfig_warn success Errors and warnings before: 1161 this patch: 1161
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 23 lines checked
netdev/build_clang_rust success No Rust files in patch. Skipping build
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Dipendra Khadka Nov. 10, 2023, 11:18 a.m. UTC
Syzkaller found a null pointer dereference in nsim_bpf
originating from the lack of a null check for state.

This patch fixes the issue by adding a check for state
in two functions nsim_prog_set_loaded() and nsim_setup_prog_hw_checks()

Reported-by: syzbot+44c2416196b7c607f226@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com./bug?extid=44c2416196b7c607f226
Fixes: 31d3ad832948 ("netdevsim: add bpf offload support")
Signed-off-by: Dipendra Khadka <kdipendra88@gmail.com>
---
 drivers/net/netdevsim/bpf.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

Comments

Jakub Kicinski Nov. 10, 2023, 7:12 p.m. UTC | #1
On Fri, 10 Nov 2023 11:18:23 +0000 Dipendra Khadka wrote:
> Syzkaller found a null pointer dereference in nsim_bpf
> originating from the lack of a null check for state.
> 
> This patch fixes the issue by adding a check for state
> in two functions nsim_prog_set_loaded() and nsim_setup_prog_hw_checks()
> 
> Reported-by: syzbot+44c2416196b7c607f226@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com./bug?extid=44c2416196b7c607f226
> Fixes: 31d3ad832948 ("netdevsim: add bpf offload support")

Don't think so. It's probably due to Stan's extensions / reuse of 
the offload infra.

Please put more effort into figuring out when and why this started
happening. Describe your findings in the commit message.

Current patch looks too much like a bandaid.

Before you repost read:
https://www.kernel.org/doc/html/next/process/maintainer-netdev.html
Stanislav Fomichev Nov. 10, 2023, 7:20 p.m. UTC | #2
On 11/10, Jakub Kicinski wrote:
> On Fri, 10 Nov 2023 11:18:23 +0000 Dipendra Khadka wrote:
> > Syzkaller found a null pointer dereference in nsim_bpf
> > originating from the lack of a null check for state.
> > 
> > This patch fixes the issue by adding a check for state
> > in two functions nsim_prog_set_loaded() and nsim_setup_prog_hw_checks()
> > 
> > Reported-by: syzbot+44c2416196b7c607f226@syzkaller.appspotmail.com
> > Closes: https://syzkaller.appspot.com./bug?extid=44c2416196b7c607f226
> > Fixes: 31d3ad832948 ("netdevsim: add bpf offload support")
> 
> Don't think so. It's probably due to Stan's extensions / reuse of 
> the offload infra.
> 
> Please put more effort into figuring out when and why this started
> happening. Describe your findings in the commit message.
> 
> Current patch looks too much like a bandaid.
> 
> Before you repost read:
> https://www.kernel.org/doc/html/next/process/maintainer-netdev.html

I agree, I have a similar suspicion for the same report on the bpf list [0].

0: https://lore.kernel.org/bpf/ZU13dQb2z66CJlYi@google.com/
diff mbox series

Patch

diff --git a/drivers/net/netdevsim/bpf.c b/drivers/net/netdevsim/bpf.c
index f60eb97e3a62..5d755da3c736 100644
--- a/drivers/net/netdevsim/bpf.c
+++ b/drivers/net/netdevsim/bpf.c
@@ -97,7 +97,8 @@  static void nsim_prog_set_loaded(struct bpf_prog *prog, bool loaded)
 		return;
 
 	state = prog->aux->offload->dev_priv;
-	state->is_loaded = loaded;
+	if (state)
+		state->is_loaded = loaded;
 }
 
 static int
@@ -317,9 +318,11 @@  nsim_setup_prog_hw_checks(struct netdevsim *ns, struct netdev_bpf *bpf)
 	}
 
 	state = bpf->prog->aux->offload->dev_priv;
-	if (WARN_ON(strcmp(state->state, "xlated"))) {
-		NSIM_EA(bpf->extack, "offloading program in bad state");
-		return -EINVAL;
+	if (state) {
+		if (WARN_ON(strcmp(state->state, "xlated"))) {
+			NSIM_EA(bpf->extack, "offloading program in bad state");
+			return -EINVAL;
+		}
 	}
 	return 0;
 }