[Libusbx-devel] 32-bit libusbx broken on earlier 64-bit Linux kernels

classic Classic list List threaded Threaded
5 messages Options
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[Libusbx-devel] 32-bit libusbx broken on earlier 64-bit Linux kernels

Chris Dickens
Hi,

I recently had the need to compile a 32-bit version of the libusbx library (v1.0.17) on a 64-bit RHEL 6.5 system (Linux Kernel v2.6.32). What I found was that libusbx does not function properly when auto_detach_kernel_driver is set to be used. The error-handling code in detach_kernel_driver_and_claim() expects the ioctl to fail with ENOTTY when the ioctl is unsupported, and this indeed does happen from 64-bit userspace, but 32-bit userspace gets EINVAL.

Prior to the addition of the disconnect-claim feature for Linux, this failure would not have occurred.

I tested this on a 64-bit Fedora 19 installation and did not see the same issue, presumably because that kernel supports the ioctl and has the necessary translation from 32-bit userspace.

One additional note is that not using the auto_detach_kernel_driver feature is not sufficient to prevent failure. Even if a user does not use this feature, the code in op_reset_device() uses the detach_kernel_driver_and_claim() function to re-claim any claimed interfaces following the device reset.

Is it sufficient to check for EINVAL in the same manner as ENOTTY and fall back to the traditional claim_interface() call? Was there a reason that EINVAL was specifically handled in the error-handling code of detach_kernel_driver_and_claim(), and how might a user interpret a claim_interface() error of LIBUSB_ERROR_INVALID_PARAM?

Regards,
Chris

------------------------------------------------------------------------------
Rapidly troubleshoot problems before they affect your business. Most IT
organizations don't have a clear picture of how application performance
affects their revenue. With AppDynamics, you get 100% visibility into your
Java,.NET, & PHP application. Start your 15-day FREE TRIAL of AppDynamics Pro!
http://pubads.g.doubleclick.net/gampad/clk?id=84349351&iu=/4140/ostg.clktrk
_______________________________________________
libusbx-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/libusbx-devel
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [Libusbx-devel] 32-bit libusbx broken on earlier 64-bit Linux kernels

Hans de Goede
Hi Chris,

Thanks for the detailed bug report, and sorry for the slow reply.

On 12/01/2013 08:56 AM, Chris Dickens wrote:
> Hi,
>
> I recently had the need to compile a 32-bit version of the libusbx library
> (v1.0.17) on a 64-bit RHEL 6.5 system (Linux Kernel v2.6.32). What I found
> was that libusbx does not function properly when auto_detach_kernel_driver
> is set to be used. The error-handling code in
> detach_kernel_driver_and_claim() expects the ioctl to fail with ENOTTY when
> the ioctl is unsupported, and this indeed does happen from 64-bit
> userspace, but 32-bit userspace gets EINVAL.

Are you sure this is what was happening ? Can you perhaps try again with
a printf added at a strategic place to ensure this is really what is going
on?

Looking at:
https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/drivers/usb/core/devio.c?id=22763c5cf3690a681551162c15d34d935308c8d7

(22763c5c is the commit id responding to the v2.6.32 tag)

I see the following in there:

static int usbdev_ioctl(struct inode *inode, struct file *file,
                        unsigned int cmd, unsigned long arg)
{
        struct dev_state *ps = file->private_data;
        struct usb_device *dev = ps->dev;
        void __user *p = (void __user *)arg;
        int ret = -ENOTTY;
...
        switch (cmd) {
        case USBDEVFS_CONTROL:
...
        }
        return ret;
}

This code path is used for calls from both 64 bit and 32 bit userspace apps,
so 32 bit apps should see -ENOTTY to for an unknown cmd code.


