APPLIED: [SRU Focal] fget: check that the fd still exists after getting a ref to it

[Stefan Bader]

On 21.01.22 15:26, Thadeu Lima de Souza Cascardo wrote:
> From: Linus Torvalds <torvalds at linux-foundation.org>
> 
> Jann Horn points out that there is another possible race wrt Unix domain
> socket garbage collection, somewhat reminiscent of the one fixed in
> commit cbcf01128d0a ("af_unix: fix garbage collect vs MSG_PEEK").
> 
> See the extended comment about the garbage collection requirements added
> to unix_peek_fds() by that commit for details.
> 
> The race comes from how we can locklessly look up a file descriptor just
> as it is in the process of being closed, and with the right artificial
> timing (Jann added a few strategic 'mdelay(500)' calls to do that), the
> Unix domain socket garbage collector could see the reference count
> decrement of the close() happen before fget() took its reference to the
> file and the file was attached onto a new file descriptor.
> 
> This is all (intentionally) correct on the 'struct file *' side, with
> RCU lookups and lockless reference counting very much part of the
> design.  Getting that reference count out of order isn't a problem per
> se.
> 
> But the garbage collector can get confused by seeing this situation of
> having seen a file not having any remaining external references and then
> seeing it being attached to an fd.
> 
> In commit cbcf01128d0a ("af_unix: fix garbage collect vs MSG_PEEK") the
> fix was to serialize the file descriptor install with the garbage
> collector by taking and releasing the unix_gc_lock.
> 
> That's not really an option here, but since this all happens when we are
> in the process of looking up a file descriptor, we can instead simply
> just re-check that the file hasn't been closed in the meantime, and just
> re-do the lookup if we raced with a concurrent close() of the same file
> descriptor.
> 
> Reported-and-tested-by: Jann Horn <jannh at google.com>
> Acked-by: Miklos Szeredi <mszeredi at redhat.com>
> Signed-off-by: Linus Torvalds <torvalds at linux-foundation.org>
> (backported from commit 054aa8d439b9185d4f5eb9a90282d1ce74772969)
> [cascardo: __fcheck_files was renamed to files_lookup_fd_raw at
>   bebf684bf330 ("file: Rename __fcheck_files to files_lookup_fd_raw")]
> CVE-2021-4083
> Signed-off-by: Thadeu Lima de Souza Cascardo <cascardo at canonical.com>
> ---

Applied to focal:linux/master-next. Thanks.

-Stefan

>   fs/file.c | 4 ++++
>   1 file changed, 4 insertions(+)
> 
> diff --git a/fs/file.c b/fs/file.c
> index 3c4b9cf7ef17..5913a5db9c2a 100644
> --- a/fs/file.c
> +++ b/fs/file.c
> @@ -698,6 +698,10 @@ static struct file *__fget(unsigned int fd, fmode_t mask, unsigned int refs)
>   			file = NULL;
>   		else if (!get_file_rcu_many(file, refs))
>   			goto loop;
> +		else if (__fcheck_files(files, fd) != file) {
> +			fput_many(file, refs);
> +			goto loop;
> +		}
>   	}
>   	rcu_read_unlock();
>   

-------------- next part --------------
A non-text attachment was scrubbed...
Name: OpenPGP_signature
Type: application/pgp-signature
Size: 833 bytes
Desc: OpenPGP digital signature
URL: <https://lists.ubuntu.com/archives/kernel-team/attachments/20220126/585b0da1/attachment-0001.sig>