Enrico Mioso <mrkiko.rs@xxxxxxxxx> writes:
> Hi guys!
> It's my first experience, and util now I acquired a null pointer dereference,
> which kicks my kernel off! :)
> This is the patch ... I know for sure I'm doing something horrible!
> I'm a newbie !! XD
>
> diff --git a/drivers/net/usb/cdc_ncm.c b/drivers/net/usb/cdc_ncm.c
> index 4709fa3..725d892 100644
> --- a/drivers/net/usb/cdc_ncm.c
> +++ b/drivers/net/usb/cdc_ncm.c
> @@ -52,7 +52,7 @@
> #include <linux/usb/usbnet.h>
> #include <linux/usb/cdc.h>
> #include <linux/usb/cdc_ncm.h>
> -
> +#include <linux/usb/cdc-wdm.h>
> #define DRIVER_VERSION "14-Mar-2012"
>
> #if IS_ENABLED(CONFIG_USB_NET_CDC_MBIM)
> @@ -62,12 +62,41 @@ static bool prefer_mbim;
> #endif
> module_param(prefer_mbim, bool, S_IRUGO | S_IWUSR);
> MODULE_PARM_DESC(prefer_mbim, "Prefer MBIM setting on dual NCM/MBIM functions");
> -
> +atomic_t pmcount;
> static void cdc_ncm_txpath_bh(unsigned long param);
> static void cdc_ncm_tx_timeout_start(struct cdc_ncm_ctx *ctx);
> static enum hrtimer_restart cdc_ncm_tx_timer_cb(struct hrtimer *hr_timer);
> static struct usb_driver cdc_ncm_driver;
>
> +static int cdc_mbim_manage_power(struct usbnet *dev, int on)
> +{
> + int rv = 0;
> +
> +
> + if ((on && atomic_add_return(1, &pmcount) == 1) || (!on && atomic_dec_and_test(&pmcount))) {
> + /* need autopm_get/put here to ensure the usbcore sees the new value */
> + rv = usb_autopm_get_interface(dev->intf);
> + if (rv < 0)
> + goto err;
> + dev->intf->needs_remote_wakeup = on;
> + usb_autopm_put_interface(dev->intf);
> + }
> +err:
> + return rv;
> +}
> +
> +static int cdc_mbim_wdm_manage_power(struct usb_interface *intf, int status)
> +{
> + struct usbnet *dev = usb_get_intfdata(intf);
> +
> + /* can be called while disconnecting */
> + if (!dev)
> + return 0;
> +
> + return cdc_mbim_manage_power(dev, status);
> +}
> +
> +
For a simple test, I'd suggest ignoring PM for now. You can do
usb_autopm_get_interface() on bind and a usb_autopm_put_interface() to
make sure autosuspend doesn't complicate the testing.
Just make a dummy manage_power function to make the subdriver
registration happy.
> static void
> cdc_ncm_get_drvinfo(struct net_device *net, struct ethtool_drvinfo *info)
> {
> @@ -355,6 +384,7 @@ static const struct ethtool_ops cdc_ncm_ethtool_ops = {
> int cdc_ncm_bind_common(struct usbnet *dev, struct usb_interface *intf, u8 data_altsetting)
> {
> struct cdc_ncm_ctx *ctx;
> + struct usb_driver *subdriver;
> struct usb_driver *driver;
> u8 *buf;
> int len;
> @@ -507,6 +537,20 @@ advance:
> dev->rx_urb_size = ctx->rx_max;
>
> ctx->tx_speed = ctx->rx_speed = 0;
> +
> + usb_driver_release_interface(driver, ctx->control);
This does not make any sense to me...
> + if (ctx->control == NULL){
> + printk("ctx->control is NULL!\n");
> + return -ENODEV;
> + }
> + if (&dev->status->desc == NULL){
> + printk("ctx->control is NULL!\n");
> + return -ENODEV;
> + }
> + subdriver = usb_cdc_wdm_register(ctx->control,
> + &dev->status->desc,
> + le16_to_cpu(ctx->mbim_desc->wMaxControlMessage),
> + cdc_mbim_wdm_manage_power);
You cannot use ctx->mbim_desc here. It is NULL for any non MBIM
device. Just use some static max message constant instead, like qmi_wwan
does. We don't know how to guess the the correct value until we know
what the protocol is.
Bjørn
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
