Re: About consuming/releaseing ports in libpciaccess

[Sergey Bugaev]

On Sun, Nov 14, 2021 at 2:17 PM Joan Lledó <jlledom@mailfence.com> wrote:
>
> Hi,
>
> I'm trying to implement a correct release of resources in libpciaccess
> x86 module. I have this text from Sergey as reference

Hello! You're lucky to have caught me on my very last day :)

> Please take a look at map_dev_mem() implementation in my branch at [1].
>
> So, as I understood it, if the callee is supposed to take ownership of
> all given resources except the request port,

These rules are different between the server and client sides of MIG.
I was talking about the server side; but you are on the client side
here. On the client side, whether or not the call consumes a port
right is determined by the "disposition" specified in .defs (or
specified at runtime, for polymorphic types). Basically, if the
disposition is move-send, then the call does consume the right, and if
it's copy-send, then it doesn't. And copy-send is by far the most
common.

I would change the code like so:

@@ -4,12 +4,10 @@
 {
 #if defined(__GNU__)
     int err;
-    mach_port_t master_device;
-    mach_port_t devmem;
+    device_t master_device, devmem;
     mach_port_t pager;
     dev_mode_t mode = D_READ;
     vm_prot_t prot = VM_PROT_READ;
-    int pagesize;

     if (get_privileged_ports (NULL, &master_device)) {
         *dest = 0;
@@ -22,24 +20,24 @@
     }

     err = device_open (master_device, mode, "mem", &devmem);
+    mach_port_deallocate (mach_task_self (), master_device);
     if (err)
         return err;

-    pagesize = getpagesize();
-    if (mem_size % pagesize)
-        mem_size += pagesize - (mem_size % pagesize);
+    mem_size = (mem_size + vm_page_size - 1) % vm_page_size;

     err = device_map (devmem, prot, mem_offset, mem_size, &pager, 0);
+    mach_port_deallocate (mach_task_self (), devmem);
     if (err)
         return err;

-    err = vm_map (mach_task_self (), (vm_address_t *)dest, mem_size,
+    err = vm_map (mach_task_self (), (vm_address_t *) dest, mem_size,
                   (vm_address_t) 0, /* mask */
                   1, /* anywhere? */
                   pager, 0,
                   0, /* copy */
                   prot, VM_PROT_ALL, VM_INHERIT_SHARE);
-
+    mach_port_deallocate (mach_task_self (), pager);
     return err;
 #else
     int prot = PROT_READ;

device_t and vm_page_size changes are just cosmetics (do you even have
to round up the size explicitly?); the mach_port_deallocate () calls
are there to prevent leaks.

Sergey