On Wed, Jan 23, 2013 at 11:46:48AM +0100, Laszlo Ersek wrote:
> On 01/23/13 09:36, Stefan Hajnoczi wrote:
> > On Wed, Jan 16, 2013 at 07:40:19PM +0100, Laszlo Ersek wrote:
> >> static int piix3_post_load(void *opaque, int version_id)
> >> {
> >> PIIX3State *piix3 = opaque;
> >> piix3_update_irq_levels(piix3);
> >> + piix3->rcr &= 2; /* keep System Reset type only */
> >> return 0;
> >> }
> >
> > Is this necessary? I think only an evil migration source could set
> > value not in {0x0, 0x2}.
>
> I wanted to make sure in general that no write path to "piix3->rcr"
> would let an invalid value through.
>
> > And if so, it doesn't seem like our job to
> > validate that.
>
> Independently of the patch: why not?
The migration source and the guest are both untrusted. We just need to
protect against QEMU crashing or doing something unsafe which could lead
to security problems or data corruption.
An invalid value in this register means that the guest sees a junk value
but it's nothing dangerous that QEMU needs to protect against.
The migration source can already create a junk guest. Storing a junk
value in this register is no worse.
We don't validate other migrated values, so it stood out to me. I
wasn't sure if there's some subtle reason why we need to do this.
Consistently validating all register values in a separate patch would be
okay, but having just this one line is confusing.
Stefan
