Re: [PATCH] jfs : fs array-index-out-of-bounds in txCommit

[Dave Kleikamp <dave.kleikamp@oracle.com>]

On 10/5/23 12:15AM, Manas Ghandat wrote:
> On 04/10/23 00:46, Dave Kleikamp wrote:
>> The size of the xad array can be either XTROOTMAXSLOT or XTPAGEMAXSLOT 
>> depending on whether it is the root, imbedded in the inode, or not. It 
>> is currently declared with the smaller value so we can use xtpage_t 
>> within the inode.
>>
>> I had promised to address this, but haven't gotten to it yet. I'll try 
>> to carve out some time to do that.
>>
>> Thanks,
>> Shaggy
> 
> Can you guide with the workflow of how things should be done. I can try 
> to work on it and resolve the issue.
>

I was able to cobble this together. It compiles cleanly, but I haven't
tested it yet.

In order to make array bounds checking sane, provide a separate
definition of the in-inode xtree root and the external xtree page.

Signed-off-by: Dave Kleikamp <dave.kleikamp@oracle.com>
---
  fs/jfs/jfs_dinode.h |  2 +-
  fs/jfs/jfs_imap.c   |  6 +++---
  fs/jfs/jfs_incore.h |  2 +-
  fs/jfs/jfs_txnmgr.c |  4 ++--
  fs/jfs/jfs_xtree.c  |  4 ++--
  fs/jfs/jfs_xtree.h  | 37 +++++++++++++++++++++++--------------
  6 files changed, 32 insertions(+), 23 deletions(-)

