Popax21 / synaTudor

GNU Lesser General Public License v2.1
89 stars 10 forks source link

libgusb 0.4.0 breakage #7

Closed idoybh closed 1 year ago

idoybh commented 1 year ago

Hello again, After updating from libgusb version 0.3.10 to 0.4.0 the driver crashes with: fprintd.log tudor.log coredump.log

Build fails as well with:

FAILED: libfprint-tod/libtudor_tod.so.p/src_open.c.o 
ccache cc -Ilibfprint-tod/libtudor_tod.so.p -Ilibfprint-tod -I../libfprint-tod -I../libtudor/inc -I../tudor-host/inc -I../tudor-host-launcher/inc -I/usr/include/libfprint-2/tod-1 -I/usr/include/gio-unix-2.0 -I/usr/include/glib-2.0 -I/usr/lib/glib-2.0/include -I/usr/include/sysprof-4 -I/usr/include/libmount -I/usr/include/blkid -I/usr/include/gusb-1 -I/usr/include/libusb-1.0 -I/usr/include/json-glib-1.0 -I/usr/include/pixman-1 -I/usr/include/nss -I/usr/include/nspr -I/usr/include/gudev-1.0 -I/usr/include/libfprint-2 -fdiagnostics-color=always -D_FILE_OFFSET_BITS=64 -Wall -Winvalid-pch -O0 -g -fPIC -pthread -D_GNU_SOURCE -Wno-missing-braces -MD -MQ libfprint-tod/libtudor_tod.so.p/src_open.c.o -MF libfprint-tod/libtudor_tod.so.p/src_open.c.o.d -o libfprint-tod/libtudor_tod.so.p/src_open.c.o -c ../libfprint-tod/src/open.c
In file included from /usr/include/glib-2.0/glib.h:86,
                 from /usr/include/glib-2.0/gobject/gbinding.h:28,
                 from /usr/include/glib-2.0/glib-object.h:22,
                 from /usr/include/libfprint-2/tod-1/fpi-compat.h:21,
                 from /usr/include/libfprint-2/tod-1/drivers_api.h:24,
                 from ../libfprint-tod/src/device.h:5,
                 from ../libfprint-tod/src/open.h:5,
                 from ../libfprint-tod/src/open.c:3:
../libfprint-tod/src/open.c: In function ‘open_device’:
../libfprint-tod/src/open.c:234:62: error: ‘GUsbDevice’ {aka ‘struct _GUsbDevice’} has no member named ‘priv’
  234 |         g_assert_no_errno(tdev->usb_fd = dup(((int**) usb_dev->priv)[3][10 + 2 + 4 + 2 + 1 + 1])); //Cursed offset magic
      |                                                              ^~
/usr/include/glib-2.0/glib/gtestutils.h:162:54: note: in definition of macro ‘g_assert_no_errno’
  162 |                                              __ret = expr; \
      |                                                      ^~~~

when downgrading back to 0.3.10 everything works properly again.

Thank you, again, for this driver 👍🏾

Etaash-mathamsetty commented 1 year ago

having this same issue I think I almost have a solution (which is less hacky than the existing code as well!) basically we have to access the GUsbDevice private data, then the libusb_device_handle, the the os_priv variable stored in that

idoybh commented 1 year ago

having this same issue I think I almost have a solution (which is less hacky than the existing code as well!) basically we have to access the GUsbDevice private data, then the libusb_device_handle, the the os_priv variable stored in that

oh that's nice. yea, I saw that private struct in the libgusb diff

Popax21 commented 1 year ago

Honestly it was just a matter of time until that piece of code broke, however it was the only way I found to obtain the device FD without introducing all sorts of other issues. Anyway, I'll very likely just adjust the offsets / field names tomorrow, unless @Etaash-mathamsetty's solution is a cleaner one (if I understood correctly, it's the exact same thing, as my offsets basically go through the exact chain of references / fields they describe)

Etaash-mathamsetty commented 1 year ago

solution goes something like this:

GUsbDevicePrivate* usb_dev_private = (GUsbDevicePrivate*)(usb_dev + g_type_class_get_instance_private_offset(usb_dev->parent_instance.g_type_instance.g_class));
tdev->usb_fd = usb_dev_private->handle->os_priv;
Popax21 commented 1 year ago

Oh cool, I didn't know that that function existed. I'll definetly use it, thanks! Also, aren't libusb's private structures only defined in internal headers, and as such inaccesible to us?

Etaash-mathamsetty commented 1 year ago

Oh cool, I didn't know that that function existed. I'll definetly use it, thanks! Also, aren't libusb's private structures only defined in internal headers, and as such inaccesible to us?

yeah that's true, that's what I am trying to fix, I re defined them in the code to see if it would work not working atm you can get more code here: https://android.googlesource.com/platform/external/libusb/+/refs/heads/o-preview-2/libusb/os/linux_usbfs.c

Etaash-mathamsetty commented 1 year ago

I ended up with this code, we just have to find out what magic_number is:

GUsbDevicePrivate* usb_dev_private = (GUsbDevicePrivate*)(usb_dev + g_type_class_get_instance_private_offset(usb_dev->parent_instance.g_type_instance.g_class));
tdev->usb_fd = ((struct linux_device_handle_priv *)(usb_dev_private->handle + magic_number))->fd;
Etaash-mathamsetty commented 1 year ago

any updates on this?

Popax21 commented 1 year ago

I should have pushed a commit (cb0db2e65afe7e0f525133e0fd6e771eaca73555) adding compatibility just now, but I don't have a device with me right now to test if it works

Etaash-mathamsetty commented 1 year ago

it seems to be working

Popax21 commented 1 year ago

Oh good, if @idoybh can confirm this as well I'll close this issue now.

idoybh commented 1 year ago

Oh good, if @idoybh can confirm this as well I'll close this issue now.

Shouldn't the check for version check if the version is greater or equal to 0.4.0 rather than checking if it's exactly 0.4.0? Or is that function already doing that? I can confirm new HEAD works just fine at runtime and build.

Popax21 commented 1 year ago

Oh good, if @idoybh can confirm this as well I'll close this issue now.

Shouldn't the check for version check if the version is greater or equal to 0.4.0 rather than checking if it's exactly 0.4.0? Or is that function already doing that? I can confirm new HEAD works just fine at runtime and build.

The macro should only check the major version number for an exact match, the minor version can be larger. Also, am gonna close this now, thanks for reporting!

idoybh commented 1 year ago

Thank you @Popax21.

So we'll have to stay alert for 0.5 haha

Etaash-mathamsetty commented 1 year ago

Thank you @Popax21.

So we'll have to stay alert for 0.5 haha

this is a more permanent solution, unless they change something about the private data in 0.5 then it should still work