Skip to contentSkip to conversation

ubifs: fix slab-out-of-bounds in pack_inode via xattr

PATCH In review Ren Wei 3 participants 3 messages Jun 3 – Jun 4 1 message not in the archive

Subsystems (2)
Memory technology devices (MTD) UBI file system (UBIFS)
lore
Share & tools
lore

A message that is not in the archive (sent off-list or not yet received): [email protected]

Ren Wei

author
#1 lore raw

PATCH 1/1 ubifs: fix slab-out-of-bounds in pack_inode via xattr

From: Luxiao Xu <[email protected]>

An issue in fs/ubifs/journal.c was found where a malicious or corrupted ubifs image could trigger a slab-out-of-bounds write in pack_inode().

When an inode with extended attributes (xattrs) is being evicted or deleted (i.e., 'last_reference' and 'kill_xattrs' are true), ubifs_jnl_write_inode() calculates the allocation size 'write_len' based on 'ui->xattr_cnt' from the disk.

However, if 'ui->xattr_cnt' is maliciously tampered with or corrupted to be smaller than the actual number of xattr entries stored in the TNC (Tree Node Cache), kmalloc() will allocate an undersized buffer. The subsequent 'while (1)' loop continues to iterate through all actual xattr entries via ubifs_tnc_next_ent(), advancing the 'ino' pointer and invoking pack_inode() for each entry. This mismatch leads to an out-of-bounds memory write, corrupting the slab redzone and triggering a KASAN panic.

Fix this by introducing a strict counter 'xattrs_deleted' within the deletion loop. The loop is aborted with -EUCLEAN if the number of processed xattrs exceeds 'ui->xattr_cnt', preventing 'ino' from overflowing the allocated kmalloc buffer.

The bug is reachable by a non-root user via user and net namespace.

Fixes 7959cf3a7506 ("ubifs: journal: Handle xattrs like files")
Cc [email protected]
Reported-by Yuan Tan <[email protected]>
Reported-by Yifan Wu <[email protected]>
Reported-by Juefei Pu <[email protected]>
Reported-by Xin Liu <[email protected]>
Signed-off-by Luxiao Xu <[email protected]>
Signed-off-by Ren Wei <[email protected]>
---
 fs/ubifs/journal.c | 9 +++++++++
 1 file changed, 9 insertions(+)
fs/ubifs/journal.c +10 -0
index d02509920baf..78fe9d2a3178 100644
@@ -880,6 +880,7 @@ int ubifs_jnl_write_inode(struct ubifs_info *c, const struct inode *inode)
 		struct fscrypt_name nm = {0};
 		struct inode *xino;
 		struct ubifs_dent_node *xent, *pxent = NULL;
+		int xattrs_deleted = 0;
 
 		if (ui->xattr_cnt > ubifs_xattr_max_cnt(c)) {
 			err = -EPERM;

@@ -899,6 +900,14 @@ int ubifs_jnl_write_inode(struct ubifs_info *c, const struct inode *inode)
 				goto out_release;
 			}
 
+			if (xattrs_deleted >= ui->xattr_cnt) {
+				kfree(pxent);
+				kfree(xent);
+				err = -EUCLEAN;
+				ubifs_err(c, "xattr count mismatch for ino %lu", inode->i_ino);
+				goto out_release;
+			}
+
 			fname_name(&nm) = xent->name;
 			fname_len(&nm) = le16_to_cpu(xent->nlen);
 

@@ -917,6 +926,7 @@ int ubifs_jnl_write_inode(struct ubifs_info *c, const struct inode *inode)
 			clear_nlink(xino);
 			pack_inode(c, ino, xino, 0);
 			ino = (void *)ino + UBIFS_INO_NODE_SZ;
+			xattrs_deleted++;
 			iput(xino);
 
 			kfree(pxent);
Signature · 6 lines
2.43.0


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
reviewer
#2 lore raw
Quoted 34 lines · Ren Wei

From: Luxiao Xu <[email protected]>

An issue in fs/ubifs/journal.c was found where a malicious or corrupted ubifs image could trigger a slab-out-of-bounds write in pack_inode().

When an inode with extended attributes (xattrs) is being evicted or deleted (i.e., 'last_reference' and 'kill_xattrs' are true), ubifs_jnl_write_inode() calculates the allocation size 'write_len' based on 'ui->xattr_cnt' from the disk.

However, if 'ui->xattr_cnt' is maliciously tampered with or corrupted to be smaller than the actual number of xattr entries stored in the TNC (Tree Node Cache), kmalloc() will allocate an undersized buffer. The subsequent 'while (1)' loop continues to iterate through all actual xattr entries via ubifs_tnc_next_ent(), advancing the 'ino' pointer and invoking pack_inode() for each entry. This mismatch leads to an out-of-bounds memory write, corrupting the slab redzone and triggering a KASAN panic.

Fix this by introducing a strict counter 'xattrs_deleted' within the deletion loop. The loop is aborted with -EUCLEAN if the number of processed xattrs exceeds 'ui->xattr_cnt', preventing 'ino' from overflowing the allocated kmalloc buffer.

The bug is reachable by a non-root user via user and net namespace.