diff --git a/fs/jfs/jfs_dinode.h b/fs/jfs/jfs_dinode.h
index 6b231d0d0071..603aae17a693 100644
--- a/fs/jfs/jfs_dinode.h
+++ b/fs/jfs/jfs_dinode.h
@@ -96,7 +96,7 @@ struct dinode {
  #define di_gengen	u._file._u1._imap._gengen

  			union {
-				xtpage_t _xtroot;
+				xtroot_t _xtroot;
  				struct {
  					u8 unused[16];	/* 16: */
  					dxd_t _dxd;	/* 16: */
diff --git a/fs/jfs/jfs_imap.c b/fs/jfs/jfs_imap.c
index 1b267eec3f36..394e0af0e5df 100644
--- a/fs/jfs/jfs_imap.c
+++ b/fs/jfs/jfs_imap.c
@@ -670,7 +670,7 @@ int diWrite(tid_t tid, struct inode *ip)
  		 * This is the special xtree inside the directory for storing
  		 * the directory table
  		 */
-		xtpage_t *p, *xp;
+		xtroot_t *p, *xp;
  		xad_t *xad;

  		jfs_ip->xtlid = 0;
@@ -684,7 +684,7 @@ int diWrite(tid_t tid, struct inode *ip)
  		 * copy xtree root from inode to dinode:
  		 */
  		p = &jfs_ip->i_xtroot;
-		xp = (xtpage_t *) &dp->di_dirtable;
+		xp = (xtroot_t *) &dp->di_dirtable;
  		lv = ilinelock->lv;
  		for (n = 0; n < ilinelock->index; n++, lv++) {
  			memcpy(&xp->xad[lv->offset], &p->xad[lv->offset],
@@ -713,7 +713,7 @@ int diWrite(tid_t tid, struct inode *ip)
  	 *	regular file: 16 byte (XAD slot) granularity
  	 */
  	if (type & tlckXTREE) {
-		xtpage_t *p, *xp;
+		xtroot_t *p, *xp;
  		xad_t *xad;

  		/*
diff --git a/fs/jfs/jfs_incore.h b/fs/jfs/jfs_incore.h
index 721def69e732..dd4264aa9bed 100644
--- a/fs/jfs/jfs_incore.h
+++ b/fs/jfs/jfs_incore.h
@@ -66,7 +66,7 @@ struct jfs_inode_info {
  	lid_t	xtlid;		/* lid of xtree lock on directory */
  	union {
  		struct {
-			xtpage_t _xtroot;	/* 288: xtree root */
+			xtroot_t _xtroot;	/* 288: xtree root */
  			struct inomap *_imap;	/* 4: inode map header	*/
  		} file;
  		struct {
diff --git a/fs/jfs/jfs_txnmgr.c b/fs/jfs/jfs_txnmgr.c
index ce4b4760fcb1..dccc8b3f1045 100644
--- a/fs/jfs/jfs_txnmgr.c
+++ b/fs/jfs/jfs_txnmgr.c
@@ -783,7 +783,7 @@ struct tlock *txLock(tid_t tid, struct inode *ip, 
struct metapage * mp,
  			if (mp->xflag & COMMIT_PAGE)
  				p = (xtpage_t *) mp->data;
  			else
-				p = &jfs_ip->i_xtroot;
+				p = (xtpage_t *) &jfs_ip->i_xtroot;
  			xtlck->lwm.offset =
  			    le16_to_cpu(p->header.nextindex);
  		}
@@ -1676,7 +1676,7 @@ static void xtLog(struct jfs_log * log, struct 
tblock * tblk, struct lrd * lrd,

  	if (tlck->type & tlckBTROOT) {
  		lrd->log.redopage.type |= cpu_to_le16(LOG_BTROOT);
-		p = &JFS_IP(ip)->i_xtroot;
+		p = (xtpage_t *) &JFS_IP(ip)->i_xtroot;
  		if (S_ISDIR(ip->i_mode))
  			lrd->log.redopage.type |=
  			    cpu_to_le16(LOG_DIR_XTREE);
diff --git a/fs/jfs/jfs_xtree.c b/fs/jfs/jfs_xtree.c
index 2d304cee884c..5ee618d17e77 100644
--- a/fs/jfs/jfs_xtree.c
+++ b/fs/jfs/jfs_xtree.c
@@ -1213,7 +1213,7 @@ xtSplitRoot(tid_t tid,
  	struct xtlock *xtlck;
  	int rc;

-	sp = &JFS_IP(ip)->i_xtroot;
+	sp = (xtpage_t *) &JFS_IP(ip)->i_xtroot;

  	INCREMENT(xtStat.split);

@@ -2098,7 +2098,7 @@ int xtAppend(tid_t tid,		/* transaction id */
   */
  void xtInitRoot(tid_t tid, struct inode *ip)
  {
-	xtpage_t *p;
+	xtroot_t *p;

  	/*
  	 * acquire a transaction lock on the root
diff --git a/fs/jfs/jfs_xtree.h b/fs/jfs/jfs_xtree.h
index ad7592191d76..0f6cf5a1ce75 100644
--- a/fs/jfs/jfs_xtree.h
+++ b/fs/jfs/jfs_xtree.h
@@ -65,24 +65,33 @@ struct xadlist {
  #define XTPAGEMAXSLOT	256
  #define XTENTRYSTART	2

-/*
- *	xtree page:
- */
-typedef union {
-	struct xtheader {
-		__le64 next;	/* 8: */
-		__le64 prev;	/* 8: */
+struct xtheader {
+	__le64 next;	/* 8: */
+	__le64 prev;	/* 8: */

-		u8 flag;	/* 1: */
-		u8 rsrvd1;	/* 1: */
-		__le16 nextindex;	/* 2: next index = number of entries */
-		__le16 maxentry;	/* 2: max number of entries */
-		__le16 rsrvd2;	/* 2: */
+	u8 flag;	/* 1: */
+	u8 rsrvd1;	/* 1: */
+	__le16 nextindex;	/* 2: next index = number of entries */
+	__le16 maxentry;	/* 2: max number of entries */
+	__le16 rsrvd2;	/* 2: */

-		pxd_t self;	/* 8: self */
-	} header;		/* (32) */
+	pxd_t self;	/* 8: self */
+};

+/*
+ *	xtree root (in inode):
+ */
+typedef union {
+	struct xtheader header;
  	xad_t xad[XTROOTMAXSLOT];	/* 16 * maxentry: xad array */
+} xtroot_t;
+
+/*
+ *	xtree page:
+ */
+typedef union {
+	struct xtheader header;
+	xad_t xad[XTPAGEMAXSLOT];	/* 16 * maxentry: xad array */
  } xtpage_t;

  /*
-- 
2.42.0