* [PATCH v2] jbd2: fix potential double free
@ 2019-05-10 10:36 Chengguang Xu
2019-05-11 0:18 ` Theodore Ts'o
2019-05-11 1:37 ` Theodore Ts'o
0 siblings, 2 replies; 3+ messages in thread
From: Chengguang Xu @ 2019-05-10 10:36 UTC (permalink / raw)
To: jack, tytso; +Cc: linux-ext4, linux-kernel, Chengguang Xu
When fail from creating cache jbd2_inode_cache, we will
destroy previously created cache jbd2_handle_cache twice.
This patch fixes it by sperating each cache
initialization/destruction to individual function.
Signed-off-by: Chengguang Xu <cgxu519@gmail.com>
---
v2:
- Seperate cache initialization/destruction to individual function.
fs/jbd2/journal.c | 51 +++++++++++++++++++++++++++----------------
fs/jbd2/revoke.c | 32 +++++++++++++++++----------
fs/jbd2/transaction.c | 8 ++++---
include/linux/jbd2.h | 8 ++++---
4 files changed, 62 insertions(+), 37 deletions(-)
diff --git a/fs/jbd2/journal.c b/fs/jbd2/journal.c
index 37e16d969925..0f1ac43d0560 100644
--- a/fs/jbd2/journal.c
+++ b/fs/jbd2/journal.c
@@ -2299,7 +2299,7 @@ static void jbd2_journal_destroy_slabs(void)
}
}
-static int jbd2_journal_create_slab(size_t size)
+static int __init jbd2_journal_create_slab(size_t size)
{
static DEFINE_MUTEX(jbd2_slab_create_mutex);
int i = order_base_2(size) - 10;
@@ -2375,22 +2375,19 @@ static struct kmem_cache *jbd2_journal_head_cache;
static atomic_t nr_journal_heads = ATOMIC_INIT(0);
#endif
-static int jbd2_journal_init_journal_head_cache(void)
+static int __init jbd2_journal_init_journal_head_cache(void)
{
- int retval;
-
- J_ASSERT(jbd2_journal_head_cache == NULL);
+ J_ASSERT(!jbd2_journal_head_cache);
jbd2_journal_head_cache = kmem_cache_create("jbd2_journal_head",
sizeof(struct journal_head),
0, /* offset */
SLAB_TEMPORARY | SLAB_TYPESAFE_BY_RCU,
NULL); /* ctor */
- retval = 0;
if (!jbd2_journal_head_cache) {
- retval = -ENOMEM;
printk(KERN_EMERG "JBD2: no memory for journal_head cache\n");
+ return -ENOMEM;
}
- return retval;
+ return 0;
}
static void jbd2_journal_destroy_journal_head_cache(void)
@@ -2636,28 +2633,38 @@ static void __exit jbd2_remove_jbd_stats_proc_entry(void)
struct kmem_cache *jbd2_handle_cache, *jbd2_inode_cache;
+static int __init jbd2_journal_init_inode_cache(void)
+{
+ J_ASSERT(!jbd2_inode_cache);
+ jbd2_inode_cache = KMEM_CACHE(jbd2_inode, 0);
+ if (!jbd2_inode_cache) {
+ pr_emerg("JBD2: failed to create inode cache\n");
+ return -ENOMEM;
+ }
+ return 0;
+}
+
static int __init jbd2_journal_init_handle_cache(void)
{
+ J_ASSERT(!jbd2_handle_cache);
jbd2_handle_cache = KMEM_CACHE(jbd2_journal_handle, SLAB_TEMPORARY);
- if (jbd2_handle_cache == NULL) {
+ if (!jbd2_handle_cache) {
printk(KERN_EMERG "JBD2: failed to create handle cache\n");
return -ENOMEM;
}
- jbd2_inode_cache = KMEM_CACHE(jbd2_inode, 0);
- if (jbd2_inode_cache == NULL) {
- printk(KERN_EMERG "JBD2: failed to create inode cache\n");
- kmem_cache_destroy(jbd2_handle_cache);
- return -ENOMEM;
- }
return 0;
}
+static void jbd2_journal_destroy_inode_cache(void)
+{
+ kmem_cache_destroy(jbd2_inode_cache);
+ jbd2_inode_cache = NULL;
+}
+
static void jbd2_journal_destroy_handle_cache(void)
{
kmem_cache_destroy(jbd2_handle_cache);
jbd2_handle_cache = NULL;
- kmem_cache_destroy(jbd2_inode_cache);
- jbd2_inode_cache = NULL;
}
/*
@@ -2668,11 +2675,15 @@ static int __init journal_init_caches(void)
{
int ret;
- ret = jbd2_journal_init_revoke_caches();
+ ret = jbd2_journal_init_revoke_record_cache();
+ if (ret == 0)
+ ret = jbd2_journal_init_revoke_table_cache();
if (ret == 0)
ret = jbd2_journal_init_journal_head_cache();
if (ret == 0)
ret = jbd2_journal_init_handle_cache();
+ if (ret == 0)
+ ret = jbd2_journal_init_inode_cache();
if (ret == 0)
ret = jbd2_journal_init_transaction_cache();
return ret;
@@ -2680,9 +2691,11 @@ static int __init journal_init_caches(void)
static void jbd2_journal_destroy_caches(void)
{
- jbd2_journal_destroy_revoke_caches();
+ jbd2_journal_destroy_revoke_record_cache();
+ jbd2_journal_destroy_revoke_table_cache();
jbd2_journal_destroy_journal_head_cache();
jbd2_journal_destroy_handle_cache();
+ jbd2_journal_destroy_inode_cache();
jbd2_journal_destroy_transaction_cache();
jbd2_journal_destroy_slabs();
}
diff --git a/fs/jbd2/revoke.c b/fs/jbd2/revoke.c
index a1143e57a718..69b9bc329964 100644
--- a/fs/jbd2/revoke.c
+++ b/fs/jbd2/revoke.c
@@ -178,33 +178,41 @@ static struct jbd2_revoke_record_s *find_revoke_record(journal_t *journal,
return NULL;
}
-void jbd2_journal_destroy_revoke_caches(void)
+void jbd2_journal_destroy_revoke_record_cache(void)
{
kmem_cache_destroy(jbd2_revoke_record_cache);
jbd2_revoke_record_cache = NULL;
+}
+
+void jbd2_journal_destroy_revoke_table_cache(void)
+{
kmem_cache_destroy(jbd2_revoke_table_cache);
jbd2_revoke_table_cache = NULL;
}
-int __init jbd2_journal_init_revoke_caches(void)
+int __init jbd2_journal_init_revoke_record_cache(void)
{
J_ASSERT(!jbd2_revoke_record_cache);
- J_ASSERT(!jbd2_revoke_table_cache);
-
jbd2_revoke_record_cache = KMEM_CACHE(jbd2_revoke_record_s,
SLAB_HWCACHE_ALIGN|SLAB_TEMPORARY);
- if (!jbd2_revoke_record_cache)
- goto record_cache_failure;
+ if (!jbd2_revoke_record_cache) {
+ pr_emerg("JBD2: failed to create revoke_record cache\n");
+ return -ENOMEM;
+ }
+ return 0;
+}
+
+int __init jbd2_journal_init_revoke_table_cache(void)
+{
+ J_ASSERT(!jbd2_revoke_table_cache);
jbd2_revoke_table_cache = KMEM_CACHE(jbd2_revoke_table_s,
SLAB_TEMPORARY);
- if (!jbd2_revoke_table_cache)
- goto table_cache_failure;
- return 0;
-table_cache_failure:
- jbd2_journal_destroy_revoke_caches();
-record_cache_failure:
+ if (!jbd2_revoke_table_cache) {
+ pr_emerg("JBD2: failed to create revoke_table cache\n");
return -ENOMEM;
+ }
+ return 0;
}
static struct jbd2_revoke_table_s *jbd2_journal_init_revoke_table(int hash_size)
diff --git a/fs/jbd2/transaction.c b/fs/jbd2/transaction.c
index f940d31c2adc..8ca4fddc705f 100644
--- a/fs/jbd2/transaction.c
+++ b/fs/jbd2/transaction.c
@@ -42,9 +42,11 @@ int __init jbd2_journal_init_transaction_cache(void)
0,
SLAB_HWCACHE_ALIGN|SLAB_TEMPORARY,
NULL);
- if (transaction_cache)
- return 0;
- return -ENOMEM;
+ if (!transaction_cache) {
+ pr_emerg("JBD2: failed to create transaction cache\n");
+ return -ENOMEM;
+ }
+ return 0;
}
void jbd2_journal_destroy_transaction_cache(void)
diff --git a/include/linux/jbd2.h b/include/linux/jbd2.h
index c2ffff5f9ae2..6c9870e16b19 100644
--- a/include/linux/jbd2.h
+++ b/include/linux/jbd2.h
@@ -1318,7 +1318,7 @@ extern void __wait_on_journal (journal_t *);
/* Transaction cache support */
extern void jbd2_journal_destroy_transaction_cache(void);
-extern int jbd2_journal_init_transaction_cache(void);
+extern int __init jbd2_journal_init_transaction_cache(void);
extern void jbd2_journal_free_transaction(transaction_t *);
/*
@@ -1446,8 +1446,10 @@ static inline void jbd2_free_inode(struct jbd2_inode *jinode)
/* Primary revoke support */
#define JOURNAL_REVOKE_DEFAULT_HASH 256
extern int jbd2_journal_init_revoke(journal_t *, int);
-extern void jbd2_journal_destroy_revoke_caches(void);
-extern int jbd2_journal_init_revoke_caches(void);
+extern void jbd2_journal_destroy_revoke_record_cache(void);
+extern void jbd2_journal_destroy_revoke_table_cache(void);
+extern int __init jbd2_journal_init_revoke_record_cache(void);
+extern int __init jbd2_journal_init_revoke_table_cache(void);
extern void jbd2_journal_destroy_revoke(journal_t *);
extern int jbd2_journal_revoke (handle_t *, unsigned long long, struct buffer_head *);
--
2.20.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH v2] jbd2: fix potential double free
2019-05-10 10:36 [PATCH v2] jbd2: fix potential double free Chengguang Xu
@ 2019-05-11 0:18 ` Theodore Ts'o
2019-05-11 1:37 ` Theodore Ts'o
1 sibling, 0 replies; 3+ messages in thread
From: Theodore Ts'o @ 2019-05-11 0:18 UTC (permalink / raw)
To: Chengguang Xu; +Cc: jack, linux-ext4, linux-kernel
On Fri, May 10, 2019 at 06:36:48PM +0800, Chengguang Xu wrote:
> When fail from creating cache jbd2_inode_cache, we will
> destroy previously created cache jbd2_handle_cache twice.
> This patch fixes it by sperating each cache
> initialization/destruction to individual function.
>
> Signed-off-by: Chengguang Xu <cgxu519@gmail.com>
Thanks, applied.
- Ted
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH v2] jbd2: fix potential double free
2019-05-10 10:36 [PATCH v2] jbd2: fix potential double free Chengguang Xu
2019-05-11 0:18 ` Theodore Ts'o
@ 2019-05-11 1:37 ` Theodore Ts'o
1 sibling, 0 replies; 3+ messages in thread
From: Theodore Ts'o @ 2019-05-11 1:37 UTC (permalink / raw)
To: Chengguang Xu; +Cc: jack, linux-ext4, linux-kernel
On Fri, May 10, 2019 at 06:36:48PM +0800, Chengguang Xu wrote:
> diff --git a/fs/jbd2/journal.c b/fs/jbd2/journal.c
> index 37e16d969925..0f1ac43d0560 100644
> --- a/fs/jbd2/journal.c
> +++ b/fs/jbd2/journal.c
> @@ -2299,7 +2299,7 @@ static void jbd2_journal_destroy_slabs(void)
> }
> }
>
> -static int jbd2_journal_create_slab(size_t size)
> +static int __init jbd2_journal_create_slab(size_t size)
> {
> static DEFINE_MUTEX(jbd2_slab_create_mutex);
> int i = order_base_2(size) - 10;
I had to remove this hunk. It's incorrect, since jbd2_journal_load
calls this function when mounting file systems. So it can't be marked
__init.
- Ted
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2019-05-11 17:41 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-05-10 10:36 [PATCH v2] jbd2: fix potential double free Chengguang Xu
2019-05-11 0:18 ` Theodore Ts'o
2019-05-11 1:37 ` Theodore Ts'o
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).