[PATCH 5.15.y, 5.10.y] exfat: fix potential deadlock on __exfat_get_dentry_set

[PATCH 5.15.y, 5.10.y] exfat: fix potential deadlock on __exfat_get_dentry_set

[Sherry Yang]

From: Sungjong Seo <sj1557.seo@samsung.com>

commit 89fc548767a2155231128cb98726d6d2ea1256c9 upstream.

When accessing a file with more entries than ES_MAX_ENTRY_NUM, the bh-array
is allocated in __exfat_get_entry_set. The problem is that the bh-array is
allocated with GFP_KERNEL. It does not make sense. In the following cases,
a deadlock for sbi->s_lock between the two processes may occur.

       CPU0                CPU1
       ----                ----
  kswapd
   balance_pgdat
    lock(fs_reclaim)
                      exfat_iterate
                       lock(&sbi->s_lock)
                       exfat_readdir
                        exfat_get_uniname_from_ext_entry
                         exfat_get_dentry_set
                          __exfat_get_dentry_set
                           kmalloc_array
                            ...
                            lock(fs_reclaim)
    ...
    evict
     exfat_evict_inode
      lock(&sbi->s_lock)

To fix this, let's allocate bh-array with GFP_NOFS.

Fixes: a3ff29a95fde ("exfat: support dynamic allocate bh for exfat_entry_set_cache")
Cc: stable@vger.kernel.org # v6.2+
Reported-by: syzbot+412a392a2cd4a65e71db@syzkaller.appspotmail.com
Closes: https://lore.kernel.org/lkml/000000000000fef47e0618c0327f@google.com
Signed-off-by: Sungjong Seo <sj1557.seo@samsung.com>
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
[Sherry: The problematic commit was backported to 5.15.y and 5.10.y,
thus backport this fix]
Signed-off-by: Sherry Yang <sherry.yang@oracle.com>
---
 fs/exfat/dir.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/exfat/dir.c b/fs/exfat/dir.c
