ACK: [SRU][A/B][PATCH] fsnotify: Fix fsnotify_mark_connector race

[Stefan Bader]

On 27.04.2018 13:36, Seyeong Kim wrote:
> From: Robert Kolchmeyer <rkolchmeyer at google.com>
> 
> BugLink: http://bugs.launchpad.net/bugs/1765564
> 
> 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 moving the connector's 'destroy_next' field
> into the object pointer union. This should work since the object pointer
> access is protected by both a spinlock and the value of the 'flags'
> field, and the 'flags' field is cleared while holding the spinlock in
> fsnotify_put_mark() before 'destroy_next' is updated. It shouldn't be
> possible for another thread to accidentally read from the object pointer
> after the 'destroy_next' field is updated.
> 
> 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.
> 
> References: https://bugzilla.kernel.org/show_bug.cgi?id=199437
> Fixes: 08991e83b7286635167bab40927665a90fb00d81
> CC: stable at vger.kernel.org
> Signed-off-by: Robert Kolchmeyer <rkolchmeyer at google.com>
> Signed-off-by: Jan Kara <jack at suse.cz>
> (cherry picked from commit d90a10e2444ba5a351fa695917258ff4c5709fa5)
> Signed-off-by: Seyeong Kim <seyeong.kim at canonical.com>
Acked-by: Stefan Bader <stefan.bader at canonical.com>

> ---

For artful/bionic, the azure kernels got fixed individually but should be able
to sync with next rebase.

-Stefan
>  include/linux/fsnotify_backend.h | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/include/linux/fsnotify_backend.h b/include/linux/fsnotify_backend.h
> index 067d52e..d719194 100644
> --- a/include/linux/fsnotify_backend.h
> +++ b/include/linux/fsnotify_backend.h
> @@ -217,12 +217,10 @@ struct fsnotify_mark_connector {
>  	union {	/* Object pointer [lock] */
>  		struct inode *inode;
>  		struct vfsmount *mnt;
> -	};
> -	union {
> -		struct hlist_head list;
>  		/* Used listing heads to free after srcu period expires */
>  		struct fsnotify_mark_connector *destroy_next;
>  	};
> +	struct hlist_head list;
>  };
>  
>  /*
> 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: OpenPGP digital signature
URL: <https://lists.ubuntu.com/archives/kernel-team/attachments/20180522/f47d2110/attachment.sig>