* [PATCH] xfs: check the return value of krealloc() in xfs_uuid_mount
@ 2020-11-25 6:50 Qinglang Miao
2020-11-25 15:55 ` Eric Sandeen
0 siblings, 1 reply; 6+ messages in thread
From: Qinglang Miao @ 2020-11-25 6:50 UTC (permalink / raw)
To: Darrick J. Wong, linux-xfs; +Cc: linux-kernel, Qinglang Miao
krealloc() may fail to expand the memory space. Add sanity checks to it,
and WARN() if that really happened.
Fixes: 771915c4f688 ("xfs: remove kmem_realloc()")
Reported-by: Hulk Robot <hulkci@huawei.com>
Signed-off-by: Qinglang Miao <miaoqinglang@huawei.com>
---
fs/xfs/xfs_mount.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/fs/xfs/xfs_mount.c b/fs/xfs/xfs_mount.c
index 150ee5cb8..c07f48c32 100644
--- a/fs/xfs/xfs_mount.c
+++ b/fs/xfs/xfs_mount.c
@@ -80,9 +80,13 @@ xfs_uuid_mount(
}
if (hole < 0) {
- xfs_uuid_table = krealloc(xfs_uuid_table,
+ uuid_t *if_xfs_uuid_table;
+ if_xfs_uuid_table = krealloc(xfs_uuid_table,
(xfs_uuid_table_size + 1) * sizeof(*xfs_uuid_table),
GFP_KERNEL | __GFP_NOFAIL);
+ if (!if_xfs_uuid_table)
+ goto out_duplicate;
+ xfs_uuid_table = if_xfs_uuid_table;
hole = xfs_uuid_table_size++;
}
xfs_uuid_table[hole] = *uuid;
--
2.23.0
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] xfs: check the return value of krealloc() in xfs_uuid_mount 2020-11-25 6:50 [PATCH] xfs: check the return value of krealloc() in xfs_uuid_mount Qinglang Miao @ 2020-11-25 15:55 ` Eric Sandeen 2020-11-26 1:21 ` Qinglang Miao 0 siblings, 1 reply; 6+ messages in thread From: Eric Sandeen @ 2020-11-25 15:55 UTC (permalink / raw) To: Qinglang Miao, Darrick J. Wong, linux-xfs; +Cc: linux-kernel On 11/25/20 12:50 AM, Qinglang Miao wrote: > krealloc() may fail to expand the memory space. Even with __GFP_NOFAIL? * ``GFP_KERNEL | __GFP_NOFAIL`` - overrides the default allocator behavior and all allocation requests will loop endlessly until they succeed. This might be really dangerous especially for larger orders. > Add sanity checks to it, > and WARN() if that really happened. As aside, there is no WARN added in this patch for a memory failure. > Fixes: 771915c4f688 ("xfs: remove kmem_realloc()") > Reported-by: Hulk Robot <hulkci@huawei.com> > Signed-off-by: Qinglang Miao <miaoqinglang@huawei.com> > --- > fs/xfs/xfs_mount.c | 6 +++++- > 1 file changed, 5 insertions(+), 1 deletion(-) > > diff --git a/fs/xfs/xfs_mount.c b/fs/xfs/xfs_mount.c > index 150ee5cb8..c07f48c32 100644 > --- a/fs/xfs/xfs_mount.c > +++ b/fs/xfs/xfs_mount.c > @@ -80,9 +80,13 @@ xfs_uuid_mount( > } > > if (hole < 0) { > - xfs_uuid_table = krealloc(xfs_uuid_table, > + uuid_t *if_xfs_uuid_table; > + if_xfs_uuid_table = krealloc(xfs_uuid_table, > (xfs_uuid_table_size + 1) * sizeof(*xfs_uuid_table), > GFP_KERNEL | __GFP_NOFAIL); > + if (!if_xfs_uuid_table) > + goto out_duplicate; And this would emit "Filesystem has duplicate UUID" which is not correct. But anyway, the __GFP_NOFAIL in the call makes this all moot AFAICT. -Eric > + xfs_uuid_table = if_xfs_uuid_table; > hole = xfs_uuid_table_size++; > } > xfs_uuid_table[hole] = *uuid; > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] xfs: check the return value of krealloc() in xfs_uuid_mount 2020-11-25 15:55 ` Eric Sandeen @ 2020-11-26 1:21 ` Qinglang Miao 2020-11-26 2:16 ` Gao Xiang 0 siblings, 1 reply; 6+ messages in thread From: Qinglang Miao @ 2020-11-26 1:21 UTC (permalink / raw) To: Eric Sandeen, Darrick J. Wong, linux-xfs; +Cc: linux-kernel 在 2020/11/25 23:55, Eric Sandeen 写道: > On 11/25/20 12:50 AM, Qinglang Miao wrote: >> krealloc() may fail to expand the memory space. > > Even with __GFP_NOFAIL? > > * ``GFP_KERNEL | __GFP_NOFAIL`` - overrides the default allocator behavior > and all allocation requests will loop endlessly until they succeed. > This might be really dangerous especially for larger orders. > >> Add sanity checks to it, >> and WARN() if that really happened. > > As aside, there is no WARN added in this patch for a memory failure. > >> Fixes: 771915c4f688 ("xfs: remove kmem_realloc()") >> Reported-by: Hulk Robot <hulkci@huawei.com> >> Signed-off-by: Qinglang Miao <miaoqinglang@huawei.com> >> --- >> fs/xfs/xfs_mount.c | 6 +++++- >> 1 file changed, 5 insertions(+), 1 deletion(-) >> >> diff --git a/fs/xfs/xfs_mount.c b/fs/xfs/xfs_mount.c >> index 150ee5cb8..c07f48c32 100644 >> --- a/fs/xfs/xfs_mount.c >> +++ b/fs/xfs/xfs_mount.c >> @@ -80,9 +80,13 @@ xfs_uuid_mount( >> } >> >> if (hole < 0) { >> - xfs_uuid_table = krealloc(xfs_uuid_table, >> + uuid_t *if_xfs_uuid_table; >> + if_xfs_uuid_table = krealloc(xfs_uuid_table, >> (xfs_uuid_table_size + 1) * sizeof(*xfs_uuid_table), >> GFP_KERNEL | __GFP_NOFAIL); >> + if (!if_xfs_uuid_table) >> + goto out_duplicate; > > And this would emit "Filesystem has duplicate UUID" which is not correct. > > But anyway, the __GFP_NOFAIL in the call makes this all moot AFAICT. > > -Eric Hi Eric, Sorry for neglecting __GFP_NOFAIL symbol, and I would add a WARN in memory failure next time. Thanks for your advice！ > >> + xfs_uuid_table = if_xfs_uuid_table; >> hole = xfs_uuid_table_size++; >> } >> xfs_uuid_table[hole] = *uuid; >> > . > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] xfs: check the return value of krealloc() in xfs_uuid_mount 2020-11-26 1:21 ` Qinglang Miao @ 2020-11-26 2:16 ` Gao Xiang 2020-11-26 3:05 ` Qinglang Miao 0 siblings, 1 reply; 6+ messages in thread From: Gao Xiang @ 2020-11-26 2:16 UTC (permalink / raw) To: Qinglang Miao; +Cc: Eric Sandeen, Darrick J. Wong, linux-xfs, linux-kernel Hi Qinglang, On Thu, Nov 26, 2020 at 09:21:11AM +0800, Qinglang Miao wrote: > > > 在 2020/11/25 23:55, Eric Sandeen 写道: > > On 11/25/20 12:50 AM, Qinglang Miao wrote: > > > krealloc() may fail to expand the memory space. > > > > Even with __GFP_NOFAIL? > > > > * ``GFP_KERNEL | __GFP_NOFAIL`` - overrides the default allocator behavior > > and all allocation requests will loop endlessly until they succeed. > > This might be really dangerous especially for larger orders. > > > > > Add sanity checks to it, > > > and WARN() if that really happened. > > > > As aside, there is no WARN added in this patch for a memory failure. > > > > > Fixes: 771915c4f688 ("xfs: remove kmem_realloc()") > > > Reported-by: Hulk Robot <hulkci@huawei.com> > > > Signed-off-by: Qinglang Miao <miaoqinglang@huawei.com> > > > --- > > > fs/xfs/xfs_mount.c | 6 +++++- > > > 1 file changed, 5 insertions(+), 1 deletion(-) > > > > > > diff --git a/fs/xfs/xfs_mount.c b/fs/xfs/xfs_mount.c > > > index 150ee5cb8..c07f48c32 100644 > > > --- a/fs/xfs/xfs_mount.c > > > +++ b/fs/xfs/xfs_mount.c > > > @@ -80,9 +80,13 @@ xfs_uuid_mount( > > > } > > > if (hole < 0) { > > > - xfs_uuid_table = krealloc(xfs_uuid_table, > > > + uuid_t *if_xfs_uuid_table; > > > + if_xfs_uuid_table = krealloc(xfs_uuid_table, > > > (xfs_uuid_table_size + 1) * sizeof(*xfs_uuid_table), > > > GFP_KERNEL | __GFP_NOFAIL); > > > + if (!if_xfs_uuid_table) > > > + goto out_duplicate; > > > > And this would emit "Filesystem has duplicate UUID" which is not correct. > > > > But anyway, the __GFP_NOFAIL in the call makes this all moot AFAICT. > > > > -Eric > Hi Eric, > > Sorry for neglecting __GFP_NOFAIL symbol, and I would add a WARN in memory > failure next time. Sorry about my limited knowledge, but why it needs a WARN here since I think it will never fail if __GFP_NOFAIL is added (no ?). I'm not sure if Hulk CI is completely broken or not on this, also if such CI can now generate trivial patch (?) since the subject, commit message and even the variable name is quite similiar to https://lore.kernel.org/linux-xfs/20201124104531.561-2-thunder.leizhen@huawei.com in a day. And it'd be better to look into the code before sending patches... Thanks, Gao Xiang > > Thanks for your advice！ > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] xfs: check the return value of krealloc() in xfs_uuid_mount 2020-11-26 2:16 ` Gao Xiang @ 2020-11-26 3:05 ` Qinglang Miao 2020-11-26 3:13 ` Gao Xiang 0 siblings, 1 reply; 6+ messages in thread From: Qinglang Miao @ 2020-11-26 3:05 UTC (permalink / raw) To: Gao Xiang; +Cc: Eric Sandeen, Darrick J. Wong, linux-xfs, linux-kernel 在 2020/11/26 10:16, Gao Xiang 写道: > Hi Qinglang, > > On Thu, Nov 26, 2020 at 09:21:11AM +0800, Qinglang Miao wrote: >> >> >> 在 2020/11/25 23:55, Eric Sandeen 写道: >>> On 11/25/20 12:50 AM, Qinglang Miao wrote: >>>> krealloc() may fail to expand the memory space. >>> >>> Even with __GFP_NOFAIL? >>> >>> * ``GFP_KERNEL | __GFP_NOFAIL`` - overrides the default allocator behavior >>> and all allocation requests will loop endlessly until they succeed. >>> This might be really dangerous especially for larger orders. >>> >>>> Add sanity checks to it, >>>> and WARN() if that really happened. >>> >>> As aside, there is no WARN added in this patch for a memory failure. >>> >>>> Fixes: 771915c4f688 ("xfs: remove kmem_realloc()") >>>> Reported-by: Hulk Robot <hulkci@huawei.com> >>>> Signed-off-by: Qinglang Miao <miaoqinglang@huawei.com> >>>> --- >>>> fs/xfs/xfs_mount.c | 6 +++++- >>>> 1 file changed, 5 insertions(+), 1 deletion(-) >>>> >>>> diff --git a/fs/xfs/xfs_mount.c b/fs/xfs/xfs_mount.c >>>> index 150ee5cb8..c07f48c32 100644 >>>> --- a/fs/xfs/xfs_mount.c >>>> +++ b/fs/xfs/xfs_mount.c >>>> @@ -80,9 +80,13 @@ xfs_uuid_mount( >>>> } >>>> if (hole < 0) { >>>> - xfs_uuid_table = krealloc(xfs_uuid_table, >>>> + uuid_t *if_xfs_uuid_table; >>>> + if_xfs_uuid_table = krealloc(xfs_uuid_table, >>>> (xfs_uuid_table_size + 1) * sizeof(*xfs_uuid_table), >>>> GFP_KERNEL | __GFP_NOFAIL); >>>> + if (!if_xfs_uuid_table) >>>> + goto out_duplicate; >>> >>> And this would emit "Filesystem has duplicate UUID" which is not correct. >>> >>> But anyway, the __GFP_NOFAIL in the call makes this all moot AFAICT. >>> >>> -Eric >> Hi Eric, >> >> Sorry for neglecting __GFP_NOFAIL symbol, and I would add a WARN in memory >> failure next time. > > Sorry about my limited knowledge, but why it needs a WARN here since > I think it will never fail if __GFP_NOFAIL is added (no ?). 'next time' means next time when I send patches related to memory failure, not on this one. Sorry for making confusing to you. > > I'm not sure if Hulk CI is completely broken or not on this, also if > such CI can now generate trivial patch (?) since the subject, commit > message and even the variable name is quite similiar to > https://lore.kernel.org/linux-xfs/20201124104531.561-2-thunder.leizhen@huawei.com > in a day. > > And it'd be better to look into the code before sending patches... Yeah.. I should pay more attension. > > Thanks, > Gao Xiang > Thanks for your advice~ >> >> Thanks for your advice！ >> > > . > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] xfs: check the return value of krealloc() in xfs_uuid_mount 2020-11-26 3:05 ` Qinglang Miao @ 2020-11-26 3:13 ` Gao Xiang 0 siblings, 0 replies; 6+ messages in thread From: Gao Xiang @ 2020-11-26 3:13 UTC (permalink / raw) To: Qinglang Miao; +Cc: Eric Sandeen, Darrick J. Wong, linux-xfs, linux-kernel On Thu, Nov 26, 2020 at 11:05:03AM +0800, Qinglang Miao wrote: ... > > > > I'm not sure if Hulk CI is completely broken or not on this, also if > > such CI can now generate trivial patch (?) since the subject, commit > > message and even the variable name is quite similiar to > > https://lore.kernel.org/linux-xfs/20201124104531.561-2-thunder.leizhen@huawei.com > > in a day. > > > > And it'd be better to look into the code before sending patches... > Yeah.. I should pay more attension. Yeah, it'd be better to address/fix the Hulk CI false report if possible, so it won't have such warning in the future. Thanks, Gao Xiang > > > > Thanks, > > Gao Xiang > > Thanks for your advice~ > > > > > > Thanks for your advice！ > > > > > > > . > > > ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-11-26 3:14 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-11-25 6:50 [PATCH] xfs: check the return value of krealloc() in xfs_uuid_mount Qinglang Miao 2020-11-25 15:55 ` Eric Sandeen 2020-11-26 1:21 ` Qinglang Miao 2020-11-26 2:16 ` Gao Xiang 2020-11-26 3:05 ` Qinglang Miao 2020-11-26 3:13 ` Gao Xiang
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®