On Jul 16, 2015, at 9:19 AM, Stefan Hajnoczi wrote: > On Thu, Jul 09, 2015 at 10:02:26AM -0400, Programmingkid wrote: >> >> On Jul 9, 2015, at 6:52 AM, Stefan Hajnoczi wrote: >> >>> On Tue, Jul 07, 2015 at 01:33:23PM -0400, Programmingkid wrote: >>>> Make physical devices like a USB flash drive or a CDROM drive work in >>>> QEMU. With >>>> this patch I can use a USB flash drive like a hard drive. Before this >>>> patch, >>>> QEMU would just quit with a message like "resource busy". >>> >>> The commit message and the description are missing "on Mac OS X". It >>> should be clear right away that this applies to Mac only. This works >>> fine on Linux and probably other host OSes. >> >> Yeah, that should have been done. Did you see any issues with the code? > > QEMU shouldn't silently open a different file than the one given by the > user. The user should give the exact device file they want. If there > is magic behavior it needs to be documented, but I don't see a reason > why that's necessary in the case of device files.
I think you are reviewing an older patch. The newest one doesn't do that. > > QEMU shouldn't mount/unmount the CD-ROM. atexit(3) doesn't handle > crashes or abort(). Users may be confused to find their CD-ROM > unmounted in those cases and would see this as a bug. Instead we should > refuse mounted CD-ROMs so the user understands that block-level access > requires them to unmount first. That can be done. It just wouldn't be as user friendly as having QEMU do it for the user :( > > The strcpy/sprintf usage in this patch is unsafe and can lead to buffer > overflow, for example in the case of generating command-lines. The > command-line buffer is only MAXPATHLEN so prepending the command to the > filename could exceed the buffer size. > > There is also a buffer overflow in the array of devices that need to be > mounted. What happens if there are more than 7 devices? Ok. Will correct this issue.
