[PATCH 1/1] drm/amdkfd: Add IPC API

[Dave Airlie]

On Tue, 14 Jul 2020 at 14:09, Felix Kuehling <felix.kuehling at amd.com> wrote:
>
> Am 2020-07-13 um 11:28 p.m. schrieb Dave Airlie:
> > On Tue, 14 Jul 2020 at 13:14, Felix Kuehling <Felix.Kuehling at amd.com> wrote:
> >> This allows exporting and importing buffers. The API generates handles
> >> that can be used with the HIP IPC API, i.e. big numbers rather than
> >> file descriptors.
> > First up why? I get the how.
>
> The "why" is compatibility with HIP code ported from CUDA. The
> equivalent CUDA IPC API works with handles that can be communicated
> through e.g. a pipe or shared memory. You can't do that with file
> descriptors.

Okay that sort of useful information should definitely be in the patch
description.

>
> https://docs.nvidia.com/cuda/cuda-runtime-api/group__CUDART__DEVICE.html#group__CUDART__DEVICE_1g8a37f7dfafaca652391d0758b3667539
>
> https://docs.nvidia.com/cuda/cuda-runtime-api/group__CUDART__DEVICE.html#group__CUDART__DEVICE_1g01050a29fefde385b1042081ada4cde9
>
> >
> >> + * @share_handle is a 128 bit random number generated with
> >> + * @get_random_bytes. This number should be very hard to guess.
> >> + * Knowledge of the @share_handle implies authorization to access
> >> + * the shared memory. User mode should treat it like a secret key.
> >> + * It can be used to import this BO in a different process context
> >> + * for IPC buffer sharing. The handle will be valid as long as the
> >> + * underlying BO exists. If the same BO is exported multiple times,
> > Do we have any examples of any APIs in the kernel that operate like
> > this? That don't at least layer some sort of file permissions  and
> > access control on top?
>
> SystemV shared memory APIs (shmget, shmat) work similarly. There are
> some permissions that can be specified by the exporter in shmget.
> However, the handles are just numbers and much easier to guess (they are
> 32-bit integers). The most restrictive permissions would allow only the
> exporting UID to attach to the shared memory segment.
>
> I think DRM flink works similarly as well, with a global name IDR used
> for looking up GEM objects using global object names.

flink is why I asked, because flink was a mistake and not one I'd care
to make again.
shm is horrible also, but at least has some permissions on what users
can attack it.

> > The reason fd's are good is that combined with unix sockets, you can't
> > sniff it, you can't ptrace a process and find it, you can't write it
> > out in a coredump and have someone access it later.
>
> Arguably ptrace and core dumps give you access to all the memory
> contents already. So you don't need the shared memory handle to access
> memory in that case.

core dumps might not dump this memory though, but yeah ptrace would
likely already mean you have access.

> > Maybe someone who knows security can ack merging this sort of uAPI
> > design, I'm not confident in what it's doing is in any ways a good
> > idea. I might have to ask some people to take a closer look.
>
> Please do. We have tried to make this API as secure as possible within
> the constraints of the user mode API we needed to implement.

I'll see if I hear back, but also if danvet has any input like I
suppose it's UUID based buffer access, so maybe 128-bit is enough and
you have enough entropy not to create anything insanely predictable.

Dave.