[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH 03/11] Refactor virStorageFileGetMetadataFromFD to separate functionality



On Mon, Jul 12, 2010 at 02:30:40PM +0100, Daniel P. Berrange wrote:
> The virStorageFileGetMetadataFromFD did two jobs in one. First
> it probed for storage type, then it extracted metadata for the
> type. It is desirable to be able to separate these jobs, allowing
> probing without querying metadata, and querying metadata without
> probing.
> 
> To prepare for this, split out probing code into a new pair of
> methods
[...]
> +++ b/src/util/storage_file.c
> @@ -104,6 +104,9 @@ static int vmdk4GetBackingStore(char **, int *,
>  #define QCOW2_HDR_EXTENSION_END 0
>  #define QCOW2_HDR_EXTENSION_BACKING_FORMAT 0xE2792ACA
>  
> +/* VMDK needs at least this to find backing store,
> + * other formats are less */
> +#define STORAGE_MAX_HEAD (20*512)

  What about finishing the comment, less what ??? "need less" ?

[...]
> -    unsigned char head[20*512]; /* vmdk4GetBackingStore needs this much. */

  okay "need less"

[...]
> +/**
> + * virStorageFileProbeFormatFromFD:
> + *
> + * Probe for the format of 'fd' (which is an open file descriptor
> + * pointing to 'path'), returning the detected disk format.
> + *
> + * Callers are advised never to trust the returned 'format'
> + * unless it is listed as VIR_STORAGE_FILE_RAW, since a
> + * malicious guest can turn a file into any other non-raw
> + * format at will.
> + *
> + * Best option: Don't use this function

 Could this be a setting ? Not always nice to set security left as a
 setting, but if we don't trust that code ...


> +/**
> + * virStorageFileProbeFormat:
> + *
> + * Probe for the format of 'path', returning the detected
> + * disk format.
> + *
> + * Callers are advised never to trust the returned 'format'
> + * unless it is listed as VIR_STORAGE_FILE_RAW, since a
> + * malicious guest can turn a raw file into any other non-raw
> + * format at will.
> + *
> + * Best option: Don't use this function
> + */

  idem.

> + * Callers are advised never to trust the returned 'meta->format'
> + * unless it is listed as VIR_STORAGE_FILE_RAW, since a
> + * malicious guest can turn a raw file into any other non-raw
> + * format at will.

  idem

ACK to the approach but since we are left to chose between heuristics
potentially dangerous or disabling guessing, ca we defr that back to
some kind of user option ?

Daniel

-- 
Daniel Veillard      | libxml Gnome XML XSLT toolkit  http://xmlsoft.org/
daniel veillard com  | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library  http://libvirt.org/


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]