Re: [PATCH] Avoid passing libusb functions as callbacks

Javier Celaya <javier.celaya@xxxxxxxxxxx> · Tue, 21 Aug 2018 14:53:59 +0200

Hi,

El mar, 21-08-2018 a las 09:29 +0200, Victor Toso escribió:
Hi,

On Tue, Aug 21, 2018 at 03:18:35AM -0400, Frediano Ziglio wrote:

Hi,
On Fri, 2018-08-17 at 15:24 +0200, Victor Toso wrote:
Hi,

On Fri, Aug 17, 2018 at 03:12:35PM +0200, jorge.olmos@xxxxxxxxxxx
wrote:
From: Jorge Olmos <jorge.olmos@xxxxxxxxxxx>

When building spice-gtk for windows:
- libusb uses __stdcall calling convention when compiled for win32.
It does
not include an option to be compiled with __cdecl calling
convention.
Directly calling libusb functions works fine. But it is a problem
when its
functions are passed as callbacks to a function that expects other
calling
convention.
- glib uses __cdecl calling convention and expects the functions it
receives as parameters to follow __cdecl convention.

So the lines included in spice-gtk like:
     g_clear_pointer(&priv->device, libusb_unref_device);
cause libusb_unref_device (compiled with _stdcall convention) to be
called
with __cdecl convention. This causes stack corruption, and hence
crashes.

Have you raised a bug in glib? We use this libraries to help with
portability so I'd hope it is possible to fix in glib.


I don't think it is even possible to fix this cleanly in glib. A
compilation of glib can work well with __stdcall OR with __cdecl. But
not with both of them.


It's possible, the function is called just once in the macro so won't
need all the hackish casts that are currently present. I have a fixed
version of the macro (somewhere!).

Ah, that would be cool.


Right now I have just found another patch in spice-gtk, where the same
problem was solved with the same solution, which also has no
portability issues:

https://patchwork.freedesktop.org/patch/92705/


Yes, this problem can cause a stack corruption (as you confirmed) and
should be merged GLib or not.

Yes, not against merging the fix. But if we can get it fixed in
Glib, it is matter to help other projects that port code to
windows to be fixed as well, etc.

Please open a bug in Glib and add a reference to it in the code

Are you talking to Jorge or to Frediano (since he has a fixed macro)? Jorge is on holidays anyway, he will return in a couple of weeks. I can open the bug report in Glib, but if you already have a working version of the macro...

(to avoid people reverting this change while the bug is not fixed
there)

    https://gitlab.gnome.org/GNOME/glib/issues

Cheers,
Victor

---
 src/channel-usbredir.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/src/channel-usbredir.c b/src/channel-usbredir.c
index 6ffe546..1d9c380 100644
--- a/src/channel-usbredir.c
+++ b/src/channel-usbredir.c
@@ -352,7 +352,8 @@ static void spice_usbredir_channel_open_acl_cb(
         spice_usbredir_channel_open_device(channel, &err);
     }
     if (err) {
-        g_clear_pointer(&priv->device, libusb_unref_device);
+        libusb_unref_device(priv->device);
+        priv->device = NULL;
         g_boxed_free(spice_usb_device_get_type(), priv-
spice_device);
         priv->spice_device = NULL;
         priv->state  = STATE_DISCONNECTED;
@@ -383,7 +384,8 @@ _open_device_async_cb(GTask *task,
     spice_usbredir_channel_lock(channel);
 
     if (!spice_usbredir_channel_open_device(channel, &err)) {
-        g_clear_pointer(&priv->device, libusb_unref_device);
+        libusb_unref_device(priv->device);
+        priv->device = NULL;
         g_boxed_free(spice_usb_device_get_type(), priv-
spice_device);
         priv->spice_device = NULL;
     }
@@ -504,7 +506,8 @@ void
spice_usbredir_channel_disconnect_device(SpiceUsbredirChannel
*channel)
 
         /* This also closes the libusb handle we passed from
open_device */
         usbredirhost_set_device(priv->host, NULL);
-        g_clear_pointer(&priv->device, libusb_unref_device);
+        libusb_unref_device(priv->device);
+        priv->device = NULL;
         g_boxed_free(spice_usb_device_get_type(), priv-
spice_device);
         priv->spice_device = NULL;
         priv->state  = STATE_DISCONNECTED;

Frediano
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel

-- 




 





              
              
          
          
            Javier Celaya Alastrué
          
           Chief Technology Officer
      
      
        
           
        javier.celaya@xxxxxxxxxxx
        
        
          
        +34696969959 
      
          
        j_celaya
        
        
          
        Legal Information and Privacy Policy
          
      
    
    
        
            Política de confidencialidad
            Este mensaje y los ficheros anexos son confidenciales dirigiéndose exclusivamente al destinatario mencionado en el encabezamiento. Si usted ha recibido este correo por error, tenga la amabilidad de eliminarlo de su sistema y no divulgar el contenido a terceros. Los datos personales facilitados por usted o por terceros serán tratados por FLEXIBLE SOFTWARE SOLUTIONS S.L.U. con la finalidad de gestionar y mantener los contactos y relaciones que se produzcan como consecuencia de la relación que mantiene con FLEXIBLE SOFTWARE SOLUTIONS S.L.U. Normalmente, la base jurídica que legitima este tratamiento, será su consentimiento, el interés legítimo o la necesidad para gestionar una relación contractual o similar. El plazo de conservación de sus datos vendrá determinado por la relación que mantiene con nosotros. Para más información al respecto, o para ejercer sus derechos de acceso, rectificación, supresión, oposición, limitación o portabilidad, dirija una comunicación por escrito a FLEXIBLE SOFTWARE SOLUTIONS S.L.U: Avenida de Ranillas 1D, Planta 3, Oficina 3G, Zaragoza o al correo electrónico pdo@xxxxxxxxxxx. En caso de considerar vulnerado su derecho a la protección de datos personales, podrá interponer una reclamación ante la Agencia Española de Protección de Datos (www.agpd.es). 
        
    
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel




