Re: [Xen-devel] [PATCH 00/11] pl011 emulation support in Xen

[Bhupinder Thakur <bhupinder.thakur@xxxxxxxxxx>]

Hi,

Thanks for your comments.

On 3 March 2017 at 21:23, Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx> wrote:
>> The following changes were done:
>
> .. snip..
>
> Thank you for this great writeup. I took a stab at it and stopped at patch
> #2 b/c Julien said he would look in it deeper. But based on a brief
> look I would say:
>  - Please do remove most of the comments. They really do not add
>    much context besides describing the code - and we all can
>    read the code. The idea behind the comments is to describe some
>    semantics of it, or something that is not obvious at first or
>    such. Not the code.
I will remove the comments where not required.

>
>  - Comments. One line comments are:
>   /* Comment. */
>    And please do use proper case and a period.
>
I will correct the comments.

>  - Be careful about compiler optimizations and jump tables.
>    Specifically see https://xenbits.xen.org/xsa/advisory-155.html
>    The way to make sure you don't introduce an security problem
>    is to 1) use local variables 2) read once from the ring and
>    make sure you use a compiler barrier.
>
will check the guidelines.

>  - There is also some unrelated changes. Like extra newlines. One
>    way to avoid this is to send all your patches _just_ to yourself
>    and review them - but review them in reverse order and from the
>    bottom of the emails to the top. That way you can catch some of this.
>
>  - Think in terms of how one would break this. For example the guest
>    could change the HVM parameters (or maybe not?) - or find the
>    console ring and muck with the ring indexes. You need to shield
>    the code from such changes.
>
I plan to add the checks that the HVM params can only be accessed from
a privileged guest.

> Thanks!

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