diff mbox

[1/1] event_notifier: prevent accidental use after close

Message ID 20170302181308.69662-1-pasic@linux.vnet.ibm.com (mailing list archive)
State New, archived
Headers show

Commit Message

Halil Pasic March 2, 2017, 6:13 p.m. UTC
Let's set the handles to the underlying facilities to their extremal
value so no accidental misuse can happen, and to make it obvious that the
notifier is dysfunctional. E.g. if we just close an fd but do not touch
the int holding the fd eventually a read/write could succeed again when
the fd gets reused, and corrupt the file addressed by the fd.

Signed-off-by: Halil Pasic <pasic@linux.vnet.ibm.com>
---

No strong feelings about this, but obviously, I do think it's worth a
try.

The one who brought this unfortunate possibility to my attention was
Michael Tsirkin.
---
 util/event_notifier-posix.c | 2 ++
 util/event_notifier-win32.c | 1 +
 2 files changed, 3 insertions(+)

Comments

Stefan Hajnoczi March 3, 2017, 2:23 a.m. UTC | #1
On Thu, Mar 02, 2017 at 07:13:08PM +0100, Halil Pasic wrote:
> Let's set the handles to the underlying facilities to their extremal
> value so no accidental misuse can happen, and to make it obvious that the
> notifier is dysfunctional. E.g. if we just close an fd but do not touch
> the int holding the fd eventually a read/write could succeed again when
> the fd gets reused, and corrupt the file addressed by the fd.
> 
> Signed-off-by: Halil Pasic <pasic@linux.vnet.ibm.com>
> ---
> 
> No strong feelings about this, but obviously, I do think it's worth a
> try.
> 
> The one who brought this unfortunate possibility to my attention was
> Michael Tsirkin.
> ---
>  util/event_notifier-posix.c | 2 ++
>  util/event_notifier-win32.c | 1 +
>  2 files changed, 3 insertions(+)

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
diff mbox

Patch

diff --git a/util/event_notifier-posix.c b/util/event_notifier-posix.c
index 7e40252..acdbe3b 100644
--- a/util/event_notifier-posix.c
+++ b/util/event_notifier-posix.c
@@ -81,8 +81,10 @@  void event_notifier_cleanup(EventNotifier *e)
 {
     if (e->rfd != e->wfd) {
         close(e->rfd);
+        e->rfd = -1;
     }
     close(e->wfd);
+    e->wfd = -1;
 }
 
 int event_notifier_get_fd(const EventNotifier *e)
diff --git a/util/event_notifier-win32.c b/util/event_notifier-win32.c
index 519fb59..62c53b0 100644
--- a/util/event_notifier-win32.c
+++ b/util/event_notifier-win32.c
@@ -25,6 +25,7 @@  int event_notifier_init(EventNotifier *e, int active)
 void event_notifier_cleanup(EventNotifier *e)
 {
     CloseHandle(e->event);
+    e->event = NULL;
 }
 
 HANDLE event_notifier_get_handle(EventNotifier *e)