Hi,
These are improvements, not fixing any experienced issue, just looking correct
to me from the code point of view.
Changes since v1
================
1. Split from the fix.
Testing
=======
Under QEMU only. The NFC/LLCP code was not really tested on a device.
Best regards,
Krzysztof
Krzysztof Kozlowski (6):
nfc: llcp: nullify llcp_sock->dev on connect() error paths
nfc: llcp: simplify llcp_sock_connect() error paths
nfc: llcp: use centralized exiting of bind on errors
nfc: llcp: use test_bit()
nfc: llcp: protect nfc_llcp_sock_unlink() calls
nfc: llcp: Revert "NFC: Keep socket alive until the DISC PDU is
actually sent"
net/nfc/llcp.h | 1 -
net/nfc/llcp_core.c | 9 +--------
net/nfc/llcp_sock.c | 49 ++++++++++++++++++++++-----------------------
3 files changed, 25 insertions(+), 34 deletions(-)
--
2.32.0
nfc_llcp_sock_link() is called in all paths (bind/connect) as a last
action, still protected with lock_sock(). When cleaning up in
llcp_sock_release(), call nfc_llcp_sock_unlink() in a mirrored way:
earlier and still under the lock_sock().
Signed-off-by: Krzysztof Kozlowski <[email protected]>
---
net/nfc/llcp_sock.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/net/nfc/llcp_sock.c b/net/nfc/llcp_sock.c
index c9d5c427f035..5c5705f5028b 100644
--- a/net/nfc/llcp_sock.c
+++ b/net/nfc/llcp_sock.c
@@ -631,6 +631,11 @@ static int llcp_sock_release(struct socket *sock)
}
}
+ if (sock->type == SOCK_RAW)
+ nfc_llcp_sock_unlink(&local->raw_sockets, sk);
+ else
+ nfc_llcp_sock_unlink(&local->sockets, sk);
+
if (llcp_sock->reserved_ssap < LLCP_SAP_MAX)
nfc_llcp_put_ssap(llcp_sock->local, llcp_sock->ssap);
@@ -643,11 +648,6 @@ static int llcp_sock_release(struct socket *sock)
if (sk->sk_state == LLCP_DISCONNECTING)
return err;
- if (sock->type == SOCK_RAW)
- nfc_llcp_sock_unlink(&local->raw_sockets, sk);
- else
- nfc_llcp_sock_unlink(&local->sockets, sk);
-
out:
sock_orphan(sk);
sock_put(sk);
--
2.32.0
This reverts commit 17f7ae16aef1f58bc4af4c7a16b8778a91a30255.
The commit brought a new socket state LLCP_DISCONNECTING, which was
never set, only read, so socket could never set to such state.
Remove the dead code.
Signed-off-by: Krzysztof Kozlowski <[email protected]>
---
net/nfc/llcp.h | 1 -
net/nfc/llcp_core.c | 7 -------
net/nfc/llcp_sock.c | 7 -------
3 files changed, 15 deletions(-)
diff --git a/net/nfc/llcp.h b/net/nfc/llcp.h
index d49d4bf2e37c..c1d9be636933 100644
--- a/net/nfc/llcp.h
+++ b/net/nfc/llcp.h
@@ -6,7 +6,6 @@
enum llcp_state {
LLCP_CONNECTED = 1, /* wait_for_packet() wants that */
LLCP_CONNECTING,
- LLCP_DISCONNECTING,
LLCP_CLOSED,
LLCP_BOUND,
LLCP_LISTEN,
diff --git a/net/nfc/llcp_core.c b/net/nfc/llcp_core.c
index b70d5042bf74..3364caabef8b 100644
--- a/net/nfc/llcp_core.c
+++ b/net/nfc/llcp_core.c
@@ -737,13 +737,6 @@ static void nfc_llcp_tx_work(struct work_struct *work)
print_hex_dump_debug("LLCP Tx: ", DUMP_PREFIX_OFFSET,
16, 1, skb->data, skb->len, true);
- if (ptype == LLCP_PDU_DISC && sk != NULL &&
- sk->sk_state == LLCP_DISCONNECTING) {
- nfc_llcp_sock_unlink(&local->sockets, sk);
- sock_orphan(sk);
- sock_put(sk);
- }
-
if (ptype == LLCP_PDU_I)
copy_skb = skb_copy(skb, GFP_ATOMIC);
diff --git a/net/nfc/llcp_sock.c b/net/nfc/llcp_sock.c
index 5c5705f5028b..4ca35791c93b 100644
--- a/net/nfc/llcp_sock.c
+++ b/net/nfc/llcp_sock.c
@@ -641,13 +641,6 @@ static int llcp_sock_release(struct socket *sock)
release_sock(sk);
- /* Keep this sock alive and therefore do not remove it from the sockets
- * list until the DISC PDU has been actually sent. Otherwise we would
- * reply with DM PDUs before sending the DISC one.
- */
- if (sk->sk_state == LLCP_DISCONNECTING)
- return err;
-
out:
sock_orphan(sk);
sock_put(sk);
--
2.32.0
The llcp_sock_connect() error paths were using a mixed way of central
exit (goto) and cleanup
Signed-off-by: Krzysztof Kozlowski <[email protected]>
---
net/nfc/llcp_sock.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/net/nfc/llcp_sock.c b/net/nfc/llcp_sock.c
index e92440c0c4c7..fdf0856182c6 100644
--- a/net/nfc/llcp_sock.c
+++ b/net/nfc/llcp_sock.c
@@ -712,10 +712,8 @@ static int llcp_sock_connect(struct socket *sock, struct sockaddr *_addr,
llcp_sock->local = nfc_llcp_local_get(local);
llcp_sock->ssap = nfc_llcp_get_local_ssap(local);
if (llcp_sock->ssap == LLCP_SAP_MAX) {
- nfc_llcp_local_put(llcp_sock->local);
- llcp_sock->local = NULL;
ret = -ENOMEM;
- goto put_dev;
+ goto sock_llcp_put_local;
}
llcp_sock->reserved_ssap = llcp_sock->ssap;
@@ -760,11 +758,13 @@ static int llcp_sock_connect(struct socket *sock, struct sockaddr *_addr,
sock_llcp_release:
nfc_llcp_put_ssap(local, llcp_sock->ssap);
+
+sock_llcp_put_local:
nfc_llcp_local_put(llcp_sock->local);
llcp_sock->local = NULL;
+ llcp_sock->dev = NULL;
put_dev:
- llcp_sock->dev = NULL;
nfc_put_device(dev);
error:
--
2.32.0
Hello:
This series was applied to netdev/net-next.git (master)
by David S. Miller <[email protected]>:
On Wed, 2 Mar 2022 20:25:17 +0100 you wrote:
> Hi,
>
> These are improvements, not fixing any experienced issue, just looking correct
> to me from the code point of view.
>
> Changes since v1
> ================
> 1. Split from the fix.
>
> [...]
Here is the summary with links:
- [RESEND,v2,1/6] nfc: llcp: nullify llcp_sock->dev on connect() error paths
https://git.kernel.org/netdev/net-next/c/13a3585b264b
- [RESEND,v2,2/6] nfc: llcp: simplify llcp_sock_connect() error paths
https://git.kernel.org/netdev/net-next/c/ec10fd154d93
- [RESEND,v2,3/6] nfc: llcp: use centralized exiting of bind on errors
https://git.kernel.org/netdev/net-next/c/4dbbf673f7d7
- [RESEND,v2,4/6] nfc: llcp: use test_bit()
https://git.kernel.org/netdev/net-next/c/a736491239f4
- [RESEND,v2,5/6] nfc: llcp: protect nfc_llcp_sock_unlink() calls
https://git.kernel.org/netdev/net-next/c/a06b8044169f
- [RESEND,v2,6/6] nfc: llcp: Revert "NFC: Keep socket alive until the DISC PDU is actually sent"
https://git.kernel.org/netdev/net-next/c/44cd5765495b
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html