Analyzing a Use-After-Free Panic in the USB Gadget Path

Good morning, everyone, and thank you for joining on a Saturday. I'm Tom from SQE. This is a smaller group than usual, because the issue is very specific: a kernel panic in the USB path on Orion. We have Kernel and BSP, Google's Android kernel team, and Qorvix's USB team. Let's do quick introductions, starting with our guests.

Hi, I'm Nathan Brooks from the Android kernel team at Google, joining from Seattle. I work on the common kernel branches, including the USB subsystem. Kevin from partner engineering asked me to join because the crash might be in common code.

Ken Tsai from Qorvix in Taipei. I maintain the USB controller driver for the Orion chipset.

Jiwoo Lee, Kernel and BSP. After the storage panic, I seem to have become the go-to person for use-after-free bugs.

Hyunsoo Bae, also Kernel and BSP. I run our KASAN builds, so I'll walk through the report.

Here's the symptom. If a user is using USB tethering or transferring files to a computer, and they unplug the cable in the middle of a transfer, the phone occasionally panics and reboots. In the field, the rate is low, about two crashes per ten thousand unplugs, but because it happens to a lot of users every day, it's now the third most common kernel panic signature on Orion.

To reproduce it in the lab, we built a simple rig with a relay that connects and disconnects the USB cable every few seconds while a large file transfer runs. With twenty devices running overnight, we get a crash roughly every two hours. That's slow, but it's enough to test hypotheses.

That's a very useful rig. Does it reproduce with tethering only, file transfer only, or does it need both?

Either one is enough, but it's about three times more frequent when both are active at the same time. That's what made us suspect something shared between the two USB functions, rather than a bug in one of them.

The production dumps showed a crash in the completion callback of a USB request, with a pointer that looked like garbage. That's a classic sign of memory corruption, but it doesn't tell you who corrupted it. So we flashed the rig devices with a KASAN build. For anyone who hasn't used it: KASAN is the kernel address sanitizer. It tracks every allocation and every free, and when code touches memory that has already been freed, it stops immediately and prints a report.

The KASAN report is very clear. It shows three stack traces. The first one is the bad access: the USB controller driver's interrupt handler calls the completion callback for a request. The second one shows where that request was allocated: by the tethering function when the cable was connected. And the third one shows where it was freed: by the same tethering function, in its disable path, which runs when the cable is unplugged.

So the sequence is: the cable is unplugged, the tethering function frees its request, and then, a moment later, the controller driver completes the same request and calls back into memory that no longer exists. It's very similar in shape to the storage bug: a completion arriving after the owner has already cleaned up.

Then the question is whose responsibility it is to prevent that. Should the function wait for outstanding requests before freeing them, or should the controller driver make sure it never completes a request after the function is disabled?

According to the gadget framework's contract, it's a shared responsibility, but the order is clearly defined. When a function is disabled, it must first dequeue all its outstanding requests from the controller. Dequeue is supposed to be synchronous: when it returns, the controller has either completed the request with a cancellation status or guaranteed it never will. Only after that can the function free the memory.

And our controller driver does implement dequeue that way, synchronously. So if the function calls dequeue before freeing, this shouldn't happen from our side.

That's where it gets interesting. The tethering function does call dequeue, but only for requests on the receive endpoint. For the transmit endpoint, it relies on a disconnect callback from the controller to clean up. And the timing of that callback relative to disable is where the race happens.

So is this a bug in the common tethering function, which is Google's code, or in how Qorvix's driver delivers the disconnect callback?

I looked at this before the call, and I believe it's in the common code. The transmit path in that function makes an assumption that held true for older controller drivers, but isn't guaranteed by the framework. In fact, a fix was merged into the upstream long-term stable branch for 6.1 about three weeks ago. Another partner reported a similar crash on a different chipset.

Three weeks ago. That explains why we don't have it. Our Orion kernel is based on the 6.1 stable release from three months ago. We merge stable updates quarterly, and the next merge isn't scheduled until October.

I'm relieved, honestly. But to be thorough, I'll still review our controller driver's disconnect path. If our timing makes the race more likely than on other chipsets, I'd like to know.

That's a good idea. On the other partner's chipset, the crash rate was about ten times lower than what you're describing. So your controller may be completing transmit requests more quickly after disconnect, which widens the window.

Before we decide anything, I want proof that the upstream fix actually solves our problem. How quickly can we test it?

The patch is small, about fifteen lines, and it applies cleanly to our tree. I can have a KASAN build with the fix on the rig this afternoon. If we run twenty devices for forty-eight hours, which would normally produce about twenty-four crashes, and we see zero KASAN reports, I'd consider that strong evidence.

I'd also run a non-KASAN build in parallel. KASAN changes timing, because every memory access is slower. Occasionally, a race disappears under KASAN simply because everything is slower, not because it's fixed. Testing both builds protects us from fooling ourselves.

Good point. I'll split the rig: ten devices with KASAN plus the fix, ten with the production configuration plus the fix. And I'll keep a few on the old build as a control, just like we did for storage.

While we wait for the test results, let's think about the users who are hitting this today. Even if the fix goes into the September build, it won't reach most devices until mid-September. Is there anything we can do in the meantime?

