Skip site navigation (1)Skip section navigation (2)
Date:      Sat, 15 Jul 95 19:51:57 MDT
From:      terry@cs.weber.edu (Terry Lambert)
To:        hackers@freebsd.org
Subject:   Patches for minor MFS bug + discussion
Message-ID:  <9507160151.AA08444@cs.weber.edu>

next in thread | raw e-mail | index | archive | help
Rod complained that I didn't send patches with my last email.


Here are some patches for MFS.  There are two locations in the mountroot
code where the mfs_buflist in the MFS specific data portion of the mount
structure is set incorrectly when a failure occurs.  This corrects both
of them.

I would appreciate some testing of theis patched code, particularly
in an "emergency boot floppy, MFS" configuration.

There is also the addition of an mfs_mountfs routine that is identical
(for now) to the ffs_mountfs routine.  This will be changing in the
future.

I believe it is also necessary to add an mfs_unmount routine in place
of the ffs_unmount routine in use here.  This is because the current
unmount code will probably cause buffer pool corruption on the unmount
of an MFS file system (if it doesn't cause a panic outright; I'm treating
free as opaque for the purposes of the discussion).

THE mfs_unmount PATCH IS NOT INCLUDED IN THIS PATCH SET!!!

If one looks in mfs_vfsops.c on line 112 (post-patch), you will see the
MFS specific data being allocated like so:

	mfsp = malloc(sizeof *mfsp, M_MFSNODE, M_WAITOK);

The equivalent FFS line is in ffs_vfsops.c at line 430:

	ump = malloc(sizeof *ump, M_UFSMNT, M_WAITOK);

The common unmount code (ffs_vfsops.c, line 572) returns either buffer
to the *UFS* memory pool:

	free(ump, M_UFSMNT);

Clearly, this is broken.

Rather than hack all this at once, I've left the pool allocation in the
MFS alone at this time; it's recommended that you not mount and unmount
the MFS file systems more than necessary, preferrably ONCE.

In addition, there are header file changes that support the mfs_mountfs
forward declaration, as well as correcting the non-declaration of the
mfs_mountroot routine.

Please let me know if this code is incorporated ASAP, so I'll know
whether to carry around these changes as well for the next set of
diffs (upcoming).


					Regards,
					Terry Lambert
					terry@cs.weber.edu
---
Any opinions in this posting are my own and not those of my present
or previous employers.
===============================================================================
===============================================================================
===============================================================================
*** SAVE/mfs_extern.h	Sat Jul 15 15:57:34 1995
--- mfs_extern.h	Sun Jul 16 18:41:08 1995
***************
*** 56,61 ****
--- 56,63 ----
  int	mfs_ioctl __P((struct vop_ioctl_args *));
  int	mfs_mount __P((struct mount *mp,
  	    char *path, caddr_t data, struct nameidata *ndp, struct proc *p));
+ int	mfs_mountfs __P((struct vnode *, struct mount *, struct proc *));
+ int	mfs_mountroot __P((void));
  int	mfs_open __P((struct vop_open_args *));
  int	mfs_print __P((struct vop_print_args *)); /* XXX */
  int	mfs_start __P((struct mount *mp, int flags, struct proc *p));
*** SAVE/mfs_vfsops.c	Sat Jul 15 15:57:34 1995
--- mfs_vfsops.c	Sun Jul 16 18:43:55 1995
***************
*** 40,50 ****
--- 40,56 ----
  #include <sys/kernel.h>
  #include <sys/proc.h>
  #include <sys/buf.h>
+ #include <sys/mbuf.h>
+ #include <sys/file.h>
+ #include <sys/disklabel.h>
+ #include <sys/ioctl.h>
  #include <sys/mount.h>
  #include <sys/signalvar.h>
  #include <sys/vnode.h>
  #include <sys/malloc.h>
  
+ #include <miscfs/specfs/specdev.h>
+ 
  #include <ufs/ufs/quota.h>
  #include <ufs/ufs/inode.h>
  #include <ufs/ufs/ufsmount.h>
***************
*** 118,130 ****
  	mfsp->mfs_vnode = rootvp;
  	mfsp->mfs_pid = p->p_pid;
  	mfsp->mfs_buflist = (struct buf *)0;
