From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:37069) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZIeSc-0000qZ-SD for qemu-devel@nongnu.org; Fri, 24 Jul 2015 11:00:32 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZIeSW-0006gr-HP for qemu-devel@nongnu.org; Fri, 24 Jul 2015 11:00:30 -0400 Date: Fri, 24 Jul 2015 16:00:20 +0100 From: Stefan Hajnoczi Message-ID: <20150724150020.GA26843@stefanha-thinkpad.redhat.com> References: <4E49353C-F0D9-43CA-B4FE-BB5E85FCEEDC@gmail.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="tKW2IUtsqtDRztdT" Content-Disposition: inline In-Reply-To: <4E49353C-F0D9-43CA-B4FE-BB5E85FCEEDC@gmail.com> Subject: Re: [Qemu-devel] [PATCH v3] raw-posix.c: Make physical devices usable in QEMU under Mac OS X host List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Programmingkid Cc: Kevin Wolf , qemu-devel qemu-devel , Qemu-block --tKW2IUtsqtDRztdT Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Fri, Jul 17, 2015 at 08:19:16PM -0400, Programmingkid wrote: > @@ -2014,7 +2015,9 @@ kern_return_t GetBSDPath( io_iterator_t mediaIterat= or, char *bsdPath, CFIndex ma > if ( bsdPathAsCFString ) { > size_t devPathLength; > strcpy( bsdPath, _PATH_DEV ); > - strcat( bsdPath, "r" ); > + if (flags & BDRV_O_NOCACHE) { > + strcat(bsdPath, "r"); > + } > devPathLength =3D strlen( bsdPath ); > if ( CFStringGetCString( bsdPathAsCFString, bsdPath + devPat= hLength, maxPathSize - devPathLength, kCFStringEncodingASCII ) ) { > kernResult =3D KERN_SUCCESS; This hunk should be a separate patch. It fixes raw-posix alignment probing by only using the raw char device (which has alignment constraints) if BDRV_O_NOCACHE was given. > @@ -2027,7 +2030,35 @@ kern_return_t GetBSDPath( io_iterator_t mediaItera= tor, char *bsdPath, CFIndex ma > return kernResult; > } > =20 > -#endif > +/* Sets up a real cdrom for use in QEMU */ > +static bool setupCDROM(char *bsdPath) > +{ > + int index, numOfTestPartitions =3D 2, fd; > + char testPartition[MAXPATHLEN]; > + bool partitionFound =3D false; > + > + /* look for a working partition */ > + for (index =3D 0; index < numOfTestPartitions; index++) { > + pstrcpy(testPartition, strlen(bsdPath)+1, bsdPath); The safe way to use pstrcpy is: char dest[LEN]; pstrcpy(dest, sizeof(dest), src); Use the destination buffer size since that's what needs to be checked to prevent buffer overflow. Using the source buffer size could cause an overflow if the source buffer is larger than the destination buffer. Even if that's not the case today, it's bad practice because it could lead to bugs if code is modified. > + snprintf(testPartition, MAXPATHLEN, "%ss%d", testPartition, inde= x); Using the same buffer as the destination and a format string argument is questionable. I wouldn't be surprised if some snprintf() implementations produce garbage when you make them read from the same buffer they are writing to. Please replace pstrcpy() and snprintf() with a single call: snprintf(testPartition, sizeof(testPartition), "%ss%d", bsdPath, index); > + fd =3D qemu_open(testPartition, O_RDONLY | O_BINARY | O_LARGEFIL= E); > + if (fd >=3D 0) { > + partitionFound =3D true; > + qemu_close(fd); > + break; > + } > + } > + > + /* if a working partition on the device was not found */ > + if (partitionFound =3D=3D false) { > + printf("Error: Failed to find a working partition on disc!\n"); > + } else { > + DPRINTF("Using %s as optical disc\n", testPartition); > + pstrcpy(bsdPath, strlen(testPartition)+1, testPartition); Please use MAXPATHLEN instead of strlen(testPartition)+1. --tKW2IUtsqtDRztdT Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQEcBAEBAgAGBQJVslMEAAoJEJykq7OBq3PIFxwH/0tzTOMgbJjdDfU4ZLguXBOV 9hnwcKJHyAVeJzc6w2KOFoEqUj/PTPvoa1b9Au6tFvmGdxLxGhHzLkxT8aUuTdTC A32gpN6YLN5LrhBNzUXK352tU+qxExxxNW7rtkfg8JmIhHf9ThEz/M1DvLJhPqWv 429xZuXJJKGdNyyMCceW8xx7JvFYKddeltS5M/Ko8cPq4Aa9krOTBuM2Jr7Na9ot xXzJgF1MTLH7HmJkBzspEa/EVJSXjf9pWduUZJygXHWdYcjj1PXvPAcBscGLJjSn 2+d5C575J1DXhZzBTSg3p4S7faA/LX41olqGPHjZjY6j3gOntWPLCDoeaR788nQ= =AWQj -----END PGP SIGNATURE----- --tKW2IUtsqtDRztdT--