Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1591078 > unrolled thread
| Started by | Colin Ian King <colin.king@canonical.com> |
|---|---|
| First post | 2017-03-02 14:40 +0100 |
| Last post | 2017-03-02 18:00 +0100 |
| Articles | 2 — 2 participants |
Back to article view | Back to linux.kernel
re: rxrpc: Fix deadlock between call creation and sendmsg/recvmsg Colin Ian King <colin.king@canonical.com> - 2017-03-02 14:40 +0100
Re: rxrpc: Fix deadlock between call creation and sendmsg/recvmsg David Howells <dhowells@redhat.com> - 2017-03-02 18:00 +0100
| From | Colin Ian King <colin.king@canonical.com> |
|---|---|
| Date | 2017-03-02 14:40 +0100 |
| Subject | re: rxrpc: Fix deadlock between call creation and sendmsg/recvmsg |
| Message-ID | <tgz7A-2SA-17@gated-at.bofh.it> |
I think the following part of the patch is problematic:
call = rxrpc_find_call_by_user_ID(rx, user_call_ID);
if (!call) {
+ ret = -EBADSLT;
if (cmd != RXRPC_CMD_SEND_DATA)
- return -EBADSLT;
+ goto error_release_sock;
+ ret = -EBUSY;
At this point call is null, so the following code is performing a null
pointer dereference on call when accessing call->state.
Detected by CoverityScan CID#1414316 ("Dereference after null check")
+ if (call->state == RXRPC_CALL_UNINITIALISED ||
+ call->state == RXRPC_CALL_CLIENT_AWAIT_CONN ||
+ call->state == RXRPC_CALL_SERVER_PREALLOC ||
+ call->state == RXRPC_CALL_SERVER_SECURING ||
+ call->state == RXRPC_CALL_SERVER_ACCEPTING)
+ goto error_release_sock;
call = rxrpc_new_client_call_for_sendmsg(rx, msg,
user_call_ID,
[toc] | [next] | [standalone]
| From | David Howells <dhowells@redhat.com> |
|---|---|
| Date | 2017-03-02 18:00 +0100 |
| Message-ID | <tgCf8-56J-25@gated-at.bofh.it> |
| In reply to | #1591078 |
Colin Ian King <colin.king@canonical.com> wrote:
> I think the following part of the patch is problematic:
>
> call = rxrpc_find_call_by_user_ID(rx, user_call_ID);
> if (!call) {
> + ret = -EBADSLT;
> if (cmd != RXRPC_CMD_SEND_DATA)
> - return -EBADSLT;
> + goto error_release_sock;
> + ret = -EBUSY;
>
> At this point call is null, so the following code is performing a null
> pointer dereference on call when accessing call->state.
>
> Detected by CoverityScan CID#1414316 ("Dereference after null check")
>
> + if (call->state == RXRPC_CALL_UNINITIALISED ||
> + call->state == RXRPC_CALL_CLIENT_AWAIT_CONN ||
> + call->state == RXRPC_CALL_SERVER_PREALLOC ||
> + call->state == RXRPC_CALL_SERVER_SECURING ||
> + call->state == RXRPC_CALL_SERVER_ACCEPTING)
> + goto error_release_sock;
> call = rxrpc_new_client_call_for_sendmsg(rx, msg,
> user_call_ID,
Good catch, thanks, I put the state check in the wrong half of the
if-statement.
David
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web