RE: [PATCH] hv: hyperv.h: Replace one-element array with flexible-array member

[Michael Kelley (LINUX)]

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