* [PATCH 1/2] ext2: introduce helper for xattr header validation
@ 2019-05-10 10:37 Chengguang Xu
2019-05-10 10:37 ` [PATCH 2/2] ext2: introduce helper for xattr entry validation Chengguang Xu
2019-05-11 2:42 ` [PATCH 1/2] ext2: introduce helper for xattr header validation Andreas Dilger
0 siblings, 2 replies; 4+ messages in thread
From: Chengguang Xu @ 2019-05-10 10:37 UTC (permalink / raw)
To: jack; +Cc: linux-ext4, Chengguang Xu
Introduce helper function ext2_xattr_header_valid()
for xattr header validation and clean up the header
check ralated code.
Signed-off-by: Chengguang Xu <cgxu519@gmail.com>
---
fs/ext2/xattr.c | 31 ++++++++++++++++++++-----------
1 file changed, 20 insertions(+), 11 deletions(-)
diff --git a/fs/ext2/xattr.c b/fs/ext2/xattr.c
index 1e33e0ac8cf1..6e0b2b0f333f 100644
--- a/fs/ext2/xattr.c
+++ b/fs/ext2/xattr.c
@@ -134,6 +134,16 @@ ext2_xattr_handler(int name_index)
return handler;
}
+static bool
+ext2_xattr_header_valid(struct buffer_head *bh)
+{
+ if (HDR(bh)->h_magic != cpu_to_le32(EXT2_XATTR_MAGIC) ||
+ HDR(bh)->h_blocks != cpu_to_le32(1))
+ return false;
+
+ return true;
+}
+
/*
* ext2_xattr_get()
*
@@ -176,9 +186,9 @@ ext2_xattr_get(struct inode *inode, int name_index, const char *name,
ea_bdebug(bh, "b_count=%d, refcount=%d",
atomic_read(&(bh->b_count)), le32_to_cpu(HDR(bh)->h_refcount));
end = bh->b_data + bh->b_size;
- if (HDR(bh)->h_magic != cpu_to_le32(EXT2_XATTR_MAGIC) ||
- HDR(bh)->h_blocks != cpu_to_le32(1)) {
-bad_block: ext2_error(inode->i_sb, "ext2_xattr_get",
+ if (!ext2_xattr_header_valid(bh)) {
+bad_block:
+ ext2_error(inode->i_sb, "ext2_xattr_get",
"inode %ld: bad block %d", inode->i_ino,
EXT2_I(inode)->i_file_acl);
error = -EIO;
@@ -266,9 +276,9 @@ ext2_xattr_list(struct dentry *dentry, char *buffer, size_t buffer_size)
ea_bdebug(bh, "b_count=%d, refcount=%d",
atomic_read(&(bh->b_count)), le32_to_cpu(HDR(bh)->h_refcount));
end = bh->b_data + bh->b_size;
- if (HDR(bh)->h_magic != cpu_to_le32(EXT2_XATTR_MAGIC) ||
- HDR(bh)->h_blocks != cpu_to_le32(1)) {
-bad_block: ext2_error(inode->i_sb, "ext2_xattr_list",
+ if (!ext2_xattr_header_valid(bh)) {
+bad_block:
+ ext2_error(inode->i_sb, "ext2_xattr_list",
"inode %ld: bad block %d", inode->i_ino,
EXT2_I(inode)->i_file_acl);
error = -EIO;
@@ -406,9 +416,9 @@ ext2_xattr_set(struct inode *inode, int name_index, const char *name,
le32_to_cpu(HDR(bh)->h_refcount));
header = HDR(bh);
end = bh->b_data + bh->b_size;
- if (header->h_magic != cpu_to_le32(EXT2_XATTR_MAGIC) ||
- header->h_blocks != cpu_to_le32(1)) {
-bad_block: ext2_error(sb, "ext2_xattr_set",
+ if (!ext2_xattr_header_valid(bh)) {
+bad_block:
+ ext2_error(sb, "ext2_xattr_set",
"inode %ld: bad block %d", inode->i_ino,
EXT2_I(inode)->i_file_acl);
error = -EIO;
@@ -784,8 +794,7 @@ ext2_xattr_delete_inode(struct inode *inode)
goto cleanup;
}
ea_bdebug(bh, "b_count=%d", atomic_read(&(bh->b_count)));
- if (HDR(bh)->h_magic != cpu_to_le32(EXT2_XATTR_MAGIC) ||
- HDR(bh)->h_blocks != cpu_to_le32(1)) {
+ if (!ext2_xattr_header_valid(bh)) {
ext2_error(inode->i_sb, "ext2_xattr_delete_inode",
"inode %ld: bad block %d", inode->i_ino,
EXT2_I(inode)->i_file_acl);
--
2.20.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH 2/2] ext2: introduce helper for xattr entry validation
2019-05-10 10:37 [PATCH 1/2] ext2: introduce helper for xattr header validation Chengguang Xu
@ 2019-05-10 10:37 ` Chengguang Xu
2019-05-11 2:46 ` Andreas Dilger
2019-05-11 2:42 ` [PATCH 1/2] ext2: introduce helper for xattr header validation Andreas Dilger
1 sibling, 1 reply; 4+ messages in thread
From: Chengguang Xu @ 2019-05-10 10:37 UTC (permalink / raw)
To: jack; +Cc: linux-ext4, Chengguang Xu
Introduce helper function ext2_xattr_entry_valid()
for xattr entry validation and clean up the entry
check ralated code.
Signed-off-by: Chengguang Xu <cgxu519@gmail.com>
---
fs/ext2/xattr.c | 22 ++++++++++++++++------
1 file changed, 16 insertions(+), 6 deletions(-)
diff --git a/fs/ext2/xattr.c b/fs/ext2/xattr.c
index 6e0b2b0f333f..e40fff8ab543 100644
--- a/fs/ext2/xattr.c
+++ b/fs/ext2/xattr.c
@@ -144,6 +144,20 @@ ext2_xattr_header_valid(struct buffer_head *bh)
return true;
}
+static bool
+ext2_xattr_entry_valid(struct inode *inode, struct ext2_xattr_entry *entry,
+ size_t size)
+{
+ if (entry->e_value_block != 0)
+ return false;
+
+ if (size > inode->i_sb->s_blocksize ||
+ le16_to_cpu(entry->e_value_offs) + size > inode->i_sb->s_blocksize)
+ return false;
+
+ return true;
+}
+
/*
* ext2_xattr_get()
*
@@ -214,11 +228,8 @@ ext2_xattr_get(struct inode *inode, int name_index, const char *name,
goto cleanup;
found:
/* check the buffer size */
- if (entry->e_value_block != 0)
- goto bad_block;
size = le32_to_cpu(entry->e_value_size);
- if (size > inode->i_sb->s_blocksize ||
- le16_to_cpu(entry->e_value_offs) + size > inode->i_sb->s_blocksize)
+ if (!ext2_xattr_entry_valid(inode, entry, size))
goto bad_block;
if (ext2_xattr_cache_insert(ea_block_cache, bh))
@@ -483,8 +494,7 @@ ext2_xattr_set(struct inode *inode, int name_index, const char *name,
if (!here->e_value_block && here->e_value_size) {
size_t size = le32_to_cpu(here->e_value_size);
- if (le16_to_cpu(here->e_value_offs) + size >
- sb->s_blocksize || size > sb->s_blocksize)
+ if (!ext2_xattr_entry_valid(inode, here, size))
goto bad_block;
free += EXT2_XATTR_SIZE(size);
}
--
2.20.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH 1/2] ext2: introduce helper for xattr header validation
2019-05-10 10:37 [PATCH 1/2] ext2: introduce helper for xattr header validation Chengguang Xu
2019-05-10 10:37 ` [PATCH 2/2] ext2: introduce helper for xattr entry validation Chengguang Xu
@ 2019-05-11 2:42 ` Andreas Dilger
1 sibling, 0 replies; 4+ messages in thread
From: Andreas Dilger @ 2019-05-11 2:42 UTC (permalink / raw)
To: Chengguang Xu; +Cc: jack, linux-ext4
It seems like this would be more useful if you passed it the header
like ext2_xattr_header_valid(HDR(bh)).
Cheers, Andreas
> On May 10, 2019, at 04:37, Chengguang Xu <cgxu519@gmail.com> wrote:
>
> Introduce helper function ext2_xattr_header_valid()
> for xattr header validation and clean up the header
> check ralated code.
>
> Signed-off-by: Chengguang Xu <cgxu519@gmail.com>
> ---
> fs/ext2/xattr.c | 31 ++++++++++++++++++++-----------
> 1 file changed, 20 insertions(+), 11 deletions(-)
>
> diff --git a/fs/ext2/xattr.c b/fs/ext2/xattr.c
> index 1e33e0ac8cf1..6e0b2b0f333f 100644
> --- a/fs/ext2/xattr.c
> +++ b/fs/ext2/xattr.c
> @@ -134,6 +134,16 @@ ext2_xattr_handler(int name_index)
> return handler;
> }
>
> +static bool
> +ext2_xattr_header_valid(struct buffer_head *bh)
> +{
> + if (HDR(bh)->h_magic != cpu_to_le32(EXT2_XATTR_MAGIC) ||
> + HDR(bh)->h_blocks != cpu_to_le32(1))
> + return false;
> +
> + return true;
> +}
> +
> /*
> * ext2_xattr_get()
> *
> @@ -176,9 +186,9 @@ ext2_xattr_get(struct inode *inode, int name_index, const char *name,
> ea_bdebug(bh, "b_count=%d, refcount=%d",
> atomic_read(&(bh->b_count)), le32_to_cpu(HDR(bh)->h_refcount));
> end = bh->b_data + bh->b_size;
> - if (HDR(bh)->h_magic != cpu_to_le32(EXT2_XATTR_MAGIC) ||
> - HDR(bh)->h_blocks != cpu_to_le32(1)) {
> -bad_block: ext2_error(inode->i_sb, "ext2_xattr_get",
> + if (!ext2_xattr_header_valid(bh)) {
> +bad_block:
> + ext2_error(inode->i_sb, "ext2_xattr_get",
> "inode %ld: bad block %d", inode->i_ino,
> EXT2_I(inode)->i_file_acl);
> error = -EIO;
> @@ -266,9 +276,9 @@ ext2_xattr_list(struct dentry *dentry, char *buffer, size_t buffer_size)
> ea_bdebug(bh, "b_count=%d, refcount=%d",
> atomic_read(&(bh->b_count)), le32_to_cpu(HDR(bh)->h_refcount));
> end = bh->b_data + bh->b_size;
> - if (HDR(bh)->h_magic != cpu_to_le32(EXT2_XATTR_MAGIC) ||
> - HDR(bh)->h_blocks != cpu_to_le32(1)) {
> -bad_block: ext2_error(inode->i_sb, "ext2_xattr_list",
> + if (!ext2_xattr_header_valid(bh)) {
> +bad_block:
> + ext2_error(inode->i_sb, "ext2_xattr_list",
> "inode %ld: bad block %d", inode->i_ino,
> EXT2_I(inode)->i_file_acl);
> error = -EIO;
> @@ -406,9 +416,9 @@ ext2_xattr_set(struct inode *inode, int name_index, const char *name,
> le32_to_cpu(HDR(bh)->h_refcount));
> header = HDR(bh);
> end = bh->b_data + bh->b_size;
> - if (header->h_magic != cpu_to_le32(EXT2_XATTR_MAGIC) ||
> - header->h_blocks != cpu_to_le32(1)) {
> -bad_block: ext2_error(sb, "ext2_xattr_set",
> + if (!ext2_xattr_header_valid(bh)) {
> +bad_block:
> + ext2_error(sb, "ext2_xattr_set",
> "inode %ld: bad block %d", inode->i_ino,
> EXT2_I(inode)->i_file_acl);
> error = -EIO;
> @@ -784,8 +794,7 @@ ext2_xattr_delete_inode(struct inode *inode)
> goto cleanup;
> }
> ea_bdebug(bh, "b_count=%d", atomic_read(&(bh->b_count)));
> - if (HDR(bh)->h_magic != cpu_to_le32(EXT2_XATTR_MAGIC) ||
> - HDR(bh)->h_blocks != cpu_to_le32(1)) {
> + if (!ext2_xattr_header_valid(bh)) {
> ext2_error(inode->i_sb, "ext2_xattr_delete_inode",
> "inode %ld: bad block %d", inode->i_ino,
> EXT2_I(inode)->i_file_acl);
> --
> 2.20.1
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH 2/2] ext2: introduce helper for xattr entry validation
2019-05-10 10:37 ` [PATCH 2/2] ext2: introduce helper for xattr entry validation Chengguang Xu
@ 2019-05-11 2:46 ` Andreas Dilger
0 siblings, 0 replies; 4+ messages in thread
From: Andreas Dilger @ 2019-05-11 2:46 UTC (permalink / raw)
To: Chengguang Xu; +Cc: jack, linux-ext4
Similarly, this would be more useful if it took the blocksize directly as an
argument rather than the inode. That would allow it to be used to check
entries in the extended inode space or an external inode.
Cheers, Andreas
> On May 10, 2019, at 04:37, Chengguang Xu <cgxu519@gmail.com> wrote:
>
> Introduce helper function ext2_xattr_entry_valid()
> for xattr entry validation and clean up the entry
> check ralated code.
>
> Signed-off-by: Chengguang Xu <cgxu519@gmail.com>
> ---
> fs/ext2/xattr.c | 22 ++++++++++++++++------
> 1 file changed, 16 insertions(+), 6 deletions(-)
>
> diff --git a/fs/ext2/xattr.c b/fs/ext2/xattr.c
> index 6e0b2b0f333f..e40fff8ab543 100644
> --- a/fs/ext2/xattr.c
> +++ b/fs/ext2/xattr.c
> @@ -144,6 +144,20 @@ ext2_xattr_header_valid(struct buffer_head *bh)
> return true;
> }
>
> +static bool
> +ext2_xattr_entry_valid(struct inode *inode, struct ext2_xattr_entry *entry,
> + size_t size)
> +{
> + if (entry->e_value_block != 0)
> + return false;
> +
> + if (size > inode->i_sb->s_blocksize ||
> + le16_to_cpu(entry->e_value_offs) + size > inode->i_sb->s_blocksize)
> + return false;
> +
> + return true;
> +}
> +
> /*
> * ext2_xattr_get()
> *
> @@ -214,11 +228,8 @@ ext2_xattr_get(struct inode *inode, int name_index, const char *name,
> goto cleanup;
> found:
> /* check the buffer size */
> - if (entry->e_value_block != 0)
> - goto bad_block;
> size = le32_to_cpu(entry->e_value_size);
> - if (size > inode->i_sb->s_blocksize ||
> - le16_to_cpu(entry->e_value_offs) + size > inode->i_sb->s_blocksize)
> + if (!ext2_xattr_entry_valid(inode, entry, size))
> goto bad_block;
>
> if (ext2_xattr_cache_insert(ea_block_cache, bh))
> @@ -483,8 +494,7 @@ ext2_xattr_set(struct inode *inode, int name_index, const char *name,
> if (!here->e_value_block && here->e_value_size) {
> size_t size = le32_to_cpu(here->e_value_size);
>
> - if (le16_to_cpu(here->e_value_offs) + size >
> - sb->s_blocksize || size > sb->s_blocksize)
> + if (!ext2_xattr_entry_valid(inode, here, size))
> goto bad_block;
> free += EXT2_XATTR_SIZE(size);
> }
> --
> 2.20.1
>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2019-05-11 2:46 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-05-10 10:37 [PATCH 1/2] ext2: introduce helper for xattr header validation Chengguang Xu
2019-05-10 10:37 ` [PATCH 2/2] ext2: introduce helper for xattr entry validation Chengguang Xu
2019-05-11 2:46 ` Andreas Dilger
2019-05-11 2:42 ` [PATCH 1/2] ext2: introduce helper for xattr header validation Andreas Dilger
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).