RE: [EXT] [PATCH net-next v2 1/2] nfc: llcp_core: Hold a ref to llcp_local->dev when holding a ref to llcp_local

From: Suman Ghosh
Date: Mon Dec 04 2023 - 04:33:57 EST


>>
>> >@@ -180,6 +183,7 @@ int nfc_llcp_local_put(struct nfc_llcp_local
>> >*local)
>> >     if (local == NULL)
>> >         return 0;
>> >
>> >+    nfc_put_device(local->dev);
>> [Suman] One question here, if we consider the path,
>> nfc_llcp_mac_is_down() ->
>> nfc_llcp_socket_release() -> nfc_llcp_local_put(), then inside
>> nfc_llcp_socket_release() we are already doing nfc_put_device() if
>> "sk->sk_state == LLCP_CONNECTED", with this change we are doing it
>again.
>> I guess you need to add some check to avoid that. Let me know if I am
>> missing something.
>
>The socket state is set to LLCP_CONNECTED in just two places:
>nfc_llcp_recv_connect() and nfc_llcp_recv_cc().
>
>nfc_get_device() is used prior to setting the socket state to
>LLCP_CONNECTED in nfc_llcp_recv_connect(). After that, it calls
>nfc_llcp_send_cc(), which I suppose is a connection PDU by some Google-
>fu (NFC specs is paywalled).
>
>In nfc_llcp_recv_cc(), we do not use nfc_get_device(), but since one
>must send cc (which is done in nfc_llcp_recv_connect()), I think we are
>good here.
>
>This patch change doesn't touch any other refcounting. We increment the
>refcount whenever we get the local, and decrement when we put it.
>nfc_llcp_find_local() involves getting it, so all users of that function
>increment the refcount, which is also the case with
>nfc_llcp_mac_is_down(). The last nfc_llcp_local_put() then correctly
>decrements the refcount.
>
>If there is indeed a refcount error due to LLCP_CONNECTED, it probably
>exists without this patch too.
[Suman] Thanks for the explanation.