[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-block] [PATCH] raw-posix.c: Make physical devices usable in QE
From: |
Stefan Hajnoczi |
Subject: |
Re: [Qemu-block] [PATCH] raw-posix.c: Make physical devices usable in QEMU |
Date: |
Thu, 16 Jul 2015 14:19:28 +0100 |
User-agent: |
Mutt/1.5.23 (2014-03-12) |
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.
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.
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?
pgp5xir130Ho2.pgp
Description: PGP signature