Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1590218 > unrolled thread
| Started by | Elena Reshetova <elena.reshetova@intel.com> |
|---|---|
| First post | 2017-03-01 11:50 +0100 |
| Last post | 2017-03-01 11:50 +0100 |
| Articles | 4 — 1 participant |
Back to article view | Back to linux.kernel
[PATCH 0/5] fs genetic refcounter conversions Elena Reshetova <elena.reshetova@intel.com> - 2017-03-01 11:50 +0100
[PATCH 5/5] fs: convert userfaultfd_ctx.refcount from atomic_t to refcount_t Elena Reshetova <elena.reshetova@intel.com> - 2017-03-01 11:50 +0100
[PATCH 3/5] fs: convert mnt_namespace.count from atomic_t to refcount_t Elena Reshetova <elena.reshetova@intel.com> - 2017-03-01 11:50 +0100
[PATCH 4/5] fs: convert super_block.s_active from atomic_t to refcount_t Elena Reshetova <elena.reshetova@intel.com> - 2017-03-01 11:50 +0100
| From | Elena Reshetova <elena.reshetova@intel.com> |
|---|---|
| Date | 2017-03-01 11:50 +0100 |
| Subject | [PATCH 0/5] fs genetic refcounter conversions |
| Message-ID | <tg9Zv-1Pu-5@gated-at.bofh.it> |
Now when new refcount_t type and API are finally merged (see include/linux/refcount.h), the following patches convert various refcounters in the generic fs code from atomic_t to refcount_t. By doing this we prevent intentional or accidental underflows or overflows that can led to use-after-free vulnerabilities. The below patches are fully independent and can be cherry-picked separately. Since we convert all kernel subsystems in the same fashion, resulting in about 300 patches, we have to group them for sending at least in some fashion to be manageable. Please excuse the long cc list. Run-time tested using xfstests with xfs and ext4 filesystems. refcount WARNs were on and no issues related to changes were encounted during running the tests. Elena Reshetova (5): fs: convert mb_cache_entry.e_refcnt from atomic_t to refcount_t fs: convert posix_acl.a_refcount from atomic_t to refcount_t fs: convert mnt_namespace.count from atomic_t to refcount_t fs: convert super_block.s_active from atomic_t to refcount_t fs: convert userfaultfd_ctx.refcount from atomic_t to refcount_t drivers/staging/lustre/lustre/llite/llite_lib.c | 2 +- fs/cifs/cifsfs.c | 2 +- fs/devpts/inode.c | 2 +- fs/f2fs/acl.c | 2 +- fs/gfs2/super.c | 2 +- fs/kernfs/mount.c | 2 +- fs/mbcache.c | 16 ++++++++-------- fs/mount.h | 5 +++-- fs/namespace.c | 8 ++++---- fs/nfs/super.c | 4 ++-- fs/posix_acl.c | 6 +++--- fs/super.c | 10 +++++----- fs/userfaultfd.c | 11 ++++++----- include/linux/fs.h | 3 ++- include/linux/mbcache.h | 6 +++--- include/linux/posix_acl.h | 7 ++++--- 16 files changed, 46 insertions(+), 42 deletions(-) -- 2.7.4
[toc] | [next] | [standalone]
| From | Elena Reshetova <elena.reshetova@intel.com> |
|---|---|
| Date | 2017-03-01 11:50 +0100 |
| Subject | [PATCH 5/5] fs: convert userfaultfd_ctx.refcount from atomic_t to refcount_t |
| Message-ID | <tg9Zv-1Pu-9@gated-at.bofh.it> |
| In reply to | #1590218 |
refcount_t type and corresponding API should be
used instead of atomic_t when the variable is used as
a reference counter. This allows to avoid accidental
refcounter overflows that might lead to use-after-free
situations.
Signed-off-by: Elena Reshetova <elena.reshetova@intel.com>
Signed-off-by: Hans Liljestrand <ishkamiel@gmail.com>
Signed-off-by: Kees Cook <keescook@chromium.org>
Signed-off-by: David Windsor <dwindsor@gmail.com>
---
fs/userfaultfd.c | 11 ++++++-----
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/fs/userfaultfd.c b/fs/userfaultfd.c
index 3c421d0..06c334a 100644
--- a/fs/userfaultfd.c
+++ b/fs/userfaultfd.c
@@ -28,6 +28,7 @@
#include <linux/ioctl.h>
#include <linux/security.h>
#include <linux/hugetlb.h>
+#include <linux/refcount.h>
static struct kmem_cache *userfaultfd_ctx_cachep __read_mostly;
@@ -52,7 +53,7 @@ struct userfaultfd_ctx {
/* a refile sequence protected by fault_pending_wqh lock */
struct seqcount refile_seq;
/* pseudo fd refcounting */
- atomic_t refcount;
+ refcount_t refcount;
/* userfaultfd syscall flags */
unsigned int flags;
/* features requested from the userspace */
@@ -142,7 +143,7 @@ static int userfaultfd_wake_function(wait_queue_t *wq, unsigned mode,
*/
static void userfaultfd_ctx_get(struct userfaultfd_ctx *ctx)
{
- if (!atomic_inc_not_zero(&ctx->refcount))
+ if (!refcount_inc_not_zero(&ctx->refcount))
BUG();
}
@@ -156,7 +157,7 @@ static void userfaultfd_ctx_get(struct userfaultfd_ctx *ctx)
*/
static void userfaultfd_ctx_put(struct userfaultfd_ctx *ctx)
{
- if (atomic_dec_and_test(&ctx->refcount)) {
+ if (refcount_dec_and_test(&ctx->refcount)) {
VM_BUG_ON(spin_is_locked(&ctx->fault_pending_wqh.lock));
VM_BUG_ON(waitqueue_active(&ctx->fault_pending_wqh));
VM_BUG_ON(spin_is_locked(&ctx->fault_wqh.lock));
@@ -607,7 +608,7 @@ int dup_userfaultfd(struct vm_area_struct *vma, struct list_head *fcs)
return -ENOMEM;
}
- atomic_set(&ctx->refcount, 1);
+ refcount_set(&ctx->refcount, 1);
ctx->flags = octx->flags;
ctx->state = UFFD_STATE_RUNNING;
ctx->features = octx->features;
@@ -1840,7 +1841,7 @@ static struct file *userfaultfd_file_create(int flags)
if (!ctx)
goto out;
- atomic_set(&ctx->refcount, 1);
+ refcount_set(&ctx->refcount, 1);
ctx->flags = flags;
ctx->features = 0;
ctx->state = UFFD_STATE_WAIT_API;
--
2.7.4
[toc] | [prev] | [next] | [standalone]
| From | Elena Reshetova <elena.reshetova@intel.com> |
|---|---|
| Date | 2017-03-01 11:50 +0100 |
| Subject | [PATCH 3/5] fs: convert mnt_namespace.count from atomic_t to refcount_t |
| Message-ID | <tg9Zv-1Pu-11@gated-at.bofh.it> |
| In reply to | #1590218 |
refcount_t type and corresponding API should be
used instead of atomic_t when the variable is used as
a reference counter. This allows to avoid accidental
refcounter overflows that might lead to use-after-free
situations.
Signed-off-by: Elena Reshetova <elena.reshetova@intel.com>
Signed-off-by: Hans Liljestrand <ishkamiel@gmail.com>
Signed-off-by: Kees Cook <keescook@chromium.org>
Signed-off-by: David Windsor <dwindsor@gmail.com>
---
fs/mount.h | 5 +++--
fs/namespace.c | 4 ++--
2 files changed, 5 insertions(+), 4 deletions(-)
diff --git a/fs/mount.h b/fs/mount.h
index 2826543..106240d 100644
--- a/fs/mount.h
+++ b/fs/mount.h
@@ -3,9 +3,10 @@
#include <linux/poll.h>
#include <linux/ns_common.h>
#include <linux/fs_pin.h>
+#include <linux/refcount.h>
struct mnt_namespace {
- atomic_t count;
+ refcount_t count;
struct ns_common ns;
struct mount * root;
struct list_head list;
@@ -110,7 +111,7 @@ static inline void detach_mounts(struct dentry *dentry)
static inline void get_mnt_ns(struct mnt_namespace *ns)
{
- atomic_inc(&ns->count);
+ refcount_inc(&ns->count);
}
extern seqlock_t mount_lock;
diff --git a/fs/namespace.c b/fs/namespace.c
index 8bfad42..85cc8ca 100644
--- a/fs/namespace.c
+++ b/fs/namespace.c
@@ -2888,7 +2888,7 @@ static struct mnt_namespace *alloc_mnt_ns(struct user_namespace *user_ns)
}
new_ns->ns.ops = &mntns_operations;
new_ns->seq = atomic64_add_return(1, &mnt_ns_seq);
- atomic_set(&new_ns->count, 1);
+ refcount_set(&new_ns->count, 1);
new_ns->root = NULL;
INIT_LIST_HEAD(&new_ns->list);
init_waitqueue_head(&new_ns->poll);
@@ -3278,7 +3278,7 @@ void __init mnt_init(void)
void put_mnt_ns(struct mnt_namespace *ns)
{
- if (!atomic_dec_and_test(&ns->count))
+ if (!refcount_dec_and_test(&ns->count))
return;
drop_collected_mounts(&ns->root->mnt);
free_mnt_ns(ns);
--
2.7.4
[toc] | [prev] | [next] | [standalone]
| From | Elena Reshetova <elena.reshetova@intel.com> |
|---|---|
| Date | 2017-03-01 11:50 +0100 |
| Subject | [PATCH 4/5] fs: convert super_block.s_active from atomic_t to refcount_t |
| Message-ID | <tg9Zx-1Pu-37@gated-at.bofh.it> |
| In reply to | #1590218 |
refcount_t type and corresponding API should be
used instead of atomic_t when the variable is used as
a reference counter. This allows to avoid accidental
refcounter overflows that might lead to use-after-free
situations.
Signed-off-by: Elena Reshetova <elena.reshetova@intel.com>
Signed-off-by: Hans Liljestrand <ishkamiel@gmail.com>
Signed-off-by: Kees Cook <keescook@chromium.org>
Signed-off-by: David Windsor <dwindsor@gmail.com>
---
drivers/staging/lustre/lustre/llite/llite_lib.c | 2 +-
fs/cifs/cifsfs.c | 2 +-
fs/devpts/inode.c | 2 +-
fs/gfs2/super.c | 2 +-
fs/kernfs/mount.c | 2 +-
fs/namespace.c | 4 ++--
fs/nfs/super.c | 4 ++--
fs/super.c | 10 +++++-----
include/linux/fs.h | 3 ++-
9 files changed, 16 insertions(+), 15 deletions(-)
diff --git a/drivers/staging/lustre/lustre/llite/llite_lib.c b/drivers/staging/lustre/lustre/llite/llite_lib.c
index b229cbc..46d7b2a 100644
--- a/drivers/staging/lustre/lustre/llite/llite_lib.c
+++ b/drivers/staging/lustre/lustre/llite/llite_lib.c
@@ -1995,7 +1995,7 @@ void ll_umount_begin(struct super_block *sb)
struct l_wait_info lwi;
CDEBUG(D_VFSTRACE, "VFS Op: superblock %p count %d active %d\n", sb,
- sb->s_count, atomic_read(&sb->s_active));
+ sb->s_count, refcount_read(&sb->s_active));
obd = class_exp2obd(sbi->ll_md_exp);
if (!obd) {
diff --git a/fs/cifs/cifsfs.c b/fs/cifs/cifsfs.c
index 15e1db8..73d29b7 100644
--- a/fs/cifs/cifsfs.c
+++ b/fs/cifs/cifsfs.c
@@ -101,7 +101,7 @@ cifs_sb_active(struct super_block *sb)
struct cifs_sb_info *server = CIFS_SB(sb);
if (atomic_inc_return(&server->active) == 1)
- atomic_inc(&sb->s_active);
+ refcount_inc(&sb->s_active);
}
void
diff --git a/fs/devpts/inode.c b/fs/devpts/inode.c
index 108df2e..fba89ab 100644
--- a/fs/devpts/inode.c
+++ b/fs/devpts/inode.c
@@ -164,7 +164,7 @@ struct pts_fs_info *devpts_acquire(struct file *filp)
/*
* pty code needs to hold extra references in case of last /dev/tty close
*/
- atomic_inc(&sb->s_active);
+ refcount_inc(&sb->s_active);
result = DEVPTS_SB(sb);
out:
diff --git a/fs/gfs2/super.c b/fs/gfs2/super.c
index e3ee387..49af2ef 100644
--- a/fs/gfs2/super.c
+++ b/fs/gfs2/super.c
@@ -954,7 +954,7 @@ void gfs2_freeze_func(struct work_struct *work)
struct gfs2_sbd *sdp = container_of(work, struct gfs2_sbd, sd_freeze_work);
struct super_block *sb = sdp->sd_vfs;
- atomic_inc(&sb->s_active);
+ refcount_inc(&sb->s_active);
error = gfs2_glock_nq_init(sdp->sd_freeze_gl, LM_ST_SHARED, 0,
&freeze_gh);
if (error) {
diff --git a/fs/kernfs/mount.c b/fs/kernfs/mount.c
index d5b149a..8d61bcf 100644
--- a/fs/kernfs/mount.c
+++ b/fs/kernfs/mount.c
@@ -319,7 +319,7 @@ struct super_block *kernfs_pin_sb(struct kernfs_root *root, const void *ns)
list_for_each_entry(info, &root->supers, node) {
if (info->ns == ns) {
sb = info->sb;
- if (!atomic_inc_not_zero(&info->sb->s_active))
+ if (!refcount_inc_not_zero(&info->sb->s_active))
sb = ERR_PTR(-EINVAL);
break;
}
diff --git a/fs/namespace.c b/fs/namespace.c
index 85cc8ca..9a6107f 100644
--- a/fs/namespace.c
+++ b/fs/namespace.c
@@ -1051,7 +1051,7 @@ static struct mount *clone_mnt(struct mount *old, struct dentry *root,
(!(flag & CL_EXPIRE) || list_empty(&old->mnt_expire)))
mnt->mnt.mnt_flags |= MNT_LOCKED;
- atomic_inc(&sb->s_active);
+ refcount_inc(&sb->s_active);
mnt->mnt.mnt_sb = sb;
mnt->mnt.mnt_root = dget(root);
mnt->mnt_mountpoint = mnt->mnt.mnt_root;
@@ -3015,7 +3015,7 @@ struct dentry *mount_subtree(struct vfsmount *mnt, const char *name)
/* trade a vfsmount reference for active sb one */
s = path.mnt->mnt_sb;
- atomic_inc(&s->s_active);
+ refcount_inc(&s->s_active);
mntput(path.mnt);
/* lock the sucker */
down_write(&s->s_umount);
diff --git a/fs/nfs/super.c b/fs/nfs/super.c
index 6bca178..58e4b51 100644
--- a/fs/nfs/super.c
+++ b/fs/nfs/super.c
@@ -412,10 +412,10 @@ bool nfs_sb_active(struct super_block *sb)
{
struct nfs_server *server = NFS_SB(sb);
- if (!atomic_inc_not_zero(&sb->s_active))
+ if (!refcount_inc_not_zero(&sb->s_active))
return false;
if (atomic_inc_return(&server->active) != 1)
- atomic_dec(&sb->s_active);
+ refcount_dec(&sb->s_active);
return true;
}
EXPORT_SYMBOL_GPL(nfs_sb_active);
diff --git a/fs/super.c b/fs/super.c
index b8b6a08..7e1511c 100644
--- a/fs/super.c
+++ b/fs/super.c
@@ -240,7 +240,7 @@ static struct super_block *alloc_super(struct file_system_type *type, int flags,
*/
down_write_nested(&s->s_umount, SINGLE_DEPTH_NESTING);
s->s_count = 1;
- atomic_set(&s->s_active, 1);
+ refcount_set(&s->s_active, 1);
mutex_init(&s->s_vfs_rename_mutex);
lockdep_set_class(&s->s_vfs_rename_mutex, &type->s_vfs_rename_key);
mutex_init(&s->s_dquot.dqio_mutex);
@@ -303,7 +303,7 @@ static void put_super(struct super_block *sb)
void deactivate_locked_super(struct super_block *s)
{
struct file_system_type *fs = s->s_type;
- if (atomic_dec_and_test(&s->s_active)) {
+ if (refcount_dec_and_test(&s->s_active)) {
cleancache_invalidate_fs(s);
unregister_shrinker(&s->s_shrink);
fs->kill_sb(s);
@@ -335,7 +335,7 @@ EXPORT_SYMBOL(deactivate_locked_super);
*/
void deactivate_super(struct super_block *s)
{
- if (!atomic_add_unless(&s->s_active, -1, 1)) {
+ if (!refcount_dec_not_one(&s->s_active)) {
down_write(&s->s_umount);
deactivate_locked_super(s);
}
@@ -361,7 +361,7 @@ static int grab_super(struct super_block *s) __releases(sb_lock)
s->s_count++;
spin_unlock(&sb_lock);
down_write(&s->s_umount);
- if ((s->s_flags & MS_BORN) && atomic_inc_not_zero(&s->s_active)) {
+ if ((s->s_flags & MS_BORN) && refcount_inc_not_zero(&s->s_active)) {
put_super(s);
return 1;
}
@@ -1385,7 +1385,7 @@ int freeze_super(struct super_block *sb)
{
int ret;
- atomic_inc(&sb->s_active);
+ refcount_inc(&sb->s_active);
down_write(&sb->s_umount);
if (sb->s_writers.frozen != SB_UNFROZEN) {
deactivate_locked_super(sb);
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 79fc59b..6b83a08 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -31,6 +31,7 @@
#include <linux/workqueue.h>
#include <linux/percpu-rwsem.h>
#include <linux/delayed_call.h>
+#include <linux/refcount.h>
#include <asm/byteorder.h>
#include <uapi/linux/fs.h>
@@ -1308,7 +1309,7 @@ struct super_block {
struct dentry *s_root;
struct rw_semaphore s_umount;
int s_count;
- atomic_t s_active;
+ refcount_t s_active;
#ifdef CONFIG_SECURITY
void *s_security;
#endif
--
2.7.4
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web