This reverts upstream commit 58c9b016e12855286370dfb704c08498edbc857a, which was imported by commit b44bae1bd6e953127cc01e86aaa3bb2b09a0dc2d
Eliminates the racy refcount-based disposal and restores the previous epmutex serialization between ep_free and eventpoll_release_file. It also removes the struct fields, which the next commit adds back. Signed-off-by: Eva Kurchatova <[email protected]> https://virtuozzo.atlassian.net/browse/VSTOR-137490 Feature: fix epoll cve --- fs/eventpoll.c | 195 ++++++++++++++++++------------------------------- 1 file changed, 72 insertions(+), 123 deletions(-) diff --git a/fs/eventpoll.c b/fs/eventpoll.c index 5675fcbd0901..7e6a5b68c8ba 100644 --- a/fs/eventpoll.c +++ b/fs/eventpoll.c @@ -57,7 +57,13 @@ * we need a lock that will allow us to sleep. This lock is a * mutex (ep->mtx). It is acquired during the event transfer loop, * during epoll_ctl(EPOLL_CTL_DEL) and during eventpoll_release_file(). - * The epmutex is acquired when inserting an epoll fd onto another epoll + * Then we also need a global mutex to serialize eventpoll_release_file() + * and ep_free(). + * This mutex is acquired by ep_free() during the epoll file + * cleanup path and it is also acquired by eventpoll_release_file() + * if a file has been pushed inside an epoll set and it is then + * close()d without a previous call to epoll_ctl(EPOLL_CTL_DEL). + * It is also acquired when inserting an epoll fd onto another epoll * fd. We do this so that we walk the epoll tree and ensure that this * insertion does not create a cycle of epoll file descriptors, which * could lead to deadlock. We need a global mutex to prevent two @@ -147,13 +153,6 @@ struct epitem { /* The file descriptor information this item refers to */ struct epoll_filefd ffd; - /* - * Protected by file->f_lock, true for to-be-released epitem already - * removed from the "struct file" items list; together with - * eventpoll->refcount orchestrates "struct eventpoll" disposal - */ - bool dying; - /* List containing poll wait queues */ struct eppoll_entry *pwqlist; @@ -219,12 +218,6 @@ struct eventpoll { struct hlist_head refs; u8 loop_check_depth; - /* - * usage count, used together with epitem->dying to - * orchestrate the disposal of this struct - */ - refcount_t refcount; - #ifdef CONFIG_NET_RX_BUSY_POLL /* used to track busy poll napi_id */ unsigned int napi_id; @@ -248,7 +241,9 @@ struct ep_pqueue { /* Maximum number of epoll watched descriptors, per user */ static long max_user_watches __read_mostly; -/* Used for cycles detection */ +/* + * This mutex is used to serialize ep_free() and eventpoll_release_file(). + */ static DEFINE_MUTEX(epmutex); static u64 loop_check_gen = 0; @@ -563,7 +558,8 @@ static void ep_remove_wait_queue(struct eppoll_entry *pwq) /* * This function unregisters poll callbacks from the associated file - * descriptor. Must be called with "mtx" held. + * descriptor. Must be called with "mtx" held (or "epmutex" if called from + * ep_free). */ static void ep_unregister_pollwait(struct eventpoll *ep, struct epitem *epi) { @@ -686,40 +682,11 @@ static void epi_rcu_free(struct rcu_head *head) kmem_cache_free(epi_cache, epi); } -static void ep_get(struct eventpoll *ep) -{ - refcount_inc(&ep->refcount); -} - -/* - * Returns true if the event poll can be disposed - */ -static bool ep_refcount_dec_and_test(struct eventpoll *ep) -{ - if (!refcount_dec_and_test(&ep->refcount)) - return false; - - WARN_ON_ONCE(!RB_EMPTY_ROOT(&ep->rbr.rb_root)); - return true; -} - -static void ep_free(struct eventpoll *ep) -{ - mutex_destroy(&ep->mtx); - free_uid(ep->user); - wakeup_source_unregister(ep->ws); - kfree(ep); -} - /* * Removes a "struct epitem" from the eventpoll RB tree and deallocates * all the associated resources. Must be called with "mtx" held. - * If the dying flag is set, do the removal only if force is true. - * This prevents ep_clear_and_put() from dropping all the ep references - * while running concurrently with eventpoll_release_file(). - * Returns true if the eventpoll can be disposed. */ -static bool __ep_remove(struct eventpoll *ep, struct epitem *epi, bool force) +static int ep_remove(struct eventpoll *ep, struct epitem *epi) { struct file *file = epi->ffd.file; struct epitems_head *to_free; @@ -734,11 +701,6 @@ static bool __ep_remove(struct eventpoll *ep, struct epitem *epi, bool force) /* Remove the current item from the list of epoll hooks */ spin_lock(&file->f_lock); - if (epi->dying && !force) { - spin_unlock(&file->f_lock); - return false; - } - to_free = NULL; head = file->f_ep; if (head->first == &epi->fllink && !epi->fllink.next) { @@ -772,28 +734,28 @@ static bool __ep_remove(struct eventpoll *ep, struct epitem *epi, bool force) call_rcu(&epi->rcu, epi_rcu_free); percpu_counter_dec(&ep->user->epoll_watches); - return ep_refcount_dec_and_test(ep); -} -/* - * ep_remove variant for callers owing an additional reference to the ep - */ -static void ep_remove_safe(struct eventpoll *ep, struct epitem *epi) -{ - WARN_ON_ONCE(__ep_remove(ep, epi, false)); + return 0; } -static void ep_clear_and_put(struct eventpoll *ep) +static void ep_free(struct eventpoll *ep) { - struct rb_node *rbp, *next; + struct rb_node *rbp; struct epitem *epi; - bool dispose; /* We need to release all tasks waiting for these file */ if (waitqueue_active(&ep->poll_wait)) ep_poll_safewake(ep, NULL, 0); - mutex_lock(&ep->mtx); + /* + * We need to lock this because we could be hit by + * eventpoll_release_file() while we're freeing the "struct eventpoll". + * We do not need to hold "ep->mtx" here because the epoll file + * is on the way to be removed and no one has references to it + * anymore. The only hit might come from eventpoll_release_file() but + * holding "epmutex" is sufficient here. + */ + mutex_lock(&epmutex); /* * Walks through the whole tree by unregistering poll callbacks. @@ -806,25 +768,26 @@ static void ep_clear_and_put(struct eventpoll *ep) } /* - * Walks through the whole tree and try to free each "struct epitem". - * Note that ep_remove_safe() will not remove the epitem in case of a - * racing eventpoll_release_file(); the latter will do the removal. - * At this point we are sure no poll callbacks will be lingering around. - * Since we still own a reference to the eventpoll struct, the loop can't - * dispose it. + * Walks through the whole tree by freeing each "struct epitem". At this + * point we are sure no poll callbacks will be lingering around, and also by + * holding "epmutex" we can be sure that no file cleanup code will hit + * us during this operation. So we can avoid the lock on "ep->lock". + * We do not need to lock ep->mtx, either, we only do it to prevent + * a lockdep warning. */ - for (rbp = rb_first_cached(&ep->rbr); rbp; rbp = next) { - next = rb_next(rbp); + mutex_lock(&ep->mtx); + while ((rbp = rb_first_cached(&ep->rbr)) != NULL) { epi = rb_entry(rbp, struct epitem, rbn); - ep_remove_safe(ep, epi); + ep_remove(ep, epi); cond_resched(); } - - dispose = ep_refcount_dec_and_test(ep); mutex_unlock(&ep->mtx); - if (dispose) - ep_free(ep); + mutex_unlock(&epmutex); + mutex_destroy(&ep->mtx); + free_uid(ep->user); + wakeup_source_unregister(ep->ws); + kfree(ep); } static int ep_eventpoll_release(struct inode *inode, struct file *file) @@ -832,7 +795,7 @@ static int ep_eventpoll_release(struct inode *inode, struct file *file) struct eventpoll *ep = file->private_data; if (ep) - ep_clear_and_put(ep); + ep_free(ep); return 0; } @@ -980,34 +943,33 @@ void eventpoll_release_file(struct file *file) { struct eventpoll *ep; struct epitem *epi; - bool dispose; + struct hlist_node *next; /* - * Use the 'dying' flag to prevent a concurrent ep_clear_and_put() from - * touching the epitems list before eventpoll_release_file() can access - * the ep->mtx. + * We don't want to get "file->f_lock" because it is not + * necessary. It is not necessary because we're in the "struct file" + * cleanup path, and this means that no one is using this file anymore. + * So, for example, epoll_ctl() cannot hit here since if we reach this + * point, the file counter already went to zero and fget() would fail. + * The only hit might come from ep_free() but by holding the mutex + * will correctly serialize the operation. We do need to acquire + * "ep->mtx" after "epmutex" because ep_remove() requires it when called + * from anywhere but ep_free(). + * + * Besides, ep_remove() acquires the lock, so we can't hold it here. */ -again: - spin_lock(&file->f_lock); - if (file->f_ep && file->f_ep->first) { - epi = hlist_entry(file->f_ep->first, struct epitem, fllink); - epi->dying = true; - spin_unlock(&file->f_lock); - - /* - * ep access is safe as we still own a reference to the ep - * struct - */ + mutex_lock(&epmutex); + if (unlikely(!file->f_ep)) { + mutex_unlock(&epmutex); + return; + } + hlist_for_each_entry_safe(epi, next, file->f_ep, fllink) { ep = epi->ep; - mutex_lock(&ep->mtx); - dispose = __ep_remove(ep, epi, true); + mutex_lock_nested(&ep->mtx, 0); + ep_remove(ep, epi); mutex_unlock(&ep->mtx); - - if (dispose) - ep_free(ep); - goto again; } - spin_unlock(&file->f_lock); + mutex_unlock(&epmutex); } static int ep_alloc(struct eventpoll **pep) @@ -1030,7 +992,6 @@ static int ep_alloc(struct eventpoll **pep) ep->rbr = RB_ROOT_CACHED; ep->ovflist = EP_UNACTIVE_PTR; ep->user = user; - refcount_set(&ep->refcount, 1); *pep = ep; @@ -1299,10 +1260,10 @@ static int ep_poll_callback(wait_queue_entry_t *wait, unsigned mode, int sync, v */ list_del_init(&wait->entry); /* - * ->whead != NULL protects us from the race with - * ep_clear_and_put() or ep_remove(), ep_remove_wait_queue() - * takes whead->lock held by the caller. Once we nullify it, - * nothing protects ep/epi or even wait. + * ->whead != NULL protects us from the race with ep_free() + * or ep_remove(), ep_remove_wait_queue() takes whead->lock + * held by the caller. Once we nullify it, nothing protects + * ep/epi or even wait. */ smp_store_release(&ep_pwq_from_wait(wait)->whead, NULL); } @@ -1572,22 +1533,16 @@ static int ep_insert(struct eventpoll *ep, const struct epoll_event *event, if (tep) mutex_unlock(&tep->mtx); - /* - * ep_remove_safe() calls in the later error paths can't lead to - * ep_free() as the ep file itself still holds an ep reference. - */ - ep_get(ep); - /* now check if we've created too many backpaths */ if (unlikely(full_check && reverse_path_check())) { - ep_remove_safe(ep, epi); + ep_remove(ep, epi); return -EINVAL; } if (epi->event.events & EPOLLWAKEUP) { error = ep_create_wakeup_source(epi); if (error) { - ep_remove_safe(ep, epi); + ep_remove(ep, epi); return error; } } @@ -1611,7 +1566,7 @@ static int ep_insert(struct eventpoll *ep, const struct epoll_event *event, * high memory pressure. */ if (unlikely(!epq.epi)) { - ep_remove_safe(ep, epi); + ep_remove(ep, epi); return -ENOMEM; } @@ -2117,7 +2072,7 @@ static int do_epoll_create(int flags) out_free_fd: put_unused_fd(fd); out_free_ep: - ep_clear_and_put(ep); + ep_free(ep); return error; } @@ -2259,16 +2214,10 @@ int do_epoll_ctl(int epfd, int op, int fd, struct epoll_event *epds, error = -EEXIST; break; case EPOLL_CTL_DEL: - if (epi) { - /* - * The eventpoll itself is still alive: the refcount - * can't go to zero here. - */ - ep_remove_safe(ep, epi); - error = 0; - } else { + if (epi) + error = ep_remove(ep, epi); + else error = -ENOENT; - } break; case EPOLL_CTL_MOD: if (epi) { -- 2.55.0 _______________________________________________ Devel mailing list [email protected] https://lists.openvz.org/mailman/listinfo/devel
