WARNING in batadv_iv_send_outstanding_bat_ogm_packet

WARNING in batadv_iv_send_outstanding_bat_ogm_packet

[Tetsuo Handa]

syzbot is hitting

  WARN_ON(forw_packet->if_outgoing->soft_iface != soft_iface)

at batadv_iv_ogm_emit() [1].

This WARN_ON() was added by commit ef0a937f7a1450d3 ("batman-adv: consider
outgoing interface in OGM sending") but it is unclear why it needs to use
WARN_ON(). Is this fatal error worth crashing the kernel?

Please consider replacing with pr_err() etc. like commit b4142fc4d52d051d
("drm/vkms: fix misuse of WARN_ON") does if this is not a kernel bug.

[1] https://syzkaller.appspot.com/bug?id=9dc0c4cd70ad72df352243e887fd7e18901e7cee

[PATCH] batman-adv: don't warn when enslaving hard interface failed

[Tetsuo Handa]

syzbot is hitting

  WARN_ON(forw_packet->if_outgoing->soft_iface != soft_iface)

at batadv_iv_ogm_emit() [1], for forw_packet->if_outgoing->soft_iface
can remain NULL if batadv_hardif_enable_interface() failed due to e.g.
memory allocation fault injection.

Link: https://syzkaller.appspot.com/bug?id=9dc0c4cd70ad72df352243e887fd7e18901e7cee [1]
Reported-by: syzbot <syzbot+c0b807de416427ff3dd1@syzkaller.appspotmail.com>
Tested-by: syzbot <syzbot+c0b807de416427ff3dd1@syzkaller.appspotmail.com>
Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
Fixes: ef0a937f7a1450d3 ("batman-adv: consider outgoing interface in OGM sending")
---
 net/batman-adv/bat_iv_ogm.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/net/batman-adv/bat_iv_ogm.c b/net/batman-adv/bat_iv_ogm.c
index 789f257be24f..d24853c16ea5 100644
--- a/net/batman-adv/bat_iv_ogm.c
+++ b/net/batman-adv/bat_iv_ogm.c
@@ -409,6 +409,9 @@ static void batadv_iv_ogm_emit(struct batadv_forw_packet *forw_packet)
 	if (WARN_ON(!forw_packet->if_outgoing))
 		return;
 
+	if (!forw_packet->if_outgoing->soft_iface)
+		return;
+
 	if (WARN_ON(forw_packet->if_outgoing->soft_iface != soft_iface))
 		return;
 
-- 
2.18.4

Re: [PATCH] batman-adv: don't warn when enslaving hard interface failed

[Sven Eckelmann]

[-- Attachment #1: Type: text/plain, Size: 1617 bytes --]

On Sunday, 6 June 2021 16:28:17 CEST Tetsuo Handa wrote:
> syzbot is hitting
> 
>   WARN_ON(forw_packet->if_outgoing->soft_iface != soft_iface)
> 
> at batadv_iv_ogm_emit() [1], for forw_packet->if_outgoing->soft_iface
> can remain NULL if batadv_hardif_enable_interface() failed due to e.g.
> memory allocation fault injection.

Cannot apply this because following (conflicting) is already queued up in 
batadv/net:  
https://git.open-mesh.org/linux-merge.git/blobdiff/b741596468b010af2846b75f5e75a842ce344a6e..9f460ae31c4435fd022c443a6029352217a16ac1:/net/batman-adv/bat_iv_ogm.c

    Applying patch #18340 using 'git am -s'
    Description: batman-adv: don't warn when enslaving hard interface failed
    Applying: batman-adv: don't warn when enslaving hard interface failed
    error: patch failed: net/batman-adv/bat_iv_ogm.c:409
    error: net/batman-adv/bat_iv_ogm.c: patch does not apply
    Patch failed at 0001 batman-adv: don't warn when enslaving hard interface failed
    hint: Use 'git am --show-current-patch=diff' to see the failed patch
    When you have resolved this problem, run "git am --continue".
    If you prefer to skip this patch, run "git am --skip" instead.
    To restore the original branch and stop patching, run "git am --abort".
    'git am' failed with exit status 128

Please rebase your patch in case it is really needed.

And the explanation you give seems to be bogus. Or am I missing some error 
handling in batadv_hardif_enable_interface [1]?

Kind regards,
	Sven

[1] https://git.open-mesh.org/linux-merge.git/blob/refs/heads/batadv/net-next:/net/batman-adv/hard-interface.c

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH] batman-adv: don't warn when enslaving hard interface failed

[Tetsuo Handa]

On 2021/06/06 23:35, Sven Eckelmann wrote:
> Please rebase your patch in case it is really needed.

Oh, I didn't know you already applied that change, for that commit is
not yet visible from linux-next.git as of next-20210604.

> 
> And the explanation you give seems to be bogus. Or am I missing some error 
> handling in batadv_hardif_enable_interface [1]?

I told syzbot to try https://syzkaller.appspot.com/text?tag=Patch&x=100b083fd00000
and the response ( https://syzkaller.appspot.com/text?tag=CrashLog&x=1456f0ffd00000 ) was

   batman_adv: forw_packet->if_outgoing->soft_iface=0000000000000000 forw_packet->if_incoming->soft_iface=0000000039fa85b7

indicating that if_outgoing->soft_iface was NULL, and there was a memory allocation
fault injection immediately before this result.

Since if_outgoing->soft_iface becomes non-NULL if batadv_hardif_enable_interface()
succeeds, this situation indicates that batadv_hardif_enable_interface() failure
caused forw_packet->if_outgoing->soft_iface to remain NULL.

> 
> Kind regards,
> 	Sven
> 
> [1] https://git.open-mesh.org/linux-merge.git/blob/refs/heads/batadv/net-next:/net/batman-adv/hard-interface.c
> 

Re: [PATCH] batman-adv: don't warn when enslaving hard interface failed

[Sven Eckelmann]

[-- Attachment #1: Type: text/plain, Size: 1324 bytes --]

On Sunday, 6 June 2021 17:48:25 CEST Tetsuo Handa wrote:
> > And the explanation you give seems to be bogus. Or am I missing some error 
> > handling in batadv_hardif_enable_interface [1]?
> 
> I told syzbot to try https://syzkaller.appspot.com/text?tag=Patch&x=100b083fd00000
> and the response ( https://syzkaller.appspot.com/text?tag=CrashLog&x=1456f0ffd00000 ) was
> 
>    batman_adv: forw_packet->if_outgoing->soft_iface=0000000000000000 forw_packet->if_incoming->soft_iface=0000000039fa85b7
> 
> indicating that if_outgoing->soft_iface was NULL, and there was a memory allocation
> fault injection immediately before this result.
> 
> Since if_outgoing->soft_iface becomes non-NULL if batadv_hardif_enable_interface()
> succeeds, this situation indicates that batadv_hardif_enable_interface() failure
> caused forw_packet->if_outgoing->soft_iface to remain NULL.

Ok, then I misread the commit message. I've understood is as "soft_iface" 
allocation failed (which doesn't happen here anymore) in 
batadv_hardif_enable_interface. But you meant that was that hard_iface->soft_iface is 
set to the correct value, the OGM transmission is started up by batadv_iv_ogm_schedule_buff
but soft_iface is changed back immediately in batadv_hardif_enable_interface because
netdev_master_upper_dev_link failed.

Kind regards,
	Sven

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 833 bytes --]