NULL pointers and PV guests.

NULL pointers and PV guests.

[Tim Deegan]

Hi,

After XSA-109 (a null function-pointer dereference) we've been
thinking about things we can do to make null pointers less dangerous
in PV guests.  This is a problem for pure PV only - when Xen is
running HVM and PVH guests null pointer dereferences will fault.

[ Disclaimer: it's sadly clear that I'm not going to have time to work
  on any of these ideas myself. :(  But we could at least put them on
  the wish list. ]

Idea 1: track PV pagetables so that we can tell which pagetables
might map the zero address -- e.g. by adding a flag or new types at
each level to indicate that we've seen this pagetable referenced
from slot zero of a higer-level pagetable that also has the flag set.
Then we could refuse any potential mapping of the bottom virtual 4k.

This is probably OK as a general feature because most PV OSes will
want to keep the bottom 4k free so that their own null pointers work.
But it would potentially mean that the guest couldn't alias the same
L1/2/3 pagetable at address 0 and some other address.

Linux/BSD people, can you comment on how likely that is to be a
problem?  Is it totally mad?

Idea 2: manually audit and fix all structs of function pointers
in Xen so that they always point to one of:
 - a useful function;
 - a noop stub (for cases where we currently test for != NULL); or
 - a function that calls BUG().
That seems like it would be a good idea, but it only helps for
functions and not for data pointers, and we might easily introduce
more null function pointers in new code.

Idea 3: #2 plus some sort of preprocessor wrappers (like we
have for guest handles or gfn_ts) to help us maintain discipline.
Uglier, but maybe better?

Idea 4: build-time support, with something like a clang analysis
pass or coccinelle, for finding uninitialised function pointers,
or for automatically inserting checks on indirect jumps.
Anyone know of existing tools that could help here?

Anything else we should consider?

Cheers,

Tim.

Re: NULL pointers and PV guests.

[Razvan Cojocaru]

On 03/26/2015 06:23 PM, Tim Deegan wrote:
> Idea 4: build-time support, with something like a clang analysis
> pass or coccinelle, for finding uninitialised function pointers,
> or for automatically inserting checks on indirect jumps.
> Anyone know of existing tools that could help here?

Scan-build is quite nice:

http://clang-analyzer.llvm.org/scan-build.html


HTH,
Razvan

Re: NULL pointers and PV guests.

[Tim Deegan]

At 18:31 +0200 on 26 Mar (1427394672), Razvan Cojocaru wrote:
> On 03/26/2015 06:23 PM, Tim Deegan wrote:
> > Idea 4: build-time support, with something like a clang analysis
> > pass or coccinelle, for finding uninitialised function pointers,
> > or for automatically inserting checks on indirect jumps.
> > Anyone know of existing tools that could help here?
> 
> Scan-build is quite nice:
> 
> http://clang-analyzer.llvm.org/scan-build.html

Yep - I have run Xen through scan-build once before, but both Xen and
clang have changed a fair amount since then. :)  In my imaginary
copious free time it would be nice to get that build lint-free...

Tim.

Re: NULL pointers and PV guests.

[Andrew Cooper]

On 26/03/15 16:23, Tim Deegan wrote:
> Hi,
>
> After XSA-109 (a null function-pointer dereference) we've been
> thinking about things we can do to make null pointers less dangerous
> in PV guests.  This is a problem for pure PV only - when Xen is
> running HVM and PVH guests null pointer dereferences will fault.
>
> [ Disclaimer: it's sadly clear that I'm not going to have time to work
>    on any of these ideas myself. :(  But we could at least put them on
>    the wish list. ]
>
> Idea 1: track PV pagetables so that we can tell which pagetables
> might map the zero address -- e.g. by adding a flag or new types at
> each level to indicate that we've seen this pagetable referenced
> from slot zero of a higer-level pagetable that also has the flag set.
> Then we could refuse any potential mapping of the bottom virtual 4k.
>
> This is probably OK as a general feature because most PV OSes will
> want to keep the bottom 4k free so that their own null pointers work.
> But it would potentially mean that the guest couldn't alias the same
> L1/2/3 pagetable at address 0 and some other address.
>
> Linux/BSD people, can you comment on how likely that is to be a
> problem?  Is it totally mad?

While this would be a very good idea (as would preventing mappings on 
the boundaries of the canonical region), such a change would break all 
minios based guests which start .text at 0

As a result I don't think this is a feasible option, although it might 
be a very good idea to have an opt-in restriction for guests which 
actively wish to play nice.

>
> Idea 2: manually audit and fix all structs of function pointers
> in Xen so that they always point to one of:
>   - a useful function;
>   - a noop stub (for cases where we currently test for != NULL); or
>   - a function that calls BUG().
> That seems like it would be a good idea, but it only helps for
> functions and not for data pointers, and we might easily introduce
> more null function pointers in new code.
>
> Idea 3: #2 plus some sort of preprocessor wrappers (like we
> have for guest handles or gfn_ts) to help us maintain discipline.
> Uglier, but maybe better?
>
> Idea 4: build-time support, with something like a clang analysis
> pass or coccinelle, for finding uninitialised function pointers,
> or for automatically inserting checks on indirect jumps.
> Anyone know of existing tools that could help here?

