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: [email protected]
Signed-off-by: Pavel Skripkin <[email protected]>
---
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
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: [email protected]
> Signed-off-by: Pavel Skripkin <[email protected]>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.
>
On Wed, 23 Jun 2021 09:13:07 -0500
Dave Kleikamp <[email protected]> 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:
> > [email protected]
> > Signed-off-by: Pavel Skripkin <[email protected]>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
On 6/23/21 11:38 AM, Pavel Skripkin wrote:
> On Wed, 23 Jun 2021 09:13:07 -0500
> Dave Kleikamp <[email protected]> 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:
>>> [email protected]
>>> Signed-off-by: Pavel Skripkin <[email protected]>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
>