Re: [PATCH] net: wireless: convert WARN_ON() to pr_warn() in cfg80211_sme_connect

[Du Cheng <ducheng2@gmail.com>]

Le Wed, Apr 07, 2021 at 05:51:05PM +0200, Johannes Berg a écrit :
> Hi,
> 
> > I have spent some time to understand the netlink subsystem as a IPC mechanism.
> > What I have now is a reliable sequence of steps to reproduce the crash, by
> > condensing the syzkaller C reproducer:
> > [link to reproducer: https://syzkaller.appspot.com/text?tag=ReproC&x=1414c997900000]
> > 
> > * hwsim80211_create_device (sendmsg: HWSIM_CMD_NEW_RADIO)
> > * nl80211_set_interface (sendmsg: NL80211_CMD_SET_INTERFACE)
> > * set_interface_state (ioctl: SIOCSIFFLAGS)
> > * nl80211_join_ibss (sendmsg: NL80211_CMD_JOIN_IBSS)
> > * sendmsg: NL80211_CMD_SET_INTERFACE
> > * 1st sendmsg: NL80211_CMD_CONNECT
> > * 2nd sendmsg: NL80211_CMD_CONNECT <- this triggers the WARN_ON(wdev->conn)
> > * (if kernel not panic yet) more sendmsg: NL80211_CMD_CONNECT ...
> > 
> > If the code skips WARN_ON() and instead returns -EINPROGRESS, user application
> > will receive error from the following recv(sock, ...). User application can hence
> > choose to handle this error accordingly while kernel soldiers on without panicking.
> > 
> > If dev_warn() is added, for every subsequent NL80211_CMD_CONNECT, the console is
> > flooded with the printout.
> > 
> > Maybe it is ok to silently return -EINPROGRESS for the 2nd NL80211_CMD_CONNECT
> > and beyond.
> > 
> 
> Yeah, I think the right thing to do is to just drop the WARN_ON
> entirely. In fact, I can't really seem to figure out now why it was
> added there (even if I probably did that myself), nothing else seems to
> prevent getting to this code path multiple times directly one after
> another.
> 
> johannes
> 
Hi Johannes,

Thanks for your input! I will send a v2 that drops the WARN_ON().

Regards,
Du Cheng