[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

[Hans de Goede]

Hi Chris,

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

Post by 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.

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.

Post by Chris Dickens
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.

Post by Chris Dickens
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.

Post by Chris Dickens
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

[Chris Dickens]

Hi Hans,

Are you sure this is what was happening ? Can you perhaps try again with

Post by Hans de Goede
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

Post by Hans de Goede
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)
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) {
...
}
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.

Regards,
Chris

[Hans de Goede]

Hi Chris,

Post by 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?
[ 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
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)
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) {
...
}
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)
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

[Chris Dickens]

Hi Hans,

Patch works as expected, thanks!

Regards,
Chris

Post by Hans de Goede
Hi Chris,

Post by 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?
[ 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
https://git.kernel.org/cgit/linux/kernel/git/torvalds/
linux.git/tree/drivers/usb/core/devio.c?id=22763c5cf3690a681551162c15d34d
935308c8d7
(22763c5c is the commit id responding to the v2.6.32 tag)
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) {
...
}
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
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=22763c5cf3690a681551162c15d34d
935308c8d7#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
https://lists.sourceforge.net/lists/listinfo/libusbx-devel