[DRAFT] nvme_driver: fix overflow computing queue size and don't wait for bad devices forever

[gurasinghMS]

When running the fuzzer locally I came across two bugs causing a panic and a timeout respectively.

• Computing the queue size for the I/O queues can cause an addition overflow related panic if the size of the queue is MAX_U16. To fix this, the code now uses saturating_add.
• A timeout related bug popped up when the driver tries resetting the underlying device due to failures. If the CFG bit is set, the driver never times out of the retry attempts, instead it loops forever waiting for the controller to report a NOT_READY state.

[chris-oo]

please add a description for the change.

[mattkur]

please add a description for the change.

I would also ask you to add a test here, but I know that you found these with changes in the nvme driver fuzzer you're working on. So, I think getting that fuzzer changes is a decent test and the best test ROI.

[jstarks]

The caller seems to rely on this actually resetting the device before it frees buffers. With this change, you might now return with the device still referencing these buffers. And that can lead to memory corruption.

[jstarks]

            // Hold onto responses until the reset completes so that waiting IOs do
            // not think the memory is unaliased by the device.

[jstarks]

The PR title is misleading--it implies that this is primarily a change to the fuzzer. But the meaningful changes are to the driver. It should be prefixed with "nvme_driver", not "fuzz_nvme_driver".

[mattkur]

The PR title is misleading--it implies that this is primarily a change to the fuzzer. But the meaningful changes are to the driver. It should be prefixed with "nvme_driver", not "fuzz_nvme_driver".

I was already changing it, but @gurasinghMS : I want to echo this feedback for future reference (see my suggested new title as an example)

[jstarks]

Are we worried about the timeout causing additional issues in a virtualized environment under load? What's the to value in practice for the devices we care about?

[mattkur]

Are we worried about the timeout causing additional issues in a virtualized environment under load? What's the to value in practice for the devices we care about?

Yeah, I am concerned about this. But mostly in the sense that we'd read some huge value and hang for a very long time. That reduces down to the current behavior if you shrink your observation time window (we currently hang forever...).

Are you concerned about a CAP.TO that's too low such that we time out before the device has actually had a chance to get ready?

[jstarks]

Are you concerned about a CAP.TO that's too low such that we time out before the device has actually had a chance to get ready?

Yes.

[gurasinghMS]

Pausing work on the PCIe interface fuzzing and marking this PR as a Draft for the time being. Will pick this work back up after working on the NVMe interface fuzzing.