[REGRESSION] nvme: code command_id with a genctr for use-after-free validation crashes apple T2 SSD

[Orlando Chamberlain]

On 26/9/21 03:16, Keith Busch wrote:
> On Sat, Sep 25, 2021 at 01:10:42PM +0000, Orlando Chamberlain wrote:
>> Commit e7006de6c238 causes the SSD controller on Apple T2 computers to crash
>> and prevents linux from booting.
>>
>> This commit implemented a counter that is stored within the NVMe command_id,
>> however this counter makes the command_id higher than normal, causing a panic
>> on the T2 security chip that functions as the SSD controller, which then
>> causes the system to power off after a few seconds.
> 
> Ah, yet another spec non-complainat quirk from these controllers.

Apple does like to think different.

>> This is the entry in lspci -nn for the ssd:
>>
>> 04:00.0 Mass storage controller [0180]: Apple Inc. ANS2 NVMe Controller [106b:2005] (rev 01)
>>
>> This commit was included in 5.14.6 and backported to 5.10.67, but does not
>> occur in 5.14.5 and 5.10.66. I am on a MacBookPro16,1, the crash has been
>> reproduced on a MacBookPro16,2 as well.
> 
> Is the PCI VID:DID the same from in your lspci output for all affected
> macbooks?

Yes, they all have 106b:2005

>> I've tried to modify the genctr so that it is in the other side of the
>> command_id (which I thought might make the command_id's lower) with the patch
>> below, but it did not prevent the crash.
> 
> That might mean the h/w is using the command id as an index into
> internal structures. That is not spec compliant, so it sounds like
> we'll need to introduce another quirk for the macs.
> 

I've managed to get it to boot by commenting out the counter increment, which might work
as a quirk:

--- a/drivers/nvme/host/core.c

+++ b/drivers/nvme/host/core.c

@@ -1027,7 +1027,7 @@ blk_status_t nvme_setup_cmd(struct nvme_ns *ns, struct request *req)

                return BLK_STS_IOERR;

        }

 

-       nvme_req(req)->genctr++;

+       //nvme_req(req)->genctr++;

        cmd->common.command_id = nvme_cid(req);

        trace_nvme_setup_cmd(req, cmd);

        return ret;