[PATCH][request stable 3.10 inclusion] CVE-2014-8172

[PATCH][request stable 3.10 inclusion] CVE-2014-8172

[Wang Kai]

Hi,Greg

The following patch fix CVE-2014-8172, that backport form upstream
commit eee5cc2702929fd41cce28058dc6d6717f723f87.

Hi, Al

Could you help to review this patch? Thanks.


Al Viro (1):
  get rid of s_files and files_lock

 fs/file_table.c    | 122 -----------------------------------------------------
 fs/internal.h      |   3 --
 fs/open.c          |   2 -
 fs/super.c         |  23 +---------
 include/linux/fs.h |   8 ----
 5 files changed, 2 insertions(+), 156 deletions(-)

-- 
1.8.5.5

[PATCH] get rid of s_files and files_lock

[Wang Kai]

From: Al Viro <viro@zeniv.linux.org.uk>

commit eee5cc2702929fd41cce28058dc6d6717f723f87 upstream.

The only thing we need it for is alt-sysrq-r (emergency remount r/o)
and these days we can do just as well without going through the
list of files.

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
[wangkai: backport to 3.10
 - address CVE-2014-8172
 - adjust context
 - leave fu.fu_list as still used in 3.10]
Signed-off-by: Wang Kai <morgan.wang@huawei.com>
---
 fs/file_table.c    | 122 -----------------------------------------------------
 fs/internal.h      |   3 --
 fs/open.c          |   2 -
 fs/super.c         |  23 +---------
 include/linux/fs.h |   8 ----
 5 files changed, 2 insertions(+), 156 deletions(-)

diff --git a/fs/file_table.c b/fs/file_table.c
index 54a34be..86f7cf8 100644
--- a/fs/file_table.c
+++ b/fs/file_table.c
@@ -36,8 +36,6 @@ struct files_stat_struct files_stat = {
 	.max_files = NR_FILE
 };
 
-DEFINE_STATIC_LGLOCK(files_lglock);
-
 /* SLAB cache for file structures */
 static struct kmem_cache *filp_cachep __read_mostly;
 
@@ -308,7 +306,6 @@ void fput(struct file *file)
 		struct task_struct *task = current;
 		unsigned long flags;
 