index be7570d01ae1..0a1b1de032ef 100644
--- a/fs/exfat/dir.c
+++ b/fs/exfat/dir.c
@@ -878,7 +878,7 @@ struct exfat_entry_set_cache *exfat_get_dentry_set(struct super_block *sb,
 
 	num_bh = EXFAT_B_TO_BLK_ROUND_UP(off + num_entries * DENTRY_SIZE, sb);
 	if (num_bh > ARRAY_SIZE(es->__bh)) {
-		es->bh = kmalloc_array(num_bh, sizeof(*es->bh), GFP_KERNEL);
+		es->bh = kmalloc_array(num_bh, sizeof(*es->bh), GFP_NOFS);
 		if (!es->bh) {
 			brelse(bh);
 			kfree(es);
-- 
2.46.0

Re: [PATCH 5.15.y, 5.10.y] exfat: fix potential deadlock on __exfat_get_dentry_set

[Sasha Levin]

[ Sasha's backport helper bot ]

Hi,

The upstream commit SHA1 provided is correct: 89fc548767a2155231128cb98726d6d2ea1256c9

WARNING: Author mismatch between patch and upstream commit:
Backport author: Sherry Yang <sherry.yang@oracle.com>
Commit author: Sungjong Seo <sj1557.seo@samsung.com>


Status in newer kernel trees:
6.12.y | Present (exact SHA1)
6.6.y | Present (different SHA1: a7ac198f8dba)
6.1.y | Not found
5.15.y | Not found

Note: The patch differs from the upstream commit:
---
1:  89fc548767a2 < -:  ------------ exfat: fix potential deadlock on __exfat_get_dentry_set
-:  ------------ > 1:  9b4fc692990f exfat: fix potential deadlock on __exfat_get_dentry_set
---

Results of testing on various branches:

| Branch                    | Patch Apply | Build Test |
|---------------------------|-------------|------------|
| stable/linux-5.15.y       |  Success    |  Failed    |
| stable/linux-5.10.y       |  Success    |  Failed    |

Build Errors:
Build error for stable/linux-5.15.y:
    

Build error for stable/linux-5.10.y:
    make: *** No rule to make target 'allmodconfig'.  Stop.
    make: *** No targets specified and no makefile found.  Stop.

Re: [PATCH 5.15.y, 5.10.y] exfat: fix potential deadlock on __exfat_get_dentry_set

[Sherry Yang]

Hi, 

> On Dec 14, 2024, at 6:26 AM, Sasha Levin <sashal@kernel.org> wrote:
> 
> [ Sasha's backport helper bot ]
> 
> Hi,
> 
> The upstream commit SHA1 provided is correct: 89fc548767a2155231128cb98726d6d2ea1256c9
> 
> WARNING: Author mismatch between patch and upstream commit:
> Backport author: Sherry Yang <sherry.yang@oracle.com>
> Commit author: Sungjong Seo <sj1557.seo@samsung.com>
> 
> 
> Status in newer kernel trees:
> 6.12.y | Present (exact SHA1)
> 6.6.y | Present (different SHA1: a7ac198f8dba)
> 6.1.y | Not found
> 5.15.y | Not found

I didn’t backport the commit to linux-stable-6.1.y, because 6.1.y didn’t backport the culprit commit 
a3ff29a95fde ("exfat: support dynamic allocate bh for exfat_entry_set_cache”), so not influenced.

However, both linux-stable-5.15.y and linux-stable-5.10.y actually backported the culprit commit. So I’m trying to fix it on 5.15.y and 5.10.y.

Let me know if you have more questions about it.

Thanks,
Sherry

> 
> Note: The patch differs from the upstream commit:
> ---
> 1:  89fc548767a2 < -:  ------------ exfat: fix potential deadlock on __exfat_get_dentry_set
> -:  ------------ > 1:  9b4fc692990f exfat: fix potential deadlock on __exfat_get_dentry_set
> ---
> 
> Results of testing on various branches:
> 
> | Branch                    | Patch Apply | Build Test |
> |---------------------------|-------------|------------|
> | stable/linux-5.15.y       |  Success    |  Failed    |
> | stable/linux-5.10.y       |  Success    |  Failed    |
> 
> Build Errors:
> Build error for stable/linux-5.15.y:
> 
> 
> Build error for stable/linux-5.10.y:
>    make: *** No rule to make target 'allmodconfig'.  Stop.
>    make: *** No targets specified and no makefile found.  Stop.

Re: [PATCH 5.15.y, 5.10.y] exfat: fix potential deadlock on __exfat_get_dentry_set

[Greg KH]

On Sat, Dec 14, 2024 at 05:57:01PM +0000, Sherry Yang wrote:
> Hi, 
> 
> > On Dec 14, 2024, at 6:26 AM, Sasha Levin <sashal@kernel.org> wrote:
> > 
> > [ Sasha's backport helper bot ]
> > 
> > Hi,
> > 
> > The upstream commit SHA1 provided is correct: 89fc548767a2155231128cb98726d6d2ea1256c9
> > 
> > WARNING: Author mismatch between patch and upstream commit:
> > Backport author: Sherry Yang <sherry.yang@oracle.com>
> > Commit author: Sungjong Seo <sj1557.seo@samsung.com>
> > 
> > 
> > Status in newer kernel trees:
> > 6.12.y | Present (exact SHA1)
> > 6.6.y | Present (different SHA1: a7ac198f8dba)
> > 6.1.y | Not found
> > 5.15.y | Not found
> 
> I didn’t backport the commit to linux-stable-6.1.y, because 6.1.y didn’t backport the culprit commit 
> a3ff29a95fde ("exfat: support dynamic allocate bh for exfat_entry_set_cache”), so not influenced.
> 
> However, both linux-stable-5.15.y and linux-stable-5.10.y actually backported the culprit commit. So I’m trying to fix it on 5.15.y and 5.10.y.
> 
> Let me know if you have more questions about it.

That's confusing, why doesn't 6.1.y have that commit?  Shouldn't we also
add it there along with this one?

thanks,

greg k-h

Re: [PATCH 5.15.y, 5.10.y] exfat: fix potential deadlock on __exfat_get_dentry_set

[Harshit Mogalapalli]

Hi Greg,

On 14/12/24 23:36, Greg KH wrote:
> On Sat, Dec 14, 2024 at 05:57:01PM +0000, Sherry Yang wrote:
>> Hi,
>>
>>> On Dec 14, 2024, at 6:26 AM, Sasha Levin <sashal@kernel.org> wrote:
>>>
>>> [ Sasha's backport helper bot ]
>>>
>>> Hi,
>>>
>>> The upstream commit SHA1 provided is correct: 89fc548767a2155231128cb98726d6d2ea1256c9
>>>
>>> WARNING: Author mismatch between patch and upstream commit:
>>> Backport author: Sherry Yang <sherry.yang@oracle.com>
>>> Commit author: Sungjong Seo <sj1557.seo@samsung.com>
>>>
>>>
>>> Status in newer kernel trees:
>>> 6.12.y | Present (exact SHA1)
>>> 6.6.y | Present (different SHA1: a7ac198f8dba)
>>> 6.1.y | Not found
>>> 5.15.y | Not found
>>
>> I didn’t backport the commit to linux-stable-6.1.y, because 6.1.y didn’t backport the culprit commit
>> a3ff29a95fde ("exfat: support dynamic allocate bh for exfat_entry_set_cache”), so not influenced.
>>
>> However, both linux-stable-5.15.y and linux-stable-5.10.y actually backported the culprit commit. So I’m trying to fix it on 5.15.y and 5.10.y.
>>
>> Let me know if you have more questions about it.
> 
> That's confusing, why doesn't 6.1.y have that commit?  Shouldn't we also
> add it there along with this one?
> 

https://lore.kernel.org/all/20230809103650.353831735@linuxfoundation.org/#t

Commit a3ff29a95fde ("exfat: support dynamic allocate bh for 
exfat_entry_set_cache”) which is present in 5.10.y and 5.15.y but not in 
6.1.y is added as a stable-dependency "Stable-dep-of: d42334578eba 
("exfat: check if filename entries exceeds max filename length") ", but 
this(d42334578eba - filename length check) is present in 6.1.y without 
commit a3ff29a95fde , so probably stable-dep-of is not accurate.

Given that now we already have a3ff29a95fde ("exfat: support dynamic 
allocate bh for exfat_entry_set_cache”) to 5.15.y and 5.10.y, I think we 
should add it and the fix to 6.1.y as well.

For 6.1.y here are the upstream commits: (Starting from 1 -- cleanly 
applies and builds fine, haven't done any exfat related testing though.)
1. a3ff29a95fde ("exfat: support dynamic allocate bh for 
exfat_entry_set_cache”)
2. commit: 89fc548767a2 ("exfat: fix potential deadlock on 
__exfat_get_dentry_set")

Let me know if you want me to send two patches instead, I can do that.

Thanks,
Harshit

> thanks,
> 
> greg k-h
> 

Re: [PATCH 5.15.y, 5.10.y] exfat: fix potential deadlock on __exfat_get_dentry_set

[Greg KH]

On Sun, Dec 15, 2024 at 03:11:16AM +0530, Harshit Mogalapalli wrote:
> Hi Greg,
> 
> On 14/12/24 23:36, Greg KH wrote:
> > On Sat, Dec 14, 2024 at 05:57:01PM +0000, Sherry Yang wrote:
> > > Hi,
> > > 
> > > > On Dec 14, 2024, at 6:26 AM, Sasha Levin <sashal@kernel.org> wrote:
> > > > 
> > > > [ Sasha's backport helper bot ]
> > > > 
> > > > Hi,
> > > > 
> > > > The upstream commit SHA1 provided is correct: 89fc548767a2155231128cb98726d6d2ea1256c9
> > > > 
> > > > WARNING: Author mismatch between patch and upstream commit:
> > > > Backport author: Sherry Yang <sherry.yang@oracle.com>
> > > > Commit author: Sungjong Seo <sj1557.seo@samsung.com>
> > > > 
> > > > 
> > > > Status in newer kernel trees:
> > > > 6.12.y | Present (exact SHA1)
> > > > 6.6.y | Present (different SHA1: a7ac198f8dba)
> > > > 6.1.y | Not found
> > > > 5.15.y | Not found
> > > 
> > > I didn’t backport the commit to linux-stable-6.1.y, because 6.1.y didn’t backport the culprit commit
> > > a3ff29a95fde ("exfat: support dynamic allocate bh for exfat_entry_set_cache”), so not influenced.
> > > 
> > > However, both linux-stable-5.15.y and linux-stable-5.10.y actually backported the culprit commit. So I’m trying to fix it on 5.15.y and 5.10.y.
> > > 
> > > Let me know if you have more questions about it.
> > 
> > That's confusing, why doesn't 6.1.y have that commit?  Shouldn't we also
> > add it there along with this one?
> > 
> 
> https://lore.kernel.org/all/20230809103650.353831735@linuxfoundation.org/#t
> 
> Commit a3ff29a95fde ("exfat: support dynamic allocate bh for
> exfat_entry_set_cache”) which is present in 5.10.y and 5.15.y but not in
> 6.1.y is added as a stable-dependency "Stable-dep-of: d42334578eba ("exfat:
> check if filename entries exceeds max filename length") ", but
> this(d42334578eba - filename length check) is present in 6.1.y without
> commit a3ff29a95fde , so probably stable-dep-of is not accurate.
> 
> Given that now we already have a3ff29a95fde ("exfat: support dynamic
> allocate bh for exfat_entry_set_cache”) to 5.15.y and 5.10.y, I think we
> should add it and the fix to 6.1.y as well.
> 
> For 6.1.y here are the upstream commits: (Starting from 1 -- cleanly applies
> and builds fine, haven't done any exfat related testing though.)
> 1. a3ff29a95fde ("exfat: support dynamic allocate bh for
> exfat_entry_set_cache”)
> 2. commit: 89fc548767a2 ("exfat: fix potential deadlock on
> __exfat_get_dentry_set")
> 
> Let me know if you want me to send two patches instead, I can do that.

Yes, please do!

thanks,

greg k-h