2019-02-14 11:28:56

by Sheng Yong

Subject: [PATCH v2] staging: erofs: fix memleak of inode's shared xattr array

If it fails to read a shared xattr page, the inode's shared xattr array
is not freed. The next time the inode's xattr is accessed, the previously
allocated array is leaked.

Signed-off-by: Sheng Yong <[email protected]>
---
v2: * s/node/inode in commit message
* add prefix `staging:' to the subject
---
drivers/staging/erofs/xattr.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/erofs/xattr.c b/drivers/staging/erofs/xattr.c
index 80dca6a4adbe..e3f8b904a680 100644
--- a/drivers/staging/erofs/xattr.c
+++ b/drivers/staging/erofs/xattr.c
@@ -92,8 +92,11 @@ static int init_inode_xattrs(struct inode *inode)

it.page = erofs_get_meta_page(sb,
++it.blkaddr, S_ISDIR(inode->i_mode));
- if (IS_ERR(it.page))
+ if (IS_ERR(it.page)) {
+ kfree(vi->xattr_shared_xattrs);
+ vi->xattr_shared_xattrs = NULL;
return PTR_ERR(it.page);
+ }

it.kaddr = kmap_atomic(it.page);
atomic_map = true;
--
2.17.1

2019-02-14 11:33:04

by Gao Xiang

Subject: Re: [PATCH v2] staging: erofs: fix memleak of inode's shared xattr array

On 2019/2/14 14:46, Sheng Yong wrote:
> If it fails to read a shared xattr page, the inode's shared xattr array
> is not freed. The next time the inode's xattr is accessed, the previously
> allocated array is leaked.
>
> Signed-off-by: Sheng Yong <[email protected]>

LGTM,

Fixes: b17500a0fdba ("staging: erofs: introduce xattr & acl support")
Cc: <[email protected]> # 4.19+
Reviewed-by: Gao Xiang <[email protected]>

[and there is also another race condition in it, but different root cause.
let me fix it later independently...]

Thanks,
Gao Xiang

2019-02-15 02:36:45

by Chao Yu

Subject: Re: [PATCH v2] staging: erofs: fix memleak of inode's shared xattr array

On 2019/2/14 15:10, Gao Xiang wrote:
>
>
> On 2019/2/14 14:46, Sheng Yong wrote:
>> If it fails to read a shared xattr page, the inode's shared xattr array
>> is not freed. The next time the inode's xattr is accessed, the previously
>> allocated array is leaked.

Nice catch!

>>
>> Signed-off-by: Sheng Yong <[email protected]>
>
> LGTM,
>
> Fixes: b17500a0fdba ("staging: erofs: introduce xattr & acl support")
> Cc: <[email protected]> # 4.19+
> Reviewed-by: Gao Xiang <[email protected]>

Reviewed-by: Chao Yu <[email protected]>

Thanks,

>
> [and there is also another race condition in it, but different root cause.
> let me fix it later independently...]
>
> Thanks,
> Gao Xiang
>
> .
>