On Mon, 2004-11-01 at 15:30, Richard Allen wrote:
> On Mon, Nov 01, 2004 at 09:01:11PM +0000, Richard Allen wrote:
> > On Mon, Nov 01, 2004 at 08:39:58PM +0000, Richard Allen wrote:
> > 
> > > Continue? (Y/N) Y
> > > The specified disk did not finish formatting.
> > > Internal error3: Format failed rc=ffffffff
> > 
> > Running mkfs.jfs in strace gives:
> > 
> > write(3, "!Ce\207\1\0\0\0\0\0\0\0\0 \0\0\0\20\0\0\f\0\0\0\0\t \20"..., 4096) = -1 
> > EFAULT (Bad address)
> 
> I'm sorry if I'm mailbombing you guys.    But I think I found the problem.
> I traced the problem to ujfs_rw_diskblocks() in libfs/devices.c
> EFAULT in write() means the buffer to write is outside my accessible address
> space.   
> 
> The function jfs_logform() has a structure defined called log_sup of type
> logsuper and it's defined as static.   It's then used as argument 4 to the
> function ujfs_rw_diskblocks() which is located in another file.

The problem is that struct logsuper is smaller than 4K, and we are
writing 4K from that structure.  I'm not sure why you're the first to
report the problem.  Maybe it's a newer compiler or linker.

>         rc = ujfs_rw_diskblocks(fd, (log_begin + LOGPSIZE),
>                                 (unsigned) LOGPSIZE, (char *) &log_sup, PUT);
> 
> in ujfs_rw_diskblocks argument 4 becomes void *data_buffer which is used
> in read and write functions.
> 
>         case PUT:
>                 Bytes_Transferred = write(dev_ptr, data_buffer, disk_count);
>                 break;
> 
> After making the structure non static, it formats perfectly:

There's still a problem with this.  For an external journal, we are
going to read 4K into logsup to verify that the uuid matches, and this
will cause other data on the stack to be overwritten.  With the static
declaration, somehow we've been lucky and not run into a problem before
(at least one that has been reported).  Even if we're not reading, we
will still be writing some random stack contents to the disk after the
superblock, which is not so good.

The right fix would be to either pad (and zero) struct jfslog to be 4K
bytes long, or to change jfslog to a pointer and malloc a 4K buffer.

This patch does a calloc instead of declaring the structure statically. 
Does it fix your problem?

? jfsutils/mkfs/.initmap.c.swp
Index: jfsutils/libfs/logform.c
===================================================================
RCS file: /usr/cvs/jfs/jfsutils/libfs/logform.c,v
retrieving revision 1.15
diff -u -p -u -r1.15 logform.c
--- jfsutils/libfs/logform.c    23 Sep 2002 21:07:43 -0000      1.15
+++ jfsutils/libfs/logform.c    1 Nov 2004 22:24:44 -0000
@@ -63,7 +63,7 @@ int jfs_logform(int fd,               /* this is a fi
        int npages, rc, k;
        char logpages[4 * LOGPSIZE];
        struct logpage *logp;   /* array of 4 log pages */
-       static struct logsuper log_sup;
+       static struct logsuper *log_sup;
        struct lrd *lrd_ptr;
        int64_t log_begin;      /* the location of the beginning of the log
                                 * inside of the file system. ( in bytes )
@@ -73,6 +73,12 @@ int jfs_logform(int fd,              /* this is a fi
        int Working_counter;
        char *Working[5];
 
+       log_sup = calloc(1, LOGPSIZE);
+       if (log_sup == NULL) {
+               printf("logform: calloc failed!\n");
+               return -1;
+       }
+
        /* find the log superblock location 
         */
        log_begin = log_start << s_l2bsize;
@@ -106,12 +112,12 @@ int jfs_logform(int fd,           /* this is a fi
                else {
                        /* Verify existing uuid matches uuid passed in */
                        rc = ujfs_rw_diskblocks(fd, (log_begin + LOGPSIZE),
-                                               (unsigned) LOGPSIZE, (char *) 
&log_sup, GET);
+                                               (unsigned) LOGPSIZE, (char *) log_sup, 
GET);
                        if (rc)
                                return -1;
-                       ujfs_swap_logsuper(&log_sup);
+                       ujfs_swap_logsuper(log_sup);
 
-                       if (!uuid_compare(log_sup.uuid, uuid)) {
+                       if (!uuid_compare(log_sup->uuid, uuid)) {
                                printf("Invalid log device\n");
                                return -1;
                        }
@@ -125,36 +131,36 @@ int jfs_logform(int fd,           /* this is a fi
        /* 
         * init log superblock: log page 1
         */
-       log_sup.magic = LOGMAGIC;
-       log_sup.version = LOGVERSION;
-       log_sup.state = LOGREDONE;
+       log_sup->magic = LOGMAGIC;
+       log_sup->version = LOGVERSION;
+       log_sup->state = LOGREDONE;
        /* Assign fs s_flag to log superblock.
         * Currently s_flag carries the inlinelog info and commit option
         * ( i.e. group commit or lazy commit, etc.. )
         */
-       log_sup.flag = s_flag;
-       log_sup.size = npages;
-       log_sup.bsize = aggr_blk_size;
-       log_sup.l2bsize = s_l2bsize;
-       log_sup.end = 2 * LOGPSIZE + LOGPHDRSIZE + LOGRDSIZE;
+       log_sup->flag = s_flag;
+       log_sup->size = npages;
+       log_sup->bsize = aggr_blk_size;
+       log_sup->l2bsize = s_l2bsize;
+       log_sup->end = 2 * LOGPSIZE + LOGPHDRSIZE + LOGRDSIZE;
 
        if (uuid)
-               uuid_copy(log_sup.uuid, uuid);
+               uuid_copy(log_sup->uuid, uuid);
        else
-               uuid_clear(log_sup.uuid);       /* Inline log */
+               uuid_clear(log_sup->uuid);      /* Inline log */
 
        if (label) {
-               strncpy((char *) &log_sup.label, label, sizeof (log_sup.label));
+               strncpy((char *) &log_sup->label, label, sizeof (log_sup->label));
        }
 
        for (k = 0; k < MAX_ACTIVE; k++)
-               uuid_clear(log_sup.active[k]);
+               uuid_clear(log_sup->active[k]);
 
        /* swap if on big endian machine */
-       ujfs_swap_logsuper(&log_sup);
+       ujfs_swap_logsuper(log_sup);
 
        rc = ujfs_rw_diskblocks(fd, (log_begin + LOGPSIZE),
-                               (unsigned) LOGPSIZE, (char *) &log_sup, PUT);
+                               (unsigned) LOGPSIZE, (char *) log_sup, PUT);
        if (rc != 0)
                return -1;
 


_______________________________________________
Jfs-discussion mailing list
[EMAIL PROTECTED]
http://www-124.ibm.com/developerworks/oss/mailman/listinfo/jfs-discussion

Reply via email to