| Message ID | 20230925194040.68592-4-vsementsov@yandex-team.ru |
|---|---|
| State | New |
| Headers | show |
| Series | coverity fixes | expand |
On Mon, 25 Sept 2023 at 20:43, Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> wrote: > > Prefer clear assertions instead of possible array overflow. > > Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> > --- > util/filemonitor-inotify.c | 21 +++++++++++++-------- > 1 file changed, 13 insertions(+), 8 deletions(-) > > diff --git a/util/filemonitor-inotify.c b/util/filemonitor-inotify.c > index 2c45f7f176..09ef240174 100644 > --- a/util/filemonitor-inotify.c > +++ b/util/filemonitor-inotify.c > @@ -81,16 +81,21 @@ static void qemu_file_monitor_watch(void *arg) > > /* Loop over all events in the buffer */ > while (used < len) { > - struct inotify_event *ev = > - (struct inotify_event *)(buf + used); > - const char *name = ev->len ? ev->name : ""; > - QFileMonitorDir *dir = g_hash_table_lookup(mon->idmap, > - GINT_TO_POINTER(ev->wd)); > - uint32_t iev = ev->mask & > - (IN_CREATE | IN_MODIFY | IN_DELETE | IN_IGNORED | > - IN_MOVED_TO | IN_MOVED_FROM | IN_ATTRIB); > + const char *name; > + QFileMonitorDir *dir; > + uint32_t iev; > int qev; > gsize i; > + struct inotify_event *ev = (struct inotify_event *)(buf + used); > + > + assert(len - used >= sizeof(struct inotify_event)); > + assert(len - used - sizeof(struct inotify_event) >= ev->len); So this is something we can assert because we trust the kernel (which is what's on the other end of the inotify fd) to only give us a valid buffer with complete event records in it, not partial pieces, right? If so, then we should say so in a comment. (FWIW in the online Coverity Scan we just marked this one as a false-positive.) thanks -- PMM
diff --git a/util/filemonitor-inotify.c b/util/filemonitor-inotify.c index 2c45f7f176..09ef240174 100644 --- a/util/filemonitor-inotify.c +++ b/util/filemonitor-inotify.c @@ -81,16 +81,21 @@ static void qemu_file_monitor_watch(void *arg) /* Loop over all events in the buffer */ while (used < len) { - struct inotify_event *ev = - (struct inotify_event *)(buf + used); - const char *name = ev->len ? ev->name : ""; - QFileMonitorDir *dir = g_hash_table_lookup(mon->idmap, - GINT_TO_POINTER(ev->wd)); - uint32_t iev = ev->mask & - (IN_CREATE | IN_MODIFY | IN_DELETE | IN_IGNORED | - IN_MOVED_TO | IN_MOVED_FROM | IN_ATTRIB); + const char *name; + QFileMonitorDir *dir; + uint32_t iev; int qev; gsize i; + struct inotify_event *ev = (struct inotify_event *)(buf + used); + + assert(len - used >= sizeof(struct inotify_event)); + assert(len - used - sizeof(struct inotify_event) >= ev->len); + + name = ev->len ? ev->name : ""; + dir = g_hash_table_lookup(mon->idmap, GINT_TO_POINTER(ev->wd)); + iev = ev->mask & + (IN_CREATE | IN_MODIFY | IN_DELETE | IN_IGNORED | + IN_MOVED_TO | IN_MOVED_FROM | IN_ATTRIB); used += sizeof(struct inotify_event) + ev->len;
Prefer clear assertions instead of possible array overflow. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> --- util/filemonitor-inotify.c | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-)