| Message ID | 52763f964626ec61f78c66d4d757331d62311a5b.1661421007.git.asml.silence@gmail.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | io_uring/net: fix uninitialised addr | expand |
On 8/25/22 11:11, Pavel Begunkov wrote: > Don't forget to initialise and set addr in io_sendzc(), so if it goes > async we can copy it. Jens, can you amend it into the last commit? ("io_uring/net: save address for sendzc async execution") > Signed-off-by: Pavel Begunkov <asml.silence@gmail.com> > --- > io_uring/net.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/io_uring/net.c b/io_uring/net.c > index 4eaeb805e720..0af8a02df580 100644 > --- a/io_uring/net.c > +++ b/io_uring/net.c > @@ -975,7 +975,7 @@ static int io_sg_from_iter(struct sock *sk, struct sk_buff *skb, > > int io_sendzc(struct io_kiocb *req, unsigned int issue_flags) > { > - struct sockaddr_storage __address, *addr; > + struct sockaddr_storage __address, *addr = NULL; > struct io_ring_ctx *ctx = req->ctx; > struct io_sendzc *zc = io_kiocb_to_cmd(req, struct io_sendzc); > struct io_notif_slot *notif_slot; > @@ -1012,12 +1012,13 @@ int io_sendzc(struct io_kiocb *req, unsigned int issue_flags) > if (req_has_async_data(req)) { > struct io_async_msghdr *io = req->async_data; > > - msg.msg_name = &io->addr; > + msg.msg_name = addr = &io->addr; > } else { > ret = move_addr_to_kernel(zc->addr, zc->addr_len, &__address); > if (unlikely(ret < 0)) > return ret; > msg.msg_name = (struct sockaddr *)&__address; > + addr = &__address; > } > msg.msg_namelen = zc->addr_len; > }
On 8/25/22 4:13 AM, Pavel Begunkov wrote: > On 8/25/22 11:11, Pavel Begunkov wrote: >> Don't forget to initialise and set addr in io_sendzc(), so if it goes >> async we can copy it. > > Jens, can you amend it into the last commit? > ("io_uring/net: save address for sendzc async execution") Yes, I'll amend it. But do we have a test case that hits this path? Because it seems like that would've blown up immediately.
On 8/25/22 14:52, Jens Axboe wrote: > On 8/25/22 4:13 AM, Pavel Begunkov wrote: >> On 8/25/22 11:11, Pavel Begunkov wrote: >>> Don't forget to initialise and set addr in io_sendzc(), so if it goes >>> async we can copy it. >> >> Jens, can you amend it into the last commit? >> ("io_uring/net: save address for sendzc async execution") > > Yes, I'll amend it. But do we have a test case that hits this path? > Because it seems like that would've blown up immediately. Apparently a test I have only hits io_sendzc_prep_async() callback and the large buffer test doesn't trigger it. Hard to trigger it with udp and addresses don't make sense with tcp.
diff --git a/io_uring/net.c b/io_uring/net.c index 4eaeb805e720..0af8a02df580 100644 --- a/io_uring/net.c +++ b/io_uring/net.c @@ -975,7 +975,7 @@ static int io_sg_from_iter(struct sock *sk, struct sk_buff *skb, int io_sendzc(struct io_kiocb *req, unsigned int issue_flags) { - struct sockaddr_storage __address, *addr; + struct sockaddr_storage __address, *addr = NULL; struct io_ring_ctx *ctx = req->ctx; struct io_sendzc *zc = io_kiocb_to_cmd(req, struct io_sendzc); struct io_notif_slot *notif_slot; @@ -1012,12 +1012,13 @@ int io_sendzc(struct io_kiocb *req, unsigned int issue_flags) if (req_has_async_data(req)) { struct io_async_msghdr *io = req->async_data; - msg.msg_name = &io->addr; + msg.msg_name = addr = &io->addr; } else { ret = move_addr_to_kernel(zc->addr, zc->addr_len, &__address); if (unlikely(ret < 0)) return ret; msg.msg_name = (struct sockaddr *)&__address; + addr = &__address; } msg.msg_namelen = zc->addr_len; }
Don't forget to initialise and set addr in io_sendzc(), so if it goes async we can copy it. Signed-off-by: Pavel Begunkov <asml.silence@gmail.com> --- io_uring/net.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)