Re: [Xen-devel] [PATCH v4 07/33] xen: guestcopy: Provide an helper to safely copy string from guest

[Julien Grall <julien.grall@xxxxxxxxxx>]

On 31/03/15 14:49, Andrew Cooper wrote:
> On 31/03/15 14:30, Julien Grall wrote:
>>
>>> Furthermore, two size parameters serves no useful purpose.  The caller
>>> must always be in a position to decide a plausible upper bound.
>> I don't understand the problem to have two size parameters...
>>
>> The first one is the size given by the guest while the second one if the
>> upper bound.
>>
>> The maximum size may change from every caller. Hence the second size
>> parameter.
> 
> The caller shouldn't even be calling safe_copy_string_from_guest() with
> a guest-controlled-implausibly-large size.
> 
> The caller should be doing something like:
> 
> if ( usersize > PLAUSIBLE_UPPER_BOUND )
>   ... fail
> else
>   data = safe_copy_string_from_guest(hnd, usersize).
> 
> 
> Mixing plausibility checks and string copying in a single function is a
> antipattern, and IMO should not be moved into a common helper function
> like this.

Why it's an antipattern? It's exactly the same as checking the validity
of the buffer in copy_from_guest...

safe_copy-string_from_guest will fail if the size is too high.

Caller of this function may forget to do the check and introduce a
security issue. Having the check in safe_copy_string_from_guest avoid
this problem.

Regards,

-- 
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel