<html><head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8">
</head>
<body>
Am 05.05.22 um 10:10 schrieb Jason Ekstrand:<br>
<blockquote type="cite" cite="mid:CAOFGe948_qtwrLd1DiHOJOkxK-iT_qU-3FG+uz4bGGnL7oKpCw@mail.gmail.com">
<div dir="ltr">
<div class="gmail_quote">
<div dir="ltr" class="gmail_attr">On Thu, May 5, 2022 at 1:25
AM Christian König <<a href="mailto:christian.koenig@amd.com" moz-do-not-send="true" class="moz-txt-link-freetext">christian.koenig@amd.com</a>>
wrote:<br>
</div>
<blockquote class="gmail_quote" style="margin:0px 0px 0px
0.8ex;border-left:1px solid
rgb(204,204,204);padding-left:1ex">[SNIP]<br>
> + fd_install(fd, sync_file->file);<br>
> +<br>
> + arg.fd = fd;<br>
> + if (copy_to_user(user_data, &arg,
sizeof(arg)))<br>
> + return -EFAULT;<br>
<br>
I know we had that discussion before, but I'm not 100% any
more what the <br>
outcome was.<br>
<br>
The problem here is that when the copy_to_user fails we have
a file <br>
descriptor which is valid, but userspace doesn't know
anything about it.<br>
<br>
I only see a few possibilities here:<br>
1. Keep it like this and just assume that a process which
you can't copy <br>
the fd to is also dying (a bit to much assumption for my
taste).<br>
<br>
2. Close the file descriptor when this happens (not ideal
either).<br>
<br>
3. Instead of returning the fd in the parameter structure
return it as <br>
IOCTL result.<br>
<br>
Number 3 is what drm_prime_handle_to_fd_ioctl() is doing as
well and <br>
IIRC we said that this is probably the best option.<br>
</blockquote>
<div><br>
</div>
<div>I don't have a strong preference here, so I'll go with
whatever in the end but let me at least explain my
reasoning. First, this was based on the FD import/export in
syncobj which stuffs the FD in the args struct. If
`copy_to_user` is a problem here, it's a problem there as
well. Second, the only way `copy_to_user` can fail is if
the client gives us a read-only page or somehow manages to
race removing the page from their address space (via
unmap(), for instance) with this ioctl. Both of those seem
like pretty serious client errors to me. That, or the
client is in the process of dying, in which case we really
don't care.</div>
</div>
</div>
</blockquote>
<br>
Yeah, I know about that copy_to_user() issue in the syncobj and also
some driver specific handling.<br>
<br>
That's why we discussed this before and IIRC somebody indeed ran
into an issue with -EFAULT and that was the reason all this bubbled
up.<br>
<br>
I don't have a strong preference either, but I think we should try
to learn from previous mistakes and design new interfaces based on
such experience.<br>
<br>
Christian.<br>
<br>
<blockquote type="cite" cite="mid:CAOFGe948_qtwrLd1DiHOJOkxK-iT_qU-3FG+uz4bGGnL7oKpCw@mail.gmail.com">
<div dir="ltr">
<div class="gmail_quote">
<div><br>
</div>
<div>--Jason</div>
<br>
</div>
</div>
</blockquote>
<br>
</body>
</html>