Re: [PATCH] jfs : fs array-index-out-of-bounds in txCommit
From: Manas Ghandat
Date: Fri Oct 13 2023 - 05:50:00 EST
On 05/10/23 19:50, Dave Kleikamp wrote:
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@xxxxxxxxxx>
---
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;
/*
I tested this patch and it has not triggered any bug.