From d561ecfbc9a17ca47721cd6d7742d2b1835f8ae3 Mon Sep 17 00:00:00 2001 From: Mathieu Desnoyers Date: Thu, 26 Jun 2014 14:52:03 -0400 Subject: [PATCH] Fix: statedump: check whether "files" is NULL, RCU semantic fix We need to check if p->files is NULL before passing it to files_fdtable(). Moreover, since the fdt is now protected by RCU, we have to assume it can change between the read from lttng_enumerate_task_fd() and the internal in-kernel read in iterate_fd(). Therefore, move this rcu dereference into lttng_dump_one_fd(), and perform the appropriate checks on max fds. Fixes #799 Signed-off-by: Mathieu Desnoyers --- lttng-statedump-impl.c | 24 ++++++++++++++++++++---- 1 file changed, 20 insertions(+), 4 deletions(-) diff --git a/lttng-statedump-impl.c b/lttng-statedump-impl.c index d0e641a9..dad51dda 100644 --- a/lttng-statedump-impl.c +++ b/lttng-statedump-impl.c @@ -79,7 +79,7 @@ struct lttng_fd_ctx { char *page; struct lttng_session *session; struct task_struct *p; - struct fdtable *fdt; + struct files_struct *files; }; /* @@ -219,13 +219,24 @@ int lttng_dump_one_fd(const void *p, struct file *file, unsigned int fd) const struct lttng_fd_ctx *ctx = p; const char *s = d_path(&file->f_path, ctx->page, PAGE_SIZE); unsigned int flags = file->f_flags; + struct fdtable *fdt; /* * We don't expose kernel internal flags, only userspace-visible * flags. */ flags &= ~FMODE_NONOTIFY; - if (test_bit(fd, ctx->fdt->close_on_exec)) + fdt = files_fdtable(ctx->files); + /* + * We need to check here again whether fd is within the fdt + * max_fds range, because we might be seeing a different + * files_fdtable() than iterate_fd(), assuming only RCU is + * protecting the read. In reality, iterate_fd() holds + * file_lock, which should ensure the fdt does not change while + * the lock is taken, but we are not aware whether this is + * guaranteed or not, so play safe. + */ + if (fd < fdt->max_fds && test_bit(fd, fdt->close_on_exec)) flags |= O_CLOEXEC; if (IS_ERR(s)) { struct dentry *dentry = file->f_path.dentry; @@ -248,10 +259,15 @@ void lttng_enumerate_task_fd(struct lttng_session *session, struct task_struct *p, char *tmp) { struct lttng_fd_ctx ctx = { .page = tmp, .session = session, .p = p }; + struct files_struct *files; task_lock(p); - ctx.fdt = files_fdtable(p->files); - lttng_iterate_fd(p->files, 0, lttng_dump_one_fd, &ctx); + files = p->files; + if (!files) + goto end; + ctx.files = files; + lttng_iterate_fd(files, 0, lttng_dump_one_fd, &ctx); +end: task_unlock(p); } -- 2.34.1