From patchwork Mon Apr 17 14:03:26 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Tetsuo Handa X-Patchwork-Id: 13214086 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id C9AD1C77B70 for ; Mon, 17 Apr 2023 14:04:10 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 622276B0071; Mon, 17 Apr 2023 10:04:10 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 5AB256B0072; Mon, 17 Apr 2023 10:04:10 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 4505F8E0001; Mon, 17 Apr 2023 10:04:10 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0012.hostedemail.com [216.40.44.12]) by kanga.kvack.org (Postfix) with ESMTP id 2DCE96B0071 for ; Mon, 17 Apr 2023 10:04:10 -0400 (EDT) Received: from smtpin30.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay03.hostedemail.com (Postfix) with ESMTP id C4794A0530 for ; Mon, 17 Apr 2023 14:04:09 +0000 (UTC) X-FDA: 80691052218.30.4A63E77 Received: from www262.sakura.ne.jp (www262.sakura.ne.jp [202.181.97.72]) by imf20.hostedemail.com (Postfix) with ESMTP id C7E191C0030 for ; Mon, 17 Apr 2023 14:04:05 +0000 (UTC) Authentication-Results: imf20.hostedemail.com; dkim=none; dmarc=none; spf=none (imf20.hostedemail.com: domain of penguin-kernel@I-love.SAKURA.ne.jp has no SPF policy when checking 202.181.97.72) smtp.mailfrom=penguin-kernel@I-love.SAKURA.ne.jp ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1681740246; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=ACOZ5IduHudgt/mkxF/AfQVNCpg83etlj0ATzKYREAQ=; b=W29R3jCadi8A/tEphhEN4vijhMXCWSeqN70IxeR0cau53noyz+tqsM3c7grbAYCHK/HASu wQneUCyoXWQumACIB9/TdjIghISYOhYYGIso2Z34Azvz3pq+FzhUjYSOqde0gwcgPviElD qZn4eDBKeGLWMAr0tF7Sd3CawMpx3vA= ARC-Authentication-Results: i=1; imf20.hostedemail.com; dkim=none; dmarc=none; spf=none (imf20.hostedemail.com: domain of penguin-kernel@I-love.SAKURA.ne.jp has no SPF policy when checking 202.181.97.72) smtp.mailfrom=penguin-kernel@I-love.SAKURA.ne.jp ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1681740246; a=rsa-sha256; cv=none; b=mziwHA0KveP0BYsjbuaqdJZU8lR0KR3uc6+7THvGgTiLtOHDZ7oDZjNGaWFWMQpuzsXjTY lrlULlp2gTIL7XBi+jsZ2oZUEJ4wSKAZslg7XVvDCgWab85DFQuHR67U51y/UVXbHZ/kB5 eE2qZ8SD/o/9u3Bjsa++luMYLhcLSn8= Received: from fsav112.sakura.ne.jp (fsav112.sakura.ne.jp [27.133.134.239]) by www262.sakura.ne.jp (8.15.2/8.15.2) with ESMTP id 33HE3Toj003234; Mon, 17 Apr 2023 23:03:29 +0900 (JST) (envelope-from penguin-kernel@I-love.SAKURA.ne.jp) Received: from www262.sakura.ne.jp (202.181.97.72) by fsav112.sakura.ne.jp (F-Secure/fsigk_smtp/550/fsav112.sakura.ne.jp); Mon, 17 Apr 2023 23:03:29 +0900 (JST) X-Virus-Status: clean(F-Secure/fsigk_smtp/550/fsav112.sakura.ne.jp) Received: from [192.168.1.6] (M106072142033.v4.enabler.ne.jp [106.72.142.33]) (authenticated bits=0) by www262.sakura.ne.jp (8.15.2/8.15.2) with ESMTPSA id 33HE3T8F003231 (version=TLSv1.2 cipher=AES256-GCM-SHA384 bits=256 verify=NO); Mon, 17 Apr 2023 23:03:29 +0900 (JST) (envelope-from penguin-kernel@I-love.SAKURA.ne.jp) Message-ID: Date: Mon, 17 Apr 2023 23:03:26 +0900 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101 Thunderbird/102.10.0 Subject: [PATCH] vfs: allow using kernel buffer during fiemap operation Content-Language: en-US From: Tetsuo Handa To: syzbot , ntfs3@lists.linux.dev, syzkaller-bugs@googlegroups.com, Mark Fasheh , Alexander Viro , Christian Brauner , Konstantin Komarov , linux-fsdevel Cc: Hillf Danton , linux-mm , trix@redhat.com, ndesaulniers@google.com, nathan@kernel.org References: <000000000000e2102c05eeaf9113@google.com> <00000000000031b80705ef5d33d1@google.com> In-Reply-To: X-Rspamd-Server: rspam07 X-Rspamd-Queue-Id: C7E191C0030 X-Rspam-User: X-Stat-Signature: zwj47oke1tfmzptz9xxbqobe9hyrdisb X-HE-Tag: 1681740245-433752 X-HE-Meta: U2FsdGVkX18yyVIP9o+zUMS5xm0SIWqQCynbiXYbrn70XqB+6a32FWVeqKILEZ2CHvFKSAH/Svce4WdDXbcqaryk4ie564Y0BpaN/uxjCDw+dDggTQdXpu4snsk/ASYshT8WeNVA8pVW+KYPx/YMY2VJVYhTOm+G8HBUo47+gKkj7V0y/q/49IgcbOt0obDFjffUeiGNHCY58Qd7HigSlyaJDUKxnY3G7vAfLNG1z6idfTxwcUQtqEchlKZ4TXMmhXSFphu3MZ5uWvgVISVJazakIY3uIWzBYjgo6La/ax6wHULewtCQb9RivNLes4JIBMOlIL+tNZkWVVvrTsgrEI9azPnc25P+lIDw1KSnXVVm5etpAnu6Xut9iDzIBcbbzHY3fnaXWxHlg7TdkcXRvGVmhjGz/2Nh6HqJ2tNj4XMoQTsQS3s9m+pGyeOdICgMKGKstVjqw2UXStqjqzZtxdKNScnqKCFlB4CaqVdZttnkdHUpWsNsD4OOyW4MbD0GOKnUvPxgczcJHe4uaDpZxpPbejtHCWvIuqT8rDgY38ws+MHQnKVZJ6UaopSztcV/n+LIp4cK1ByTU/pEPVYhSk/Lz4rGMuawkMK09bJE6lZab5ncN1KiUrTY+6B/wpvCL5vmnx08eP3gXhmCk8BGr3+//no5PfatRKdnSHbf58cozC8ACOYQrjuAEvByoGIuxxwwiIBEM5Qx582uOCMdiNaJ63zHhdDpio3cAY6ZeDWassts7/9t84RWY3Mh0OHikGgrju/ppx+DnIgMH+jiRafVRvA9VdcjhKmzLSsCfpmfa0thCLmUg8E7dYyv/iBQij1s8uUji9gefrGqORpsSQoZvqolQKmzxewaMSFlC93V4fqCfffoA86tD1lhiGOCzYIsZOnLDzGRfqjAIg34Zx0MyiqSpBAs6yb9by6RRJ8tPGF3KNc/gDnkMKDKamT28tycgxr07yZbKbahjXc fHVtFTeB CpxFvfU+kf33FV23VyxIKZoe/NDA2PsFP7fVD1Uy9zR/u72ZCAHtQe02vU5KQSyrpYu2Z2HxjrsjTIgWN/Kd7r+L8gG4UZNJOJ3ABbFhrdW0Sb/+S1bDKN6O1DJQfBJlukRcw/K6y5Lojm6n+j4yuGo/SndK1toiCQxmFvw5GXOUA7tv0lWmeYQ+sBG93d220YBibQvL2iLFB0tLUbxOUuS6csBZkltI8xQC5TYSTf0ugVLxpFN8JoO1ZAs2Zswp/HX7mealZLb39WleC/3t0UZk1uB8wLMdAMTwfYzqakTKTKf/SGYg4WFN9QW0431xe11U2VEklwLpAkS1KKC1jB1/I6pumGjem5MfygC9dUnmHNdtA4dgNrW3Z1NJmyr/MNSdKqJpVnhkDgC4C1e5qV39CoSz+jAiXrBYeKJYBhRxZ3oNe7RdpyKDMpSSy5MFMYSiS+VGhgwmzzBI= X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: syzbot is reporting circular locking dependency between ntfs_file_mmap() (which has mm->mmap_lock => ni->ni_lock => ni->file.run_lock dependency) and ntfs_fiemap() (which has ni->ni_lock => ni->file.run_lock => mm->mmap_lock dependency), for commit c4b929b85bdb ("vfs: vfs-level fiemap interface") implemented fiemap_fill_next_extent() using copy_to_user() where direct mm->mmap_lock dependency is inevitable. Since ntfs3 does not want to release ni->ni_lock and/or ni->file.run_lock in order to make sure that "struct ATTRIB" does not change during ioctl(FS_IOC_FIEMAP) request, let's make it possible to call fiemap_fill_next_extent() with filesystem locks held. This patch adds fiemap_fill_next_kernel_extent() which spools "struct fiemap_extent" to dynamically allocated kernel buffer, and fiemap_copy_kernel_extent() which copies spooled "struct fiemap_extent" to userspace buffer after filesystem locks are released. Reported-by: syzbot Link: https://syzkaller.appspot.com/bug?extid=96cee7d33ca3f87eee86 Reported-by: syzbot Link: https://syzkaller.appspot.com/bug?extid=c300ab283ba3bc072439 Fixes: 4342306f0f0d ("fs/ntfs3: Add file operations and implementation") Signed-off-by: Tetsuo Handa --- fs/ioctl.c | 52 ++++++++++++++++++++++++++++++++++++------ fs/ntfs3/file.c | 4 ++++ fs/ntfs3/frecord.c | 10 ++++---- include/linux/fiemap.h | 24 +++++++++++++++++-- 4 files changed, 76 insertions(+), 14 deletions(-) diff --git a/fs/ioctl.c b/fs/ioctl.c index 5b2481cd4750..60ddc2760932 100644 --- a/fs/ioctl.c +++ b/fs/ioctl.c @@ -112,11 +112,10 @@ static int ioctl_fibmap(struct file *filp, int __user *p) #define SET_UNKNOWN_FLAGS (FIEMAP_EXTENT_DELALLOC) #define SET_NO_UNMOUNTED_IO_FLAGS (FIEMAP_EXTENT_DATA_ENCRYPTED) #define SET_NOT_ALIGNED_FLAGS (FIEMAP_EXTENT_DATA_TAIL|FIEMAP_EXTENT_DATA_INLINE) -int fiemap_fill_next_extent(struct fiemap_extent_info *fieinfo, u64 logical, - u64 phys, u64 len, u32 flags) +int do_fiemap_fill_next_extent(struct fiemap_extent_info *fieinfo, u64 logical, + u64 phys, u64 len, u32 flags, bool is_kernel) { struct fiemap_extent extent; - struct fiemap_extent __user *dest = fieinfo->fi_extents_start; /* only count the extents */ if (fieinfo->fi_extents_max == 0) { @@ -140,16 +139,55 @@ int fiemap_fill_next_extent(struct fiemap_extent_info *fieinfo, u64 logical, extent.fe_length = len; extent.fe_flags = flags; - dest += fieinfo->fi_extents_mapped; - if (copy_to_user(dest, &extent, sizeof(extent))) - return -EFAULT; + if (!is_kernel) { + struct fiemap_extent __user *dest = fieinfo->fi_extents_start; + + dest += fieinfo->fi_extents_mapped; + if (copy_to_user(dest, &extent, sizeof(extent))) + return -EFAULT; + } else { + struct fiemap_extent_list *entry = kmalloc(sizeof(*entry), GFP_NOFS); + + if (!entry) + return -ENOMEM; + memmove(&entry->extent, &extent, sizeof(extent)); + list_add_tail(&entry->list, &fieinfo->fi_extents_list); + } fieinfo->fi_extents_mapped++; if (fieinfo->fi_extents_mapped == fieinfo->fi_extents_max) return 1; return (flags & FIEMAP_EXTENT_LAST) ? 1 : 0; } -EXPORT_SYMBOL(fiemap_fill_next_extent); +EXPORT_SYMBOL(do_fiemap_fill_next_extent); + +int fiemap_copy_kernel_extent(struct fiemap_extent_info *fieinfo, int err) +{ + struct fiemap_extent __user *dest; + struct fiemap_extent_list *entry, *tmp; + unsigned int len = 0; + + list_for_each_entry(entry, &fieinfo->fi_extents_list, list) + len++; + if (!len) + return err; + fieinfo->fi_extents_mapped -= len; + dest = fieinfo->fi_extents_start + fieinfo->fi_extents_mapped; + list_for_each_entry(entry, &fieinfo->fi_extents_list, list) { + if (copy_to_user(dest, &entry->extent, sizeof(entry->extent))) { + err = -EFAULT; + break; + } + dest++; + fieinfo->fi_extents_mapped++; + } + list_for_each_entry_safe(entry, tmp, &fieinfo->fi_extents_list, list) { + list_del(&entry->list); + kfree(entry); + } + return err; +} +EXPORT_SYMBOL(fiemap_copy_kernel_extent); /** * fiemap_prep - check validity of requested flags for fiemap diff --git a/fs/ntfs3/file.c b/fs/ntfs3/file.c index e9bdc1ff08c9..1a3e28f71599 100644 --- a/fs/ntfs3/file.c +++ b/fs/ntfs3/file.c @@ -1145,12 +1145,16 @@ int ntfs_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo, if (err) return err; + INIT_LIST_HEAD(&fieinfo->fi_extents_list); + ni_lock(ni); err = ni_fiemap(ni, fieinfo, start, len); ni_unlock(ni); + err = fiemap_copy_kernel_extent(fieinfo, err); + return err; } diff --git a/fs/ntfs3/frecord.c b/fs/ntfs3/frecord.c index f1df52dfab74..b70f9dfb71ab 100644 --- a/fs/ntfs3/frecord.c +++ b/fs/ntfs3/frecord.c @@ -1941,8 +1941,7 @@ int ni_fiemap(struct ntfs_inode *ni, struct fiemap_extent_info *fieinfo, } if (!attr || !attr->non_res) { - err = fiemap_fill_next_extent( - fieinfo, 0, 0, + err = fiemap_fill_next_kernel_extent(fieinfo, 0, 0, attr ? le32_to_cpu(attr->res.data_size) : 0, FIEMAP_EXTENT_DATA_INLINE | FIEMAP_EXTENT_LAST | FIEMAP_EXTENT_MERGED); @@ -2037,8 +2036,8 @@ int ni_fiemap(struct ntfs_inode *ni, struct fiemap_extent_info *fieinfo, if (vbo + dlen >= end) flags |= FIEMAP_EXTENT_LAST; - err = fiemap_fill_next_extent(fieinfo, vbo, lbo, dlen, - flags); + err = fiemap_fill_next_kernel_extent(fieinfo, vbo, lbo, + dlen, flags); if (err < 0) break; if (err == 1) { @@ -2058,7 +2057,8 @@ int ni_fiemap(struct ntfs_inode *ni, struct fiemap_extent_info *fieinfo, if (vbo + bytes >= end) flags |= FIEMAP_EXTENT_LAST; - err = fiemap_fill_next_extent(fieinfo, vbo, lbo, bytes, flags); + err = fiemap_fill_next_kernel_extent(fieinfo, vbo, lbo, bytes, + flags); if (err < 0) break; if (err == 1) { diff --git a/include/linux/fiemap.h b/include/linux/fiemap.h index c50882f19235..10cb33ed80a9 100644 --- a/include/linux/fiemap.h +++ b/include/linux/fiemap.h @@ -5,17 +5,37 @@ #include #include +struct fiemap_extent_list { + struct list_head list; + struct fiemap_extent extent; +}; + struct fiemap_extent_info { unsigned int fi_flags; /* Flags as passed from user */ unsigned int fi_extents_mapped; /* Number of mapped extents */ unsigned int fi_extents_max; /* Size of fiemap_extent array */ struct fiemap_extent __user *fi_extents_start; /* Start of fiemap_extent array */ + struct list_head fi_extents_list; /* List of fiemap_extent_list */ }; int fiemap_prep(struct inode *inode, struct fiemap_extent_info *fieinfo, u64 start, u64 *len, u32 supported_flags); -int fiemap_fill_next_extent(struct fiemap_extent_info *info, u64 logical, - u64 phys, u64 len, u32 flags); +int do_fiemap_fill_next_extent(struct fiemap_extent_info *fieinfo, u64 logical, + u64 phys, u64 len, u32 flags, bool is_kernel); + +static inline int fiemap_fill_next_extent(struct fiemap_extent_info *info, + u64 logical, u64 phys, u64 len, u32 flags) +{ + return do_fiemap_fill_next_extent(info, logical, phys, len, flags, false); +} + +static inline int fiemap_fill_next_kernel_extent(struct fiemap_extent_info *info, + u64 logical, u64 phys, u64 len, u32 flags) +{ + return do_fiemap_fill_next_extent(info, logical, phys, len, flags, true); +} + +int fiemap_copy_kernel_extent(struct fiemap_extent_info *info, int err); #endif /* _LINUX_FIEMAP_H 1 */