[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH 02/22] libxc: introduce xc_dom_seg_to_ptr_pages

On 10/06/13 15:02, Ian Jackson wrote:
> Andrew Cooper writes ("Re: [PATCH 02/22] libxc: introduce 
> xc_dom_seg_to_ptr_pages"):
>> On 10/06/13 14:40, Ian Jackson wrote:
>>> Or to put it another way: doing it this way makes it easier to see
>>> that the resulting code is correct.
>> I absolutely agree for unstable, but am arguing this around a minimal
>> set of changes for a security fix.
> The reasoning behind security fixes having a minimal set of changes
> is as follows:
> 1. We want security fixes to have a low probability of mistakes
>    (both regressions and failures to fix the whole problem).
> 2. Therefore we want security fixes to be easy to review.
> 3. Therefore, and directly from (1), security fixes should be as
>    obviously correct as possible.
> 4. Normally the best way to make a patch or series more obviously
>    correct is to make it shorter.
> The goal of making security fixes short (4) exists only to serve the
> goals of review (3) and correctness (1).  If it is easier to assure
> correctness of a longer series, then that longer series is desirable.
> As I say:
>>> Or to put it another way: doing it this way makes it easier to see
>>> that the resulting code is correct.
> Indeed this whole series is much bigger, textually, than it could have
> been.  Folding the patches into a single diff would make the result
> "smaller" by a factor of two.  Using a different approach such as
> trying to add specific range checking at every pointer computation
> site might well have produced a smaller patch, but it would be much
> harder to see whether the results were correct.
>> In practice, I would suggest that xc_dom_seg_to_ptr() be updated to have
>> the pages count, and all callsites updated appropriately.
> When you say "have the pages count" what do you mean ?  You mean to
> _take_ the pages count ?  But the pages count can usefully be computed
> centrally in xc_dom_seg_to_ptr.
> Ian.

Having agreed about this patch offline,

Reviewed-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>

Xen-devel mailing list



Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.