Bluescreen when sending a constant stream of URBs about usbip-win

[cezuni]

@tatokis : Your report may imply that removing urbr does not guarantee mutual exclusion and incur memory corruption. If list related fields of urbr are NULL, the memory has been already broken. So I greatly appreciate your patch but it seems to be not needed in normal case.

One of unpleasant things is fixed out queue length in userspace forward routine. 😢 Anyway, your application manage to work by increasing OUT_Q_LEN.
https://github.com/cezuni/usbip-win/blob/94b35a0916962874f062076176ec597a612aaa77/userspace/lib/usbip_forward.c#L251

[tatokis]

I will definitely try that, thank you. That said, it still shouldn't trigger a bluescreen, and it doesn't seem to happen without that commit.

That aside, this sounds more like a "double free" issue (RemoveEntryList getting called twice on the same pointer), rather than memory corruption.
I believe if there was corruption, the fields wouldn't be null, but some random values, and there would also possibly be corruption that wouldn't let me view the struct fields inside WinDbg.

[cezuni]

@tatokis : After inspecting a code, list_state NULL case was found. So list_state should be initialized when urbr is created. Could you try with the following diff?

diff --git a/driver/vhci/usbreq.c b/driver/vhci/usbreq.c
index db5bb0c..508f187 100644
--- a/driver/vhci/usbreq.c
+++ b/driver/vhci/usbreq.c
@@ -161,6 +161,7 @@ create_urbr(pusbip_vpdo_dev_t vpdo, PIRP irp, unsigned long seq_num_unlink)
        urbr->vpdo = vpdo;
        urbr->irp = irp;
        urbr->seq_num_unlink = seq_num_unlink;
+       InitializeListHead(&urbr->list_state);
        return urbr;
 }
 

[tatokis]

That fixes the issue, and the system no longer crashes on unlink. Thank you :)