| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-Id: <1199141688.13731.35.camel@localhost>
Date: Mon, 31 Dec 2007 14:54:48 -0800
From: Dave Hansen <haveblue@...ibm.com>
To: Christoph Hellwig <hch@....de>
Cc: "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: getting rid of filp search in fs_may_remount_ro()
On Mon, 2007-12-31 at 11:54 -0800, Dave Hansen wrote:
> On Wed, 2007-12-26 at 15:12 +0100, Christoph Hellwig wrote:
> > Btw, I just noticed in current -mm fs_may_remount_ro() is still around
> > and not replaced by ther per-sb writers count. That surely sounds like
> > some kind of mismerge..
>
> I was actually leaving that for later. Getting rid of the filp search
> is a great benefit of the r/o bind patches, but it isn't strictly
> necessary and it doesn't really hurt anything to keep it.
>
> The reason that it was contentious was that we need some way to be able
> to do an sb-to-mount mapping. When remounting the sb, we need to
> determine whether *any* of the mounts of that sb have any writers.
>
> We don't currently have any mechanisms to do direct lookups from sb to
> mount. The only alternative I can see right now is to walk over all
> tasks, then walk over all vfs namespaces, and walk each mount tree to
> see if any mounts are of the sb we're looking for. This needs to be
> done while already holding the mnt_writers[] locks so that no new mnt
> writers can come in.
>
> *THAT* is going to be a heavyweight operation. I need to go look in
> detail at how the mount trees are kept, and we'll need some kind of
> mechanism to keep track of which vfs namespaces we've looked at during
> the search so we don't search them twice.
>
> Can you think of a simpler way to do it?
Here's one blatantly untested idea I have. The idea is to keep track if
anyone might be writing to a mnt. We keep track on a flag in the mnt.
When we set the flag, we increment a counter in the sb and decrement
when the flag is cleared.
We can't simply look at mnt->__mnt_writers because there might be
"checked-out" writers in the mnt_writers[] array. We also have to keep
new writers from coming in while we do this, so we use the spinlocks in
the mnt_writers[] array for exclusion. This is a pretty heavyweight
lock, but it only gets used at rw->ro transitions.
-- Dave
--- ./fs/file_table.c.orig 2007-12-31 11:14:59.000000000 -0800
+++ ./fs/file_table.c 2007-12-31 14:38:19.000000000 -0800
@@ -376,26 +376,12 @@
int fs_may_remount_ro(struct super_block *sb)
{
- struct file *file;
-
- /* Check that no files are currently opened for writing. */
- file_list_lock();
- list_for_each_entry(file, &sb->s_files, f_u.fu_list) {
- struct inode *inode = file->f_path.dentry->d_inode;
-
- /* File with pending delete? */
- if (inode->i_nlink == 0)
- goto too_bad;
-
- /* Writeable file? */
- if (S_ISREG(inode->i_mode) && (file->f_mode & FMODE_WRITE))
- goto too_bad;
- }
- file_list_unlock();
- return 1; /* Tis' cool bro. */
-too_bad:
- file_list_unlock();
- return 0;
+ int ret = 1;
+ lock_mnt_writers();
+ if (atomic_read(&sb->__s_mnt_writers))
+ ret = 0;
+ unlock_mnt_writers();
+ return ret;
}
void __init files_init(unsigned long mempages)
--- ./fs/namespace.c.orig 2007-12-31 13:52:36.000000000 -0800
+++ ./fs/namespace.c 2007-12-31 14:53:56.000000000 -0800
@@ -138,7 +138,7 @@
}
fs_initcall(init_mnt_writers);
-static void mnt_unlock_cpus(void)
+void mnt_unlock_cpus(void)
{
int cpu;
struct mnt_writer *cpu_writer;
@@ -149,6 +149,22 @@
}
}
+static void mark_mnt_has_writer(struct vfsmount *mnt)
+{
+ if (!test_and_set_bit(ilog2(MNT_MAY_HAVE_WRITERS), &mnt->mnt_flags))
+ atomic_inc(&mnt->mnt_sb->s_possible_mnt_writers);
+}
+
+static void check_mnt_for_writers(struct vfsmount *mnt)
+{
+ int bitnr = ilog2(MNT_MAY_HAVE_WRITERS);
+
+ if (atomic_read(&mnt->__mnt_writers))
+ mark_mnt_has_writer(mnt);
+ else if (test_and_clear_bit(bitnr, &mnt->mnt_flags))
+ atomic_dec(&mnt->mnt_sb->s_possible_mnt_writers);
+}
+
static inline void __clear_mnt_count(struct mnt_writer *cpu_writer)
{
if (!cpu_writer->mnt)
@@ -199,6 +215,7 @@
}
use_cpu_writer_for_mount(cpu_writer, mnt);
cpu_writer->count++;
+ mark_mnt_has_writer(mnt);
out:
spin_unlock(&cpu_writer->lock);
put_cpu_var(mnt_writers);
@@ -215,11 +232,33 @@
cpu_writer = &per_cpu(mnt_writers, cpu);
spin_lock(&cpu_writer->lock);
__clear_mnt_count(cpu_writer);
+ /*
+ * __mnt_writers may temporarily hit zero
+ * (and trigger MNT_MAY_HAVE_WRITERS to get
+ * cleared), but it will get set again if
+ * and when another mnt_writer[] has an
+ * entry for that mnt later in this loop.
+ */
+ check_mnt_has_writers(cpu_writer->mnt);
cpu_writer->mnt = NULL;
}
}
/*
+ * This is just an external interface. I want
+ * to use the long names in here, but leave the
+ * simpler names for external users.
+ */
+void lock_mnt_writers()
+{
+ lock_and_coalesce_cpu_mnt_writer_counts();
+}
+void unlock_mnt_writers()
+{
+ mnt_unlock_cpus();
+}
+
+/*
* These per-cpu write counts are not guaranteed to have
* matched increments and decrements on any given cpu.
* A file open()ed for write on one cpu and close()d on
--- ./include/linux/fs.h.orig 2007-12-31 14:08:20.000000000 -0800
+++ ./include/linux/fs.h 2007-12-31 14:31:12.000000000 -0800
@@ -1005,6 +1005,9 @@
#ifdef CONFIG_SECURITY
void *s_security;
#endif
+ atomic_t __s_mnt_writers;/* how many mounts of this sb might
+ * have writers. Only stable with
+ * all mnt_writers[] locks held */
struct xattr_handler **s_xattr;
struct list_head s_inodes; /* all inodes */
--- ./include/linux/mount.h.orig 2007-12-31 14:00:13.000000000 -0800
+++ ./include/linux/mount.h 2007-12-31 14:33:44.000000000 -0800
@@ -33,6 +33,7 @@
#define MNT_SHRINKABLE 0x100
#define MNT_IMBALANCED_WRITE_COUNT 0x200 /* just for debugging */
+#define MNT_MAY_HAVE_WRITERS 0x400 /* did this ever have a writer? */
#define MNT_SHARED 0x1000 /* if the vfsmount is a shared mount */
#define MNT_UNBINDABLE 0x2000 /* if the vfsmount is a unbindable mount */
@@ -80,6 +81,8 @@
extern int mnt_want_write(struct vfsmount *mnt);
extern void mnt_drop_write(struct vfsmount *mnt);
+extern void lock_mnt_writers(void);
+extern void unlock_mnt_writers(void);
extern void mntput_no_expire(struct vfsmount *mnt);
extern void mnt_pin(struct vfsmount *mnt);
extern void mnt_unpin(struct vfsmount *mnt);
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists