[Qemu-devel] Recursion in cpu_physical_memory_rw

[Qemu-devel] Recursion in cpu_physical_memory_rw

[Herbert Xu]

Hi:

A number of qemu driver backends (such as rtl8139) call the function
cpu_physical_memory_rw to read/write guest memory.  The target guest
memory address is often supplied by the guest.  This opens up the
possibility of a guest giving an address which happens to be an MMIO
address which can potentially lead to infinite recursion involving
cpu_physical_memory_rw.

Since these driver backends really only need to access system memory,
we could simply provide a new access interface that does not allow
MMIO addresses.

Any comments on this problem?

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Recursion in cpu_physical_memory_rw

[Herbert Xu]

Hi:

A number of qemu driver backends (such as rtl8139) call the function
cpu_physical_memory_rw to read/write guest memory.  The target guest
memory address is often supplied by the guest.  This opens up the
possibility of a guest giving an address which happens to be an MMIO
address which can potentially lead to infinite recursion involving
cpu_physical_memory_rw.

Since these driver backends really only need to access system memory,
we could simply provide a new access interface that does not allow
MMIO addresses.

Any comments on this problem?

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: [Qemu-devel] Recursion in cpu_physical_memory_rw

[Paul Brook]

On Wednesday 15 November 2006 00:43, Herbert Xu wrote:
> Hi:
>
> A number of qemu driver backends (such as rtl8139) call the function
> cpu_physical_memory_rw to read/write guest memory.  The target guest
> memory address is often supplied by the guest.  This opens up the
> possibility of a guest giving an address which happens to be an MMIO
> address which can potentially lead to infinite recursion involving
> cpu_physical_memory_rw.
>
> Since these driver backends really only need to access system memory,
> we could simply provide a new access interface that does not allow
> MMIO addresses.

It isn't always system memory. Some DMA controllers deliberately write to 
device FIFOs. There are also several devices which map areas of onboard RAM. 
At minimum you need to make those to use RAM mappings rather than MMIO.

If a device is recursively writing to itself I'd take this as sign that the 
guest OS is already pretty screwed. I'm not sure what happens in this 
situation on real hardware, but I wouldn't be surprised if it caused similar 
effects by flooding the bus.

Paul

Re: Recursion in cpu_physical_memory_rw

[Paul Brook]

On Wednesday 15 November 2006 00:43, Herbert Xu wrote:
> Hi:
>
> A number of qemu driver backends (such as rtl8139) call the function
> cpu_physical_memory_rw to read/write guest memory.  The target guest
> memory address is often supplied by the guest.  This opens up the
> possibility of a guest giving an address which happens to be an MMIO
> address which can potentially lead to infinite recursion involving
> cpu_physical_memory_rw.
>
> Since these driver backends really only need to access system memory,
> we could simply provide a new access interface that does not allow
> MMIO addresses.

It isn't always system memory. Some DMA controllers deliberately write to 
device FIFOs. There are also several devices which map areas of onboard RAM. 
At minimum you need to make those to use RAM mappings rather than MMIO.

If a device is recursively writing to itself I'd take this as sign that the 
guest OS is already pretty screwed. I'm not sure what happens in this 
situation on real hardware, but I wouldn't be surprised if it caused similar 
effects by flooding the bus.

Paul

Re: [Qemu-devel] Recursion in cpu_physical_memory_rw

[Herbert Xu]

On Wed, Nov 15, 2006 at 12:57:24AM +0000, Paul Brook wrote:
> 
> It isn't always system memory. Some DMA controllers deliberately write to 
> device FIFOs. There are also several devices which map areas of onboard RAM. 
> At minimum you need to make those to use RAM mappings rather than MMIO.

I'm not suggesting that we change all existing users of cpu_physical_*
to a new interface that only accessed RAM.  However, for cases where it
is obvious that only system RAM is intended (e.g., rtl8139), it makes
sense to bypass MMIO handlers.

> If a device is recursively writing to itself I'd take this as sign that the 
> guest OS is already pretty screwed. I'm not sure what happens in this 
> situation on real hardware, but I wouldn't be surprised if it caused similar 
> effects by flooding the bus.

