David Miller <davem@xxxxxxxxxxxxx> writes:
> The problem is in cdc_ncm_bind_common().
>
> It seems to leave dangling interface data pointers in some cases, and
> then branches just to "error" so that they don't get cleared back out.
Sorry, but I fail to see this as well. I see one "return" and two "goto
error", but all are well before any intfdata pointer is set:
364 ctx = kzalloc(sizeof(*ctx), GFP_KERNEL);
365 if (!ctx)
366 return -ENOMEM;
367
[..]
453 /* check if we got everything */
454 if ((ctx->control == NULL) || (ctx->data == NULL) ||
455 ((!ctx->mbim_desc) && ((ctx->ether_desc == NULL) || (ctx->control != intf))))
456 goto error;
457
458 /* claim data interface, if different from control */
459 if (ctx->data != ctx->control) {
460 temp = usb_driver_claim_interface(driver, ctx->data, dev);
461 if (temp)
462 goto error;
463 }
[..]
490 usb_set_intfdata(ctx->data, dev);
491 usb_set_intfdata(ctx->control, dev);
492 usb_set_intfdata(ctx->intf, dev);
[..]
510 return 0;
511
512 error2:
513 usb_set_intfdata(ctx->control, NULL);
514 usb_set_intfdata(ctx->data, NULL);
515 if (ctx->data != ctx->control)
516 usb_driver_release_interface(driver, ctx->data);
517 error:
518 cdc_ncm_free((struct cdc_ncm_ctx *)dev->data[0]);
519 dev->data[0] = 0;
520 dev_info(&dev->udev->dev, "bind() failure\n");
521 return -ENODEV;
522 }
This could (and given this thread, probably should) certainly be
refactored and cleaned up a bit. But I do not see how it leaves any
dangling pointers. The pointers are only set near the end of the
function, and the only exit points after that are either success or
through the "error2" label.
Side note: It is definitely confusing that we set 3 pointers, but only
clean up 2. The reason is that there are never more than 2 interfaces
involved here. We always have ctx->intf == ctx->control. I'd really
like to get rid of that redundant ctx->intf pointer. That's one issue
to cleanup throughout this driver.
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