Fixes 7959cf3a7506 ("ubifs: journal: Handle xattrs like files")
Cc [email protected]
Reported-by Yuan Tan <[email protected]>
Reported-by Yifan Wu <[email protected]>
Reported-by Juefei Pu <[email protected]>
Reported-by Xin Liu <[email protected]>
Signed-off-by Luxiao Xu <[email protected]>
Signed-off-by Ren Wei <[email protected]>
Quoted 4 lines
---
  fs/ubifs/journal.c | 9 +++++++++
  1 file changed, 9 insertions(+)

Hi Luxiao,
Each metadata(inode/dentry) entry will checked with crc while loading from flash(see ubifs_read_node->ubifs_check_node), I don't think corrupted image will trigger the problem, because ubifs_lookup->ubifs_iget will fail after detecting crc error. Unless someone manually inject wong number into 'ui->xattr_cnt' fieled and update the whole ino node crc, and I don't think it is a real problem need to be fixed. In that injection method, no one filesystem is okay. The bitflips happen both in node field(eg. xattr_cnt) and crc field is an almost impossible thing.

Quoted 29 lines
fs/ubifs/journal.c +9 -0
index d02509920baf..78fe9d2a3178 100644
@@ -880,6 +880,7 @@ int ubifs_jnl_write_inode(struct ubifs_info *c, const struct inode *inode)
  		struct fscrypt_name nm = {0};
  		struct inode *xino;
  		struct ubifs_dent_node *xent, *pxent = NULL;
+		int xattrs_deleted = 0;
 
  		if (ui->xattr_cnt > ubifs_xattr_max_cnt(c)) {
  			err = -EPERM;

@@ -899,6 +900,14 @@ int ubifs_jnl_write_inode(struct ubifs_info *c, const struct inode *inode)
  				goto out_release;
  			}
 
+			if (xattrs_deleted >= ui->xattr_cnt) {
+				kfree(pxent);
+				kfree(xent);
+				err = -EUCLEAN;
+				ubifs_err(c, "xattr count mismatch for ino %lu", inode->i_ino);
+				goto out_release;
+			}
+
  			fname_name(&nm) = xent->name;
  			fname_len(&nm) = le16_to_cpu(xent->nlen);

fs/ubifs/journal.c int ubifs_jnl_write_inode(struct ubifs_info *c, const struct inode *inode) · line 932

Quoted 7 lines
fs/ubifs/journal.c +1 -0
@@ -917,6 +926,7 @@ int ubifs_jnl_write_inode(struct ubifs_info *c, const struct inode *inode)
  			clear_nlink(xino);
  			pack_inode(c, ino, xino, 0);
  			ino = (void *)ino + UBIFS_INO_NODE_SZ;
+			xattrs_deleted++;
  			iput(xino);
 
  			kfree(pxent);

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

maintainer
#3 lore raw

----- Ursprüngliche Mail -----

Quoted 27 lines

Von: "chengzhihao1" <[email protected]>

Quoted 25 lines · Ren Wei

From: Luxiao Xu <[email protected]>

An issue in fs/ubifs/journal.c was found where a malicious or corrupted ubifs image could trigger a slab-out-of-bounds write in pack_inode().

When an inode with extended attributes (xattrs) is being evicted or deleted (i.e., 'last_reference' and 'kill_xattrs' are true), ubifs_jnl_write_inode() calculates the allocation size 'write_len' based on 'ui->xattr_cnt' from the disk.

However, if 'ui->xattr_cnt' is maliciously tampered with or corrupted to be smaller than the actual number of xattr entries stored in the TNC (Tree Node Cache), kmalloc() will allocate an undersized buffer. The subsequent 'while (1)' loop continues to iterate through all actual xattr entries via ubifs_tnc_next_ent(), advancing the 'ino' pointer and invoking pack_inode() for each entry. This mismatch leads to an out-of-bounds memory write, corrupting the slab redzone and triggering a KASAN panic.

Fix this by introducing a strict counter 'xattrs_deleted' within the deletion loop. The loop is aborted with -EUCLEAN if the number of processed xattrs exceeds 'ui->xattr_cnt', preventing 'ino' from overflowing the allocated kmalloc buffer.

The bug is reachable by a non-root user via user and net namespace.

As I wrote in my reply to 0/1, I'm not so sure about that. To load a MTD and mount UBIFS you need to be the real root.

Quoted 14 lines
Quoted 9 lines
Fixes 7959cf3a7506 ("ubifs: journal: Handle xattrs like files")
Cc [email protected]
Reported-by Yuan Tan <[email protected]>
Reported-by Yifan Wu <[email protected]>
Reported-by Juefei Pu <[email protected]>
Reported-by Xin Liu <[email protected]>
Signed-off-by Luxiao Xu <[email protected]>
Signed-off-by Ren Wei <[email protected]>
Quoted 4 lines
---
  fs/ubifs/journal.c | 9 +++++++++
  1 file changed, 9 insertions(+)
Quoted 10 lines

Hi Luxiao,
Each metadata(inode/dentry) entry will checked with crc while loading from flash(see ubifs_read_node->ubifs_check_node), I don't think corrupted image will trigger the problem, because ubifs_lookup->ubifs_iget will fail after detecting crc error. Unless someone manually inject wong number into 'ui->xattr_cnt' fieled and update the whole ino node crc, and I don't think it is a real problem need to be fixed. In that injection method, no one filesystem is okay. The bitflips happen both in node field(eg. xattr_cnt) and crc field is an almost impossible thing.

Well, if we can easily detect this case, let's fix it. UBIFS tries hard to make sure the underlaying data structures are correct.

Thanks,
//richard

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/