[PATCH 1/1] ext4: better error handling of kstrdup()

[PATCH 1/1] ext4: better error handling of kstrdup()

[Taesoo Kim]

Upon memory pressure, kstrdup() might fail and correctly
handle memory error, although current implementation do not
refer orig_data.

NOTE. fortunately the correct impl works, other than a
corner case where kstrdup() fails and kzalloc() succeeds;
it might record 'NULL' in the log.

Signed-off-by: Taesoo Kim <tsgatesv@gmail.com>
---
 fs/ext4/super.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index e061e66..e2a609c 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -3408,7 +3408,7 @@ static int ext4_reserve_clusters(struct ext4_sb_info *sbi, ext4_fsblk_t count)

 static int ext4_fill_super(struct super_block *sb, void *data, int silent)
 {
-	char *orig_data = kstrdup(data, GFP_KERNEL);
+	char *orig_data;
 	struct buffer_head *bh;
 	struct ext4_super_block *es = NULL;
 	struct ext4_sb_info *sbi;
@@ -3431,6 +3431,10 @@ static int ext4_fill_super(struct super_block *sb, void *data, int silent)
 	unsigned int journal_ioprio = DEFAULT_JOURNAL_IOPRIO;
 	ext4_group_t first_not_zeroed;

+	orig_data = kstrdup(data, GFP_KERNEL);
+	if (!orig_data)
+		return -ENOMEM;
+
 	sbi = kzalloc(sizeof(*sbi), GFP_KERNEL);
 	if (!sbi)
 		goto out_free_orig;
@@ -4843,6 +4847,8 @@ static int ext4_remount(struct super_block *sb, int *flags, char *data)
 	int i, j;
 #endif
 	char *orig_data = kstrdup(data, GFP_KERNEL);
+	if (!orig_data)
+		return -ENOMEM;

 	/* Store the original options */
 	old_sb_flags = sb->s_flags;
--
2.3.3

Re: [PATCH 1/1] ext4: better error handling of kstrdup()

[Theodore Ts'o]

On Fri, Mar 20, 2015 at 05:21:54PM -0400, Taesoo Kim wrote:
> Upon memory pressure, kstrdup() might fail and correctly
> handle memory error, although current implementation do not
> refer orig_data.
> 
> NOTE. fortunately the correct impl works, other than a
> corner case where kstrdup() fails and kzalloc() succeeds;
> it might record 'NULL' in the log.
> 
> Signed-off-by: Taesoo Kim <tsgatesv@gmail.com>

Applied, thanks.

					- Ted

Re: [PATCH 1/1] ext4: better error handling of kstrdup()

[Theodore Ts'o]

On Fri, Mar 20, 2015 at 05:21:54PM -0400, Taesoo Kim wrote:
> Upon memory pressure, kstrdup() might fail and correctly
> handle memory error, although current implementation do not
> refer orig_data.
> 
> NOTE. fortunately the correct impl works, other than a
> corner case where kstrdup() fails and kzalloc() succeeds;
> it might record 'NULL' in the log.
> 
> Signed-off-by: Taesoo Kim <tsgatesv@gmail.com>

Did you test this patch?  If there are no mount options (such as when
mounting the root file system, data is NULL, so orig_data is NULL),
and the mount fails.  So a kernel won't boot with this patch applied.

					- Ted

Re: [PATCH 1/1] ext4: better error handling of kstrdup()

[Taesoo Kim]

Hi Ted,

It's my fault. I didn't properly check the case of having data = NULL
to kstrdup() as my system has long mounting options.. Sorry for the
trouble. I think there is no clean way to handle this error (better),
other than implicitly letting kstrdup() fail (and ignore).

Taesoo

On 04/03/15 at 01:09am, Theodore Ts'o wrote:
> On Fri, Mar 20, 2015 at 05:21:54PM -0400, Taesoo Kim wrote:
> > Upon memory pressure, kstrdup() might fail and correctly
> > handle memory error, although current implementation do not
> > refer orig_data.
> > 
> > NOTE. fortunately the correct impl works, other than a
> > corner case where kstrdup() fails and kzalloc() succeeds;
> > it might record 'NULL' in the log.
> > 
> > Signed-off-by: Taesoo Kim <tsgatesv@gmail.com>
> 
> Did you test this patch?  If there are no mount options (such as when
> mounting the root file system, data is NULL, so orig_data is NULL),
> and the mount fails.  So a kernel won't boot with this patch applied.
> 
> 					- Ted