Re: [PATCH net] netdevsim: avoid NULL dereference after failed probe

[Daniel Machon]

Hi,

> device_register() reports whether device registration succeeded, not
> whether the matching driver's probe succeeded. If nsim_drv_probe()
> fails, the driver core leaves the nsim_bus_dev registered while the probe
> error path clears its driver data.
> 
> new_device_store() subsequently marks the nsim_bus_dev initialized. A
> write to its new_port or del_port attribute therefore passes the init
> check and calls nsim_drv_port_add() or nsim_drv_port_del() with no valid
> nsim_dev. Both helpers pass the NULL driver data to priv_to_devlink(),
> leading to a NULL pointer dereference.
> 
> This can be reproduced by creating a netdevsim device for which
> nsim_drv_probe() fails and then writing to the new_port or del_port
> attribute of the registered device.
> 
> Serialize the driver data check and the port operation with the device
> lock, and reject the operation with -ENODEV when no driver data is
> present. This also prevents driver unbind from freeing nsim_dev between
> the check and its use, and follows the locking used by sriov_numvfs.

I agree with Hangbin that unregistering after a failed probe seems like a better
solution, but since  you already explored that, lets not go there.

There might be another approach though. Did you explore nsim_driver.dev_groups?
Adding nsim_bus_dev_new_port_attr/nsim_bus_dev_del_port_attr there might work,
and the fix will be simpler. I cant claim to be an expert in this area, but it
might be wort investigating.

/Daniel