| Message ID | 20240531211211.12628-2-krisman@suse.de (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
| Series | io_uring: support IORING_OP_BIND and IORING_OP_LISTEN | expand |
On 5/31/24 3:12 PM, Gabriel Krisman Bertazi wrote: > move_addr_to_kernel can fail, like if the user provides a bad sockaddr > pointer. In this case where the failure happens on ->prep() we don't > have a chance to clean the request later, so handle it here. Hmm, that should still get freed in the cleanup path? It'll eventually go on the compl_reqs list, and it has REQ_F_ASYNC_DATA set. Yes it'll be slower than the recycling it, but that should not matter as it's an erred request.
Jens Axboe <axboe@kernel.dk> writes: > On 5/31/24 3:12 PM, Gabriel Krisman Bertazi wrote: >> move_addr_to_kernel can fail, like if the user provides a bad sockaddr >> pointer. In this case where the failure happens on ->prep() we don't >> have a chance to clean the request later, so handle it here. > > Hmm, that should still get freed in the cleanup path? It'll eventually > go on the compl_reqs list, and it has REQ_F_ASYNC_DATA set. Yes it'll > be slower than the recycling it, but that should not matter as it's > an erred request. Hm right. I actually managed to reproduce some kind of memory exhaustion yesterday that I thought was fixed by this patch. But I see your point and I'm failing to trigger it today. Please disregard this patch. I'll look further to figure out what I did there.
On 5/31/24 5:01 PM, Gabriel Krisman Bertazi wrote: > Jens Axboe <axboe@kernel.dk> writes: > >> On 5/31/24 3:12 PM, Gabriel Krisman Bertazi wrote: >>> move_addr_to_kernel can fail, like if the user provides a bad sockaddr >>> pointer. In this case where the failure happens on ->prep() we don't >>> have a chance to clean the request later, so handle it here. >> >> Hmm, that should still get freed in the cleanup path? It'll eventually >> go on the compl_reqs list, and it has REQ_F_ASYNC_DATA set. Yes it'll >> be slower than the recycling it, but that should not matter as it's >> an erred request. > > Hm right. I actually managed to reproduce some kind of memory > exhaustion yesterday that I thought was fixed by this patch. But I see > your point and I'm failing to trigger it today. > > Please disregard this patch. I'll look further to figure out what I did > there. Maybe enable KMEMLEAK? It's pretty handy for testing. If there is a leak there, you should be able to reliably get info by doing: # ./reproducer (should be easy, just bogus addr) # echo scan > /sys/kernel/debug/kmemleak # sleep 5 # echo scan > /sys/kernel/debug/kmemleak
diff --git a/io_uring/net.c b/io_uring/net.c index 0a48596429d9..c3377e70aeeb 100644 --- a/io_uring/net.c +++ b/io_uring/net.c @@ -1657,6 +1657,7 @@ int io_connect_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe) { struct io_connect *conn = io_kiocb_to_cmd(req, struct io_connect); struct io_async_msghdr *io; + int ret; if (sqe->len || sqe->buf_index || sqe->rw_flags || sqe->splice_fd_in) return -EINVAL; @@ -1669,7 +1670,10 @@ int io_connect_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe) if (unlikely(!io)) return -ENOMEM; - return move_addr_to_kernel(conn->addr, conn->addr_len, &io->addr); + ret = move_addr_to_kernel(conn->addr, conn->addr_len, &io->addr); + if (ret) + io_netmsg_recycle(req, 0); + return ret; } int io_connect(struct io_kiocb *req, unsigned int issue_flags)
move_addr_to_kernel can fail, like if the user provides a bad sockaddr pointer. In this case where the failure happens on ->prep() we don't have a chance to clean the request later, so handle it here. Signed-off-by: Gabriel Krisman Bertazi <krisman@suse.de> --- io_uring/net.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)