[PATCH] f2fs: correct removexattr behavior for null valued extended attribute

[PATCH] f2fs: correct removexattr behavior for null valued extended attribute

[Chao Yu]

From: Daeho Jeong <daeho.jeong@samsung.com>

__vfs_removexattr() transfers "NULL" value to the setxattr handler of
the f2fs filesystem in order to remove the extended attribute. But,
__f2fs_setxattr() just ignores the removal request when the value of
the extended attribute is already NULL. We have to remove the extended
attribute itself even if the value of that is already NULL.

We can reporduce this bug with the below:

1. touch file
2. setfattr -n "user.foo" file
3. setfattr -x "user.foo" file
4. getfattr -d file
> user.foo

Signed-off-by: Daeho Jeong <daeho.jeong@samsung.com>
Signed-off-by: Youngjin Gil <youngjin.gil@samsung.com>
Tested-by: Hobin Woo <hobin.woo@samsung.com>
Tested-by: Chao Yu <yuchao0@huawei.com>
Reviewed-by: Chao Yu <yuchao0@huawei.com>
Signed-off-by: Chao Yu <yuchao0@huawei.com>
---
 fs/f2fs/xattr.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/f2fs/xattr.c b/fs/f2fs/xattr.c
index 600162f4ddbf..ae2dfa709f5d 100644
--- a/fs/f2fs/xattr.c
+++ b/fs/f2fs/xattr.c
@@ -600,7 +600,7 @@ static int __f2fs_setxattr(struct inode *inode, int index,
 			goto exit;
 		}
 
