Skip to contentSkip to conversation

nfc: st95hf: drop nfcdev_free

PATCH In review Daniel Mack 1 participant 3 messages Jun 29, 2018 – Jul 17, 2018 NFC

lore
Share & tools
lore

Daniel Mack

author
#0 lore raw

PATCH 1/2 nfc: st95hf: drop nfcdev_free

This flag is unneccesary. We can just nullify `ddev' instead after we freed it.

Signed-off-by Daniel Mack <[email protected]>
---
 drivers/nfc/st95hf/core.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)
drivers/nfc/st95hf/core.c +3 -5
index a50a95cfcfd8..ef91ca8b53a4 100644
@@ -220,7 +220,6 @@ struct st95_digital_cmd_complete_arg {
  *	from threaded ISR. Usage of this mutex avoids any race between
  *	deletion of the object from st95hf_remove() and its access from
  *	the threaded ISR.
- * @nfcdev_free: flag to have the state of nfc device object.
  *	[alive | died]
  * @current_protocol: current nfc protocol.
  * @current_rf_tech: current rf technology.

@@ -237,7 +236,6 @@ struct st95hf_context {
 	unsigned char sendrcv_trflag;
 	struct semaphore exchange_lock;
 	struct mutex rm_lock;
-	bool nfcdev_free;
 	u8 current_protocol;
 	u8 current_rf_tech;
 	int fwi;

@@ -822,8 +820,8 @@ static irqreturn_t st95hf_irq_thread_handler(int irq, void  *st95hfcontext)
 		goto end;
 	}
 
-	/* if stcontext->nfcdev_free is true, it means remove already ran */
-	if (stcontext->nfcdev_free) {
+	/* if stcontext->ddev is %NULL, it means remove already ran */
+	if (!stcontext->ddev) {
 		result = -ENODEV;
 		goto end;
 	}

@@ -1222,7 +1220,7 @@ static int st95hf_remove(struct spi_device *nfc_spi_dev)
 
 	nfc_digital_unregister_device(stcontext->ddev);
 	nfc_digital_free_device(stcontext->ddev);
-	stcontext->nfcdev_free = true;
+	stcontext->ddev = NULL;
 
 	mutex_unlock(&stcontext->rm_lock);
Signature · 1 line
2.17.1
author
#1 lore raw

PATCH 2/2 nfc: st95hf: drop another illegal kfree_skb()

In the error path of the IRQ handler, don't free the skb in flight. The callback in the digital core will do that for us, so this is another double-free that leads to memory corruptions.

The assignment of 'wtx' doesn't make sense as the variable is not read after it is written. Drop it.

Signed-off-by Daniel Mack <[email protected]>
---
 drivers/nfc/st95hf/core.c | 2 --
 1 file changed, 2 deletions(-)
drivers/nfc/st95hf/core.c +0 -2
index ef91ca8b53a4..e651e1aae5a3 100644
@@ -868,8 +868,6 @@ static irqreturn_t st95hf_irq_thread_handler(int irq, void  *st95hfcontext)
 	return IRQ_HANDLED;
 
 end:
-	kfree_skb(skb_resp);
-	wtx = false;
 	cb_arg->rats = false;
 	skb_resp = ERR_PTR(result);
 	/* call of callback with error */
Signature · 1 line
2.17.1
author
#2 lore raw

Re: [PATCH 2/2] nfc: st95hf: drop another illegal kfree_skb()

Hi,

I'll resend the two patches in this series as part of a bigger series soon, please ignore them for now.

Thanks,
Daniel

Quoted 26 lines · Daniel Mack

In the error path of the IRQ handler, don't free the skb in flight. The callback in the digital core will do that for us, so this is another double-free that leads to memory corruptions.

The assignment of 'wtx' doesn't make sense as the variable is not read after it is written. Drop it.

Signed-off-by Daniel Mack <[email protected]>
---
  drivers/nfc/st95hf/core.c | 2 --
  1 file changed, 2 deletions(-)
drivers/nfc/st95hf/core.c +0 -2
index ef91ca8b53a4..e651e1aae5a3 100644
@@ -868,8 +868,6 @@ static irqreturn_t st95hf_irq_thread_handler(int irq, void  *st95hfcontext)
  	return IRQ_HANDLED;
 
  end:
-	kfree_skb(skb_resp);
-	wtx = false;
  	cb_arg->rats = false;
  	skb_resp = ERR_PTR(result);
  	/* call of callback with error */