>
> Prior to the addition of the disconnect-claim feature for Linux, this
> failure would not have occurred.
>
> I tested this on a 64-bit Fedora 19 installation and did not see the same
> issue, presumably because that kernel supports the ioctl and has the
> necessary translation from 32-bit userspace.
>
> One additional note is that not using the auto_detach_kernel_driver feature
> is not sufficient to prevent failure. Even if a user does not use this
> feature, the code in op_reset_device() uses the
> detach_kernel_driver_and_claim() function to re-claim any claimed
> interfaces following the device reset.

Right.

> Is it sufficient to check for EINVAL in the same manner as ENOTTY and fall
> back to the traditional claim_interface() call? Was there a reason that
> EINVAL was specifically handled in the error-handling code of
> detach_kernel_driver_and_claim(),

EINVAL will also be returned when the user passes in an invalid interface number
number. But we could still try the fallback then as the op_detach_kernel_driver
will also fail in case of an invalid interface number, and we would still
end up returning LIBUSB_ERROR_INVALID_PARAM. This means an extra kernel call,
but a cheap one and only if the user passes in garbage.

So yes handing EINVAL as ENOTTY should work, but first I would like to be really,
really sure that what you think you're seeing is really happening.

> and how might a user interpret a claim_interface() error of LIBUSB_ERROR_INVALID_PARAM?

As any other case where LIBUSB_ERROR_INVALID_PARAM is returned it usually means
that things are failing because the code calling libusb is passing in an
invalid parameter :)

Regards,

Hans

------------------------------------------------------------------------------
Learn Graph Databases - Download FREE O'Reilly Book
"Graph Databases" is the definitive new guide to graph databases and their
applications. Written by three acclaimed leaders in the field,
this first edition is now available. Download your free book today!
http://p.sf.net/sfu/13534_NeoTech
_______________________________________________
libusbx-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/libusbx-devel
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [Libusbx-devel] 32-bit libusbx broken on earlier 64-bit Linux kernels

Chris Dickens
Hi Hans,

Are you sure this is what was happening ? Can you perhaps try again with
a printf added at a strategic place to ensure this is really what is going
on?

Here's the debug output right after the DISCONNECT_CLAIM fails:

[ 0.004913] [00007d79] libusb: debug [libusb_claim_interface] interface 1
[ 0.004930] [00007d79] libusb: debug [detach_kernel_driver_and_claim] ioctl DISCONNECT_CLAIM error -1 errno 22

Looking at:
https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/drivers/usb/core/devio.c?id=22763c5cf3690a681551162c15d34d935308c8d7

(22763c5c is the commit id responding to the v2.6.32 tag)

I see the following in there:

static int usbdev_ioctl(struct inode *inode, struct file *file,
                        unsigned int cmd, unsigned long arg)
{
        struct dev_state *ps = file->private_data;
        struct usb_device *dev = ps->dev;
        void __user *p = (void __user *)arg;
        int ret = -ENOTTY;
...
        switch (cmd) {
        case USBDEVFS_CONTROL:
...
        }
        return ret;
}

This code path is used for calls from both 64 bit and 32 bit userspace apps,
so 32 bit apps should see -ENOTTY to for an unknown cmd code.

The code for RHEL's 2.6.32 kernel is a bit different, but essentially boils down to the same thing. (I was referencing http://ftp.redhat.com/redhat/linux/enterprise/6Workstation/en/os/SRPMS/kernel-2.6.32-431.5.1.el6.src.rpm

Doing a bit more research, I've found this is actually happening further up the call stack, before usbfs is ever involved. I noticed this in the kernel log:

ioctl32(pcs:32121): Unknown cmd fd(11) cmd(8108551b){t:'U';sz:264} arg(f59b4c50) on /dev/bus/usb/001/079