The scenario here is a compromised guest attempting to harm a host such
as Xen.

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: Re: [Qemu-devel] Recursion in cpu_physical_memory_rw

[Keir Fraser]

On 15/11/06 2:58 am, "Herbert Xu" <herbert@gondor.apana.org.au> wrote:

>> It isn't always system memory. Some DMA controllers deliberately write to
>> device FIFOs. There are also several devices which map areas of onboard RAM.
>> At minimum you need to make those to use RAM mappings rather than MMIO.
> 
> I'm not suggesting that we change all existing users of cpu_physical_*
> to a new interface that only accessed RAM.  However, for cases where it
> is obvious that only system RAM is intended (e.g., rtl8139), it makes
> sense to bypass MMIO handlers.

Could we add a recursion counter to the memory-access functions, and bail if
it reaches some limit?

 -- Keir

Re: [Xen-devel] Re: [Qemu-devel] Recursion in cpu_physical_memory_rw

[Keir Fraser]

On 15/11/06 2:58 am, "Herbert Xu" <herbert@gondor.apana.org.au> wrote:

>> It isn't always system memory. Some DMA controllers deliberately write to
>> device FIFOs. There are also several devices which map areas of onboard RAM.
>> At minimum you need to make those to use RAM mappings rather than MMIO.
> 
> I'm not suggesting that we change all existing users of cpu_physical_*
> to a new interface that only accessed RAM.  However, for cases where it
> is obvious that only system RAM is intended (e.g., rtl8139), it makes
> sense to bypass MMIO handlers.

Could we add a recursion counter to the memory-access functions, and bail if
it reaches some limit?

 -- Keir

Re: [Xen-devel] Re: [Qemu-devel] Recursion in cpu_physical_memory_rw

[Herbert Xu]

On Wed, Nov 15, 2006 at 07:55:48AM +0000, Keir Fraser wrote:
>
> > I'm not suggesting that we change all existing users of cpu_physical_*
> > to a new interface that only accessed RAM.  However, for cases where it
> > is obvious that only system RAM is intended (e.g., rtl8139), it makes
> > sense to bypass MMIO handlers.
> 
> Could we add a recursion counter to the memory-access functions, and bail if
> it reaches some limit?

Yes that would work too.  However, chips such as rtl8139 should never
do MMIO in this case (the real hardware would never allow that to occur)
so we should do that accordingly.

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: Re: [Qemu-devel] Recursion in cpu_physical_memory_rw

[Herbert Xu]

On Wed, Nov 15, 2006 at 07:55:48AM +0000, Keir Fraser wrote:
>
> > I'm not suggesting that we change all existing users of cpu_physical_*
> > to a new interface that only accessed RAM.  However, for cases where it
> > is obvious that only system RAM is intended (e.g., rtl8139), it makes
> > sense to bypass MMIO handlers.
> 
> Could we add a recursion counter to the memory-access functions, and bail if
> it reaches some limit?

Yes that would work too.  However, chips such as rtl8139 should never
do MMIO in this case (the real hardware would never allow that to occur)
so we should do that accordingly.

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: Re: [Qemu-devel] Recursion in cpu_physical_memory_rw

[Keir Fraser]

On 15/11/06 11:12, "Herbert Xu" <herbert@gondor.apana.org.au> wrote:

>> Could we add a recursion counter to the memory-access functions, and bail if
>> it reaches some limit?
> 
> Yes that would work too.  However, chips such as rtl8139 should never
> do MMIO in this case (the real hardware would never allow that to occur)
> so we should do that accordingly.

We'd take a patch for Xen to do it the 'proper way' with an extended
memory-access API if that is also acceptable for the upstream Qemu
maintainers.

 -- Keir

Re: [Xen-devel] Re: [Qemu-devel] Recursion in cpu_physical_memory_rw

[Keir Fraser]

On 15/11/06 11:12, "Herbert Xu" <herbert@gondor.apana.org.au> wrote:

