Re: [syzbot] WARNING in batadv_nc_mesh_free

[Sven Eckelmann]

On Friday, 22 October 2021 22:58:15 CEST Pavel Skripkin wrote:
[...]
> > Oh, ok. Next clean up call in batadv_nc_mesh_free() caused GPF, since
> > fields are not initialized. Let's try to clean up one by one and do not
> > break dependencies.
> > 
> > Quite ugly one, but idea is correct, I guess
> > 
> > Also, make each *_init() call clean up all allocated stuff to not call
> > corresponding *_free() on error handling path, since it introduces
> > problems, as syzbot reported

Thanks for the patch + syzbot interactions. I just wanted to implement a 
change - which would most likely have ended up the same way. Can you please 
send it to netdev and Cc b.a.t.m.a.n@xxxxxxxxxxxxxxxxxxx? We don't have 
anything else to submit at the moment for netdev and this patch can be applied 
by netdev directly. I will add my Acked-by in this process.

Not sure about the Fixes. It is definitely wrong in the initial commit.... but 
it got only really problematic when other features got introduced. I would 
still say that the initial one should be mentioned.

Fixes: c6c8fea29769 ("net: Add batman-adv meshing protocol")

@Linus, @Marek, @Antonio: Please check whether it is ok to move the 
batadv_v_mesh_init after batadv_tt_init + batadv_originator_init. 
batadv_v_mesh_init is basically there to initialize:

* bat_priv->bat_v.ogm_buff(|_len|_mutex)
* bat_priv->bat_v.ogm_seqno
* bat_priv->bat_v.ogm_wq

batadv_originator_init is there to initialize the 

* bat_priv->orig_hash
* bat_priv->orig_work (batadv_purge_orig) + queue it up

batadv_tt_init is a lot more complex but should in theory not interact with 
ogm specific algo ops.

I wouldn't know why there could be a problem but I would leave it to the 
experts.

Kind regards,
	Sven

Attachment: signature.asc
Description: This is a digitally signed message part.