Re: [Xen-devel] [PATCH v8.1 04/27] xen/xsplice: Hypervisor implementation of XEN_XSPLICE_op

["Jan Beulich" <jbeulich@xxxxxxxx>]

>>> Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx> 04/18/16 9:50 AM >>>
>On Sun, Apr 17, 2016 at 02:05:10AM -0600, Jan Beulich wrote:
>> >>> Konrad Rzeszutek Wilk <konrad@xxxxxxxxxx> 04/15/16 4:29 AM >>>
>> >On Thu, Apr 14, 2016 at 10:36:46AM -0600, Jan Beulich wrote:
>> >> >>> Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx> 04/14/16 12:05 AM >>>
>> >> > +    spin_lock(&payload_lock);
>> >> > +
>> >> > +    found = find_payload(n);
>> >> > +    if ( IS_ERR(found) )
>> >> > +    {
>> >> > +        rc = PTR_ERR(found);
>> >> > +        goto out;
>> >> > +    }
>> >> > +    else if ( found )
>> >> > +    {
>> >> > +        rc = -EEXIST;
>> >> > +        goto out;
>> >> > +    }
>> >> > +
>> >> > +    data = xzalloc(struct payload);
>> >> 
>> >> I generally advocate for not doing allocations with locks held, and I 
>> >> don't think
>> >> it would severely complicate the code here doing so.
>> >
>> >I can certainly unlock and then lock again (when adding
>> >it to the list).
>> 
>> That would create a race again afaict. Instead what I have been trying to 
>> hint
>> at is that the allocation should be done before taking the lock, freeing the 
>> object
>> again if in the end it turned out it's not going to be needed. Hence the 
>> referral to
>
>What if I get -ENOMEM and that the user supplied an payload we already
>have? In that case I would return -ENOMEM while I would expect us to
>return -EEXIST.
>
>Unless I add some extra checks to continue on?

Or unless you didn't check the allocation result right after the allocation 
call,
but only where you check it now.

>Also one could do a bit of memory DoS (perhaps by mistake) by continously
>uploading the same and same payload and us first allocating the memory,
>and then doing the check for the payload existence (which would then
>free the memory). Since the allocation is outside the lock we can
>eat a bit of memory.

Why that? You'd free the memory right away on the error path.

Jan


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