Skip to contentSkip to conversation

f2fs: correctly check empty xattr key

PATCH In review Taesoo Kim 3 participants 3 messages Mar 20, 2015 – Mar 23, 2015 F2FS file system

lore
Share & tools
lore

Taesoo Kim

author
#0 lore raw

PATCH 1/1 f2fs: correctly check empty xattr key

When xattr name (key) is empty (""), correctly return -EINVAL error. xattr_advise_set/get() seem to make the same mistake.

Signed-off-by Taesoo Kim <[email protected]>
---
 fs/f2fs/xattr.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
fs/f2fs/xattr.c +2 -2
index 5072bf9..6b1fbc4 100644
@@ -132,7 +132,7 @@ static int f2fs_xattr_advise_get(struct dentry *dentry, const char *name,
 {
 	struct inode *inode = dentry->d_inode;
 
-	if (strcmp(name, "") != 0)
+	if (strcmp(name, "") == 0)
 		return -EINVAL;
 
 	*((char *)buffer) = F2FS_I(inode)->i_advise;

@@ -144,7 +144,7 @@ static int f2fs_xattr_advise_set(struct dentry *dentry, const char *name,
 {
 	struct inode *inode = dentry->d_inode;
 
-	if (strcmp(name, "") != 0)
+	if (strcmp(name, "") == 0)
 		return -EINVAL;
 	if (!inode_owner_or_capable(inode))
 		return -EPERM;
Signature · 1 line
2.3.3
maintainer
#1 lore raw

Hi Taesoo,

Quoted 2 lines · Taesoo Kim

When xattr name (key) is empty (""), correctly return -EINVAL error. xattr_advise_set/get() seem to make the same mistake.

The intention here is to disallow any xattr name, since this is used to get or set the i_advise value by users.

Thanks,

Quoted 30 lines
Signed-off-by Taesoo Kim <[email protected]>
---
 fs/f2fs/xattr.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
fs/f2fs/xattr.c +2 -2
index 5072bf9..6b1fbc4 100644
@@ -132,7 +132,7 @@ static int f2fs_xattr_advise_get(struct dentry *dentry, const char *name,
 {
 	struct inode *inode = dentry->d_inode;
 
-	if (strcmp(name, "") != 0)
+	if (strcmp(name, "") == 0)
 		return -EINVAL;
 
 	*((char *)buffer) = F2FS_I(inode)->i_advise;

@@ -144,7 +144,7 @@ static int f2fs_xattr_advise_set(struct dentry *dentry, const char *name,
 {
 	struct inode *inode = dentry->d_inode;
 
-	if (strcmp(name, "") != 0)
+	if (strcmp(name, "") == 0)
 		return -EINVAL;
 	if (!inode_owner_or_capable(inode))
 		return -EPERM;
Signature · 1 line
2.3.3
#2 lore raw

Hi Jaegeuk,

Thank you for letting us know. We have another question:

@f2fs_sync_file()

if (unlikely(f2fs_readonly(inode->i_sb)))

     return 0;

Isn't it better to report '-EROFS' as ext3/4? I suspect that f2fs returns 0 as fsync() man page doesn't specify such error condition. We are wondering if it is the real reason for f2fs's design decision.

Thank you,
Taesoo

Quoted 41 lines · Jaegeuk Kim

Hi Taesoo,

Quoted 2 lines · Taesoo Kim

When xattr name (key) is empty (""), correctly return -EINVAL error. xattr_advise_set/get() seem to make the same mistake.

The intention here is to disallow any xattr name, since this is used to get or set the i_advise value by users.

Thanks,

Quoted 30 lines
Signed-off-by Taesoo Kim <[email protected]>
---
 fs/f2fs/xattr.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
fs/f2fs/xattr.c +2 -2
index 5072bf9..6b1fbc4 100644
@@ -132,7 +132,7 @@ static int f2fs_xattr_advise_get(struct dentry *dentry, const char *name,
 {
 	struct inode *inode = dentry->d_inode;
 
-	if (strcmp(name, "") != 0)
+	if (strcmp(name, "") == 0)
 		return -EINVAL;
 
 	*((char *)buffer) = F2FS_I(inode)->i_advise;

@@ -144,7 +144,7 @@ static int f2fs_xattr_advise_set(struct dentry *dentry, const char *name,
 {
 	struct inode *inode = dentry->d_inode;
 
-	if (strcmp(name, "") != 0)
+	if (strcmp(name, "") == 0)
 		return -EINVAL;
 	if (!inode_owner_or_capable(inode))
 		return -EPERM;
Signature · 1 line
2.3.3