>> Could we add a recursion counter to the memory-access functions, and bail if
>> it reaches some limit?
> 
> Yes that would work too.  However, chips such as rtl8139 should never
> do MMIO in this case (the real hardware would never allow that to occur)
> so we should do that accordingly.

We'd take a patch for Xen to do it the 'proper way' with an extended
memory-access API if that is also acceptable for the upstream Qemu
maintainers.

 -- Keir

Re: [Xen-devel] Re: [Qemu-devel] Recursion in cpu_physical_memory_rw

[Paul Brook]

> However, chips such as rtl8139 should never
> do MMIO in this case (the real hardware would never allow that to occur)

Really? Why wouldn't it work on real hardware?

Paul

Re: [Xen-devel] Re: Recursion in cpu_physical_memory_rw

[Paul Brook]

> However, chips such as rtl8139 should never
> do MMIO in this case (the real hardware would never allow that to occur)

Really? Why wouldn't it work on real hardware?

Paul

Re: [Xen-devel] Re: [Qemu-devel] Recursion in cpu_physical_memory_rw

[Herbert Xu]

On Wed, Nov 15, 2006 at 03:02:02PM +0000, Paul Brook wrote:
> > However, chips such as rtl8139 should never
> > do MMIO in this case (the real hardware would never allow that to occur)
> 
> Really? Why wouldn't it work on real hardware?

For rtl8139 it would cause a Master Abort.

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: Re: [Qemu-devel] Recursion in cpu_physical_memory_rw

[Herbert Xu]

On Wed, Nov 15, 2006 at 03:02:02PM +0000, Paul Brook wrote:
> > However, chips such as rtl8139 should never
> > do MMIO in this case (the real hardware would never allow that to occur)
> 
> Really? Why wouldn't it work on real hardware?

For rtl8139 it would cause a Master Abort.

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: [Qemu-devel] Recursion in cpu_physical_memory_rw

[Anthony Liguori]

Herbert Xu wrote:
> On Wed, Nov 15, 2006 at 12:57:24AM +0000, Paul Brook wrote:
>> It isn't always system memory. Some DMA controllers deliberately write to 
>> device FIFOs. There are also several devices which map areas of onboard RAM. 
>> At minimum you need to make those to use RAM mappings rather than MMIO.
> 
> I'm not suggesting that we change all existing users of cpu_physical_*
> to a new interface that only accessed RAM.  However, for cases where it
> is obvious that only system RAM is intended (e.g., rtl8139), it makes
> sense to bypass MMIO handlers.
> 
>> If a device is recursively writing to itself I'd take this as sign that the 
>> guest OS is already pretty screwed. I'm not sure what happens in this 
>> situation on real hardware, but I wouldn't be surprised if it caused similar 
>> effects by flooding the bus.
> 
> The scenario here is a compromised guest attempting to harm a host such
> as Xen.

The only "harm" done to a host is that the process will take as much CPU 
as it can get.  This is really only a problem in Xen because the device 
model is in Domain-0.  Once the device model is in a different domain, 
it doesn't matter anymore as the normal scheduler parameters can be used 
to ensure that no other hosts are harmed.

Regards,

Anthony Liguori


> Cheers,

Re: [Qemu-devel] Recursion in cpu_physical_memory_rw

[Anthony Liguori]

Herbert Xu wrote:
> On Wed, Nov 15, 2006 at 12:57:24AM +0000, Paul Brook wrote:
>> It isn't always system memory. Some DMA controllers deliberately write to 
>> device FIFOs. There are also several devices which map areas of onboard RAM. 
>> At minimum you need to make those to use RAM mappings rather than MMIO.
> 
> I'm not suggesting that we change all existing users of cpu_physical_*
> to a new interface that only accessed RAM.  However, for cases where it
> is obvious that only system RAM is intended (e.g., rtl8139), it makes
> sense to bypass MMIO handlers.
> 
>> If a device is recursively writing to itself I'd take this as sign that the 
>> guest OS is already pretty screwed. I'm not sure what happens in this 
>> situation on real hardware, but I wouldn't be surprised if it caused similar 
>> effects by flooding the bus.
> 
> The scenario here is a compromised guest attempting to harm a host such
> as Xen.

The only "harm" done to a host is that the process will take as much CPU 
as it can get.  This is really only a problem in Xen because the device 
model is in Domain-0.  Once the device model is in a different domain, 
it doesn't matter anymore as the normal scheduler parameters can be used 
to ensure that no other hosts are harmed.

Regards,

Anthony Liguori


> Cheers,

Re: [Qemu-devel] Recursion in cpu_physical_memory_rw

[Herbert Xu]

On Wed, Nov 15, 2006 at 01:03:34PM -0600, Anthony Liguori wrote:
>
> >The scenario here is a compromised guest attempting to harm a host such
> >as Xen.
> 
> The only "harm" done to a host is that the process will take as much CPU 
> as it can get.  This is really only a problem in Xen because the device 
> model is in Domain-0.  Once the device model is in a different domain, 
> it doesn't matter anymore as the normal scheduler parameters can be used 
> to ensure that no other hosts are harmed.

Actually it'll still be a problem in a driver domain unless it (and the
hardware) is dedicated to a single guest.

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: Re: [Qemu-devel] Recursion in cpu_physical_memory_rw

[Keir Fraser]

On 16/11/06 5:11 am, "Herbert Xu" <herbert@gondor.apana.org.au> wrote:

>> The only "harm" done to a host is that the process will take as much CPU
>> as it can get.  This is really only a problem in Xen because the device
>> model is in Domain-0.  Once the device model is in a different domain,
>> it doesn't matter anymore as the normal scheduler parameters can be used
>> to ensure that no other hosts are harmed.
> 
> Actually it'll still be a problem in a driver domain unless it (and the
> hardware) is dedicated to a single guest.

Each qemu 'stub domain' will be dedicated to a single guest. Adding a
recursion counter to the memory access functions sounds possibly useful even
just from a debugging p.o.v. though.

 -- Keir

Re: [Xen-devel] Re: [Qemu-devel] Recursion in cpu_physical_memory_rw

[Keir Fraser]

On 16/11/06 5:11 am, "Herbert Xu" <herbert@gondor.apana.org.au> wrote:

>> The only "harm" done to a host is that the process will take as much CPU
>> as it can get.  This is really only a problem in Xen because the device
>> model is in Domain-0.  Once the device model is in a different domain,
>> it doesn't matter anymore as the normal scheduler parameters can be used
>> to ensure that no other hosts are harmed.
> 
> Actually it'll still be a problem in a driver domain unless it (and the
> hardware) is dedicated to a single guest.

Each qemu 'stub domain' will be dedicated to a single guest. Adding a
recursion counter to the memory access functions sounds possibly useful even
just from a debugging p.o.v. though.

 -- Keir

Re: [Xen-devel] Re: [Qemu-devel] Recursion in cpu_physical_memory_rw

[Herbert Xu]

On Thu, Nov 16, 2006 at 07:52:45AM +0000, Keir Fraser wrote:
> 
> Each qemu 'stub domain' will be dedicated to a single guest. Adding a

You're right of course.  Somehow I was thinking of having the physical
NIC in the qemu domain which obviously isn't the case.

> recursion counter to the memory access functions sounds possibly useful even
> just from a debugging p.o.v. though.

After thinking about it for a while this does seem to be the safest
and easiest strategy.

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: Re: [Qemu-devel] Recursion in cpu_physical_memory_rw

[Herbert Xu]

On Thu, Nov 16, 2006 at 07:52:45AM +0000, Keir Fraser wrote:
> 
> Each qemu 'stub domain' will be dedicated to a single guest. Adding a

You're right of course.  Somehow I was thinking of having the physical
NIC in the qemu domain which obviously isn't the case.

> recursion counter to the memory access functions sounds possibly useful even
> just from a debugging p.o.v. though.

After thinking about it for a while this does seem to be the safest
and easiest strategy.

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt