Re: [Xen-devel] [PATCH v5 6/8] xen/arm: introduce GNTTABOP_cache_flush

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 16.10.14 at 16:19, <stefano.stabellini@xxxxxxxxxxxxx> wrote:
> On Thu, 16 Oct 2014, Jan Beulich wrote:
>> >>> On 16.10.14 at 12:55, <stefano.stabellini@xxxxxxxxxxxxx> wrote:
>> > On Tue, 14 Oct 2014, Jan Beulich wrote:
>> >> >>> On 13.10.14 at 16:58, <stefano.stabellini@xxxxxxxxxxxxx> wrote:
>> >> > +static bool_t grant_map_exists(const struct domain *ld,
>> >> > +                        struct grant_table *rgt,
>> >> > +                        unsigned long mfn)
>> >> > +{
>> >> > +    const struct active_grant_entry *act;
>> >> > +    grant_ref_t ref;
>> >> > +
>> >> > +    ASSERT(spin_is_locked(&rgt->lock));
>> >> > +
>> >> > +    for ( ref = 0; ref != nr_grant_entries(rgt); ref++ )
>> >> 
>> >> Unless I overlooked something here or in an earlier patch I think
>> >> you're _still_ not enforcing a suitably low upper limit on the
>> >> variable that controls the maximum iteration count here. Whether
>> >> you enforce this the hard way (by just capping an eventual
>> >> command line specified value) or the soft way (by issuing a
>> >> warning that such a setting may hamper security, plus
>> >> documenting the supported value range) is up to you.
>> > 
>> > What value range do you suggest?
>> 
>> Whatever is suitable to limit the loop to no more than a few thousand
>> iterations.
> 
> To limit the number of iterations to 5000, we need 10 grant frames max.
> I'll use that as limit, that also means changing
> DEFAULT_MAX_NR_GRANT_FRAMES to 10.

This may be fine (provided that constant doesn't affect Dom0's
maptrack size anymore), but I'm not really certain going below the
current default is a good thing to do.

Jan


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