* [Qemu-devel] [PATCH v2 0/2] qga umask fix addenda
@ 2013-05-08 15:31 Laszlo Ersek
2013-05-08 15:31 ` [Qemu-devel] [PATCH v2 1/2] qga: distinguish binary modes in "guest_file_open_modes" map Laszlo Ersek
` (3 more replies)
0 siblings, 4 replies; 10+ messages in thread
From: Laszlo Ersek @ 2013-05-08 15:31 UTC (permalink / raw)
To: eblake, aliguori, peter.maydell, mdroth, qemu-devel
I should have paid more attention to portability and error path cleanup
in the CVE-2013-2007 fix.
(We continue to assume, like the rest of qemu code, that
qemu_set_cloexec() never fails internally. This should be a reasonable
assumption when the input fd is valid.)
Laszlo Ersek (2):
qga: distinguish binary modes in "guest_file_open_modes" map
qga: unlink just created guest-file if fchmod() or fdopen() fails on
it
qga/commands-posix.c | 25 +++++++++++++++++++------
1 files changed, 19 insertions(+), 6 deletions(-)
^ permalink raw reply [flat|nested] 10+ messages in thread* [Qemu-devel] [PATCH v2 1/2] qga: distinguish binary modes in "guest_file_open_modes" map 2013-05-08 15:31 [Qemu-devel] [PATCH v2 0/2] qga umask fix addenda Laszlo Ersek @ 2013-05-08 15:31 ` Laszlo Ersek 2013-05-08 15:31 ` [Qemu-devel] [PATCH v2 2/2] qga: unlink just created guest-file if fchmod() or fdopen() fails on it Laszlo Ersek ` (2 subsequent siblings) 3 siblings, 0 replies; 10+ messages in thread From: Laszlo Ersek @ 2013-05-08 15:31 UTC (permalink / raw) To: eblake, aliguori, peter.maydell, mdroth, qemu-devel In Windows guests this may make a difference. Since the original patch (commit c689b4f1) sought to be pedantic and to consider theoretical corner cases of portability, we should fix it up where it failed to come through in that pursuit. Suggested-by: Eric Blake <eblake@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Signed-off-by: Laszlo Ersek <lersek@redhat.com> --- qga/commands-posix.c | 22 ++++++++++++++++------ 1 files changed, 16 insertions(+), 6 deletions(-) diff --git a/qga/commands-posix.c b/qga/commands-posix.c index 04c6951..2eec712 100644 --- a/qga/commands-posix.c +++ b/qga/commands-posix.c @@ -242,17 +242,27 @@ static GuestFileHandle *guest_file_handle_find(int64_t id, Error **err) typedef const char * const ccpc; +#ifndef O_BINARY +#define O_BINARY 0 +#endif + /* http://pubs.opengroup.org/onlinepubs/9699919799/functions/fopen.html */ static const struct { ccpc *forms; int oflag_base; } guest_file_open_modes[] = { - { (ccpc[]){ "r", "rb", NULL }, O_RDONLY }, - { (ccpc[]){ "w", "wb", NULL }, O_WRONLY | O_CREAT | O_TRUNC }, - { (ccpc[]){ "a", "ab", NULL }, O_WRONLY | O_CREAT | O_APPEND }, - { (ccpc[]){ "r+", "rb+", "r+b", NULL }, O_RDWR }, - { (ccpc[]){ "w+", "wb+", "w+b", NULL }, O_RDWR | O_CREAT | O_TRUNC }, - { (ccpc[]){ "a+", "ab+", "a+b", NULL }, O_RDWR | O_CREAT | O_APPEND } + { (ccpc[]){ "r", NULL }, O_RDONLY }, + { (ccpc[]){ "rb", NULL }, O_RDONLY | O_BINARY }, + { (ccpc[]){ "w", NULL }, O_WRONLY | O_CREAT | O_TRUNC }, + { (ccpc[]){ "wb", NULL }, O_WRONLY | O_CREAT | O_TRUNC | O_BINARY }, + { (ccpc[]){ "a", NULL }, O_WRONLY | O_CREAT | O_APPEND }, + { (ccpc[]){ "ab", NULL }, O_WRONLY | O_CREAT | O_APPEND | O_BINARY }, + { (ccpc[]){ "r+", NULL }, O_RDWR }, + { (ccpc[]){ "rb+", "r+b", NULL }, O_RDWR | O_BINARY }, + { (ccpc[]){ "w+", NULL }, O_RDWR | O_CREAT | O_TRUNC }, + { (ccpc[]){ "wb+", "w+b", NULL }, O_RDWR | O_CREAT | O_TRUNC | O_BINARY }, + { (ccpc[]){ "a+", NULL }, O_RDWR | O_CREAT | O_APPEND }, + { (ccpc[]){ "ab+", "a+b", NULL }, O_RDWR | O_CREAT | O_APPEND | O_BINARY } }; static int -- 1.7.1 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* [Qemu-devel] [PATCH v2 2/2] qga: unlink just created guest-file if fchmod() or fdopen() fails on it 2013-05-08 15:31 [Qemu-devel] [PATCH v2 0/2] qga umask fix addenda Laszlo Ersek 2013-05-08 15:31 ` [Qemu-devel] [PATCH v2 1/2] qga: distinguish binary modes in "guest_file_open_modes" map Laszlo Ersek @ 2013-05-08 15:31 ` Laszlo Ersek 2013-05-08 17:07 ` Eric Blake 2013-05-10 13:29 ` [Qemu-devel] [PATCH v2 0/2] qga umask fix addenda Luiz Capitulino 2013-05-10 19:30 ` mdroth 3 siblings, 1 reply; 10+ messages in thread From: Laszlo Ersek @ 2013-05-08 15:31 UTC (permalink / raw) To: eblake, aliguori, peter.maydell, mdroth, qemu-devel We shouldn't allow guest filesystem pollution on error paths. Suggested-by: Eric Blake <eblake@redhat.com> Signed-off-by: Laszlo Ersek <lersek@redhat.com> --- qga/commands-posix.c | 3 +++ 1 files changed, 3 insertions(+), 0 deletions(-) diff --git a/qga/commands-posix.c b/qga/commands-posix.c index 2eec712..e199738 100644 --- a/qga/commands-posix.c +++ b/qga/commands-posix.c @@ -355,6 +355,9 @@ safe_open_or_create(const char *path, const char *mode, Error **err) } close(fd); + if (oflag & O_CREAT) { + unlink(path); + } } } -- 1.7.1 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH v2 2/2] qga: unlink just created guest-file if fchmod() or fdopen() fails on it 2013-05-08 15:31 ` [Qemu-devel] [PATCH v2 2/2] qga: unlink just created guest-file if fchmod() or fdopen() fails on it Laszlo Ersek @ 2013-05-08 17:07 ` Eric Blake 0 siblings, 0 replies; 10+ messages in thread From: Eric Blake @ 2013-05-08 17:07 UTC (permalink / raw) To: Laszlo Ersek; +Cc: peter.maydell, aliguori, mdroth, qemu-devel [-- Attachment #1: Type: text/plain, Size: 892 bytes --] On 05/08/2013 09:31 AM, Laszlo Ersek wrote: > We shouldn't allow guest filesystem pollution on error paths. > > Suggested-by: Eric Blake <eblake@redhat.com> > Signed-off-by: Laszlo Ersek <lersek@redhat.com> > --- > qga/commands-posix.c | 3 +++ > 1 files changed, 3 insertions(+), 0 deletions(-) Reviewed-by: Eric Blake <eblake@redhat.com> > > diff --git a/qga/commands-posix.c b/qga/commands-posix.c > index 2eec712..e199738 100644 > --- a/qga/commands-posix.c > +++ b/qga/commands-posix.c > @@ -355,6 +355,9 @@ safe_open_or_create(const char *path, const char *mode, Error **err) > } > > close(fd); > + if (oflag & O_CREAT) { > + unlink(path); > + } > } > } > > -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 621 bytes --] ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH v2 0/2] qga umask fix addenda 2013-05-08 15:31 [Qemu-devel] [PATCH v2 0/2] qga umask fix addenda Laszlo Ersek 2013-05-08 15:31 ` [Qemu-devel] [PATCH v2 1/2] qga: distinguish binary modes in "guest_file_open_modes" map Laszlo Ersek 2013-05-08 15:31 ` [Qemu-devel] [PATCH v2 2/2] qga: unlink just created guest-file if fchmod() or fdopen() fails on it Laszlo Ersek @ 2013-05-10 13:29 ` Luiz Capitulino 2013-05-10 19:30 ` mdroth 3 siblings, 0 replies; 10+ messages in thread From: Luiz Capitulino @ 2013-05-10 13:29 UTC (permalink / raw) To: Laszlo Ersek; +Cc: peter.maydell, aliguori, mdroth, qemu-devel On Wed, 8 May 2013 17:31:34 +0200 Laszlo Ersek <lersek@redhat.com> wrote: > I should have paid more attention to portability and error path cleanup > in the CVE-2013-2007 fix. > > (We continue to assume, like the rest of qemu code, that > qemu_set_cloexec() never fails internally. This should be a reasonable > assumption when the input fd is valid.) Series: Reviewed-by: Luiz Capitulino <lcapitulino@redhat.com> > > Laszlo Ersek (2): > qga: distinguish binary modes in "guest_file_open_modes" map > qga: unlink just created guest-file if fchmod() or fdopen() fails on > it > > qga/commands-posix.c | 25 +++++++++++++++++++------ > 1 files changed, 19 insertions(+), 6 deletions(-) > > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH v2 0/2] qga umask fix addenda 2013-05-08 15:31 [Qemu-devel] [PATCH v2 0/2] qga umask fix addenda Laszlo Ersek ` (2 preceding siblings ...) 2013-05-10 13:29 ` [Qemu-devel] [PATCH v2 0/2] qga umask fix addenda Luiz Capitulino @ 2013-05-10 19:30 ` mdroth 2013-05-10 19:53 ` Laszlo Ersek 3 siblings, 1 reply; 10+ messages in thread From: mdroth @ 2013-05-10 19:30 UTC (permalink / raw) To: Laszlo Ersek; +Cc: peter.maydell, aliguori, qemu-devel On Wed, May 08, 2013 at 05:31:34PM +0200, Laszlo Ersek wrote: > I should have paid more attention to portability and error path cleanup > in the CVE-2013-2007 fix. > > (We continue to assume, like the rest of qemu code, that > qemu_set_cloexec() never fails internally. This should be a reasonable > assumption when the input fd is valid.) > > Laszlo Ersek (2): > qga: distinguish binary modes in "guest_file_open_modes" map > qga: unlink just created guest-file if fchmod() or fdopen() fails on > it Thanks, applied to qga branch: https://github.com/mdroth/qemu/commits/qga > > qga/commands-posix.c | 25 +++++++++++++++++++------ > 1 files changed, 19 insertions(+), 6 deletions(-) > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH v2 0/2] qga umask fix addenda 2013-05-10 19:30 ` mdroth @ 2013-05-10 19:53 ` Laszlo Ersek 2013-05-10 20:09 ` mdroth 0 siblings, 1 reply; 10+ messages in thread From: Laszlo Ersek @ 2013-05-10 19:53 UTC (permalink / raw) To: mdroth; +Cc: peter.maydell, aliguori, qemu-devel On 05/10/13 21:30, mdroth wrote: > On Wed, May 08, 2013 at 05:31:34PM +0200, Laszlo Ersek wrote: >> I should have paid more attention to portability and error path cleanup >> in the CVE-2013-2007 fix. >> >> (We continue to assume, like the rest of qemu code, that >> qemu_set_cloexec() never fails internally. This should be a reasonable >> assumption when the input fd is valid.) >> >> Laszlo Ersek (2): >> qga: distinguish binary modes in "guest_file_open_modes" map >> qga: unlink just created guest-file if fchmod() or fdopen() fails on >> it > > Thanks, applied to qga branch: > > https://github.com/mdroth/qemu/commits/qga Thanks! Can you reword the second commit to include Eric's R-b? <http://lists.nongnu.org/archive/html/qemu-devel/2013-05/msg01179.html> Thanks! Laszlo ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH v2 0/2] qga umask fix addenda 2013-05-10 19:53 ` Laszlo Ersek @ 2013-05-10 20:09 ` mdroth 2013-05-12 16:47 ` Andreas Färber 0 siblings, 1 reply; 10+ messages in thread From: mdroth @ 2013-05-10 20:09 UTC (permalink / raw) To: Laszlo Ersek; +Cc: peter.maydell, aliguori, qemu-devel On Fri, May 10, 2013 at 09:53:27PM +0200, Laszlo Ersek wrote: > On 05/10/13 21:30, mdroth wrote: > > On Wed, May 08, 2013 at 05:31:34PM +0200, Laszlo Ersek wrote: > >> I should have paid more attention to portability and error path cleanup > >> in the CVE-2013-2007 fix. > >> > >> (We continue to assume, like the rest of qemu code, that > >> qemu_set_cloexec() never fails internally. This should be a reasonable > >> assumption when the input fd is valid.) > >> > >> Laszlo Ersek (2): > >> qga: distinguish binary modes in "guest_file_open_modes" map > >> qga: unlink just created guest-file if fchmod() or fdopen() fails on > >> it > > > > Thanks, applied to qga branch: > > > > https://github.com/mdroth/qemu/commits/qga > > Thanks! > > Can you reword the second commit to include Eric's R-b? > <http://lists.nongnu.org/archive/html/qemu-devel/2013-05/msg01179.html> Sure, missed that one. Should be fixed in tree now. > > Thanks! > Laszlo > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH v2 0/2] qga umask fix addenda 2013-05-10 20:09 ` mdroth @ 2013-05-12 16:47 ` Andreas Färber 2013-05-13 11:33 ` mdroth 0 siblings, 1 reply; 10+ messages in thread From: Andreas Färber @ 2013-05-12 16:47 UTC (permalink / raw) To: mdroth, Laszlo Ersek; +Cc: peter.maydell, aliguori, qemu-devel, qemu-stable Am 10.05.2013 22:09, schrieb mdroth: > On Fri, May 10, 2013 at 09:53:27PM +0200, Laszlo Ersek wrote: >> On 05/10/13 21:30, mdroth wrote: >>> On Wed, May 08, 2013 at 05:31:34PM +0200, Laszlo Ersek wrote: >>>> I should have paid more attention to portability and error path cleanup >>>> in the CVE-2013-2007 fix. >>>> >>>> (We continue to assume, like the rest of qemu code, that >>>> qemu_set_cloexec() never fails internally. This should be a reasonable >>>> assumption when the input fd is valid.) >>>> >>>> Laszlo Ersek (2): >>>> qga: distinguish binary modes in "guest_file_open_modes" map >>>> qga: unlink just created guest-file if fchmod() or fdopen() fails on >>>> it >>> >>> Thanks, applied to qga branch: >>> >>> https://github.com/mdroth/qemu/commits/qga >> >> Thanks! >> >> Can you reword the second commit to include Eric's R-b? >> <http://lists.nongnu.org/archive/html/qemu-devel/2013-05/msg01179.html> > > Sure, missed that one. Should be fixed in tree now. Shouldn't at least the unlinking be backported to stable as well? Andreas -- SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH v2 0/2] qga umask fix addenda 2013-05-12 16:47 ` Andreas Färber @ 2013-05-13 11:33 ` mdroth 0 siblings, 0 replies; 10+ messages in thread From: mdroth @ 2013-05-13 11:33 UTC (permalink / raw) To: Andreas Färber Cc: peter.maydell, aliguori, Laszlo Ersek, qemu-devel, qemu-stable On Sun, May 12, 2013 at 06:47:05PM +0200, Andreas Färber wrote: > Am 10.05.2013 22:09, schrieb mdroth: > > On Fri, May 10, 2013 at 09:53:27PM +0200, Laszlo Ersek wrote: > >> On 05/10/13 21:30, mdroth wrote: > >>> On Wed, May 08, 2013 at 05:31:34PM +0200, Laszlo Ersek wrote: > >>>> I should have paid more attention to portability and error path cleanup > >>>> in the CVE-2013-2007 fix. > >>>> > >>>> (We continue to assume, like the rest of qemu code, that > >>>> qemu_set_cloexec() never fails internally. This should be a reasonable > >>>> assumption when the input fd is valid.) > >>>> > >>>> Laszlo Ersek (2): > >>>> qga: distinguish binary modes in "guest_file_open_modes" map > >>>> qga: unlink just created guest-file if fchmod() or fdopen() fails on > >>>> it > >>> > >>> Thanks, applied to qga branch: > >>> > >>> https://github.com/mdroth/qemu/commits/qga > >> > >> Thanks! > >> > >> Can you reword the second commit to include Eric's R-b? > >> <http://lists.nongnu.org/archive/html/qemu-devel/2013-05/msg01179.html> > > > > Sure, missed that one. Should be fixed in tree now. > > Shouldn't at least the unlinking be backported to stable as well? Yes, these are basically updates to the CVE fix, so I think they should all be applied to stable. I'll send PULL today so hopefully we can get them into 1.5 prior to patch freeze for 1.4.2. Otherwise I'll backport from the qga tree. > > Andreas > > -- > SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany > GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg > ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2013-05-13 11:35 UTC | newest] Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2013-05-08 15:31 [Qemu-devel] [PATCH v2 0/2] qga umask fix addenda Laszlo Ersek 2013-05-08 15:31 ` [Qemu-devel] [PATCH v2 1/2] qga: distinguish binary modes in "guest_file_open_modes" map Laszlo Ersek 2013-05-08 15:31 ` [Qemu-devel] [PATCH v2 2/2] qga: unlink just created guest-file if fchmod() or fdopen() fails on it Laszlo Ersek 2013-05-08 17:07 ` Eric Blake 2013-05-10 13:29 ` [Qemu-devel] [PATCH v2 0/2] qga umask fix addenda Luiz Capitulino 2013-05-10 19:30 ` mdroth 2013-05-10 19:53 ` Laszlo Ersek 2013-05-10 20:09 ` mdroth 2013-05-12 16:47 ` Andreas Färber 2013-05-13 11:33 ` mdroth
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.