Tigran Aivazian wrote:
>
> +/* moves the inode into nullfs and bads it, called on every open file.
> + * The 'root' argument is passed so that we don't down root->i_sem twice
> + */
> +static void disable_fd(struct task_struct *tsk, int fd, struct dentry *root)
> +{
> + struct files_struct *files;
> + struct file *file;
> + struct inode *inode;
> + mode_t saved_mode;
> + struct file_operations *fop;
> +
> + files = tsk->files;
> + file = files->fd[fd];
> + inode = file->f_dentry->d_inode;
> + fop = file->f_op;
> +
> + /* serialize with operations on this inode */
> + if (inode != root->d_inode) /* root is down'd by the caller */
> + down(&inode->i_sem);
> +
Is it possible that disable_fd is called for a directory? Then this
could dead lock with a concurrent rename. (double_down, triple_down,...)
> + lock_kernel();
> + locks_remove_posix(file, files);
> + locks_remove_flock(file);
> + unlock_kernel();
Now superflous, locks_remove_xy calls lock_kernel internally.
> +
> + /* flush and release fs-specific resources */
> + if (fop->flush)
> + fop->flush(file);
> + if (fop->release)
> + fop->release(inode, file);
This one could cause crashes if another thread is within kernel space.
E.g. coda frees allocated memory in coda_release.
> +
> +void disable_filesystem(struct vfsmount *mnt)
> +{
Please use get_task_struct(), task_lock() and atomic_inc(ref_count).
Your code contains the same bug as the old access_process_vm code that
caused crashes in 2.3.51(+-2)
> diff -urN -X dontdiff linux/fs/super.c nullfs/fs/super.c
> --- linux/fs/super.c Tue Jul 11 19:26:50 2000
> +++ nullfs/fs/super.c Tue Jul 18 17:29:34 2000
> @@ -979,6 +979,9 @@
> return retval;
> }
>
> + if (flags&MNT_FORCE)
> + disable_filesystem(mnt);
> +
Hmmm.
What about module unload races?
thread 1:
calls sys_read for an nfs file.
nfs enters uninterruptible sleep.
Thread 2: calls do_umount(MNT_FORCE) for that filesystem.
Succeeds. filp got killed.
umount decrements the ref count for nfs.
do_umount returns.
user calls rmmod nfs. nfs module removed.
Thread 1:
the nfs server replies, and nfs leaves the uninterruptible sleep.
crash, the module is gone.
As I wrote before, IMHO the design is flawed: you can't kill an inode
while another thread is working with that inode.
I would prefer break_uninterruptible_sleep + SIGSTOP_INTERNAL [as
SIGSTOP, but without notifying childs/parents]
-- Manfred- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.rutgers.edu Please read the FAQ at http://www.tux.org/lkml/
This archive was generated by hypermail 2b29 : Sun Jul 23 2000 - 21:00:11 EST