-		if (f2fs_xattr_value_same(here, value, size))
+		if (value && f2fs_xattr_value_same(here, value, size))
 			goto exit;
 	} else if ((flags & XATTR_REPLACE)) {
 		error = -ENODATA;
-- 
2.14.1.145.gb3622a4ee


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot

Re: [PATCH] f2fs: correct removexattr behavior for null valued extended attribute

[Jaegeuk Kim]

Thanks,

On 01/20, Chao Yu wrote:
> From: Daeho Jeong <daeho.jeong@samsung.com>
> 
> __vfs_removexattr() transfers "NULL" value to the setxattr handler of
> the f2fs filesystem in order to remove the extended attribute. But,
> __f2fs_setxattr() just ignores the removal request when the value of
> the extended attribute is already NULL. We have to remove the extended
> attribute itself even if the value of that is already NULL.
> 
> We can reporduce this bug with the below:
> 
> 1. touch file
> 2. setfattr -n "user.foo" file
> 3. setfattr -x "user.foo" file
> 4. getfattr -d file
> > user.foo
> 
> Signed-off-by: Daeho Jeong <daeho.jeong@samsung.com>
> Signed-off-by: Youngjin Gil <youngjin.gil@samsung.com>
> Tested-by: Hobin Woo <hobin.woo@samsung.com>
> Tested-by: Chao Yu <yuchao0@huawei.com>
> Reviewed-by: Chao Yu <yuchao0@huawei.com>
> Signed-off-by: Chao Yu <yuchao0@huawei.com>
> ---
>  fs/f2fs/xattr.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/fs/f2fs/xattr.c b/fs/f2fs/xattr.c
> index 600162f4ddbf..ae2dfa709f5d 100644
> --- a/fs/f2fs/xattr.c
> +++ b/fs/f2fs/xattr.c
> @@ -600,7 +600,7 @@ static int __f2fs_setxattr(struct inode *inode, int index,
>  			goto exit;
>  		}
>  
> -		if (f2fs_xattr_value_same(here, value, size))
> +		if (value && f2fs_xattr_value_same(here, value, size))
>  			goto exit;
>  	} else if ((flags & XATTR_REPLACE)) {
>  		error = -ENODATA;
> -- 
> 2.14.1.145.gb3622a4ee

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot

[PATCH] f2fs: correct removexattr behavior for null valued extended attribute

[Daeho Jeong]

__vfs_removexattr() transfers "NULL" value to the setxattr handler of
the f2fs filesystem in order to remove the extended attribute. But,
__f2fs_setxattr() just ignores the removal request when the value of
the extended attribute is already NULL. We have to remove the extended
attribute itself even if the value of that is already NULL.

Signed-off-by: Daeho Jeong <daeho.jeong@samsung.com>
Signed-off-by: Youngjin Gil <youngjin.gil@samsung.com>
Tested-by: Hobin Woo <hobin.woo@samsung.com>
---
 fs/f2fs/xattr.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/f2fs/xattr.c b/fs/f2fs/xattr.c
index ec8961e..2776618 100644
--- a/fs/f2fs/xattr.c
+++ b/fs/f2fs/xattr.c
@@ -598,7 +598,7 @@ static int __f2fs_setxattr(struct inode *inode, int index,
 			goto exit;
 		}
 
-		if (f2fs_xattr_value_same(here, value, size))
+		if (value && f2fs_xattr_value_same(here, value, size))
 			goto exit;
 	} else if ((flags & XATTR_REPLACE)) {
 		error = -ENODATA;
-- 
1.9.1


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot

Re: [PATCH] f2fs: correct removexattr behavior for null valued extended attribute

[Chao Yu]

On 2018/1/9 8:22, Daeho Jeong wrote:
> __vfs_removexattr() transfers "NULL" value to the setxattr handler of
> the f2fs filesystem in order to remove the extended attribute. But,
> __f2fs_setxattr() just ignores the removal request when the value of
> the extended attribute is already NULL. We have to remove the extended

How can we build such xattr entry whose value is NULL? Can you give an
reproducer example?

Thanks,

> attribute itself even if the value of that is already NULL.
> 
> Signed-off-by: Daeho Jeong <daeho.jeong@samsung.com>
> Signed-off-by: Youngjin Gil <youngjin.gil@samsung.com>
> Tested-by: Hobin Woo <hobin.woo@samsung.com>
> ---
>  fs/f2fs/xattr.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/fs/f2fs/xattr.c b/fs/f2fs/xattr.c
> index ec8961e..2776618 100644
> --- a/fs/f2fs/xattr.c
> +++ b/fs/f2fs/xattr.c
> @@ -598,7 +598,7 @@ static int __f2fs_setxattr(struct inode *inode, int index,
>  			goto exit;
>  		}
>  
> -		if (f2fs_xattr_value_same(here, value, size))
> +		if (value && f2fs_xattr_value_same(here, value, size))
>  			goto exit;
>  	} else if ((flags & XATTR_REPLACE)) {
>  		error = -ENODATA;
> 


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot

Re: [PATCH] f2fs: correct removexattr behavior for null valued extended attribute

[Daeho Jeong]

Hi, Chao

> How can we build such xattr entry whose value is NULL? Can you give an
> reproducer example?
 
We used setxattr system call with "value" as NULL and "size" as 0.
And then vfs_setxattr will change "value" to "" and transfer "value" and "size"
to f2fs_setxattr().

Thanks,

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

Re: [PATCH] f2fs: correct removexattr behavior for null valued extended attribute

[Chao Yu]

Hi Daeho,

On 2018/1/9 17:59, Daeho Jeong wrote:
> Hi, Chao
> 
>> How can we build such xattr entry whose value is NULL? Can you give an
>> reproducer example?
>  
> We used setxattr system call with "value" as NULL and "size" as 0.
> And then vfs_setxattr will change "value" to "" and transfer "value" and "size"
> to f2fs_setxattr().

Correct, I can reproduce this bug with:

1. touch file
2. setfattr -n "user.foo" file
3. setfattr -x "user.foo" file
4. getfattr -d file
user.foo

After applying your patch, the problem can be fixed.

Could you add below test case explicitly in your commit log?

Reviewed-by: Chao Yu <yuchao0@huawei.com>
Tested-by: Chao Yu <yuchao0@huawei.com>

Thanks,

> 
> Thanks,
> 
> ------------------------------------------------------------------------------
> Check out the vibrant tech community on one of the world's most
> engaging tech sites, Slashdot.org! http://sdm.link/slashdot
> _______________________________________________
> Linux-f2fs-devel mailing list
> Linux-f2fs-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
> 

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel