>>> Konrad Rzeszutek Wilk <konrad.w...@oracle.com> 04/18/16 9:50 AM >>>
>On Sun, Apr 17, 2016 at 02:05:10AM -0600, Jan Beulich wrote:
>> >>> Konrad Rzeszutek Wilk <kon...@kernel.org> 04/15/16 4:29 AM >>>
>> >On Thu, Apr 14, 2016 at 10:36:46AM -0600, Jan Beulich wrote:
>> >> >>> Konrad Rzeszutek Wilk <konrad.w...@oracle.com> 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@lists.xen.org
http://lists.xen.org/xen-devel
