io_uring/net: silence sparse warnings on address space

Commit Message

Jens Axboe June 20, 2023, 10:55 p.m. UTC

Rather than assign the user pointer to msghdr->msg_control, assign it
to msghdr->msg_control_user to make sparse happy. They are in a union
so the end result is the same, but let's avoid new sparse warnings and
squash this one.

Reported-by: kernel test robot <[e-mail]>
Closes: https://lore.kernel.org/oe-kbuild-all/[e-mail]/
Fixes: cac9e4418f4c ("io_uring/net: save msghdr->msg_control for retries")
Signed-off-by: Jens Axboe <[e-mail]>

---

Comments

Christoph Hellwig June 21, 2023, 4:17 a.m. UTC | #1

On Tue, Jun 20, 2023 at 04:55:05PM -0600, Jens Axboe wrote:
> Rather than assign the user pointer to msghdr->msg_control, assign it
> to msghdr->msg_control_user to make sparse happy. They are in a union
> so the end result is the same, but let's avoid new sparse warnings and
> squash this one.

Te patch looks good, but I think "silence sparse warning" is a horrible
way to write a commit message.  Yes, you're silencing sparse, but sparse
only complains because we have a type mismatch.

So the much better Subject would be something like: 

io_uring/net: use the correct msghdr union member in io_sendmsg_copy_hdr

Use msg_control_user to read the control message in io_sendmsg_copy_hdr
as we expect a user pointer, not the kernel pointer in msg_control.
The end result is the same, but this avoids a sparse addres space
warning.

With that:

Reviewed-by: Christoph Hellwig <[e-mail]>

(and it's really time we ger the __user and __bitwise annotations
checked by hte actual compiler..)

Jens Axboe June 21, 2023, 1:36 p.m. UTC | #2

On 6/20/23 10:17?PM, Christoph Hellwig wrote:
> On Tue, Jun 20, 2023 at 04:55:05PM -0600, Jens Axboe wrote:
>> Rather than assign the user pointer to msghdr->msg_control, assign it
>> to msghdr->msg_control_user to make sparse happy. They are in a union
>> so the end result is the same, but let's avoid new sparse warnings and
>> squash this one.
> 
> Te patch looks good, but I think "silence sparse warning" is a horrible
> way to write a commit message.  Yes, you're silencing sparse, but sparse
> only complains because we have a type mismatch.
> 
> So the much better Subject would be something like: 
> 
> io_uring/net: use the correct msghdr union member in io_sendmsg_copy_hdr
> 
> Use msg_control_user to read the control message in io_sendmsg_copy_hdr
> as we expect a user pointer, not the kernel pointer in msg_control.
> The end result is the same, but this avoids a sparse addres space
> warning.
> 
> With that:
> 
> Reviewed-by: Christoph Hellwig <[e-mail]>

You're totally right, that is a much better subject line. I've amended
the commit.

> (and it's really time we ger the __user and __bitwise annotations
> checked by hte actual compiler..)

That would indeed be nice... I know io_uring has some sparse complaints
on the __poll_t type that have been around forever, would be nice to get
those sorted and just in general ensure it's sparse clean. Then we could
start looking for new warnings at build time.

Patch

diff --git a/io_uring/net.c b/io_uring/net.c
index 2bc2cb2f4d6c..c8a4b2ac00f7 100644
--- a/io_uring/net.c
+++ b/io_uring/net.c
@@ -203,7 +203,7 @@  static int io_sendmsg_copy_hdr(struct io_kiocb *req,
 	ret = sendmsg_copy_msghdr(&iomsg->msg, sr->umsg, sr->msg_flags,
 					&iomsg->free_iov);
 	/* save msg_control as sys_sendmsg() overwrites it */
-	sr->msg_control = iomsg->msg.msg_control;
+	sr->msg_control = iomsg->msg.msg_control_user;
 	return ret;
 }
 
@@ -302,7 +302,7 @@  int io_sendmsg(struct io_kiocb *req, unsigned int issue_flags)
 
 	if (req_has_async_data(req)) {
 		kmsg = req->async_data;
-		kmsg->msg.msg_control = sr->msg_control;
+		kmsg->msg.msg_control_user = sr->msg_control;
 	} else {
 		ret = io_sendmsg_copy_hdr(req, &iomsg);
 		if (ret)