From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-vk0-f73.google.com ([209.85.213.73]:50748 "EHLO mail-vk0-f73.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752922AbeDSAF3 (ORCPT ); Wed, 18 Apr 2018 20:05:29 -0400 Received: by mail-vk0-f73.google.com with SMTP id v145so2338361vkv.17 for ; Wed, 18 Apr 2018 17:05:29 -0700 (PDT) MIME-Version: 1.0 Date: Wed, 18 Apr 2018 17:05:25 -0700 Message-Id: <20180419000525.21673-1-rkolchmeyer@google.com> Subject: [PATCH] fsnotify: Fix fsnotify_mark_connector race From: Robert Kolchmeyer To: jack@suse.cz, amir73il@gmail.com, mszeredi@redhat.com, linux-fsdevel@vger.kernel.org, rkolchmeyer@google.com Cc: linux-fsdevel@vger.kernel.org, Robert Kolchmeyer Content-Type: text/plain; charset="UTF-8" Sender: linux-fsdevel-owner@vger.kernel.org List-ID: fsnotify() acquires a reference to a fsnotify_mark_connector through the SRCU-protected pointer to_tell->i_fsnotify_marks. However, it appears that no precautions are taken in fsnotify_put_mark() to ensure that fsnotify() drops its reference to this fsnotify_mark_connector before assigning a value to its 'destroy_next' field. This can result in fsnotify_put_mark() assigning a value to a connector's 'destroy_next' field right before fsnotify() tries to traverse the linked list referenced by the connector's 'list' field. Since these two fields are members of the same union, this behavior results in a kernel panic. This issue is resolved by calling synchronize_srcu() before updating the connector's 'destroy_next' field in fsnotify_put_mark(). Since the relevant section of fsnotify() occurs in a SRCU read-side critical section, this will force fsnotify_put_mark() to wait for fsnotify() to finish operating on the connector before updating its 'destroy_next' field. Since fsnotify_put_mark() removes references to the connector before assigning its 'destroy_next' field, it shouldn't be possible for another thread to acquire a reference to the connector while fsnotify_put_mark() waits for fsnotify() to finish its work. The offending behavior here is extremely unlikely; since fsnotify_put_mark() removes references to a connector (specifically, it ensures that the connector is unreachable from the inode it was formerly attached to) before updating its 'destroy_next' field, a sizeable chunk of code in fsnotify_put_mark() has to execute in the short window between when fsnotify() acquires the connector reference and saves the value of its 'list' field. On the HEAD kernel, I've only been able to reproduce this by inserting a udelay(1) in fsnotify(). However, I've been able to reproduce this issue without inserting a udelay(1) anywhere on older unmodified release kernels, so I believe it's worth fixing at HEAD. Fixes bug 199437: https://bugzilla.kernel.org/show_bug.cgi?id=199437 Fixes: 08991e83b7286635167bab40927665a90fb00d81 Signed-off-by: Robert Kolchmeyer --- fs/notify/mark.c | 1 + 1 file changed, 1 insertion(+) diff --git a/fs/notify/mark.c b/fs/notify/mark.c index e9191b416434..358fc7de1e86 100644 --- a/fs/notify/mark.c +++ b/fs/notify/mark.c @@ -227,6 +227,7 @@ void fsnotify_put_mark(struct fsnotify_mark *mark) iput(inode); if (free_conn) { + synchronize_srcu(&fsnotify_mark_srcu); spin_lock(&destroy_lock); conn->destroy_next = connector_destroy_list; connector_destroy_list = conn; -- 2.17.0.484.g0c8726318c-goog