| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20141008092347.GB1990@localhost>
Date: Wed, 8 Oct 2014 11:23:47 +0200
From: Johan Hovold <johan@...nel.org>
To: Octavian Purdila <octavian.purdila@...el.com>
Cc: gregkh@...uxfoundation.org, linus.walleij@...aro.org,
gnurou@...il.com, wsa@...-dreams.de, sameo@...ux.intel.com,
lee.jones@...aro.org, arnd@...db.de, johan@...nel.org,
daniel.baluta@...el.com, laurentiu.palcu@...el.com,
linux-usb@...r.kernel.org, linux-kernel@...r.kernel.org,
linux-gpio@...r.kernel.org, linux-i2c@...r.kernel.org
Subject: Re: [PATCH v6 1/4] mfd: add support for Diolan DLN-2 devices
On Thu, Sep 25, 2014 at 07:07:31PM +0300, Octavian Purdila wrote:
> +static void dln2_rx_transfer(struct dln2_dev *dln2, struct urb *urb,
> + u16 handle, u16 rx_slot)
> +{
> + struct dln2_mod_rx_slots *rxs = &dln2->mod_rx_slots[handle];
> + struct dln2_rx_context *rxc;
> + struct device *dev = &dln2->interface->dev;
> +
> + spin_lock(&rxs->lock);
You must use spin_lock_irqsave here as you call it from the completion
handler.
> +
> + rxc = &rxs->slots[rx_slot];
> +
> + if (rxc->in_use && !rxc->urb) {
> + rxc->urb = urb;
> + complete(&rxc->done);
> + /* URB will be resubmitted in free_rx_slot */
> + } else {
> + dev_warn(dev, "bad/late response %d/%d\n", handle, rx_slot);
> + dln2_submit_urb(dln2, urb, GFP_ATOMIC);
> + }
> +
> + spin_unlock(&rxs->lock);
> +}
> +static int _dln2_transfer(struct dln2_dev *dln2, u16 handle, u16 cmd,
> + const void *obuf, unsigned obuf_len,
> + void *ibuf, unsigned *ibuf_len)
> +{
> + int ret = 0;
> + u16 result;
> + int rx_slot;
> + struct dln2_response *rsp;
> + struct dln2_rx_context *rxc;
> + struct device *dev;
> + const int timeout = DLN2_USB_TIMEOUT * HZ / 1000;
> + struct dln2_mod_rx_slots *rxs = &dln2->mod_rx_slots[handle];
> +
> + spin_lock(&dln2->disconnect_lock);
How did you test this version? You never initialise disconnect_lock so
lockdep complains loudly when calling _dln2_transfer during probe.
> + if (!dln2->disconnect)
> + dln2->active_transfers++;
> + else
> + ret = -ENODEV;
> + spin_unlock(&dln2->disconnect_lock);
> +
> + if (ret)
> + return ret;
> +
> + rx_slot = alloc_rx_slot(dln2, handle);
> + if (rx_slot < 0) {
> + ret = rx_slot;
> + goto out_decr;
> + }
> +
> + dev = &dln2->interface->dev;
> +
> + ret = dln2_send_wait(dln2, handle, cmd, rx_slot, obuf, obuf_len);
> + if (ret < 0) {
> + dev_err(dev, "USB write failed: %d\n", ret);
> + goto out_free_rx_slot;
> + }
> +
> + rxc = &rxs->slots[rx_slot];
> +
> + ret = wait_for_completion_interruptible_timeout(&rxc->done, timeout);
> + if (ret <= 0) {
> + if (!ret)
> + ret = -ETIMEDOUT;
> + goto out_free_rx_slot;
> + }
> +
> + if (!rxc->urb) {
> + ret = -ENODEV;
> + goto out_free_rx_slot;
> + }
> +
> + /* if we got here we know that the response header has been checked */
> + rsp = rxc->urb->transfer_buffer;
> +
> + if (rsp->hdr.size < sizeof(*rsp)) {
> + ret = -EPROTO;
> + goto out_free_rx_slot;
> + }
> +
> + result = le16_to_cpu(rsp->result);
> + if (result) {
> + dev_dbg(dev, "%d received response with error %d\n",
> + handle, result);
> + ret = -EREMOTEIO;
> + goto out_free_rx_slot;
> + }
> +
> + if (!ibuf) {
> + ret = 0;
> + goto out_free_rx_slot;
> + }
> +
> + if (*ibuf_len > rsp->hdr.size - sizeof(*rsp))
> + *ibuf_len = rsp->hdr.size - sizeof(*rsp);
> +
> + memcpy(ibuf, rsp + 1, *ibuf_len);
> +
> +out_free_rx_slot:
> + free_rx_slot(dln2, handle, rx_slot);
> +out_decr:
> + spin_lock(&dln2->disconnect_lock);
> + dln2->active_transfers--;
> + spin_unlock(&dln2->disconnect_lock);
> + if (dln2->disconnect)
> + wake_up(&dln2->disconnect_wq);
> +
> + return ret;
> +}
> +static void dln2_disconnect(struct usb_interface *interface)
> +{
> + struct dln2_dev *dln2 = usb_get_intfdata(interface);
> + int i, j;
> +
> + /* don't allow starting new transfers */
> + spin_lock(&dln2->disconnect_lock);
> + dln2->disconnect = true;
> + spin_unlock(&dln2->disconnect_lock);
> +
> + /* cancel in progress transfers */
> + for (i = 0; i < DLN2_HANDLES; i++) {
> + struct dln2_mod_rx_slots *rxs = &dln2->mod_rx_slots[i];
> + unsigned long flags;
> +
> + spin_lock_irqsave(&rxs->lock, flags);
Just stick to spin_lock in this function.
> +
> + /* cancel all response waiters */
> + for (j = 0; j < DLN2_MAX_RX_SLOTS; j++) {
> + struct dln2_rx_context *rxc = &rxs->slots[j];
> +
> + if (rxc->in_use)
> + complete(&rxc->done);
> + }
> +
> + spin_unlock_irqrestore(&rxs->lock, flags);
> + }
> +
> + /* wait for transfers to end */
> + wait_event(dln2->disconnect_wq, !dln2->active_transfers);
> +
> + mfd_remove_devices(&interface->dev);
> +
> + dln2_free(dln2);
> +}
> +
> +static int dln2_probe(struct usb_interface *interface,
> + const struct usb_device_id *usb_id)
> +{
> + struct usb_host_interface *hostif = interface->cur_altsetting;
> + struct device *dev = &interface->dev;
> + struct dln2_dev *dln2;
> + int ret;
> + int i, j;
> + int id;
> +
> + if (hostif->desc.bInterfaceNumber != 0 ||
> + hostif->desc.bNumEndpoints < 2)
> + return -ENODEV;
> +
> + dln2 = kzalloc(sizeof(*dln2), GFP_KERNEL);
> + if (!dln2)
> + return -ENOMEM;
> +
> + dln2->ep_out = hostif->endpoint[0].desc.bEndpointAddress;
> + dln2->ep_in = hostif->endpoint[1].desc.bEndpointAddress;
> + dln2->usb_dev = usb_get_dev(interface_to_usbdev(interface));
> + dln2->interface = interface;
> + usb_set_intfdata(interface, dln2);
> + init_waitqueue_head(&dln2->disconnect_wq);
> +
> + for (i = 0; i < DLN2_HANDLES; i++) {
> + init_waitqueue_head(&dln2->mod_rx_slots[i].wq);
> + spin_lock_init(&dln2->mod_rx_slots[i].lock);
> + for (j = 0; j < DLN2_MAX_RX_SLOTS; j++)
> + init_completion(&dln2->mod_rx_slots[i].slots[j].done);
> + }
> +
> + spin_lock_init(&dln2->event_cb_lock);
> + INIT_LIST_HEAD(&dln2->event_cb_list);
The disconnect_lock is never initialised, as mentioned above.
> +
> + ret = dln2_setup_rx_urbs(dln2, hostif);
> + if (ret) {
> + dln2_free(dln2);
> + return ret;
> + }
> +
> + ret = dln2_hw_init(dln2);
> + if (ret < 0) {
> + dev_err(dev, "failed to initialize hardware\n");
> + goto out_cleanup;
> + }
> +
> + id = dln2->usb_dev->bus->busnum << 8 | dln2->usb_dev->devnum;
> + ret = mfd_add_devices(dev, id, dln2_devs, ARRAY_SIZE(dln2_devs),
> + NULL, 0, NULL);
> + if (ret != 0) {
> + dev_err(dev, "failed to add mfd devices to core\n");
> + goto out_cleanup;
> + }
> +
> + return 0;
> +
> +out_cleanup:
> + dln2_free(dln2);
> +
> + return ret;
> +}
> +
> +static const struct usb_device_id dln2_table[] = {
> + { USB_DEVICE(0xa257, 0x2013) },
> + { }
> +};
> +
> +MODULE_DEVICE_TABLE(usb, dln2_table);
> +
> +static struct usb_driver dln2_driver = {
> + .name = "usb-dln2",
Just call the usb driver "dln2".
> + .probe = dln2_probe,
> + .disconnect = dln2_disconnect,
> + .id_table = dln2_table,
> +};
Johan
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists