| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <4266e5dd-39d8-05ec-d420-4f4c78c5f016@gmail.com>
Date: Fri, 13 Oct 2023 15:19:50 +0530
From: Manas Ghandat <ghandatmanas@...il.com>
To: Dave Kleikamp <dave.kleikamp@...cle.com>
Cc: linux-kernel@...r.kernel.org, jfs-discussion@...ts.sourceforge.net,
Linux-kernel-mentees@...ts.linuxfoundation.org,
syzbot+0558d19c373e44da3c18@...kaller.appspotmail.com
Subject: Re: [PATCH] jfs : fs array-index-out-of-bounds in txCommit
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@...cle.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;
>
> /*
I tested this patch and it has not triggered any bug.
Powered by blists - more mailing lists