I have looked into coccinelle before, and it sadly cant parse our 
XEN_GUEST_HANDLE() constructs, which causes it to ignore the rest of the 
translation unit.

It would be nice if someone who spoke more Ocaml than me fixed their C 
parser, as spatch itself is a very useful tool.


Independently of fixing the NULL pointer issue, attempting to get the 
clang analysis running would be a very good thing.

>
> Anything else we should consider?

I have also tried experimenting with sparse, and also come to the 
conclusion that it is a lot of work.  (First and foremost fixing sparse 
so it understands C11's _StaticAssert(), or hopefully some kind person 
has already done this since I last checked.)

~Andrew

Re: NULL pointers and PV guests.

[Ian Campbell]

On Thu, 2015-03-26 at 16:44 +0000, Andrew Cooper wrote:
> As a result I don't think this is a feasible option, although it might 
> be a very good idea to have an opt-in restriction for guests which 
> actively wish to play nice.

opt-in isn't very useful, my malicious guest simply wouldn't opt-in...

Ian.

Re: NULL pointers and PV guests.

[Konrad Rzeszutek Wilk]

On Thu, Mar 26, 2015 at 04:23:19PM +0000, Tim Deegan wrote:
> Hi,
> 
> After XSA-109 (a null function-pointer dereference) we've been
> thinking about things we can do to make null pointers less dangerous
> in PV guests.  This is a problem for pure PV only - when Xen is
> running HVM and PVH guests null pointer dereferences will fault.
> 
> [ Disclaimer: it's sadly clear that I'm not going to have time to work
>   on any of these ideas myself. :(  But we could at least put them on
>   the wish list. ]
> 
> Idea 1: track PV pagetables so that we can tell which pagetables
> might map the zero address -- e.g. by adding a flag or new types at
> each level to indicate that we've seen this pagetable referenced
> from slot zero of a higer-level pagetable that also has the flag set.
> Then we could refuse any potential mapping of the bottom virtual 4k.
> 
> This is probably OK as a general feature because most PV OSes will
> want to keep the bottom 4k free so that their own null pointers work.
> But it would potentially mean that the guest couldn't alias the same
> L1/2/3 pagetable at address 0 and some other address.
> 
> Linux/BSD people, can you comment on how likely that is to be a
> problem?  Is it totally mad?

I would stay away from any pagetables manipulation as much as possible
in Linux. Linus is already unhappy with the SHARED_PMD flag being
disabled when running under Xen and wants to eliminate that.

The less (or none) that we touch in Linux pagetables the better.
> 
> Idea 2: manually audit and fix all structs of function pointers
> in Xen so that they always point to one of:
>  - a useful function;
>  - a noop stub (for cases where we currently test for != NULL); or
>  - a function that calls BUG().
> That seems like it would be a good idea, but it only helps for
> functions and not for data pointers, and we might easily introduce
> more null function pointers in new code.
> 
> Idea 3: #2 plus some sort of preprocessor wrappers (like we
> have for guest handles or gfn_ts) to help us maintain discipline.
> Uglier, but maybe better?
> 
> Idea 4: build-time support, with something like a clang analysis
> pass or coccinelle, for finding uninitialised function pointers,
> or for automatically inserting checks on indirect jumps.
> Anyone know of existing tools that could help here?

