Shaggy,
I'm not extremely familiar with this code, but after hunting down a
segfault related to reading invalid data off the disk, I thought I'd
take a few minutes (/hours) and try and add more sanity checking to
the log replay code. I tried to look for cases where we are trusting
data that was read off disk and using it to index/access data in
memory.
I haven't tested this (it should compile), but I think it's simple
enough that you could probably take a look and see if it's sane and
worth further work/testing.
One thing I wasn't sure about was if it is possible for a
redopage.type to have more than one bit set or not, if so, we could
just add that to the switch statement. Also, we seem to check the
and of that record with a single bit a few times and possibly should
just use equals instead? Of course, that depends on the answer to my
first question though. I can make another patch.
I mostly only looked at the jfs_logredo->doAfter->updatePage chain of
functions, I could probably also look at other paths through
jfs_logredo. Also I thought I saw some possible endian bugs in there
too...
--- jfsutils-1.1.11/libfs/log_work.c 2006-06-04 16:37:29.000000000 -0500
+++ jfsutils-1.1.11/libfs/log_work.c~new 2007-03-24 03:52:57.339219090
-0500
@@ -598,9 +598,36 @@ int doAfter(struct lrd *ld, /* pointer t
* skip log records for those which do.)
*/
vol = ld->aggregate;
+
+ /* check sanity before indexing into an array */
+ if (vol < 0 || vol >= MAX_ACTIVE) {
+ DBG_TRACE(("doAfter: invalid volume number"))
+ return (1);
+ }
+
if (vopen[vol].status == FM_CLEAN || vopen[vol].status == FM_LOGREDO)
return (0);
+ /* check sanity of redopage type */
+ switch (ld->log.redopage.type) {
+
+ case LOG_INODE :
+ case LOG_XTREE :
+ case LOG_DTREE :
+ case LOG_BTROOT :
+ case LOG_EA :
+ case LOG_ACL :
+ case LOG_DATA :
+ case LOG_NEW :
+ case LOG_EXTEND :
+ case LOG_RELOCATE :
+ case LOG_DIR_XTREE :
+ break;
+ default:
+ DBG_TRACE(("doAfter: invalid redopage.type"))
+ return (1);
+ }
+
/*
* If the redopage.type != LOG_INODE, then if there is a
* NoRedoFile filter in effect for the inode we can skip this
@@ -2428,6 +2455,13 @@ int updatePage(struct lrd *ld, int32_t l
size_dinode = sizeof (struct dinode);
segdata = (int16_t *) ((caddr_t) afterdata + ld->length);
l2linesize = ld->log.redopage.l2linesize;
+
+ /* sanity check the l2linesize */
+ if (l2linesize > L2LOGPSIZE ) {
+ DBG_TRACE(("updatePage: invalid l2linesize"))
+ return (1);
+ }
+
linesize = 1 << l2linesize;
j = 0;
seglen = 0;
@@ -2680,6 +2714,13 @@ int updatePage(struct lrd *ld, int32_t l
off = __le16_to_cpu(*--segdata); /* get offset */
segnum++;
seglen = ln << ld->log.redopage.l2linesize;
+
+ /* sanity check seglen since it comes from disk --ln and off */
+ if (seglen <= 0 || seglen > (ld->length - j)){
+ DBG_TRACE(("updatePage: invalid legnth and/or offset"))
+ return (1);
+ }
+
/*
* segdata points to the beginning of the segment
*/
-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys-and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV
_______________________________________________
Jfs-discussion mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/jfs-discussion
