| Message ID | 20220328032642.2371596-1-lv.ruyi@zte.com.cn (mailing list archive) |
|---|---|
| State | Deferred, archived |
| Headers | show |
| Series | fs: xfs: add NULL pointer check | expand |
On 3/28/22 12:26, cgel.zte@gmail.com wrote: > From: Lv Ruyi <lv.ruyi@zte.com.cn> > > kmem_zalloc() is a memory allocation function which can return NULL when > some internal memory errors happen. It is safer to check NULL pointer. > > Reported-by: Zeal Robot <zealci@zte.com.cn> > Signed-off-by: Lv Ruyi <lv.ruyi@zte.com.cn> > --- > fs/xfs/libxfs/xfs_attr_leaf.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/fs/xfs/libxfs/xfs_attr_leaf.c b/fs/xfs/libxfs/xfs_attr_leaf.c > index 014daa8c542d..e6694f49f563 100644 > --- a/fs/xfs/libxfs/xfs_attr_leaf.c > +++ b/fs/xfs/libxfs/xfs_attr_leaf.c > @@ -1571,6 +1571,8 @@ xfs_attr3_leaf_compact( > trace_xfs_attr_leaf_compact(args); > > tmpbuffer = kmem_alloc(args->geo->blksize, 0); See kmem_alloc() code: this function cannot fail (it retries the allocation until success). So checking for NULL pointer does not make sense. > + if (!tmpbuffer) > + return; > memcpy(tmpbuffer, bp->b_addr, args->geo->blksize); > memset(bp->b_addr, 0, args->geo->blksize); > leaf_src = (xfs_attr_leafblock_t *)tmpbuffer; > @@ -2290,6 +2292,8 @@ xfs_attr3_leaf_unbalance( > struct xfs_attr3_icleaf_hdr tmphdr; > > tmp_leaf = kmem_zalloc(state->args->geo->blksize, 0); > + if (!tmp_leaf) > + return; > > /* > * Copy the header into the temp leaf so that all the stuff
Sorry, please ignore the noise. thanks
On 3/28/22 12:40, Damien Le Moal wrote: > On 3/28/22 12:26, cgel.zte@gmail.com wrote: >> From: Lv Ruyi <lv.ruyi@zte.com.cn> >> >> kmem_zalloc() is a memory allocation function which can return NULL when >> some internal memory errors happen. It is safer to check NULL pointer. >> >> Reported-by: Zeal Robot <zealci@zte.com.cn> >> Signed-off-by: Lv Ruyi <lv.ruyi@zte.com.cn> >> --- >> fs/xfs/libxfs/xfs_attr_leaf.c | 4 ++++ >> 1 file changed, 4 insertions(+) >> >> diff --git a/fs/xfs/libxfs/xfs_attr_leaf.c b/fs/xfs/libxfs/xfs_attr_leaf.c >> index 014daa8c542d..e6694f49f563 100644 >> --- a/fs/xfs/libxfs/xfs_attr_leaf.c >> +++ b/fs/xfs/libxfs/xfs_attr_leaf.c >> @@ -1571,6 +1571,8 @@ xfs_attr3_leaf_compact( >> trace_xfs_attr_leaf_compact(args); >> >> tmpbuffer = kmem_alloc(args->geo->blksize, 0); > > See kmem_alloc() code: this function cannot fail (it retries the > allocation until success). So checking for NULL pointer does not make sense. Note: this comment is not 100% correct. kmem_alloc() can fail if KM_MAYFAIL is specified as a flag. But that is not the case here. > >> + if (!tmpbuffer) >> + return; >> memcpy(tmpbuffer, bp->b_addr, args->geo->blksize); >> memset(bp->b_addr, 0, args->geo->blksize); >> leaf_src = (xfs_attr_leafblock_t *)tmpbuffer; >> @@ -2290,6 +2292,8 @@ xfs_attr3_leaf_unbalance( >> struct xfs_attr3_icleaf_hdr tmphdr; >> >> tmp_leaf = kmem_zalloc(state->args->geo->blksize, 0); >> + if (!tmp_leaf) >> + return; >> >> /* >> * Copy the header into the temp leaf so that all the stuff > >
diff --git a/fs/xfs/libxfs/xfs_attr_leaf.c b/fs/xfs/libxfs/xfs_attr_leaf.c index 014daa8c542d..e6694f49f563 100644 --- a/fs/xfs/libxfs/xfs_attr_leaf.c +++ b/fs/xfs/libxfs/xfs_attr_leaf.c @@ -1571,6 +1571,8 @@ xfs_attr3_leaf_compact( trace_xfs_attr_leaf_compact(args); tmpbuffer = kmem_alloc(args->geo->blksize, 0); + if (!tmpbuffer) + return; memcpy(tmpbuffer, bp->b_addr, args->geo->blksize); memset(bp->b_addr, 0, args->geo->blksize); leaf_src = (xfs_attr_leafblock_t *)tmpbuffer; @@ -2290,6 +2292,8 @@ xfs_attr3_leaf_unbalance( struct xfs_attr3_icleaf_hdr tmphdr; tmp_leaf = kmem_zalloc(state->args->geo->blksize, 0); + if (!tmp_leaf) + return; /* * Copy the header into the temp leaf so that all the stuff