This is called from inside the compat_sys_ioctl function (https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/fs/compat_ioctl.c?id=22763c5cf3690a681551162c15d34d935308c8d7#n2857) and is called when the file operations table does not provide a compat_ioctl function and no handler is found. usbfs indeed does not provide this, thus after failing to find a handler it prints this error and as you can see right after returns EINVAL.

Regards,
Chris

------------------------------------------------------------------------------
Learn Graph Databases - Download FREE O'Reilly Book
"Graph Databases" is the definitive new guide to graph databases and their
applications. Written by three acclaimed leaders in the field,
this first edition is now available. Download your free book today!
http://p.sf.net/sfu/13534_NeoTech
_______________________________________________
libusbx-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/libusbx-devel
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [Libusbx-devel] 32-bit libusbx broken on earlier 64-bit Linux kernels

Hans de Goede
Hi Chris,

On 03/11/2014 05:27 PM, Chris Dickens wrote:

> Hi Hans,
>
>     Are you sure this is what was happening ? Can you perhaps try again with
>     a printf added at a strategic place to ensure this is really what is going
>     on?
>
>
> Here's the debug output right after the DISCONNECT_CLAIM fails:
>
> [ 0.004913] [00007d79] libusb: debug [libusb_claim_interface] interface 1
> [ 0.004930] [00007d79] libusb: debug [detach_kernel_driver_and_claim] ioctl DISCONNECT_CLAIM error -1 errno 22
>
>     Looking at:
>     https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/drivers/usb/core/devio.c?id=22763c5cf3690a681551162c15d34d935308c8d7
>
>     (22763c5c is the commit id responding to the v2.6.32 tag)
>
>     I see the following in there:
>
>     static int usbdev_ioctl(struct inode *inode, struct file *file,
>                              unsigned int cmd, unsigned long arg)
>     {
>              struct dev_state *ps = file->private_data;
>              struct usb_device *dev = ps->dev;
>              void __user *p = (void __user *)arg;
>              int ret = -ENOTTY;
>     ...
>              switch (cmd) {
>              case USBDEVFS_CONTROL:
>     ...
>              }
>              return ret;
>     }
>
>     This code path is used for calls from both 64 bit and 32 bit userspace apps,
>     so 32 bit apps should see -ENOTTY to for an unknown cmd code.
>
>
> The code for RHEL's 2.6.32 kernel is a bit different, but essentially boils down to the same thing. (I was referencing http://ftp.redhat.com/redhat/linux/enterprise/6Workstation/en/os/SRPMS/kernel-2.6.32-431.5.1.el6.src.rpm)
>
> Doing a bit more research, I've found this is actually happening further up the call stack, before usbfs is ever involved. I noticed this in the kernel log:
>
> ioctl32(pcs:32121): Unknown cmd fd(11) cmd(8108551b){t:'U';sz:264} arg(f59b4c50) on /dev/bus/usb/001/079
>
> This message is generating from https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/fs/compat_ioctl.c?id=22763c5cf3690a681551162c15d34d935308c8d7#n2769
> This is called from inside the compat_sys_ioctl function (https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/fs/compat_ioctl.c?id=22763c5cf3690a681551162c15d34d935308c8d7#n2857) and is called when the file operations table does not provide a compat_ioctl function and no handler is found. usbfs indeed does not provide this, thus after failing to find a handler it prints this error and as you can see right after returns EINVAL.
Ah yes I see now, and there is a big honking list of compatible ioctls (so which are compat without
needing translation from 32 <-> 64 bit) higher up in the file. Which of course does no contain
the new ioctls. So now we have a different return code when running in native 32 bit versus
when running 32 bits code under a 64 bit kernel how nice (not).

Ok, so treating EINVAL as ENOTTY is going to be the only solution then, can you try the
attached patch?

Thanks & Regards,

Hans

------------------------------------------------------------------------------
Learn Graph Databases - Download FREE O'Reilly Book
"Graph Databases" is the definitive new guide to graph databases and their
applications. Written by three acclaimed leaders in the field,
this first edition is now available. Download your free book today!
http://p.sf.net/sfu/13534_NeoTech
_______________________________________________
libusbx-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/libusbx-devel

0001-linux_usbfs-Treat-EINVAL-as-ENOTTY-in-cases-where-we.patch (1K) Download Attachment
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [Libusbx-devel] 32-bit libusbx broken on earlier 64-bit Linux kernels

Chris Dickens
Hi Hans,

Patch works as expected, thanks!

Regards,
Chris


On Tue, Mar 11, 2014 at 10:45 AM, Hans de Goede <[hidden email]> wrote:
Hi Chris,


On 03/11/2014 05:27 PM, Chris Dickens wrote:
Hi Hans,

    Are you sure this is what was happening ? Can you perhaps try again with
    a printf added at a strategic place to ensure this is really what is going
    on?


Here's the debug output right after the DISCONNECT_CLAIM fails:

[ 0.004913] [00007d79] libusb: debug [libusb_claim_interface] interface 1
[ 0.004930] [00007d79] libusb: debug [detach_kernel_driver_and_claim] ioctl DISCONNECT_CLAIM error -1 errno 22

    Looking at:
    https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/drivers/usb/core/devio.c?id=22763c5cf3690a681551162c15d34d935308c8d7

    (22763c5c is the commit id responding to the v2.6.32 tag)

    I see the following in there:

    static int usbdev_ioctl(struct inode *inode, struct file *file,
                             unsigned int cmd, unsigned long arg)
    {
             struct dev_state *ps = file->private_data;
             struct usb_device *dev = ps->dev;
             void __user *p = (void __user *)arg;
             int ret = -ENOTTY;
    ...
             switch (cmd) {
             case USBDEVFS_CONTROL:
    ...
             }
             return ret;
    }

    This code path is used for calls from both 64 bit and 32 bit userspace apps,
    so 32 bit apps should see -ENOTTY to for an unknown cmd code.


The code for RHEL's 2.6.32 kernel is a bit different, but essentially boils down to the same thing. (I was referencing http://ftp.redhat.com/redhat/linux/enterprise/6Workstation/en/os/SRPMS/kernel-2.6.32-431.5.1.el6.src.rpm)

Doing a bit more research, I've found this is actually happening further up the call stack, before usbfs is ever involved. I noticed this in the kernel log:

ioctl32(pcs:32121): Unknown cmd fd(11) cmd(8108551b){t:'U';sz:264} arg(f59b4c50) on /dev/bus/usb/001/079

This message is generating from https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/fs/compat_ioctl.c?id=22763c5cf3690a681551162c15d34d935308c8d7#n2769
This is called from inside the compat_sys_ioctl function (https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/fs/compat_ioctl.c?id=22763c5cf3690a681551162c15d34d935308c8d7#n2857) and is called when the file operations table does not provide a compat_ioctl function and no handler is found. usbfs indeed does not provide this, thus after failing to find a handler it prints this error and as you can see right after returns EINVAL.

Ah yes I see now, and there is a big honking list of compatible ioctls (so which are compat without
needing translation from 32 <-> 64 bit) higher up in the file. Which of course does no contain
the new ioctls. So now we have a different return code when running in native 32 bit versus
when running 32 bits code under a 64 bit kernel how nice (not).

Ok, so treating EINVAL as ENOTTY is going to be the only solution then, can you try the
attached patch?

Thanks & Regards,

Hans

------------------------------------------------------------------------------
Learn Graph Databases - Download FREE O'Reilly Book
"Graph Databases" is the definitive new guide to graph databases and their
applications. Written by three acclaimed leaders in the field,
this first edition is now available. Download your free book today!
http://p.sf.net/sfu/13534_NeoTech
_______________________________________________
libusbx-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/libusbx-devel



------------------------------------------------------------------------------
Learn Graph Databases - Download FREE O'Reilly Book
"Graph Databases" is the definitive new guide to graph databases and their
applications. Written by three acclaimed leaders in the field,
this first edition is now available. Download your free book today!
http://p.sf.net/sfu/13534_NeoTech
_______________________________________________
libusbx-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/libusbx-devel
Loading...