-		file_sb_list_del(file);
 		if (likely(!in_interrupt() && !(task->flags & PF_KTHREAD))) {
 			init_task_work(&file->f_u.fu_rcuhead, ____fput);
 			if (!task_work_add(task, &file->f_u.fu_rcuhead, true))
@@ -333,7 +330,6 @@ void __fput_sync(struct file *file)
 {
 	if (atomic_long_dec_and_test(&file->f_count)) {
 		struct task_struct *task = current;
-		file_sb_list_del(file);
 		BUG_ON(!(task->flags & PF_KTHREAD));
 		__fput(file);
 	}
@@ -345,127 +341,10 @@ void put_filp(struct file *file)
 {
 	if (atomic_long_dec_and_test(&file->f_count)) {
 		security_file_free(file);
-		file_sb_list_del(file);
 		file_free(file);
 	}
 }
 
-static inline int file_list_cpu(struct file *file)
-{
-#ifdef CONFIG_SMP
-	return file->f_sb_list_cpu;
-#else
-	return smp_processor_id();
-#endif
-}
-
-/* helper for file_sb_list_add to reduce ifdefs */
-static inline void __file_sb_list_add(struct file *file, struct super_block *sb)
-{
-	struct list_head *list;
-#ifdef CONFIG_SMP
-	int cpu;
-	cpu = smp_processor_id();
-	file->f_sb_list_cpu = cpu;
-	list = per_cpu_ptr(sb->s_files, cpu);
-#else
-	list = &sb->s_files;
-#endif
-	list_add(&file->f_u.fu_list, list);
-}
-
-/**
- * file_sb_list_add - add a file to the sb's file list
- * @file: file to add
- * @sb: sb to add it to
- *
- * Use this function to associate a file with the superblock of the inode it
- * refers to.
- */
-void file_sb_list_add(struct file *file, struct super_block *sb)
-{
-	lg_local_lock(&files_lglock);
-	__file_sb_list_add(file, sb);
-	lg_local_unlock(&files_lglock);
-}
-
-/**
- * file_sb_list_del - remove a file from the sb's file list
- * @file: file to remove
- * @sb: sb to remove it from
- *
- * Use this function to remove a file from its superblock.
- */
-void file_sb_list_del(struct file *file)
-{
-	if (!list_empty(&file->f_u.fu_list)) {
-		lg_local_lock_cpu(&files_lglock, file_list_cpu(file));
-		list_del_init(&file->f_u.fu_list);
-		lg_local_unlock_cpu(&files_lglock, file_list_cpu(file));
-	}
-}
-
-#ifdef CONFIG_SMP
-
-/*
- * These macros iterate all files on all CPUs for a given superblock.
- * files_lglock must be held globally.
- */
-#define do_file_list_for_each_entry(__sb, __file)		\
-{								\
-	int i;							\
-	for_each_possible_cpu(i) {				\
-		struct list_head *list;				\
-		list = per_cpu_ptr((__sb)->s_files, i);		\
-		list_for_each_entry((__file), list, f_u.fu_list)
-
-#define while_file_list_for_each_entry				\
-	}							\
-}
-
-#else
-
-#define do_file_list_for_each_entry(__sb, __file)		\
-{								\
-	struct list_head *list;					\
-	list = &(sb)->s_files;					\
-	list_for_each_entry((__file), list, f_u.fu_list)
-
-#define while_file_list_for_each_entry				\
-}
-
-#endif
-
-/**
- *	mark_files_ro - mark all files read-only
- *	@sb: superblock in question
- *
- *	All files are marked read-only.  We don't care about pending
- *	delete files so this should be used in 'force' mode only.
- */
-void mark_files_ro(struct super_block *sb)
-{
-	struct file *f;
-
-	lg_global_lock(&files_lglock);
-	do_file_list_for_each_entry(sb, f) {
-		if (!S_ISREG(file_inode(f)->i_mode))
-		       continue;
-		if (!file_count(f))
-			continue;
-		if (!(f->f_mode & FMODE_WRITE))
-			continue;
-		spin_lock(&f->f_lock);
-		f->f_mode &= ~FMODE_WRITE;
-		spin_unlock(&f->f_lock);
-		if (file_check_writeable(f) != 0)
-			continue;
-		__mnt_drop_write(f->f_path.mnt);
-		file_release_write(f);
-	} while_file_list_for_each_entry;
-	lg_global_unlock(&files_lglock);
-}
-
 void __init files_init(unsigned long mempages)
 { 
 	unsigned long n;
@@ -481,6 +360,5 @@ void __init files_init(unsigned long mempages)
 	n = (mempages * (PAGE_SIZE / 1024)) / 10;
 	files_stat.max_files = max_t(unsigned long, n, NR_FILE);
 	files_defer_init();
-	lg_lock_init(&files_lglock, "files_lglock");
 	percpu_counter_init(&nr_files, 0);
 } 
diff --git a/fs/internal.h b/fs/internal.h
index 6812158..2ffa65a 100644
--- a/fs/internal.h
+++ b/fs/internal.h
@@ -74,9 +74,6 @@ extern void chroot_fs_refs(const struct path *, const struct path *);
 /*
  * file_table.c
  */
-extern void file_sb_list_add(struct file *f, struct super_block *sb);
-extern void file_sb_list_del(struct file *f);
-extern void mark_files_ro(struct super_block *);
 extern struct file *get_empty_filp(void);
 
 /*
diff --git a/fs/open.c b/fs/open.c
index 86092bd..5f129683 100644
--- a/fs/open.c
+++ b/fs/open.c
@@ -674,7 +674,6 @@ static int do_dentry_open(struct file *f,
 	}
 
 	f->f_mapping = inode->i_mapping;
-	file_sb_list_add(f, inode->i_sb);
 
 	if (unlikely(f->f_mode & FMODE_PATH)) {
 		f->f_op = &empty_fops;
@@ -709,7 +708,6 @@ static int do_dentry_open(struct file *f,
 
 cleanup_all:
 	fops_put(f->f_op);
-	file_sb_list_del(f);
 	if (f->f_mode & FMODE_WRITE) {
 		if (!special_file(inode->i_mode)) {
 			/*
diff --git a/fs/super.c b/fs/super.c
index e028b50..97280e7 100644
--- a/fs/super.c
+++ b/fs/super.c
@@ -163,19 +163,6 @@ static struct super_block *alloc_super(struct file_system_type *type, int flags)
 			s = NULL;
 			goto out;
 		}
-#ifdef CONFIG_SMP
-		s->s_files = alloc_percpu(struct list_head);
-		if (!s->s_files)
-			goto err_out;
-		else {
-			int i;
-
-			for_each_possible_cpu(i)
-				INIT_LIST_HEAD(per_cpu_ptr(s->s_files, i));
-		}
-#else
-		INIT_LIST_HEAD(&s->s_files);
-#endif
 		if (init_sb_writers(s, type))
 			goto err_out;
 		s->s_flags = flags;
@@ -225,10 +212,6 @@ out:
 	return s;
 err_out:
 	security_sb_free(s);
-#ifdef CONFIG_SMP
-	if (s->s_files)
-		free_percpu(s->s_files);
-#endif
 	destroy_sb_writers(s);
 	kfree(s);
 	s = NULL;
@@ -243,9 +226,6 @@ err_out:
  */
 static inline void destroy_super(struct super_block *s)
 {
-#ifdef CONFIG_SMP
-	free_percpu(s->s_files);
-#endif
 	destroy_sb_writers(s);
 	security_sb_free(s);
 	WARN_ON(!list_empty(&s->s_mounts));
@@ -727,7 +707,8 @@ int do_remount_sb(struct super_block *sb, int flags, void *data, int force)
 	   make sure there are no rw files opened */
 	if (remount_ro) {
 		if (force) {
-			mark_files_ro(sb);
+			sb->s_readonly_remount = 1;
+			smp_wmb();
 		} else {
 			retval = sb_prepare_remount_readonly(sb);
 			if (retval)
diff --git a/include/linux/fs.h b/include/linux/fs.h
index d57bc5d..bc402a8 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -779,9 +779,6 @@ struct file {
 	 * Must not be taken from IRQ context.
 	 */
 	spinlock_t		f_lock;
-#ifdef CONFIG_SMP
-	int			f_sb_list_cpu;
-#endif
 	atomic_long_t		f_count;
 	unsigned int 		f_flags;
 	fmode_t			f_mode;
@@ -1257,11 +1254,6 @@ struct super_block {
 
 	struct list_head	s_inodes;	/* all inodes */
 	struct hlist_bl_head	s_anon;		/* anonymous dentries for (nfs) exporting */
-#ifdef CONFIG_SMP
-	struct list_head __percpu *s_files;
-#else
-	struct list_head	s_files;
-#endif
 	struct list_head	s_mounts;	/* list of mounts; _not_ for fs use */
 	/* s_dentry_lru, s_nr_dentry_unused protected by dcache.c lru locks */
 	struct list_head	s_dentry_lru;	/* unused dentry lru */
-- 
1.8.5.5

Re: [PATCH] get rid of s_files and files_lock

[Zefan Li]

On 2015/4/29 19:00, Wang Kai wrote:
> From: Al Viro <viro@zeniv.linux.org.uk>
> 
> commit eee5cc2702929fd41cce28058dc6d6717f723f87 upstream.
> 
> The only thing we need it for is alt-sysrq-r (emergency remount r/o)
> and these days we can do just as well without going through the
> list of files.
> 
> Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
> [wangkai: backport to 3.10
>  - address CVE-2014-8172
>  - adjust context
>  - leave fu.fu_list as still used in 3.10]

This doesn't sound right to me. We're removing operations on fu_list like
init/add/del, so how can it still be used?

Re: [PATCH] get rid of s_files and files_lock

[Wang Kai]

On 2015/4/30 11:24, Zefan Li wrote:
> On 2015/4/29 19:00, Wang Kai wrote:
>> From: Al Viro <viro@zeniv.linux.org.uk>
>>
>> commit eee5cc2702929fd41cce28058dc6d6717f723f87 upstream.
>>
>> The only thing we need it for is alt-sysrq-r (emergency remount r/o)
>> and these days we can do just as well without going through the
>> list of files.
>>
>> Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
>> [wangkai: backport to 3.10
>>  - address CVE-2014-8172
>>  - adjust context
>>  - leave fu.fu_list as still used in 3.10]
> 
> This doesn't sound right to me. We're removing operations on fu_list like
> init/add/del, so how can it still be used?

Thanks for review, Zefan.

The fix commit eee5cc2702 comes after 4f5e65a1cc90, which replace
fu_list with fu_llist for avoiding lock.
So, if we don't backport 4f5e65a1cc90, we should leave fu_list there.

Or, should we backport 4f5e65a1cc90 too?
IMO, as it's NOT bugfix, and just backport this one will NOT make
work easier(cherry-picked), so looks unnecessary.

Another question, this bugfix will change kernel's header files(API),
how LTS handle with this, OK with that?

> 
> 
> .
> 

Re: [PATCH] get rid of s_files and files_lock

[Zefan Li]

On 2015/4/30 12:39, Wang Kai wrote:
> On 2015/4/30 11:24, Zefan Li wrote:
>> On 2015/4/29 19:00, Wang Kai wrote:
>>> From: Al Viro <viro@zeniv.linux.org.uk>
>>>
>>> commit eee5cc2702929fd41cce28058dc6d6717f723f87 upstream.
>>>
>>> The only thing we need it for is alt-sysrq-r (emergency remount r/o)
>>> and these days we can do just as well without going through the
>>> list of files.
>>>
>>> Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
>>> [wangkai: backport to 3.10
>>>  - address CVE-2014-8172
>>>  - adjust context
>>>  - leave fu.fu_list as still used in 3.10]
>>
>> This doesn't sound right to me. We're removing operations on fu_list like
>> init/add/del, so how can it still be used?
> 
> Thanks for review, Zefan.
> 
> The fix commit eee5cc2702 comes after 4f5e65a1cc90, which replace
> fu_list with fu_llist for avoiding lock.
> So, if we don't backport 4f5e65a1cc90, we should leave fu_list there.
> 
> Or, should we backport 4f5e65a1cc90 too?

Not necessary, but is better. my 2 cents.

> IMO, as it's NOT bugfix, and just backport this one will NOT make
> work easier(cherry-picked), so looks unnecessary.
> 
> Another question, this bugfix will change kernel's header files(API),
> how LTS handle with this, OK with that?
> 

Documentation/stable_api_nonsense.txt

Re: [PATCH] get rid of s_files and files_lock

[Wang Kai]

On 2015/4/30 12:49, Zefan Li wrote:
> On 2015/4/30 12:39, Wang Kai wrote:
>> On 2015/4/30 11:24, Zefan Li wrote:
>>> On 2015/4/29 19:00, Wang Kai wrote:
>>>> From: Al Viro <viro@zeniv.linux.org.uk>
>>>>
>>>> commit eee5cc2702929fd41cce28058dc6d6717f723f87 upstream.
>>>>
>>>> The only thing we need it for is alt-sysrq-r (emergency remount r/o)
>>>> and these days we can do just as well without going through the
>>>> list of files.
>>>>
>>>> Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
>>>> [wangkai: backport to 3.10
>>>>  - address CVE-2014-8172
>>>>  - adjust context
>>>>  - leave fu.fu_list as still used in 3.10]
>>>
>>> This doesn't sound right to me. We're removing operations on fu_list like
>>> init/add/del, so how can it still be used?
>>
>> Thanks for review, Zefan.
>>
>> The fix commit eee5cc2702 comes after 4f5e65a1cc90, which replace
>> fu_list with fu_llist for avoiding lock.
>> So, if we don't backport 4f5e65a1cc90, we should leave fu_list there.
>>
>> Or, should we backport 4f5e65a1cc90 too?
> 
> Not necessary, but is better. my 2 cents.
OK, I will send a v2 soon.

> 
>> IMO, as it's NOT bugfix, and just backport this one will NOT make
>> work easier(cherry-picked), so looks unnecessary.
>>
>> Another question, this bugfix will change kernel's header files(API),
>> how LTS handle with this, OK with that?
>>
> 
> Documentation/stable_api_nonsense.txt
> 
> 
> .
>