Re: [PATCH] scsi: storvsc: Fix validation for unsolicited incoming packets
From: Andrea Parri <parri.andrea@gmail.com>
Date: 2021-10-06 13:39:39
Also in:
linux-scsi, lkml
quoted
quoted
I know you have determined experimentally that Hyper-V sends unsolicited packets with the above length, so the idea is to validate that the guest actually gets packets at least that big. But I wonder if we should think about this slightly differently. The goal is for the storvsc driver to protect itself against bad or malicious messages from Hyper-V. For the unsolicited messages, the only field that this storvsc driver needs to access is the vstor_packet->operation field.Eh, this is one piece of information I was looking for... ;-)I'm just looking at the code in storvsc_on_receive(). storvsc_on_receive() itself looks at the "operation" field, but for the REMOVE_DEVICE and ENUMERATE_BUS operations, you can see that the rest of the vstor_packet is ignored and is not passed to any called functions.quoted
quoted
So an alternate approach is to set the minimum length as small as possible while ensuring that field is valid.The fact is, I'm not sure how to do it for unsolicited messages. Current code ensures/checks != COMPLETE_IO. Your comment above and code audit suggest that we should add a check != FCHBA_DATA. I saw ENUMERATE_BUS messages, code only using their "operation".I'm not completely sure about FCHBA_DATA. That message does not seem to be unsolicited, as the guest sends out a message of that type in storvsc_channel_init() using storvsc_execute_vstor_op(). So any received messages of that type are presumably in response to the guest request, and will get handled via the test for rqst_id == VMBUS_RQST_INIT. Long Li could probably confirm. So if Hyper-V did send a FCHBA_DATA packet with rqst_id of 0, it would seem to be appropriate to reject it.quoted
And, again, this is only based on current code/observations... So, maybe you mean something like this (on top of this patch)?Yes, with a comment to explain what's going on. :-)
My (current) best guess is here: https://lkml.kernel.org/r/20211006132026.4089-1-parri.andrea@gmail.com Thanks, Andrea