On 2/15/21 10:23 PM, Sagi Grimberg wrote:
blk_mq_tag_to_rq() will always return a request if the command_id is
in the valid range. Check if the request has been started. If we
blindly process the request we might double complete a request which
can be fatal.
How did you get to this one? did the controller send a completion for
a completed/bogus request?
If that is the case, then that must mean it's possible the driver could
have started the command id just before the bogus completion check.
Data
iorruption, right?
Yes, which is why I don't think this check is very useful..
I actually view that as a valid protection against spoofed frames.
Without it it's easy to crash the machine by injecting fake
completions with random command ids.
And this doesn't help because the command can have been easily reused
and started... What is this protecting against? Note that none of the
other transports checks that, why should tcp?
Because it's particularly easy to spoof packets on tcp.
All other nvme-of transports are layered on top of other transports
which do some sanity checks already, so it becomes really hard to inject
invalid NVMe-oF frames for those.
NVMe-TCP has none of these protections, making it really easy to inject
faulty frames (or, heaven forbid, running a packet fuzzer).
And crashing the machine on invalid frames is always a bad idea; I would
have expected NVMe-TCP to drop them.
Cheers,
Hannes
--
Dr. Hannes Reinecke Kernel Storage Architect
h...@suse.de +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Felix Imendörffer
