All of lore.kernel.org
 help / color / mirror / Atom feed
From: "christophe.ricard" <christophe.ricard-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
To: Samuel Ortiz <sameo-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
Cc: linux-nfc-hn68Rpc1hR1g9hUCZPvPmw@public.gmane.org,
	christophe-h.ricard-qxv4g6HH51o@public.gmane.org,
	devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Subject: Re: [PATCH v1 04/34] NFC: st21nfca: Remove unreachable code
Date: Tue, 27 Jan 2015 00:07:48 +0100	[thread overview]
Message-ID: <54C6C8C4.4010807@gmail.com> (raw)
In-Reply-To: <20150126011001.GB28592-41CF7WKNp/FoDWY/xQGDymt3HXsI98Cx0E9HWUfgJXw@public.gmane.org>

Hi Samuel,

Unless i misunderstood the code i thought that skb is freed once used by 
an event_received hook.

For example if skb is relevant for st21nfca event_received hook, it will 
return 0. We then jump to exit_noskb in nfc_hci_event_received.
In st21nfca_dep_event_received, r = 0 is never changed or only for case 
ST21NFCA_EVT_SEND_DATA.

In case skb is not relevant for st21nfca event_received hook, it will 
return a value > 0. I have choosen 1. This way, nfc_hci_event_received
will continue to check for relevant event (aka: 
NFC_HCI_EVT_TARGET_DISCOVERED) and will then free the current skb.

So i am don't think  my there is an error in st21nfca_dep_event_received 
or there is one in nfc_hci_event_received as well ?

What do you think ?

Best Regards
Christophe

On 26/01/2015 02:10, Samuel Ortiz wrote:
> Hi Christophe,
>
> On Mon, Dec 08, 2014 at 10:08:09PM +0100, Christophe Ricard wrote:
>> kfree_skb(skb) in st21nfca_hci_event_received is never reach.
>>
>> Reported-by: Dan Carpenter <dan.carpenter-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
>> Signed-off-by: Christophe Ricard <christophe-h.ricard-qxv4g6HH51o@public.gmane.org>
>> ---
>>   drivers/nfc/st21nfca/st21nfca.c | 2 --
>>   1 file changed, 2 deletions(-)
>>
>> diff --git a/drivers/nfc/st21nfca/st21nfca.c b/drivers/nfc/st21nfca/st21nfca.c
>> index f2596c8..880193b 100644
>> --- a/drivers/nfc/st21nfca/st21nfca.c
>> +++ b/drivers/nfc/st21nfca/st21nfca.c
>> @@ -845,8 +845,6 @@ static int st21nfca_hci_event_received(struct nfc_hci_dev *hdev, u8 gate,
>>   	default:
>>   		return 1;
>>   	}
>> -	kfree_skb(skb);
>> -	return 0;
> That make sense to me, but while looking at this function, I think your
> skb free in st21nfca_dep_event_received() is incorrect.
> The HCI core manages the SKBs it sends and frees this skb.
>
> Cheers,
> Samuel.

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

  parent reply	other threads:[~2015-01-26 23:07 UTC|newest]

Thread overview: 45+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-12-08 21:08 [PATCH v1 00/34] Minor clean & Secure Element support for ST21NFCA/ST21NFCB Christophe Ricard
     [not found] ` <1418072919-10535-1-git-send-email-christophe-h.ricard-qxv4g6HH51o@public.gmane.org>
2014-12-08 21:08   ` [PATCH v1 01/34] NFC: dts: st21nfca: Fix compatible string spelling to follow other drivers Christophe Ricard
2014-12-08 21:08   ` [PATCH v1 02/34] NFC: dts: st21nfcb: " Christophe Ricard
2014-12-08 21:08   ` [PATCH v1 03/34] NFC: st21nfcb: Fix "WARNING: invalid free of devm_ allocated data" Christophe Ricard
2014-12-08 21:08   ` [PATCH v1 04/34] NFC: st21nfca: Remove unreachable code Christophe Ricard
     [not found]     ` <1418072919-10535-5-git-send-email-christophe-h.ricard-qxv4g6HH51o@public.gmane.org>
2015-01-26  1:10       ` Samuel Ortiz
     [not found]         ` <20150126011001.GB28592-41CF7WKNp/FoDWY/xQGDymt3HXsI98Cx0E9HWUfgJXw@public.gmane.org>
2015-01-26 23:07           ` christophe.ricard [this message]
2014-12-08 21:08   ` [PATCH v1 05/34] NFC: hci: Change event_received handler gate parameter to pipe Christophe Ricard
     [not found]     ` <1418072919-10535-6-git-send-email-christophe-h.ricard-qxv4g6HH51o@public.gmane.org>
2015-01-26  1:10       ` Samuel Ortiz
2014-12-08 21:08   ` [PATCH v1 06/34] NFC: hci: Add pipes table to reference them with a tuple {gate, host} Christophe Ricard
     [not found]     ` <1418072919-10535-7-git-send-email-christophe-h.ricard-qxv4g6HH51o@public.gmane.org>
2015-01-26  1:11       ` Samuel Ortiz
2014-12-08 21:08   ` [PATCH v1 07/34] NFC: hci: Change nfc_hci_send_response gate parameter to pipe Christophe Ricard
     [not found]     ` <1418072919-10535-8-git-send-email-christophe-h.ricard-qxv4g6HH51o@public.gmane.org>
2015-01-26  1:11       ` Samuel Ortiz
2014-12-08 21:08   ` [PATCH v1 08/34] NFC: hci: Reference every pipe information according to notification Christophe Ricard
     [not found]     ` <1418072919-10535-9-git-send-email-christophe-h.ricard-qxv4g6HH51o@public.gmane.org>