! 	if (error = ffs_mountfs(rootvp, mp, p)) {
  		free(mp, M_MOUNT);
  		free(mfsp, M_MFSNODE);
  		return (error);
  	}
  	if (error = vfs_lock(mp)) {
  		(void)ffs_unmount(mp, 0, p);
  		free(mp, M_MOUNT);
  		free(mfsp, M_MFSNODE);
  		return (error);
--- 124,138 ----
  	mfsp->mfs_vnode = rootvp;
  	mfsp->mfs_pid = p->p_pid;
  	mfsp->mfs_buflist = (struct buf *)0;
! 	if (error = mfs_mountfs(rootvp, mp, p)) {
! 		mfsp->mfs_buflist = (struct buf *)-1;		/* NB*/
  		free(mp, M_MOUNT);
  		free(mfsp, M_MFSNODE);
  		return (error);
  	}
  	if (error = vfs_lock(mp)) {
  		(void)ffs_unmount(mp, 0, p);
+ 		mfsp->mfs_buflist = (struct buf *)-1;		/* NB*/
  		free(mp, M_MOUNT);
  		free(mfsp, M_MFSNODE);
  		return (error);
***************
*** 233,239 ****
  	mfsp->mfs_vnode = devvp;
  	mfsp->mfs_pid = p->p_pid;
  	mfsp->mfs_buflist = (struct buf *)0;
! 	if (error = ffs_mountfs(devvp, mp, p)) {
  		mfsp->mfs_buflist = (struct buf *)-1;
  		vrele(devvp);
  		return (error);
--- 241,247 ----
  	mfsp->mfs_vnode = devvp;
  	mfsp->mfs_pid = p->p_pid;
  	mfsp->mfs_buflist = (struct buf *)0;
! 	if (error = mfs_mountfs(devvp, mp, p)) {
  		mfsp->mfs_buflist = (struct buf *)-1;
  		vrele(devvp);
  		return (error);
***************
*** 250,255 ****
--- 258,401 ----
  	(void) mfs_statfs(mp, &mp->mnt_stat, p);
  	return (0);
  }
+ 
+ /*
+  * Common code for mount and mountroot
+  *
+  * Note: moved MFS specific prepatory to common root mount code,
+  * since a correct implementation requires FFS/MFS/LFS specific
+  * option processing to take place.
+  */
+ int	ffs_sbupdate __P((struct ufsmount *, int));
+ int	ffs_oldfscompat __P((struct fs *));
+ void	ffs_vmlimits __P((struct fs *));
+ int
+ mfs_mountfs(devvp, mp, p)
+ 	register struct vnode *devvp;
+ 	struct mount *mp;
+ 	struct proc *p;
+ {
+ 	register struct ufsmount *ump;
+ 	struct buf *bp;
+ 	register struct fs *fs;
+ 	dev_t dev = devvp->v_rdev;
+ 	struct partinfo dpart;
+ 	caddr_t base, space;
+ 	int havepart = 0, blks;
+ 	int error, i, size;
+ 	int ronly;
+ 
+ 	/*
+ 	 * Disallow multiple mounts of the same device.
+ 	 * Disallow mounting of a device that is currently in use
+ 	 * (except for root, which might share swap device for miniroot).
+ 	 * Flush out any old buffers remaining from a previous use.
+ 	 */
+ 	error = vfs_mountedon(devvp);
+ 	if (error)
+ 		return (error);
+ 	if (vcount(devvp) > 1 && devvp != rootvp)
+ 		return (EBUSY);
+ 	error = vinvalbuf(devvp, V_SAVE, p->p_ucred, p, 0, 0);
+ 	if (error)
+ 		return (error);
+ 
+ 	ronly = (mp->mnt_flag & MNT_RDONLY) != 0;
+ 	error = VOP_OPEN(devvp, ronly ? FREAD : FREAD|FWRITE, FSCRED, p);
+ 	if (error)
+ 		return (error);
+ 	if (VOP_IOCTL(devvp, DIOCGPART, (caddr_t)&dpart, FREAD, NOCRED, p) != 0)
+ 		size = DEV_BSIZE;
+ 	else {
+ 		havepart = 1;
+ 		size = dpart.disklab->d_secsize;
+ 	}
+ 
+ 	bp = NULL;
+ 	ump = NULL;
+ 	error = bread(devvp, SBLOCK, SBSIZE, NOCRED, &bp);
+ 	if (error)
+ 		goto out;
+ 	fs = (struct fs *)bp->b_data;
+ 	if (fs->fs_magic != FS_MAGIC || fs->fs_bsize > MAXBSIZE ||
+ 	    fs->fs_bsize < sizeof(struct fs)) {
+ 		error = EINVAL;		/* XXX needs translation */
+ 		goto out;
+ 	}
+ 	if (!fs->fs_clean) {
+ 		if (ronly || (mp->mnt_flag & MNT_FORCE)) {
+ 			printf("WARNING: %s was not properly dismounted.\n",fs->fs_fsmnt);
+ 		} else {
+ 			printf("WARNING: R/W mount of %s denied. Filesystem is not clean - run fsck.\n",fs->fs_fsmnt);
+ 			error = EPERM;
+ 			goto out;
+ 		}
+ 	}
+ 	ump = malloc(sizeof *ump, M_UFSMNT, M_WAITOK);
+ 	bzero((caddr_t)ump, sizeof *ump);
+ 	ump->um_fs = malloc((u_long)fs->fs_sbsize, M_UFSMNT,
+ 	    M_WAITOK);
+ 	bcopy(bp->b_data, ump->um_fs, (u_int)fs->fs_sbsize);
+ 	if (fs->fs_sbsize < SBSIZE)
+ 		bp->b_flags |= B_INVAL;
+ 	brelse(bp);
+ 	bp = NULL;
+ 	fs = ump->um_fs;
+ 	fs->fs_ronly = ronly;
+ 	if (ronly == 0) {
+ 		fs->fs_fmod = 1;
+ 		fs->fs_clean = 0;
+ 	}
+ 	blks = howmany(fs->fs_cssize, fs->fs_fsize);
+ 	base = space = malloc((u_long)fs->fs_cssize, M_UFSMNT,
+ 	    M_WAITOK);
+ 	for (i = 0; i < blks; i += fs->fs_frag) {
+ 		size = fs->fs_bsize;
+ 		if (i + fs->fs_frag > blks)
+ 			size = (blks - i) * fs->fs_fsize;
+ 		error = bread(devvp, fsbtodb(fs, fs->fs_csaddr + i), size,
+ 			NOCRED, &bp);
+ 		if (error) {
+ 			free(base, M_UFSMNT);
+ 			goto out;
+ 		}
+ 		bcopy(bp->b_data, space, (u_int)size);
+ 		fs->fs_csp[fragstoblks(fs, i)] = (struct csum *)space;
+ 		space += size;
+ 		brelse(bp);
+ 		bp = NULL;
+ 	}
+ 	mp->mnt_data = (qaddr_t)ump;
+ 	mp->mnt_stat.f_fsid.val[0] = (long)dev;
+ 	mp->mnt_stat.f_fsid.val[1] = MOUNT_UFS;
+ 	mp->mnt_maxsymlinklen = fs->fs_maxsymlinklen;
+ 	mp->mnt_flag |= MNT_LOCAL;
+ 	ump->um_mountp = mp;
+ 	ump->um_dev = dev;
+ 	ump->um_devvp = devvp;
+ 	ump->um_nindir = fs->fs_nindir;
+ 	ump->um_bptrtodb = fs->fs_fsbtodb;
+ 	ump->um_seqinc = fs->fs_frag;
+ 	for (i = 0; i < MAXQUOTAS; i++)
+ 		ump->um_quotas[i] = NULLVP;
+ 	devvp->v_specflags |= SI_MOUNTEDON;
+ 	ffs_oldfscompat(fs);
+ 	ffs_vmlimits(fs);
+ 	if (ronly == 0)
+ 		ffs_sbupdate(ump, MNT_WAIT);
+ 	return (0);
+ out:
+ 	if (bp)
+ 		brelse(bp);
+ 	(void)VOP_CLOSE(devvp, ronly ? FREAD : FREAD|FWRITE, NOCRED, p);
+ 	if (ump) {
+ 		free(ump->um_fs, M_UFSMNT);
+ 		free(ump, M_UFSMNT);
+ 		mp->mnt_data = (qaddr_t)0;
+ 	}
+ 	return (error);
+ }
+ 
  
  int	mfs_pri = PWAIT | PCATCH;		/* XXX prob. temp */
  
===============================================================================
===============================================================================
===============================================================================
<EOT>



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?9507160151.AA08444>