Re: [Xen-devel] [PATCH] Do not read files at once in pygrub

[Miroslav Rezanina <mrezanin@xxxxxxxxxx>]

----- Original Message -----
> From: "M A Young" <m.a.young@xxxxxxxxxxxx>
> To: "Ian Campbell" <Ian.Campbell@xxxxxxxxxx>
> Cc: "Miroslav Rezanina" <mrezanin@xxxxxxxxxx>, "xen-devel" 
> <xen-devel@xxxxxxxxxxxxxxxxxxx>
> Sent: Tuesday, May 22, 2012 12:30:31 PM
> Subject: Re: [Xen-devel] [PATCH] Do not read files at once in pygrub
> 
> On Tue, 22 May 2012, Ian Campbell wrote:
> 
> > On Tue, 2012-05-22 at 08:37 +0100, Miroslav Rezanina wrote:
> >> If guest kernel or ramdisk image is too large to fit in the dom0
> >> memory, it causes pygrub crash with "out of memory" error.
> >> This patch reads kernel/ramdisk file in 1 MB blocks so prevent
> >> exhausting whole memory.
> >
> > Thanks, we've had a similar patch from Michael Young (CCd) too,
> > see:
> > <alpine.DEB.2.00.1205170029550.26049@xxxxxxxxxxxxxxxx>.
> or at
> http://lists.xen.org/archives/html/xen-devel/2012-05/msg01183.html
> 
> > They look functionally pretty similar, one big difference is that
> > Michael limits the size of the cfg file as well, which seems wise.
> 
> Yes, a malicious guest could have a huge grub configuration file as
> well.
> My strategy was just to read the first megabyte as I can't see why a
> legitimate configuration file would be anywhere near that long.
> 
Yeah, it should not be as big. However, I think it should use same approach
as kernel/ramdisk in case there will be such a big configuration file (even
we do not see reason now).

> > Looks like his handles errors on the os.write too?
> 
> Yes, if there are write problems my patch deletes the kernel and
> ramdisk
> temporary files and exits with an error. This isn't so important
> though as
> the calling process should clean them up afterwards though my testing
> suggested that if the write failed due to lack of space, the guest
> may try
> to run with incomplete kernel or ramdisk files, and also you would
> have a
> full filesystem for a bit longer.
>
This is useful part as I sometimes experienced leftovers in case of errors.

> > I do like splitting the read loop out into a function as you've
> > done
> > though.
> 
> Yes, that does seem neater.
> 
>       Michael Young
> 

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