[PATCH] fs/jfs: Patch with various fixes for the JFS code.

From: Tino Reichardt
Date: Fri Sep 21 2012 - 09:14:20 EST


Hello,

here is a list of the changed files and the reason of it.


fs/jfs/jfs_dmap.c:
- potential NULL dereference of 'mp' in dbUpdatePMap()

fs/jfs/jfs_dtree.c:
- potential NULL dereference of 'parent' dtExtendPage()
- potential NULL dereference of 'lh' in dtInsertEntry()
- potential NULL dereference of 'ih' in dtInsertEntry()
- potential NULL dereference of 'dlh' in dtMoveEntry()
- potential NULL dereference of 'dih' in dtMoveEntry()

fs/jfs/jfs_imap.c:
- potential NULL dereference of 'aiagp' in diNewExt()
- potential NULL dereference of 'biagp' in diNewExt()
- potential NULL dereference of 'ciagp' in diNewExt()

fs/jfs/jfs_incore.h:
- added a new struct slink within the union u of the struct jfs_inode_info
- this struct slink defines _inline data to be 256 byte, which was implicitly
used already this way for symlinks
- the following files are affected:
- jfs_imap.c in diWrite(), line 786 via 'jfs_ip->u.link._inline'
- inode.c in jfs_iget(), line 69 via 'JFS_IP(inode)->u.link._inline'
- namei.c in jfs_symlink(), line 951, via 'i_fastsymlink'

fs/jfs/jfs_txnmgr.c:
- TXN_SLEEP is changed from define to an inlined function
- as long, as it was a define, smatch complains about double locking of
jfsTxnLock via spin_lock()
- fix potential NULL dereference of 'mp' in txUpdateMap()

fs/jfs/jfs_xtree.c:
- potential NULL dereference of 'rxtlck' in xtSplitPage()
- potential NULL dereference of 'sxtlck' in xtSplitPage()
- potential NULL dereference of 'xtlck' in xtExtend()
- potential NULL dereference of 'xtlck' in xtUpdate()
- potential NULL dereference of 'tblk' in xtTruncate()

fs/jfs/xattr.c:
- ealist in ea_write() line 303 could be NULL, this was not checked

I also changed the sysv unchar type to u8, cause u8 is used everywhere
but the unchar only 4 times.


Signed-off-by: Tino Reichardt <milky-kernel@xxxxxxxxx>


---
fs/jfs/jfs_dmap.c | 7 ++++---
fs/jfs/jfs_dtree.c | 15 +++++++++++----
fs/jfs/jfs_imap.c | 17 ++++++++++++-----
fs/jfs/jfs_incore.h | 15 ++++++++++-----
fs/jfs/jfs_txnmgr.c | 17 +++++++++--------
fs/jfs/jfs_xtree.c | 10 ++++++++++
fs/jfs/namei.c | 2 +-
fs/jfs/xattr.c | 1 +
8 files changed, 58 insertions(+), 26 deletions(-)

diff --git a/fs/jfs/jfs_dmap.c b/fs/jfs/jfs_dmap.c
index 9a55f53..4027a7a 100644
--- a/fs/jfs/jfs_dmap.c
+++ b/fs/jfs/jfs_dmap.c
@@ -464,12 +464,13 @@ dbUpdatePMap(struct inode *ipbmap,
write_metapage(mp);
}

- mp = read_metapage(bmp->db_ipbmap, lblkno, PSIZE,
- 0);
- if (mp == NULL)
+ mp = read_metapage(bmp->db_ipbmap, lblkno, PSIZE, 0);
+ if (unlikely(mp == NULL))
return -EIO;
metapage_wait_for_io(mp);
}
+
+ BUG_ON(mp == NULL);
dp = (struct dmap *) mp->data;

/* determine the bit number and word within the dmap of
diff --git a/fs/jfs/jfs_dtree.c b/fs/jfs/jfs_dtree.c
index 9197a1b..6c51cb0 100644
--- a/fs/jfs/jfs_dtree.c
+++ b/fs/jfs/jfs_dtree.c
@@ -1670,6 +1670,7 @@ static int dtExtendPage(tid_t tid,

/* get parent/root page */
parent = BT_POP(btstack);
+ BUG_ON(parent == NULL);
DT_GETPAGE(ip, parent->bn, pmp, PSIZE, pp, rc);
if (rc)
return (rc);
@@ -4005,10 +4006,13 @@ static void dtInsertEntry(dtpage_t * p, int index, struct component_name * key,
/* terminate last/only segment */
if (h == t) {
/* single segment entry */
- if (p->header.flag & BT_LEAF)
+ if (p->header.flag & BT_LEAF) {
+ BUG_ON(lh == NULL);
lh->next = -1;
- else
+ } else {
+ BUG_ON(ih == NULL);
ih->next = -1;
+ }
} else
/* multi-segment entry */
t->next = -1;
@@ -4213,10 +4217,13 @@ static void dtMoveEntry(dtpage_t * sp, int si, dtpage_t * dp,
/* terminate dst last/only segment */
if (h == d) {
/* single segment entry */
- if (dp->header.flag & BT_LEAF)
+ if (dp->header.flag & BT_LEAF) {
+ BUG_ON(dlh == NULL);
dlh->next = -1;
- else
+ } else {
+ BUG_ON(dih == NULL);
dih->next = -1;
+ }
} else
/* multi-segment entry */
d->next = -1;
diff --git a/fs/jfs/jfs_imap.c b/fs/jfs/jfs_imap.c
index 1b6f15f..41eec95 100644
--- a/fs/jfs/jfs_imap.c
+++ b/fs/jfs/jfs_imap.c
@@ -2316,12 +2316,15 @@ static int diNewExt(struct inomap * imap, struct iag * iagp, int extno)
* iag from the ag free extent list.
*/
if (iagp->nfreeexts == cpu_to_le32(1)) {
- if (fwd >= 0)
+ if (fwd >= 0) {
+ BUG_ON(aiagp == NULL);
aiagp->extfreeback = iagp->extfreeback;
+ }

- if (back >= 0)
+ if (back >= 0) {
+ BUG_ON(biagp == NULL);
biagp->extfreefwd = iagp->extfreefwd;
- else
+ } else
imap->im_agctl[agno].extfree =
le32_to_cpu(iagp->extfreefwd);

@@ -2331,8 +2334,10 @@ static int diNewExt(struct inomap * imap, struct iag * iagp, int extno)
* add the iag to the ag free extent list.
*/
if (iagp->nfreeexts == cpu_to_le32(EXTSPERIAG)) {
- if (fwd >= 0)
+ if (fwd >= 0) {
+ BUG_ON(aiagp == NULL);
aiagp->extfreeback = cpu_to_le32(iagno);
+ }

iagp->extfreefwd = cpu_to_le32(fwd);
iagp->extfreeback = cpu_to_le32(-1);
@@ -2344,8 +2349,10 @@ static int diNewExt(struct inomap * imap, struct iag * iagp, int extno)
* ag free inode list.
*/
if (iagp->nfreeinos == 0) {
- if (freei >= 0)
+ if (freei >= 0) {
+ BUG_ON(ciagp == NULL);
ciagp->inofreeback = cpu_to_le32(iagno);
+ }

iagp->inofreefwd =
cpu_to_le32(imap->im_agctl[agno].inofree);
diff --git a/fs/jfs/jfs_incore.h b/fs/jfs/jfs_incore.h
index 4fa958a..019e44e 100644
--- a/fs/jfs/jfs_incore.h
+++ b/fs/jfs/jfs_incore.h
@@ -52,7 +52,7 @@ struct jfs_inode_info {
unsigned long cflag; /* commit flags */
u64 agstart; /* agstart of the containing IAG */
u16 bxflag; /* xflag of pseudo buffer? */
- unchar pad;
+ u8 pad;
signed char active_ag; /* ag currently allocating from */
lid_t blid; /* lid of pseudo buffer? */
lid_t atlhead; /* anonymous tlock list head */
@@ -85,14 +85,19 @@ struct jfs_inode_info {
dtroot_t _dtroot; /* 288: dtree root */
} dir;
struct {
- unchar _unused[16]; /* 16: */
+ u8 _unused[16]; /* 16: */
dxd_t _dxd; /* 16: */
- unchar _inline[128]; /* 128: inline symlink */
+ u8 _inline[128]; /* 128: inline symlink */
/* _inline_ea may overlay the last part of
* file._xtroot if maxentry = XTROOTINITSLOT
*/
- unchar _inline_ea[128]; /* 128: inline extended attr */
+ u8 _inline_ea[128]; /* 128: inline extended attr */
} link;
+ struct {
+ u8 _unused[16]; /* 16: */
+ dxd_t _dxd; /* 16: */
+ u8 _inline[256];/* 256: inline symlink only */
+ } slink;
} u;
u32 dev; /* will die when we get wide dev_t */
struct inode vfs_inode;
@@ -101,7 +106,7 @@ struct jfs_inode_info {
#define i_imap u.file._imap
#define i_dirtable u.dir._table
#define i_dtroot u.dir._dtroot
-#define i_inline u.link._inline
+#define i_inline u.slink._inline
#define i_inline_ea u.link._inline_ea

#define IREAD_LOCK(ip, subclass) \
diff --git a/fs/jfs/jfs_txnmgr.c b/fs/jfs/jfs_txnmgr.c
index 5fcc02e..2d00fee 100644
--- a/fs/jfs/jfs_txnmgr.c
+++ b/fs/jfs/jfs_txnmgr.c
@@ -115,11 +115,11 @@ struct tlock *TxLock; /* transaction lock table */
*/
static DEFINE_SPINLOCK(jfsTxnLock);

-#define TXN_LOCK() spin_lock(&jfsTxnLock)
-#define TXN_UNLOCK() spin_unlock(&jfsTxnLock)
+#define TXN_LOCK() spin_lock(&jfsTxnLock)
+#define TXN_UNLOCK() spin_unlock(&jfsTxnLock)

-#define LAZY_LOCK_INIT() spin_lock_init(&TxAnchor.LazyLock);
-#define LAZY_LOCK(flags) spin_lock_irqsave(&TxAnchor.LazyLock, flags)
+#define LAZY_LOCK_INIT() spin_lock_init(&TxAnchor.LazyLock);
+#define LAZY_LOCK(flags) spin_lock_irqsave(&TxAnchor.LazyLock, flags)
#define LAZY_UNLOCK(flags) spin_unlock_irqrestore(&TxAnchor.LazyLock, flags)

static DECLARE_WAIT_QUEUE_HEAD(jfs_commit_thread_wait);
@@ -140,10 +140,10 @@ static inline void TXN_SLEEP_DROP_LOCK(wait_queue_head_t * event)
remove_wait_queue(event, &wait);
}

-#define TXN_SLEEP(event)\
-{\
- TXN_SLEEP_DROP_LOCK(event);\
- TXN_LOCK();\
+static inline void TXN_SLEEP(wait_queue_head_t *event)
+{
+ TXN_SLEEP_DROP_LOCK(event);
+ TXN_LOCK();
}

#define TXN_WAKEUP(event) wake_up_all(event)
@@ -2381,6 +2381,7 @@ static void txUpdateMap(struct tblock * tblk)
}
}
if (tlck->flag & tlckFREEPAGE) {
+ BUG_ON(mp == NULL);
if (!(tblk->flag & tblkGC_LAZY)) {
/* This is equivalent to txRelease */
ASSERT(mp->lid == lid);
diff --git a/fs/jfs/jfs_xtree.c b/fs/jfs/jfs_xtree.c
index 6c50871..d8ff7a8 100644
--- a/fs/jfs/jfs_xtree.c
+++ b/fs/jfs/jfs_xtree.c
@@ -1066,6 +1066,8 @@ xtSplitPage(tid_t tid, struct inode *ip,
rp->header.nextindex = cpu_to_le16(XTENTRYSTART + 1);

if (!test_cflag(COMMIT_Nolink, ip)) {
+ BUG_ON(rxtlck == NULL);
+
/* rxtlck->lwm.offset = XTENTRYSTART; */
rxtlck->lwm.length = 1;
}
@@ -1137,6 +1139,7 @@ xtSplitPage(tid_t tid, struct inode *ip,
/* update page header */
sp->header.nextindex = cpu_to_le16(middle + 1);
if (!test_cflag(COMMIT_Nolink, ip)) {
+ BUG_ON(sxtlck == NULL);
sxtlck->lwm.offset = (sxtlck->lwm.offset) ?
min(skip, (int)sxtlck->lwm.offset) : skip;
}
@@ -1167,6 +1170,7 @@ xtSplitPage(tid_t tid, struct inode *ip,
/* update page header */
sp->header.nextindex = cpu_to_le16(middle);
if (!test_cflag(COMMIT_Nolink, ip)) {
+ BUG_ON(sxtlck == NULL);
sxtlck->lwm.offset = (sxtlck->lwm.offset) ?
min(middle, (int)sxtlck->lwm.offset) : middle;
}
@@ -1176,6 +1180,7 @@ xtSplitPage(tid_t tid, struct inode *ip,
}

if (!test_cflag(COMMIT_Nolink, ip)) {
+ BUG_ON(sxtlck == NULL);
sxtlck->lwm.length = le16_to_cpu(sp->header.nextindex) -
sxtlck->lwm.offset;

@@ -1492,6 +1497,7 @@ int xtExtend(tid_t tid, /* transaction id */
xad->flag |= XAD_EXTENDED;

if (!test_cflag(COMMIT_Nolink, ip)) {
+ BUG_ON(xtlck == NULL);
xtlck->lwm.offset =
(xtlck->lwm.offset) ? min(index,
(int)xtlck->lwm.offset) : index;
@@ -2005,6 +2011,7 @@ int xtUpdate(tid_t tid, struct inode *ip, xad_t * nxad)
if (newpage) {
/* close out old page */
if (!test_cflag(COMMIT_Nolink, ip)) {
+ BUG_ON(xtlck == NULL);
xtlck->lwm.offset = (xtlck->lwm.offset) ?
min(index0, (int)xtlck->lwm.offset) : index0;
xtlck->lwm.length =
@@ -2137,6 +2144,7 @@ printf("xtUpdate.updateLeft.split p:0x%p\n", p);

out:
if (!test_cflag(COMMIT_Nolink, ip)) {
+ BUG_ON(xtlck == NULL);
xtlck->lwm.offset = (xtlck->lwm.offset) ?
min(index0, (int)xtlck->lwm.offset) : index0;
xtlck->lwm.length = le16_to_cpu(p->header.nextindex) -
@@ -3540,6 +3548,8 @@ s64 xtTruncate(tid_t tid, struct inode *ip, s64 newsize, int flag)
* quickly remove it and add it to the end.
*/

+ BUG_ON(tblk == NULL);
+
/*
* Move parent page's tlock to the end of the tid's tlock list
*/
diff --git a/fs/jfs/namei.c b/fs/jfs/namei.c
index 3b91a7a..bbade01 100644
--- a/fs/jfs/namei.c
+++ b/fs/jfs/namei.c
@@ -881,7 +881,7 @@ static int jfs_symlink(struct inode *dip, struct dentry *dentry,
int ssize; /* source pathname size */
struct btstack btstack;
struct inode *ip = dentry->d_inode;
- unchar *i_fastsymlink;
+ u8 *i_fastsymlink;
s64 xlen = 0;
int bmask = 0, xsize;
s64 xaddr;
diff --git a/fs/jfs/xattr.c b/fs/jfs/xattr.c
index 26683e1..e5f9add 100644
--- a/fs/jfs/xattr.c
+++ b/fs/jfs/xattr.c
@@ -256,6 +256,7 @@ static int ea_write(struct inode *ip, struct jfs_ea_list *ealist, int size,
* loop over the FEALIST copying data into the buffer one page at
* a time.
*/
+ BUG_ON(ealist == NULL);
cp = (char *) ealist;
nbytes = size;
for (i = 0; i < nblocks; i += sbi->nbperpage) {
--
1.7.12.1

--
regards, TR
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/