From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1IJ9P8-00078G-1s for qemu-devel@nongnu.org; Thu, 09 Aug 2007 10:54:26 -0400 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1IJ9P7-000780-81 for qemu-devel@nongnu.org; Thu, 09 Aug 2007 10:54:25 -0400 Received: from [199.232.76.173] (helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1IJ9P7-00077x-5b for qemu-devel@nongnu.org; Thu, 09 Aug 2007 10:54:25 -0400 Received: from rv-out-0910.google.com ([209.85.198.187]) by monty-python.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1IJ9P6-0001z5-Gn for qemu-devel@nongnu.org; Thu, 09 Aug 2007 10:54:24 -0400 Received: by rv-out-0910.google.com with SMTP id k15so1427199rvb for ; Thu, 09 Aug 2007 07:54:22 -0700 (PDT) Message-ID: <46BB2A99.3030609@codemonkey.ws> Date: Thu, 09 Aug 2007 09:54:17 -0500 From: Anthony Liguori MIME-Version: 1.0 Subject: Re: [Qemu-devel] [PATCH 4/4][RFC] Add logic to QEMU to read command line options from qcow2 images References: <59abf66e0708081124g14901b01i841b70d17ae1e097@mail.gmail.com> <59abf66e0708081252of2948d7we85c9084bad245d4@mail.gmail.com> <20070808202428.GA25050@redhat.com> In-Reply-To: <20070808202428.GA25050@redhat.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Reply-To: qemu-devel@nongnu.org List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Daniel P. Berrange" , qemu-devel@nongnu.org, =?ISO-8859-1?Q?Jorge_Luc=E1ngeli_Obes?= , Avi Kivity Daniel P. Berrange wrote: > On Wed, Aug 08, 2007 at 04:52:58PM -0300, Jorge Luc?ngeli Obes wrote: > >> This patch makes QEMU check for command line options stored in qcow2 images. >> > > I think it is a bad idea from a security POV to automatically extract & use > command line args from a disk image like this without the admin explicitly > requesting this capability. > > eg If I grabbed a demo disk image from a vendors' or community website I would > certainly not trust whatever args may happen to be embedded in the disk image > and thus do not want QEMU to be automatically running using them. > > I'd recommend having some command line flag to turn this capability on. For > example a '--args PATH-TO-DISK' flag, > > qemu --args $HOME/fedora.qcow > That's pretty nasty. How do you specify which disk this is then? I do agree with you that allowing arbitrary command line arguments in an image file is probably a bad idea. I think the general idea of being able to launch a single image is useful but I suspect this is not the right way to do it. What are some people thinking would want to be stored in the file? Most of the command line options are more host specific than guest specific I think. Maybe we can store a pseudo-config instead that only contains a subset of parameters (and therefore, wouldn't pose a security risk)? Regards, Anthony Liguori > Would extract args from the disk image & us them. > > While traditional > > qemu $HOME/fedora.qcow > > would *not* extract args. > > >> diff --git a/qemu/vl.c b/qemu/vl.c >> index 4ad39f1..1d28794 100644 >> --- a/qemu/vl.c >> +++ b/qemu/vl.c >> @@ -7000,6 +7012,58 @@ int main(int argc, char **argv) >> nb_nics = 0; >> /* default mac address of the first network interface */ >> >> + bdrv_init(); >> + >> + drv = bdrv_find_format("qcow2"); >> + >> + if (argc > 1 && argv[1][0] != '-') { >> + bs = bdrv_new(""); >> + if (!bs) { >> + fprintf(stderr, "Not enough memory"); >> + exit(1); >> + } >> + if (bdrv_open2(bs, argv[1], 0, drv) < 0) { >> + fprintf(stderr, "Could not open '%s'", argv[1]); >> + bdrv_delete(bs); >> + exit(1); >> + } >> + >> + tmpannot = bdrv_get_annot(bs, "commandline_args"); >> + if (tmpannot) { >> + pstrcpy(annot, 1024, tmpannot); >> + >> + do { >> + tok = strtok(nbtoks == 0? tmpannot : NULL, " "); >> + >> + if (tok != NULL) >> + nbtoks++; >> + else >> + done = 1; >> + } while (!done); >> + >> + free(tmpannot); >> + >> + if (nbtoks > 0) { >> + char **argvprime = malloc((nbtoks + argc) * sizeof(char*)); >> + >> + for (i = 0; i < argc; i++) >> + argvprime[i] = argv[i]; >> + >> + for (i = 0; i < nbtoks; i++) >> + argvprime[i + argc] = strtok(i == 0? annot : NULL, " "); >> + >> + argv = argvprime; >> + argc = argc + nbtoks; >> + >> + for (i = 0; i < nbtoks + 2; i++) >> + printf("argv[%d] = %s\n", i, argv[i]); >> + >> + } >> + } >> + >> + bdrv_delete(bs); >> + } >> + >> optind = 1; >> for(;;) { >> if (optind >= argc) >> > > > Dan. >