Could Coverity help here?
> 
> Anything else we should consider?
> 
> Cheers,
> 
> Tim.

Re: NULL pointers and PV guests.

[George Dunlap]

On Mon, Mar 30, 2015 at 3:31 PM, Konrad Rzeszutek Wilk
<konrad.wilk@oracle.com> wrote:
> On Thu, Mar 26, 2015 at 04:23:19PM +0000, Tim Deegan wrote:
>> Hi,
>>
>> After XSA-109 (a null function-pointer dereference) we've been
>> thinking about things we can do to make null pointers less dangerous
>> in PV guests.  This is a problem for pure PV only - when Xen is
>> running HVM and PVH guests null pointer dereferences will fault.
>>
>> [ Disclaimer: it's sadly clear that I'm not going to have time to work
>>   on any of these ideas myself. :(  But we could at least put them on
>>   the wish list. ]
>>
>> Idea 1: track PV pagetables so that we can tell which pagetables
>> might map the zero address -- e.g. by adding a flag or new types at
>> each level to indicate that we've seen this pagetable referenced
>> from slot zero of a higer-level pagetable that also has the flag set.
>> Then we could refuse any potential mapping of the bottom virtual 4k.
>>
>> This is probably OK as a general feature because most PV OSes will
>> want to keep the bottom 4k free so that their own null pointers work.
>> But it would potentially mean that the guest couldn't alias the same
>> L1/2/3 pagetable at address 0 and some other address.
>>
>> Linux/BSD people, can you comment on how likely that is to be a
>> problem?  Is it totally mad?
>
> I would stay away from any pagetables manipulation as much as possible
> in Linux. Linus is already unhappy with the SHARED_PMD flag being
> disabled when running under Xen and wants to eliminate that.

I'm pretty sure Tim is talking about tracking pagetables in Xen, not
in Linux.  The only restriction Idea 1 has in Linux would be that it
couldn't, even during boot, be able to map something at VA 0, and Tim
is asking realistically how often this is likely to be a problem.

I know that *in general*, Linux doesn't allow processes to map
anything to VA 0 either, for similar reasons; but that there are
mechanisms in place to override that.  I think we're probably OK with
crashing a guest that runs one of these "I need NULL pointers"
programs (or allowing the host admin to special-case permission for
VMs she trusts); but there was a fear that there may be a phase during
boot where VA 0 gets mapped that would be more difficult to avoid.

 -George

Re: NULL pointers and PV guests.

[Tim Deegan]

Hi,

At 10:31 -0400 on 30 Mar (1427711518), Konrad Rzeszutek Wilk wrote:
> On Thu, Mar 26, 2015 at 04:23:19PM +0000, Tim Deegan wrote:
> > Idea 1: track PV pagetables so that we can tell which pagetables
> > might map the zero address -- e.g. by adding a flag or new types at
> > each level to indicate that we've seen this pagetable referenced
> > from slot zero of a higer-level pagetable that also has the flag set.
> > Then we could refuse any potential mapping of the bottom virtual 4k.
> > 
> > This is probably OK as a general feature because most PV OSes will
> > want to keep the bottom 4k free so that their own null pointers work.
> > But it would potentially mean that the guest couldn't alias the same
> > L1/2/3 pagetable at address 0 and some other address.
> > 
> > Linux/BSD people, can you comment on how likely that is to be a
> > problem?  Is it totally mad?
> 
> I would stay away from any pagetables manipulation as much as possible
> in Linux. Linus is already unhappy with the SHARED_PMD flag being
> disabled when running under Xen and wants to eliminate that.

That's about the answer I expected. :)

Between that and needing to have an opt-out for minos/mirage/&c this
line isn't worth pursuing.

> > Idea 4: build-time support, with something like a clang analysis
> > pass or coccinelle, for finding uninitialised function pointers,
> > or for automatically inserting checks on indirect jumps.
> > Anyone know of existing tools that could help here?
> 
> Could Coverity help here?

I think that between the xen project's coverity runs and other private
instances, we're getting everything we can out of coverity already.

I might have a look at other options in my copious free time -
I know it's possible to find indirect calls with an LLVM pass;
extending that to insert automatic NULL checks would be doable (but
only work if compiled with the right toolchain, of course).  Testing
for existing NULL checks might be more useful.

Cheers,

Tim.