From: Michael Kelley (LINUX) <mikelley@xxxxxxxxxxxxx> Sent: Monday, August 14, 2023 1:10 PM > From: Saurabh Singh Sengar <ssengar@xxxxxxxxxxxxx> Sent: Tuesday, August 8, 2023 2:49 AM > > [snip] > > > > Thanks for your comment, I wanted to have this discussion. > > > > Before sending this patch, I was contemplating whether or not to make this change. > > Through my analysis, I arrived at the conclusion that the initial validation code > > wasn't entirely accurate. And with the proposed changes it gets more accurate. > > IMHO it is more accurate to exclude the size of 'ranges' in the header. > > > > With my limited understanding of this driver, the current "header size validation" > > is only to make sure that header is correct. So, that we fetch the range_cnt and > > xfer_pageset_id correctly from it. For this to be done I don't find any reason > > to include the size of ranges in this check. With inclusion of ranges we are > > checking the first 'struct vmtransfer_page_range' size as well which is not required > > for fetching above two values. > > > > Once we have the count of ranges we will anyway check the sanity of ranges with > > NETVSC_XFER_HEADER_SIZE. This will check "count * (struct vmtransfer_page_range)" > > Which is present few lines after. > > > > For a ranges count = 1, I don't see there is any difference between both the checks as > > of today. > > > > Please let me know you opinion if you don't find my explanation reasonable. > > > > I don't see any other place this structure's size change will affect. > > > > Got it. I have now carefully looked at the netvsc_receive() code myself, > and I agree with you. With the 1 element array, the validation in > netvsc_receive() could have generated a false positive if the range_cnt > is zero. But I don't think a zero range_cnt ever happens, so the > false positive never happens. With the change to use a flexible array, > the validation is now correct even with a range_cnt of zero. > > Please add a note to the commit message for this patch: The validation > code in the netvsc driver is affected by changing the struct size, but the > effects have been examined and have been determined to be appropriate. > One other thought: Could this change affect user space DPDK code that is processing netvsc packets? Michael
