* [PATCH] jfs: fix GPF in diFree
@ 2021-06-06 14:24 Pavel Skripkin
2021-06-23 14:13 ` Dave Kleikamp
0 siblings, 1 reply; 4+ messages in thread
From: Pavel Skripkin @ 2021-06-06 14:24 UTC (permalink / raw)
To: shaggy
Cc: jfs-discussion, linux-kernel, Pavel Skripkin,
syzbot+0a89a7b56db04c21a656
Avoid passing inode with
JFS_SBI(inode->i_sb)->ipimap == NULL to
diFree()[1]. GFP will appear:
struct inode *ipimap = JFS_SBI(ip->i_sb)->ipimap;
struct inomap *imap = JFS_IP(ipimap)->i_imap;
JFS_IP() will return invalid pointer when ipimap == NULL
Call Trace:
diFree+0x13d/0x2dc0 fs/jfs/jfs_imap.c:853 [1]
jfs_evict_inode+0x2c9/0x370 fs/jfs/inode.c:154
evict+0x2ed/0x750 fs/inode.c:578
iput_final fs/inode.c:1654 [inline]
iput.part.0+0x3fe/0x820 fs/inode.c:1680
iput+0x58/0x70 fs/inode.c:1670
Reported-and-tested-by: syzbot+0a89a7b56db04c21a656@syzkaller.appspotmail.com
Signed-off-by: Pavel Skripkin <paskripkin@gmail.com>
---
fs/jfs/inode.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/fs/jfs/inode.c b/fs/jfs/inode.c
index 6f65bfa9f18d..b0eb9c85eea0 100644
--- a/fs/jfs/inode.c
+++ b/fs/jfs/inode.c
@@ -151,7 +151,8 @@ void jfs_evict_inode(struct inode *inode)
if (test_cflag(COMMIT_Freewmap, inode))
jfs_free_zero_link(inode);
- diFree(inode);
+ if (JFS_SBI(inode->i_sb)->ipimap)
+ diFree(inode);
/*
* Free the inode from the quota allocation.
--
2.31.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] jfs: fix GPF in diFree
2021-06-06 14:24 [PATCH] jfs: fix GPF in diFree Pavel Skripkin
@ 2021-06-23 14:13 ` Dave Kleikamp
2021-06-23 16:38 ` Pavel Skripkin
0 siblings, 1 reply; 4+ messages in thread
From: Dave Kleikamp @ 2021-06-23 14:13 UTC (permalink / raw)
To: Pavel Skripkin, shaggy
Cc: jfs-discussion, linux-kernel, syzbot+0a89a7b56db04c21a656
On 6/6/21 9:24 AM, Pavel Skripkin wrote:
> Avoid passing inode with
> JFS_SBI(inode->i_sb)->ipimap == NULL to
> diFree()[1]. GFP will appear:
I'm a little curious how we get as far as creating and freeing
non-special inodes if ipimap == NULL.
>
> struct inode *ipimap = JFS_SBI(ip->i_sb)->ipimap;
> struct inomap *imap = JFS_IP(ipimap)->i_imap;
>
> JFS_IP() will return invalid pointer when ipimap == NULL
>
> Call Trace:
> diFree+0x13d/0x2dc0 fs/jfs/jfs_imap.c:853 [1]
> jfs_evict_inode+0x2c9/0x370 fs/jfs/inode.c:154
> evict+0x2ed/0x750 fs/inode.c:578
> iput_final fs/inode.c:1654 [inline]
> iput.part.0+0x3fe/0x820 fs/inode.c:1680
> iput+0x58/0x70 fs/inode.c:1670
Is there more to the stack trace? Is this part of a failed mount()?
>
> Reported-and-tested-by: syzbot+0a89a7b56db04c21a656@syzkaller.appspotmail.com
> Signed-off-by: Pavel Skripkin <paskripkin@gmail.com>ipimap == NULL
I don't doubt that this happened, so I'll apply the patch which is
obviously safe.
> ---
> fs/jfs/inode.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/fs/jfs/inode.c b/fs/jfs/inode.c
> index 6f65bfa9f18d..b0eb9c85eea0 100644
> --- a/fs/jfs/inode.c
> +++ b/fs/jfs/inode.c
> @@ -151,7 +151,8 @@ void jfs_evict_inode(struct inode *inode)
> if (test_cflag(COMMIT_Freewmap, inode))
> jfs_free_zero_link(inode);
>
> - diFree(inode);
> + if (JFS_SBI(inode->i_sb)->ipimap)
> + diFree(inode);
>
> /*
> * Free the inode from the quota allocation.
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] jfs: fix GPF in diFree
2021-06-23 14:13 ` Dave Kleikamp
@ 2021-06-23 16:38 ` Pavel Skripkin
2021-06-23 16:46 ` Dave Kleikamp
0 siblings, 1 reply; 4+ messages in thread
From: Pavel Skripkin @ 2021-06-23 16:38 UTC (permalink / raw)
To: Dave Kleikamp
Cc: shaggy, jfs-discussion, linux-kernel, syzbot+0a89a7b56db04c21a656
On Wed, 23 Jun 2021 09:13:07 -0500
Dave Kleikamp <dave.kleikamp@oracle.com> wrote:
> On 6/6/21 9:24 AM, Pavel Skripkin wrote:
> > Avoid passing inode with
> > JFS_SBI(inode->i_sb)->ipimap == NULL to
> > diFree()[1]. GFP will appear:
>
> I'm a little curious how we get as far as creating and freeing
> non-special inodes if ipimap == NULL.
>
> >
> > struct inode *ipimap = JFS_SBI(ip->i_sb)->ipimap;
> > struct inomap *imap = JFS_IP(ipimap)->i_imap;
> >
> > JFS_IP() will return invalid pointer when ipimap == NULL
> >
> > Call Trace:
> > diFree+0x13d/0x2dc0 fs/jfs/jfs_imap.c:853 [1]
> > jfs_evict_inode+0x2c9/0x370 fs/jfs/inode.c:154
> > evict+0x2ed/0x750 fs/inode.c:578
> > iput_final fs/inode.c:1654 [inline]
> > iput.part.0+0x3fe/0x820 fs/inode.c:1680
> > iput+0x58/0x70 fs/inode.c:1670
>
> Is there more to the stack trace? Is this part of a failed mount()?
>
Hi, Dave!
Yes, it was caused by mount fail. Log:
[ 924.076873][ T8430] jfs_mount: diMount(ipaimap) failed w/rc = -5
So, it's errout21 label in jfs_mount(). I guess, It's early failure and
some fields wasn't initialized properly. I don't really remember my
debug results, because it was a long time ago, but I can do some debug
work again if needed!
Thanks for feedback!
> >
> > Reported-and-tested-by:
> > syzbot+0a89a7b56db04c21a656@syzkaller.appspotmail.com
> > Signed-off-by: Pavel Skripkin <paskripkin@gmail.com>ipimap == NULL
>
> I don't doubt that this happened, so I'll apply the patch which is
> obviously safe.
>
> > ---
> > fs/jfs/inode.c | 3 ++-
> > 1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/fs/jfs/inode.c b/fs/jfs/inode.c
> > index 6f65bfa9f18d..b0eb9c85eea0 100644
> > --- a/fs/jfs/inode.c
> > +++ b/fs/jfs/inode.c
> > @@ -151,7 +151,8 @@ void jfs_evict_inode(struct inode *inode)
> > if (test_cflag(COMMIT_Freewmap, inode))
> > jfs_free_zero_link(inode);
> >
> > - diFree(inode);
> > + if (JFS_SBI(inode->i_sb)->ipimap)
> > + diFree(inode);
> >
> > /*
> > * Free the inode from the quota
> > allocation.
> >
With regards,
Pavel Skripkin
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] jfs: fix GPF in diFree
2021-06-23 16:38 ` Pavel Skripkin
@ 2021-06-23 16:46 ` Dave Kleikamp
0 siblings, 0 replies; 4+ messages in thread
From: Dave Kleikamp @ 2021-06-23 16:46 UTC (permalink / raw)
To: Pavel Skripkin; +Cc: jfs-discussion, linux-kernel, syzbot+0a89a7b56db04c21a656
On 6/23/21 11:38 AM, Pavel Skripkin wrote:
> On Wed, 23 Jun 2021 09:13:07 -0500
> Dave Kleikamp <dave.kleikamp@oracle.com> wrote:
>
>> On 6/6/21 9:24 AM, Pavel Skripkin wrote:
>>> Avoid passing inode with
>>> JFS_SBI(inode->i_sb)->ipimap == NULL to
>>> diFree()[1]. GFP will appear:
>>
>> I'm a little curious how we get as far as creating and freeing
>> non-special inodes if ipimap == NULL.
>>
>>>
>>> struct inode *ipimap = JFS_SBI(ip->i_sb)->ipimap;
>>> struct inomap *imap = JFS_IP(ipimap)->i_imap;
>>>
>>> JFS_IP() will return invalid pointer when ipimap == NULL
>>>
>>> Call Trace:
>>> diFree+0x13d/0x2dc0 fs/jfs/jfs_imap.c:853 [1]
>>> jfs_evict_inode+0x2c9/0x370 fs/jfs/inode.c:154
>>> evict+0x2ed/0x750 fs/inode.c:578
>>> iput_final fs/inode.c:1654 [inline]
>>> iput.part.0+0x3fe/0x820 fs/inode.c:1680
>>> iput+0x58/0x70 fs/inode.c:1670
>>
>> Is there more to the stack trace? Is this part of a failed mount()?
>>
>
> Hi, Dave!
>
> Yes, it was caused by mount fail. Log:
>
> [ 924.076873][ T8430] jfs_mount: diMount(ipaimap) failed w/rc = -5
>
> So, it's errout21 label in jfs_mount(). I guess, It's early failure and
> some fields wasn't initialized properly. I don't really remember my
> debug results, because it was a long time ago, but I can do some debug
> work again if needed!
Thanks. I don't want to ask you to spend any more time on this. Sorry I
didn't get back sooner. Thanks for the fix!
Shaggy
>
>
> Thanks for feedback!
>
>>>
>>> Reported-and-tested-by:
>>> syzbot+0a89a7b56db04c21a656@syzkaller.appspotmail.com
>>> Signed-off-by: Pavel Skripkin <paskripkin@gmail.com>ipimap == NULL
>>
>> I don't doubt that this happened, so I'll apply the patch which is
>> obviously safe.
>>
>>> ---
>>> fs/jfs/inode.c | 3 ++-
>>> 1 file changed, 2 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/fs/jfs/inode.c b/fs/jfs/inode.c
>>> index 6f65bfa9f18d..b0eb9c85eea0 100644
>>> --- a/fs/jfs/inode.c
>>> +++ b/fs/jfs/inode.c
>>> @@ -151,7 +151,8 @@ void jfs_evict_inode(struct inode *inode)
>>> if (test_cflag(COMMIT_Freewmap, inode))
>>> jfs_free_zero_link(inode);
>>>
>>> - diFree(inode);
>>> + if (JFS_SBI(inode->i_sb)->ipimap)
>>> + diFree(inode);
>>>
>>> /*
>>> * Free the inode from the quota
>>> allocation.
>>>
>
>
>
>
> With regards,
> Pavel Skripkin
>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2021-06-23 16:46 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-06-06 14:24 [PATCH] jfs: fix GPF in diFree Pavel Skripkin
2021-06-23 14:13 ` Dave Kleikamp
2021-06-23 16:38 ` Pavel Skripkin
2021-06-23 16:46 ` Dave Kleikamp
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.