Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753579AbcCYO4P (ORCPT ); Fri, 25 Mar 2016 10:56:15 -0400 Received: from gw.hale.at ([83.64.51.210]:49002 "EHLO gw.hale.at" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752769AbcCYOzK (ORCPT ); Fri, 25 Mar 2016 10:55:10 -0400 X-HALE-Mailborder-Watermark: 1459522055.28489@HUyse+cBR+7kt3iSXnCwEA X-HALE-Mailborder-From: michael.thalmeier@hale.at X-HALE-Mailborder-SpamCheck: not spam, SpamAssassin (nicht zwischen gespeichert, Wertung=-2.899, benoetigt 3, autolearn=not spam, ALL_TRUSTED -1.00, BAYES_00 -1.90, URIBL_BLOCKED 0.00) X-HALE-Mailborder-IP-Protocol: IPv4 X-HALE-Mailborder: Found to be clean X-HALE-Mailborder-ID: 911112C2109.AA77C X-HALE-Mailborder-Information: Please contact your admin for more information From: Michael Thalmeier To: Samuel Ortiz , Lauro Ramos Venancio , Aloisio Almeida Jr Cc: linux-kernel@vger.kernel.org, linux-nfc@ml01.01.org, michael@thalmeier.at Subject: [RFC 2/4] NFC: pn533: fix deadlock when socket is closed while processing command Date: Fri, 25 Mar 2016 15:46:52 +0100 Message-Id: <1458917214-10693-3-git-send-email-michael.thalmeier@hale.at> X-Mailer: git-send-email 2.5.5 In-Reply-To: <1458917214-10693-1-git-send-email-michael.thalmeier@hale.at> References: <1458917214-10693-1-git-send-email-michael.thalmeier@hale.at> Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3824 Lines: 111 A deadlock can occur when the NFC raw socket is closed while the driver is processing a command. Following is the call graph of the affected situation: send data via raw_sock: ------------- rawsock_tx_work sock_hold => socket refcnt++ nfc_data_exchange => cb = rawsock_data_exchange_complete ops->im_transceive = pn533_transceive => arg->cb = db = rawsock_data_exchange_complete pn533_send_data_async => cb = pn533_data_exchange_complete __pn533_send_async => cmd->complete_cb = cb = pn533_data_exchange_complete if_ops->send_frame_async response: -------- pn533_recv_response queue_work(priv->wq, &priv->cmd_complete_work) pn533_wq_cmd_complete pn533_send_async_complete cmd->complete_cb() = pn533_data_exchange_complete() arg->cb() = rawsock_data_exchange_complete() sock_put => socket refcnt-- => If the corresponding socket gets closed in the meantime socket will be destructed sk_free __sk_free sk->sk_destruct = rawsock_destruct nfc_deactivate_target ops->deactivate_target = pn533_deactivate_target pn533_send_cmd_sync pn533_send_cmd_async __pn533_send_async list_add_tail(&cmd->queue, &dev->cmd_queue) => add to command list because a command is currently processed wait_for_completion => the workqueue thread waits here because it is the one processing the commands => deadlock To fix the deadlock pn533_deactivate_target is changed to issue the PN533_CMD_IN_RELEASE command in async mode. This way nothing blocks and the release command is executed after the current command. Signed-off-by: Michael Thalmeier --- drivers/nfc/pn533.c | 40 ++++++++++++++++++++++++++++++---------- 1 file changed, 30 insertions(+), 10 deletions(-) diff --git a/drivers/nfc/pn533.c b/drivers/nfc/pn533.c index a85830f..074f1e4 100644 --- a/drivers/nfc/pn533.c +++ b/drivers/nfc/pn533.c @@ -2263,12 +2263,35 @@ static int pn533_activate_target(struct nfc_dev *nfc_dev, return 0; } +static int pn533_deactivate_target_complete(struct pn533 *dev, void *arg, + struct sk_buff *resp) +{ + int rc = 0; + + dev_dbg(&dev->interface->dev, "%s\n", __func__); + + if (IS_ERR(resp)) { + rc = PTR_ERR(resp); + + nfc_err(&dev->interface->dev, "Target release error %d\n", rc); + + return rc; + } + + rc = resp->data[0] & PN533_CMD_RET_MASK; + if (rc != PN533_CMD_RET_SUCCESS) + nfc_err(&dev->interface->dev, + "Error 0x%x when releasing the target\n", rc); + + dev_kfree_skb(resp); + return rc; +} + static void pn533_deactivate_target(struct nfc_dev *nfc_dev, struct nfc_target *target, u8 mode) { struct pn533 *dev = nfc_get_drvdata(nfc_dev); struct sk_buff *skb; - struct sk_buff *resp; int rc; dev_dbg(&dev->interface->dev, "%s\n", __func__); @@ -2287,16 +2310,13 @@ static void pn533_deactivate_target(struct nfc_dev *nfc_dev, *skb_put(skb, 1) = 1; /* TG*/ - resp = pn533_send_cmd_sync(dev, PN533_CMD_IN_RELEASE, skb); - if (IS_ERR(resp)) - return; - - rc = resp->data[0] & PN533_CMD_RET_MASK; - if (rc != PN533_CMD_RET_SUCCESS) - nfc_err(&dev->interface->dev, - "Error 0x%x when releasing the target\n", rc); + rc = pn533_send_cmd_async(dev, PN533_CMD_IN_RELEASE, skb, + pn533_deactivate_target_complete, NULL); + if (rc < 0) { + dev_kfree_skb(skb); + nfc_err(&dev->interface->dev, "Target release error %d\n", rc); + } - dev_kfree_skb(resp); return; } -- 2.5.5