Subject: ffs fs_flags superblock update sign extension bug
To: None <tech-kern@netbsd.org>
From: Darrin B. Jewell <dbj@netbsd.org>
List: tech-kern
Date: 09/28/2003 18:20:30
Revision 1.120 of ffs_vfsops dated 2003/09/13 introduced a bug in the
superblock update code that results in fs_flags having a value of
0x7fffff00 after the update.
Specifically, line 995 of ffs_vfsops.c is this:
fs->fs_flags |= (fs->fs_old_flags & ~FS_FLAGS_UPDATED);
bug since fs_old_flags was int8_t and fs_flags is int32_t, and
FS_FLAGS_UPDATED is 0x80, the fs_old_flags value gets sign extended
before it gets copied into fs_flags. The high bit gets later stripped
by FS_INTERNAL being forced to zero. (This behavior also makes me
nervous, but is tangental to the issue addressed here.)
Although that line could be updated to remove this problem,
I think it is a better fix to declare these fields uint8_t and
uint32_t as per the patch included below.
Filesystems that were upgraded by -current kernels in the intermediate
time will have bad values of fs_flags which will cause a problem which
will not be exhibited until some future date when those flag fields
are used. If desired, I can post a simple program which can go in and
unset these flags. Since the lifespan of the incorrect code was
small, I do not recommend implementing compatibility in the kernel.
Please review the following patch. I would like to commit
it shortly.
Thanks,
Darrin
Index: sys/ufs/ffs/fs.h
===================================================================
RCS file: /cvsroot/src/sys/ufs/ffs/fs.h,v
retrieving revision 1.34
diff -u -r1.34 fs.h
--- sys/ufs/ffs/fs.h 21 Aug 2003 14:41:00 -0000 1.34
+++ sys/ufs/ffs/fs.h 28 Sep 2003 21:57:02 -0000
@@ -272,7 +272,7 @@
int8_t fs_fmod; /* super block modified flag */
int8_t fs_clean; /* file system is clean flag */
int8_t fs_ronly; /* mounted read-only flag */
- int8_t fs_old_flags; /* see FS_ flags below */
+ uint8_t fs_old_flags; /* see FS_ flags below */
u_char fs_fsmnt[MAXMNTLEN]; /* name mounted on */
u_char fs_volname[MAXVOLLEN]; /* volume name */
uint64_t fs_swuid; /* system-wide uid */
@@ -300,7 +300,7 @@
int32_t fs_avgfpdir; /* expected # of files per directory */
int32_t fs_save_cgsize; /* save real cg size to use fs_bsize */
int32_t fs_sparecon32[26]; /* reserved for future constants */
- int32_t fs_flags; /* see FS_ flags below */
+ uint32_t fs_flags; /* see FS_ flags below */
int32_t fs_contigsumsize; /* size of cluster summary array */
int32_t fs_maxsymlinklen; /* max length of an internal symlink */
int32_t fs_old_inodefmt; /* format of on-disk inodes */