On Mon, Dec 16, 2002 at 01:21:50PM -0500, Greg Troxel wrote:
> venus starts, and doesn't complain about its cachefiles.  However,
> trying to read one of them paniced the system.  From gdb'ing the core
> dump, I think coda_rdwr opened a file by inum and passed it to
> VOP_READ->ffs_read, which lost due to the type (unallocated inode I
> think).  So, I think two things need fixing:

Aww yeah, the Linux kernel module passes a filehandle to the
containerfile instead of the device/inode number pair. I wasn't
thinking there.
> 
> 1) my change should change the inode value in RVM, not decline to have
>    it match.  It really seems to need to match...

Yup, but as it is in RVM it might need a bit more than just 'cf->inode =
stat->st_ino'.

> 2) The kernel should be more paranoid about checking things, and not
>    take the 'struct cnode' inode value so freely.  As a minimum, it
>    probably should validate that it is a regular file.   (I maintain
>    that venus should not be able to panic the system).

That is a strong requirement, passing device/inode definitely cannot be
made safer (i.e. a bad venus could intentionally pass down the inode
number of /etc/passwd!!) So the venus process must always be 'trusted'
to some extend.

On Linux, venus opens the container file and passed down the file
handle. The kernel then accesses the file with the same 'credentials' as
venus, if venus doesn't have read or write permission the kernel won't
read or write the container file. Moderately more secure, except for the
fact that venus still runs as root :)

Windows passes down the full path to the container file. So it is
possible to validate within the kernel whether the passed down path is
actually a file and if it is in the venus.cache, but it is not possible
to check whether venus was actually allowed to read or write the file.

> I can send the backtrace to people if they think it helpful, and/or
> poke around with gdb more.  I am running NetBSD 1.6-stable on i386.

I guess what you need is the following code in CacheFile::ValidContainer,

    if (tstat.st_ino != inode) {
	Recov_BeginTrans();
	RVMLIB_REC_OBJECT(inode);
	inode = tstat.st_ino;
	Recov_EndTrans(MAXFP);
    }

Jan