This is a good example that emphasizes that the order in which patches
are queued to stable matters. More details in the revert commit.
Tested and intended for 4.14, 4.19, 5.4, 5.10.
Baokun Li (1):
ext4: fix use-after-free in ext4_xattr_set_entry
Ritesh Harjani (1):
ext4: remove duplicate definition of ext4_xattr_ibody_inline_set()
Tudor Ambarus (1):
Revert "ext4: fix use-after-free in ext4_xattr_set_entry"
fs/ext4/inline.c | 11 +++++------
fs/ext4/xattr.c | 26 +-------------------------
fs/ext4/xattr.h | 6 +++---
3 files changed, 9 insertions(+), 34 deletions(-)
--
2.40.0.634.g4ca3ef3211-goog
This reverts commit bb8592efcf8ef2f62947745d3182ea05b5256a15 which is
commit 67d7d8ad99beccd9fe92d585b87f1760dc9018e3 upstream.
The order in which patches are queued to stable matters. This patch
has a logical dependency on commit 310c097c2bdbea253d6ee4e064f3e65580ef93ac
upstream, and failing to queue the latter results in a null-ptr-deref
reported at the Link below.
In order to avoid conflicts on stable, revert the commit just so that we
can queue its prerequisite patch first and then queue the same after.
Link: https://syzkaller.appspot.com/bug?extid=d5ebf56f3b1268136afd
Signed-off-by: Tudor Ambarus <[email protected]>
---
fs/ext4/xattr.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/fs/ext4/xattr.c b/fs/ext4/xattr.c
index f3da1f2d4cb9..948da799abab 100644
--- a/fs/ext4/xattr.c
+++ b/fs/ext4/xattr.c
@@ -2193,9 +2193,8 @@ int ext4_xattr_ibody_find(struct inode *inode, struct ext4_xattr_info *i,
struct ext4_inode *raw_inode;
int error;
- if (!EXT4_INODE_HAS_XATTR_SPACE(inode))
+ if (EXT4_I(inode)->i_extra_isize == 0)
return 0;
-
raw_inode = ext4_raw_inode(&is->iloc);
header = IHDR(inode, raw_inode);
is->s.base = is->s.first = IFIRST(header);
@@ -2223,9 +2222,8 @@ int ext4_xattr_ibody_inline_set(handle_t *handle, struct inode *inode,
struct ext4_xattr_search *s = &is->s;
int error;
- if (!EXT4_INODE_HAS_XATTR_SPACE(inode))
+ if (EXT4_I(inode)->i_extra_isize == 0)
return -ENOSPC;
-
error = ext4_xattr_set_entry(i, s, handle, inode, false /* is_block */);
if (error)
return error;
--
2.40.0.634.g4ca3ef3211-goog
From: Ritesh Harjani <[email protected]>
[ Upstream commit 310c097c2bdbea253d6ee4e064f3e65580ef93ac ]
ext4_xattr_ibody_inline_set() & ext4_xattr_ibody_set() have the exact
same definition. Hence remove ext4_xattr_ibody_inline_set() and all
its call references. Convert the callers of it to call
ext4_xattr_ibody_set() instead.
[ Modified to preserve ext4_xattr_ibody_set() and remove
ext4_xattr_ibody_inline_set() instead. -- TYT ]
Signed-off-by: Ritesh Harjani <[email protected]>
Link: https://lore.kernel.org/r/fd566b799bbbbe9b668eb5eecde5b5e319e3694f.1622685482.git.riteshh@linux.ibm.com
Signed-off-by: Theodore Ts'o <[email protected]>
Signed-off-by: Tudor Ambarus <[email protected]>
---
fs/ext4/inline.c | 11 +++++------
fs/ext4/xattr.c | 26 +-------------------------
fs/ext4/xattr.h | 6 +++---
3 files changed, 9 insertions(+), 34 deletions(-)
diff --git a/fs/ext4/inline.c b/fs/ext4/inline.c
index 61cb50e8fcb7..0758f606f006 100644
--- a/fs/ext4/inline.c
+++ b/fs/ext4/inline.c
@@ -206,7 +206,7 @@ static int ext4_read_inline_data(struct inode *inode, void *buffer,
/*
* write the buffer to the inline inode.
* If 'create' is set, we don't need to do the extra copy in the xattr
- * value since it is already handled by ext4_xattr_ibody_inline_set.
+ * value since it is already handled by ext4_xattr_ibody_set.
* That saves us one memcpy.
*/
static void ext4_write_inline_data(struct inode *inode, struct ext4_iloc *iloc,
@@ -288,7 +288,7 @@ static int ext4_create_inline_data(handle_t *handle,
BUG_ON(!is.s.not_found);
- error = ext4_xattr_ibody_inline_set(handle, inode, &i, &is);
+ error = ext4_xattr_ibody_set(handle, inode, &i, &is);
if (error) {
if (error == -ENOSPC)
ext4_clear_inode_state(inode,
@@ -360,7 +360,7 @@ static int ext4_update_inline_data(handle_t *handle, struct inode *inode,
i.value = value;
i.value_len = len;
- error = ext4_xattr_ibody_inline_set(handle, inode, &i, &is);
+ error = ext4_xattr_ibody_set(handle, inode, &i, &is);
if (error)
goto out;
@@ -433,7 +433,7 @@ static int ext4_destroy_inline_data_nolock(handle_t *handle,
if (error)
goto out;
- error = ext4_xattr_ibody_inline_set(handle, inode, &i, &is);
+ error = ext4_xattr_ibody_set(handle, inode, &i, &is);
if (error)
goto out;
@@ -1930,8 +1930,7 @@ int ext4_inline_data_truncate(struct inode *inode, int *has_inline)
i.value = value;
i.value_len = i_size > EXT4_MIN_INLINE_DATA_SIZE ?
i_size - EXT4_MIN_INLINE_DATA_SIZE : 0;
- err = ext4_xattr_ibody_inline_set(handle, inode,
- &i, &is);
+ err = ext4_xattr_ibody_set(handle, inode, &i, &is);
if (err)
goto out_error;
}
diff --git a/fs/ext4/xattr.c b/fs/ext4/xattr.c
index 948da799abab..71e83e815258 100644
--- a/fs/ext4/xattr.c
+++ b/fs/ext4/xattr.c
@@ -2214,31 +2214,7 @@ int ext4_xattr_ibody_find(struct inode *inode, struct ext4_xattr_info *i,
return 0;
}
-int ext4_xattr_ibody_inline_set(handle_t *handle, struct inode *inode,
- struct ext4_xattr_info *i,
- struct ext4_xattr_ibody_find *is)
-{
- struct ext4_xattr_ibody_header *header;
- struct ext4_xattr_search *s = &is->s;
- int error;
-
- if (EXT4_I(inode)->i_extra_isize == 0)
- return -ENOSPC;
- error = ext4_xattr_set_entry(i, s, handle, inode, false /* is_block */);
- if (error)
- return error;
- header = IHDR(inode, ext4_raw_inode(&is->iloc));
- if (!IS_LAST_ENTRY(s->first)) {
- header->h_magic = cpu_to_le32(EXT4_XATTR_MAGIC);
- ext4_set_inode_state(inode, EXT4_STATE_XATTR);
- } else {
- header->h_magic = cpu_to_le32(0);
- ext4_clear_inode_state(inode, EXT4_STATE_XATTR);
- }
- return 0;
-}
-
-static int ext4_xattr_ibody_set(handle_t *handle, struct inode *inode,
+int ext4_xattr_ibody_set(handle_t *handle, struct inode *inode,
struct ext4_xattr_info *i,
struct ext4_xattr_ibody_find *is)
{
diff --git a/fs/ext4/xattr.h b/fs/ext4/xattr.h
index b357872ab83b..e5e36bd11f05 100644
--- a/fs/ext4/xattr.h
+++ b/fs/ext4/xattr.h
@@ -200,9 +200,9 @@ extern int ext4_xattr_ibody_find(struct inode *inode, struct ext4_xattr_info *i,
extern int ext4_xattr_ibody_get(struct inode *inode, int name_index,
const char *name,
void *buffer, size_t buffer_size);
-extern int ext4_xattr_ibody_inline_set(handle_t *handle, struct inode *inode,
- struct ext4_xattr_info *i,
- struct ext4_xattr_ibody_find *is);
+extern int ext4_xattr_ibody_set(handle_t *handle, struct inode *inode,
+ struct ext4_xattr_info *i,
+ struct ext4_xattr_ibody_find *is);
extern struct mb_cache *ext4_xattr_create_cache(void);
extern void ext4_xattr_destroy_cache(struct mb_cache *);
--
2.40.0.634.g4ca3ef3211-goog
On Wed, Apr 19, 2023 at 06:46:07AM +0000, Tudor Ambarus wrote:
> This is a good example that emphasizes that the order in which patches
> are queued to stable matters. More details in the revert commit.
> Tested and intended for 4.14, 4.19, 5.4, 5.10.
>
> Baokun Li (1):
> ext4: fix use-after-free in ext4_xattr_set_entry
>
> Ritesh Harjani (1):
> ext4: remove duplicate definition of ext4_xattr_ibody_inline_set()
>
> Tudor Ambarus (1):
> Revert "ext4: fix use-after-free in ext4_xattr_set_entry"
>
> fs/ext4/inline.c | 11 +++++------
> fs/ext4/xattr.c | 26 +-------------------------
> fs/ext4/xattr.h | 6 +++---
> 3 files changed, 9 insertions(+), 34 deletions(-)
All now queued up, thanks.
greg k-h