| Message ID | f5e1fc8131923c50d08fa30eb7136f32ddafe37d.1700943019.git.code@siddh.me (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | Fix UAF caused by racing datagram sending and freeing of nfc_dev | expand |
On 25/11/2023 21:26, Siddh Raman Pant wrote: > The block for datagram sending is a significantly bigger chunk of the > function compared to the other scenario. > > Thus, put the significantly smaller block inside the if-block. > > > + if (sk->sk_state != LLCP_BOUND) { > release_sock(sk); > - > - return nfc_llcp_send_ui_frame(llcp_sock, addr->dsap, addr->ssap, > - msg, len); > + return -ENOTCONN; > } > > - if (sk->sk_state != LLCP_CONNECTED) { > + DECLARE_SOCKADDR(struct sockaddr_nfc_llcp *, addr, msg->msg_name); No, this code is not readable. I don't think this change helps in anything. > + > + if (msg->msg_namelen < sizeof(*addr)) { > release_sock(sk); > - return -ENOTCONN; > + return -EINVAL; > } > > release_sock(sk); > > - return nfc_llcp_send_i_frame(llcp_sock, msg, len); > + return nfc_llcp_send_ui_frame(llcp_sock, addr->dsap, addr->ssap, > + msg, len); > + Stray blank line. Best regards, Krzysztof
diff --git a/net/nfc/llcp_sock.c b/net/nfc/llcp_sock.c index 603f2219b62f..3f1a39e54aa1 100644 --- a/net/nfc/llcp_sock.c +++ b/net/nfc/llcp_sock.c @@ -795,34 +795,32 @@ static int llcp_sock_sendmsg(struct socket *sock, struct msghdr *msg, return -ENODEV; } - if (sk->sk_type == SOCK_DGRAM) { - if (sk->sk_state != LLCP_BOUND) { - release_sock(sk); - return -ENOTCONN; - } + if (sk->sk_type != SOCK_DGRAM) { + release_sock(sk); - DECLARE_SOCKADDR(struct sockaddr_nfc_llcp *, addr, - msg->msg_name); + if (sk->sk_state != LLCP_CONNECTED) + return -ENOTCONN; - if (msg->msg_namelen < sizeof(*addr)) { - release_sock(sk); - return -EINVAL; - } + return nfc_llcp_send_i_frame(llcp_sock, msg, len); + } + if (sk->sk_state != LLCP_BOUND) { release_sock(sk); - - return nfc_llcp_send_ui_frame(llcp_sock, addr->dsap, addr->ssap, - msg, len); + return -ENOTCONN; } - if (sk->sk_state != LLCP_CONNECTED) { + DECLARE_SOCKADDR(struct sockaddr_nfc_llcp *, addr, msg->msg_name); + + if (msg->msg_namelen < sizeof(*addr)) { release_sock(sk); - return -ENOTCONN; + return -EINVAL; } release_sock(sk); - return nfc_llcp_send_i_frame(llcp_sock, msg, len); + return nfc_llcp_send_ui_frame(llcp_sock, addr->dsap, addr->ssap, + msg, len); + } static int llcp_sock_recvmsg(struct socket *sock, struct msghdr *msg,
The block for datagram sending is a significantly bigger chunk of the function compared to the other scenario. Thus, put the significantly smaller block inside the if-block. Signed-off-by: Siddh Raman Pant <code@siddh.me> --- net/nfc/llcp_sock.c | 32 +++++++++++++++----------------- 1 file changed, 15 insertions(+), 17 deletions(-)