Unfortunately, there's no configuration switch that avoids the race. The tethering function is either enabled or not, and disabling USB tethering entirely would be far worse than an occasional crash. The only real fix is the code change.

For context on severity, the panic doesn't cause any data loss, because file transfers over USB are written by the computer, not the phone, and an interrupted transfer simply fails. The user loses about a minute while the phone reboots. It's annoying, but it's not dangerous. So I don't think it justifies an emergency release outside the normal monthly schedule.

I agree with that assessment. Field Quality will still want to know the expected date, though, so support can give customers a real answer instead of 'we're looking into it'.

Is Orion Lite affected as well? It uses a different controller from us, an older generation, but the same tethering function from the common kernel.

Good question. The signature appears on Orion Lite too, but about fifteen times less often. That's consistent with what Nathan said about controller timing. So the same cherry-pick should go into the Orion Lite September release as well, even though it's much less urgent there.

And for Atlas and Atlas Lite, on kernel 6.12, the fix is already included, because the 6.12 branch received it before the 6.1 backport. So your new devices start clean.

That's good to hear. One less thing to worry about on Atlas.

Assuming the fix works, how do we ship it? Do we cherry-pick this one patch, or pull in the whole stable update?

For the September build, I'd cherry-pick. The content cutoff is August 25th, three days from now. The latest stable update contains about four hundred patches since our last merge. Even though stable patches are generally safe, taking four hundred of them three days before a cutoff, with a critical security fix and a camera fix already in the same build, is too much change at once.

I understand that for the short term. But I'd push back a little on the longer-term strategy. Merging stable updates only once a quarter means you're always up to three months behind on fixes like this one, including security fixes that don't get a separate bulletin entry. The partners with the fewest field crashes tend to merge stable updates monthly.

Monthly merges sound good in theory, but each merge needs a full kernel regression cycle. With our current test capacity, monthly would mean SQE spends a quarter of its kernel time just validating stable merges.

That's true today, but the kernel-focused regression suite we discussed at the GKI session would change that. If it runs automatically in twelve hours, monthly merges become much cheaper. I'd suggest we take this up properly at the vendor sync on the kernel stable strategy next week, rather than decide it here.

Agreed. For now: cherry-pick for September, and a proposal for a faster stable cadence at the meeting on the thirtieth.

One question for Nathan. If our controller completes requests faster than others, is that something we should change? I don't want to slow down our driver just to hide a race in a function driver.

No, please don't. Completing quickly is correct behavior. With the fix, the function no longer depends on the timing, so faster is fine. If anything, your controller did everyone a favor by exposing the bug more often.

This makes me think we should run KASAN builds on the stress rigs more regularly, not only when we're investigating something. If we'd had a KASAN build on a USB rig months ago, we'd have found this before it reached the field.

I'd support that. Maybe ten percent of the stress lab always runs KASAN builds, rotating across subsystems. It's slower, so it finds fewer timing bugs, but it finds memory bugs with a precise report, which saves days of analysis.

If it helps, our team publishes a list of the most frequent crash signatures we see across partners each month. If you match your field crashes against that list, you'll sometimes find a fix already exists upstream, like today.

That would have saved us a week here. Could you add our team to that distribution list?

Of course. I'll ask Kevin to add you on Monday.

Let me summarize. Hyunsoo: KASAN and production builds with the upstream fix on the rig this afternoon. Tom, that's me: split the rig, keep a control group, and report on Monday morning. Jiwoo: prepare the cherry-pick for the September build, assuming the results are clean, and draft the stable cadence proposal for the thirtieth. Ken: review the controller's disconnect path and share the timing data with Nathan. Nathan: add us to the monthly crash signature list. Hyunsoo and I will also propose the rotating KASAN group in the stress lab.

I'll have the timing analysis by Wednesday. I'll measure how long our controller takes to complete outstanding transmit requests after a disconnect, compared with the reference timing in the specification, so we can see exactly how much faster we are than other chipsets.

The builds will be on the rig by 3 p.m. I'll also send everyone the full KASAN report from this morning, with the three stack traces annotated, because it's a good example for anyone who hasn't read one before.

And I'll write the cherry-pick so it matches the upstream commit exactly, with the original commit reference in the message. That way, when we do the next stable merge, the tools will recognize it as already applied, and we won't get a conflict.

Thank you, everyone, especially Nathan and Ken for joining on a weekend. Our next call is Monday, August 24th, at 9 a.m. Korea time, which is Sunday at 5 p.m. in Seattle. If the overnight results are clean, it should be a short call.

Works for me. Thanks for the clear report; it made this much easier. Honestly, the reproduction rig and the KASAN report together are exactly what we hope to receive from partners. Most crash reports we get are a single stack trace and a sentence, and those take ten times longer to resolve.

That's good to hear. I'll share the rig design with our other labs, so they can build the same thing for other connectors, like the charging port and the headphone adapter.

Thanks, everyone. It's a nice change to have a bug that someone else already fixed. Let's make sure the next one gets found by matching against Nathan's list before we spend a week on it.