* [PATCH 1/2] nvmet-tcp: fix possible NULL deref
@ 2019-08-08 20:55 Sagi Grimberg
2019-08-08 20:55 ` [PATCH 2/2] nvmet-tcp: fix possible memory leak Sagi Grimberg
2019-08-11 14:57 ` [PATCH 1/2] nvmet-tcp: fix possible NULL deref Max Gurtovoy
0 siblings, 2 replies; 5+ messages in thread
From: Sagi Grimberg @ 2019-08-08 20:55 UTC (permalink / raw
We must only call sgl_free for sgl that we actually
allocated.
Signed-off-by: Sagi Grimberg <sagi at grimberg.me>
---
drivers/nvme/target/tcp.c | 12 ++++++++----
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/drivers/nvme/target/tcp.c b/drivers/nvme/target/tcp.c
index 69b83fa0c76c..0d63f3da0117 100644
--- a/drivers/nvme/target/tcp.c
+++ b/drivers/nvme/target/tcp.c
@@ -348,7 +348,8 @@ static int nvmet_tcp_map_data(struct nvmet_tcp_cmd *cmd)
return 0;
err:
- sgl_free(cmd->req.sg);
+ if (cmd->req.sg_cnt)
+ sgl_free(cmd->req.sg);
return NVME_SC_INTERNAL;
}
@@ -553,7 +554,8 @@ static int nvmet_try_send_data(struct nvmet_tcp_cmd *cmd)
if (queue->nvme_sq.sqhd_disabled) {
kfree(cmd->iov);
- sgl_free(cmd->req.sg);
+ if (cmd->req.sg_cnt)
+ sgl_free(cmd->req.sg);
}
return 1;
@@ -584,7 +586,8 @@ static int nvmet_try_send_response(struct nvmet_tcp_cmd *cmd,
return -EAGAIN;
kfree(cmd->iov);
- sgl_free(cmd->req.sg);
+ if (cmd->req.sg_cnt)
+ sgl_free(cmd->req.sg);
cmd->queue->snd_cmd = NULL;
nvmet_tcp_put_cmd(cmd);
return 1;
@@ -1306,7 +1309,8 @@ static void nvmet_tcp_finish_cmd(struct nvmet_tcp_cmd *cmd)
{
nvmet_req_uninit(&cmd->req);
nvmet_tcp_unmap_pdu_iovec(cmd);
- sgl_free(cmd->req.sg);
+ if (cmd->req.sg_cnt)
+ sgl_free(cmd->req.sg);
}
static void nvmet_tcp_uninit_data_in_cmds(struct nvmet_tcp_queue *queue)
--
2.17.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH 2/2] nvmet-tcp: fix possible memory leak
2019-08-08 20:55 [PATCH 1/2] nvmet-tcp: fix possible NULL deref Sagi Grimberg
@ 2019-08-08 20:55 ` Sagi Grimberg
2019-08-11 14:58 ` Max Gurtovoy
2019-08-11 14:57 ` [PATCH 1/2] nvmet-tcp: fix possible NULL deref Max Gurtovoy
1 sibling, 1 reply; 5+ messages in thread
From: Sagi Grimberg @ 2019-08-08 20:55 UTC (permalink / raw
when we uninit a command in error flow we also need to
free an iovec if it was allocated.
Signed-off-by: Sagi Grimberg <sagi at grimberg.me>
---
drivers/nvme/target/tcp.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/nvme/target/tcp.c b/drivers/nvme/target/tcp.c
index 0d63f3da0117..76e43750b9e5 100644
--- a/drivers/nvme/target/tcp.c
+++ b/drivers/nvme/target/tcp.c
@@ -1309,6 +1309,7 @@ static void nvmet_tcp_finish_cmd(struct nvmet_tcp_cmd *cmd)
{
nvmet_req_uninit(&cmd->req);
nvmet_tcp_unmap_pdu_iovec(cmd);
+ kfree(cmd->iov);
if (cmd->req.sg_cnt)
sgl_free(cmd->req.sg);
}
--
2.17.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH 1/2] nvmet-tcp: fix possible NULL deref
2019-08-08 20:55 [PATCH 1/2] nvmet-tcp: fix possible NULL deref Sagi Grimberg
2019-08-08 20:55 ` [PATCH 2/2] nvmet-tcp: fix possible memory leak Sagi Grimberg
@ 2019-08-11 14:57 ` Max Gurtovoy
2019-08-13 6:33 ` Sagi Grimberg
1 sibling, 1 reply; 5+ messages in thread
From: Max Gurtovoy @ 2019-08-11 14:57 UTC (permalink / raw
On 8/8/2019 11:55 PM, Sagi Grimberg wrote:
> We must only call sgl_free for sgl that we actually
> allocated.
>
> Signed-off-by: Sagi Grimberg <sagi at grimberg.me>
> ---
> drivers/nvme/target/tcp.c | 12 ++++++++----
> 1 file changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/nvme/target/tcp.c b/drivers/nvme/target/tcp.c
> index 69b83fa0c76c..0d63f3da0117 100644
> --- a/drivers/nvme/target/tcp.c
> +++ b/drivers/nvme/target/tcp.c
> @@ -348,7 +348,8 @@ static int nvmet_tcp_map_data(struct nvmet_tcp_cmd *cmd)
>
> return 0;
> err:
> - sgl_free(cmd->req.sg);
> + if (cmd->req.sg_cnt)
> + sgl_free(cmd->req.sg);
what about zeroing the sg_cnt during nvmet_tcp_get_cmd ?
and NULLify the cmd->req.sg as well in nvmet_tcp_get_cmd ?
> return NVME_SC_INTERNAL;
> }
>
> @@ -553,7 +554,8 @@ static int nvmet_try_send_data(struct nvmet_tcp_cmd *cmd)
>
> if (queue->nvme_sq.sqhd_disabled) {
> kfree(cmd->iov);
> - sgl_free(cmd->req.sg);
> + if (cmd->req.sg_cnt)
> + sgl_free(cmd->req.sg);
> }
>
> return 1;
> @@ -584,7 +586,8 @@ static int nvmet_try_send_response(struct nvmet_tcp_cmd *cmd,
> return -EAGAIN;
>
> kfree(cmd->iov);
> - sgl_free(cmd->req.sg);
> + if (cmd->req.sg_cnt)
> + sgl_free(cmd->req.sg);
> cmd->queue->snd_cmd = NULL;
> nvmet_tcp_put_cmd(cmd);
> return 1;
> @@ -1306,7 +1309,8 @@ static void nvmet_tcp_finish_cmd(struct nvmet_tcp_cmd *cmd)
> {
> nvmet_req_uninit(&cmd->req);
> nvmet_tcp_unmap_pdu_iovec(cmd);
> - sgl_free(cmd->req.sg);
> + if (cmd->req.sg_cnt)
> + sgl_free(cmd->req.sg);
> }
>
> static void nvmet_tcp_uninit_data_in_cmds(struct nvmet_tcp_queue *queue)
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH 2/2] nvmet-tcp: fix possible memory leak
2019-08-08 20:55 ` [PATCH 2/2] nvmet-tcp: fix possible memory leak Sagi Grimberg
@ 2019-08-11 14:58 ` Max Gurtovoy
0 siblings, 0 replies; 5+ messages in thread
From: Max Gurtovoy @ 2019-08-11 14:58 UTC (permalink / raw
On 8/8/2019 11:55 PM, Sagi Grimberg wrote:
> when we uninit a command in error flow we also need to
> free an iovec if it was allocated.
>
> Signed-off-by: Sagi Grimberg <sagi at grimberg.me>
> ---
> drivers/nvme/target/tcp.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/nvme/target/tcp.c b/drivers/nvme/target/tcp.c
> index 0d63f3da0117..76e43750b9e5 100644
> --- a/drivers/nvme/target/tcp.c
> +++ b/drivers/nvme/target/tcp.c
> @@ -1309,6 +1309,7 @@ static void nvmet_tcp_finish_cmd(struct nvmet_tcp_cmd *cmd)
> {
> nvmet_req_uninit(&cmd->req);
> nvmet_tcp_unmap_pdu_iovec(cmd);
> + kfree(cmd->iov);
> if (cmd->req.sg_cnt)
> sgl_free(cmd->req.sg);
> }
Looks good,
Reviewed-by: Max Gurtovoy <maxg at mellanox.com>
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH 1/2] nvmet-tcp: fix possible NULL deref
2019-08-11 14:57 ` [PATCH 1/2] nvmet-tcp: fix possible NULL deref Max Gurtovoy
@ 2019-08-13 6:33 ` Sagi Grimberg
0 siblings, 0 replies; 5+ messages in thread
From: Sagi Grimberg @ 2019-08-13 6:33 UTC (permalink / raw
> what about zeroing the sg_cnt during nvmet_tcp_get_cmd ?
>
> and NULLify the cmd->req.sg as well in nvmet_tcp_get_cmd ?
Its set in nvmet_req_init()
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2019-08-13 6:33 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-08-08 20:55 [PATCH 1/2] nvmet-tcp: fix possible NULL deref Sagi Grimberg
2019-08-08 20:55 ` [PATCH 2/2] nvmet-tcp: fix possible memory leak Sagi Grimberg
2019-08-11 14:58 ` Max Gurtovoy
2019-08-11 14:57 ` [PATCH 1/2] nvmet-tcp: fix possible NULL deref Max Gurtovoy
2019-08-13 6:33 ` Sagi Grimberg
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.