JeffLIrion / adb_shell

A Python implementation of ADB with shell and FileSync functionality.
Apache License 2.0
530 stars 60 forks source link

Issue in connecting adb over usb #194

Closed cyy60rg closed 2 years ago

cyy60rg commented 2 years ago

Traceback (most recent call last): File "/home/**/main.py", line 16, in device.connect(rsa_keys=[signer], auth_timeout_s=10) File "/home/**//venv/lib/python3.8/site-packages/adb_shell/adb_device.py", line 649, in connect self._available, self._maxdata = self._io_manager.connect(self._banner, rsa_keys, auth_timeout_s, auth_callback, adb_info) File "/home/**//venv/lib/python3.8/site-packages/adb_shell/adb_device.py", line 195, in connect self._transport.connect(adb_info.transport_timeout_s) File "/home/**//venv/lib/python3.8/site-packages/adb_shell/transport/usb_transport.py", line 230, in connect assert read_endpoint is not None AssertionError

botherder commented 2 years ago

Ditto, we're getting reports of the same issue.

irezwi commented 2 years ago

I've also experienced this issue when running example code. Let me know if I can provide any debugging info.

maybe-sybr commented 2 years ago

This may be my bad. I've also hit issues with 0.4.1 in the last day (probably since getting v0.4.1) which manifest as either a segfault in libusb1_open with python-libusb1 < 2.0 and a pthread_mutex_lock assertion failing for python-libusb1 ~= 2.0.

Reverting e7d8b59 fixes the issue for me and that change does seem to smell bad in hindsight. I wonder if using the USB context as a context manager is calling some release method in its __exit__() which was previously not being called immediately (or ever?). Perhaps there is some circular reference/GC related stuff which was preventing the context from going away while transports holding USB devices derived from the context were alive.

@botherder (or your original reporter @sivom): Would you please try reverting e7d8b59 or pinning adb_shell 0.4.0 and let me know if that fixes your reported issues? My changes are the only ones relevant to the USB transports in 0.4.1, so I'd be pretty confident that e7d8b59 is to blame if using 0.4.0 fixes the issues. Also, thanks for your work on MVT, it's really cool :)

@JeffLIrion: I'm about to do a little experimentation with my own failures to see if I can find the smoking gun here. But in any case, since e7d8b59 was purely to silence a deprecation warning, I'd be happy for you to revert it and land a 0.4.1.post or 0.4.2 release. Let me know what you think.

maybe-sybr commented 2 years ago

Okay, so the issue pretty much stems from the fact that we aren't managing the lifecycle of the libusb1 context properly. Prior to e7d8b59 landing, we would create a new context object each time we tried to find ADB devices, and would then just kind of leak it and the GC would probably handle it for us eventually, or at least it'd be torn down on interpreter exit. This meant that the libusb1 C context wasn't closed and it was fine for code which called into adb_shell to continue using the devices (mostly?).

With that chance, I introduced an explicit lifecycle to the libusb1 context object which meant that once it got closed, doing pretty much anything with the devices would explode in nasty ways. This is probably actually a UAF condition, so IMO we'll need to yank 0.4.1 from pypi, revert e7d8b59, and push out a 0.4.2 to address the potential security issue.

As a pathway forward, there'll probably need to be some work done to manage the USB context lifecycle properly in adb_shell since leaking contexts and relying on the GC seems nasty. Even if we just have an idempotent usb1 context jammed into the USBTransport class, that'd probably be Good Enough :tm: .

maybe-sybr commented 2 years ago

I typoed your handle above @JeffLIrion , so just pinging again to make sure you get an email for this thread :)

JeffLIrion commented 2 years ago

Please let me know whether this is fixed in v0.4.2.

cyy60rg commented 2 years ago

Hi team,

The error is fixed I am struggling with another error.

Traceback (most recent call last): File "/home/../main.py", line 21, in device.connect(rsa_keys=[signer], auth_timeout_s=0.1) File "/home/../venv/lib/python3.8/site-packages/adb_shell/adb_device.py", line 649, in connect self._available, self._maxdata = self._io_manager.connect(self._banner, rsa_keys, auth_timeout_s, auth_callback, adb_info) File "/home/../venv/lib/python3.8/site-packages/adb_shell/adb_device.py", line 195, in connect self._transport.connect(adb_info.transport_timeout_s) File "/home/../venv/lib/python3.8/site-packages/adb_shell/transport/usb_transport.py", line 254, in connect self._transport.claimInterface(self._interface_number) File "/home/../venv/lib/python3.8/site-packages/usb1/init.py", line 1147, in claimInterface mayRaiseUSBError( File "/home/../venv/lib/python3.8/site-packages/usb1/init.py", line 128, in mayRaiseUSBError raiseUSBError(value) File "/home/../venv/lib/python3.8/site-packages/usb1/init.py", line 120, in raiseUSBError raise STATUS_TO_EXCEPTION_DICT.get(value, __USBError)(value) usb1.USBErrorBusy: LIBUSB_ERROR_BUSY [-6]

am I doing something wrong? I have used the same code present in the readme

JeffLIrion commented 2 years ago

@cyy60rg please open a new issue. I'm going to close this one.

daniellee630911 commented 1 year ago

Hi @cyy60rg Do you fixed this issue? I'm facing same issue now.

Thanks,

Daniel