2015-01-26  1:11       ` Samuel Ortiz
2014-12-08 21:08   ` [PATCH v1 09/34] NFC: hci: Add cmd_received handler Christophe Ricard
2014-12-08 21:08   ` [PATCH v1 10/34] NFC: pn544: Change event_received gate parameter to pipe Christophe Ricard
2014-12-08 21:08   ` [PATCH v1 11/34] NFC: microread: " Christophe Ricard
2014-12-08 21:08   ` [PATCH v1 12/34] NFC: hci: Remove nfc_hci_pipe2gate function Christophe Ricard
2014-12-08 21:08   ` [PATCH v1 13/34] NFC: nfc_enable_se Remove useless blank line at beginning of function Christophe Ricard
2014-12-08 21:08   ` [PATCH v1 14/34] NFC: nfc_disable_se " Christophe Ricard
2014-12-08 21:08   ` [PATCH v1 15/34] NFC: st21nfca: Adding support for secure element Christophe Ricard
2014-12-08 21:08   ` [PATCH v1 16/34] NFC: st21nfca: Remove useless ST21NFCA_SE_HOST_EVT_HOT_PLUG macro Christophe Ricard
2014-12-08 21:08   ` [PATCH v1 17/34] NFC: st21nfca: Remove skb_pipe_list and skb_pipe_info useless allocation Christophe Ricard
2014-12-08 21:08   ` [PATCH v1 18/34] NFC: st21nfca: Remove checkpatch.pl warning Possible unnecessary 'out of memory' message Christophe Ricard
2014-12-08 21:08   ` [PATCH v1 19/34] NFC: dts: st21nfca: Document ese-present & uicc-present DTS property Christophe Ricard
2014-12-08 21:08   ` [PATCH v1 20/34] NFC: nci: Add dynamic conn_id NCI concept Christophe Ricard
     [not found]     ` <1418072919-10535-21-git-send-email-christophe-h.ricard-qxv4g6HH51o@public.gmane.org>
2015-01-27  0:44       ` Samuel Ortiz
2014-12-08 21:08   ` [PATCH v1 21/34] NFC: nci: Make nci_request available for nfc driver Christophe Ricard
2014-12-08 21:08   ` [PATCH v1 22/34] NFC: nci: Add NCI NFCEE constant Christophe Ricard
2014-12-08 21:08   ` [PATCH v1 23/34] NFC: nci: Add nci_nfcee_discover handler command/response/notification Christophe Ricard
2014-12-08 21:08   ` [PATCH v1 24/34] NFC: nci: Add nci_nfcee_mode_set handler command/response Christophe Ricard
2014-12-08 21:08   ` [PATCH v1 25/34] NFC: nci: Add nci_core_conn_create " Christophe Ricard
2014-12-08 21:08   ` [PATCH v1 26/34] NFC: nci: Add nci_core_conn_close " Christophe Ricard
2014-12-08 21:08   ` [PATCH v1 27/34] NFC: st21nfcb: Add HCI protocol over NCI protocol support Christophe Ricard
     [not found]     ` <1418072919-10535-28-git-send-email-christophe-h.ricard-qxv4g6HH51o@public.gmane.org>
2015-01-27  0:49       ` Samuel Ortiz
     [not found]         ` <20150127004958.GB31396-41CF7WKNp/FoDWY/xQGDymt3HXsI98Cx0E9HWUfgJXw@public.gmane.org>
2015-01-27 22:16           ` christophe.ricard
     [not found]             ` <54C80E40.7020707-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2015-01-28  0:04               ` Samuel Ortiz
2014-12-08 21:08   ` [PATCH v1 28/34] NFC: st21nfcb: Adding support for secure element Christophe Ricard
2014-12-08 21:08   ` [PATCH v1 29/34] NFC: Forward NFC_EVT_TRANSACTION up to user space Christophe Ricard
2014-12-08 21:08   ` [PATCH v1 30/34] NFC: nci: Add support RF_NFCEE_ACTION_NTF Christophe Ricard
2014-12-08 21:08   ` [PATCH v1 31/34] NFC: nci: Change NCI state machine to LISTEN_ACTIVE and ignore parameters in rf_intf_activated_ntf Christophe Ricard
2014-12-08 21:08   ` [PATCH v1 32/34] NFC: st21nfcb: Add support for HCI event transaction Christophe Ricard
2014-12-08 21:08   ` [PATCH v1 33/34] NFC: st21nfca: " Christophe Ricard
2014-12-08 21:08   ` [PATCH v1 34/34] NFC: st21nfca: Fix some skb memory leaks Christophe Ricard

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=54C6C8C4.4010807@gmail.com \
    --to=christophe.ricard-re5jqeeqqe8avxtiumwx3w@public.gmane.org \
    --cc=christophe-h.ricard-qxv4g6HH51o@public.gmane.org \
    --cc=devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=linux-nfc-hn68Rpc1hR1g9hUCZPvPmw@public.gmane.org \
    --cc=sameo-VuQAYsv1563Yd54FQh9/CA@public.gmane.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.