Skip to contentSkip to conversation

ocfs2: fix warning 'ocfs2_orphan_del' uses dynamic stack allocation

PATCH Superseded Joseph Qi 2 participants 3 messages Jan 26, 2015 – Mar 11, 2015 OCFS2

lore
Share & tools
lore

Joseph Qi

author
#0 lore raw

PATCH v2 1/1 ocfs2: fix warning 'ocfs2_orphan_del' uses dynamic stack allocation

In ocfs2_orphan_del it uses dynamic stack allocation for orphan entry name. Fix it by using dynamic heap allocation.

Signed-off-by Joseph Qi <[email protected]>
Reviewed-by Xuejiufei <[email protected]>
Reviewed-by alex chen <[email protected]>
---
 fs/ocfs2/namei.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)
fs/ocfs2/namei.c +7 -2
index 873b40a..5fe3af9 100644
@@ -2298,18 +2298,22 @@ int ocfs2_orphan_del(struct ocfs2_super *osb,
 {
 	int namelen = dio ? OCFS2_DIO_ORPHAN_PREFIX_LEN + OCFS2_ORPHAN_NAMELEN :
 			OCFS2_ORPHAN_NAMELEN;
-	char name[namelen + 1];
+	char *name;
 	struct ocfs2_dinode *orphan_fe;
 	int status = 0;
 	struct ocfs2_dir_lookup_result lookup = { NULL, };
 
+	name = kmalloc(namelen + 1, GFP_NOFS);
+	if (!name)
+		goto leave;
+
 	if (dio) {
 		status = snprintf(name, OCFS2_DIO_ORPHAN_PREFIX_LEN + 1, "%s",
 				OCFS2_DIO_ORPHAN_PREFIX);
 		if (status != OCFS2_DIO_ORPHAN_PREFIX_LEN) {
 			status = -EINVAL;
 			mlog_errno(status);
-			return status;
+			goto leave;
 		}
 
 		status = ocfs2_blkno_stringify(OCFS2_I(inode)->ip_blkno,

@@ -2357,6 +2361,7 @@ int ocfs2_orphan_del(struct ocfs2_super *osb,
 	ocfs2_journal_dirty(handle, orphan_dir_bh);
 
 leave:
+	kfree(name);
 	ocfs2_free_dir_lookup_result(&lookup);
 
 	if (status)
Signature · 1 line
1.8.4.3
#1 lore raw

PATCH v2 1/1 ocfs2: fix warning 'ocfs2_orphan_del' uses dynamic stack allocation

Quoted 33 lines · On Mon, 26 Jan 2015 11:20:47 +0800 Joseph Qi <[email protected]> wrote:

In ocfs2_orphan_del it uses dynamic stack allocation for orphan entry name. Fix it by using dynamic heap allocation.

...

fs/ocfs2/namei.c +6 -2
@@ -2298,18 +2298,22 @@ int ocfs2_orphan_del(struct ocfs2_super *osb,
 {
 	int namelen = dio ? OCFS2_DIO_ORPHAN_PREFIX_LEN + OCFS2_ORPHAN_NAMELEN :
 			OCFS2_ORPHAN_NAMELEN;
-	char name[namelen + 1];
+	char *name;
 	struct ocfs2_dinode *orphan_fe;
 	int status = 0;
 	struct ocfs2_dir_lookup_result lookup = { NULL, };
 
+	name = kmalloc(namelen + 1, GFP_NOFS);
+	if (!name)
+		goto leave;
+
 	if (dio) {
 		status = snprintf(name, OCFS2_DIO_ORPHAN_PREFIX_LEN + 1, "%s",
 				OCFS2_DIO_ORPHAN_PREFIX);
 		if (status != OCFS2_DIO_ORPHAN_PREFIX_LEN) {
 			status = -EINVAL;
 			mlog_errno(status);
-			return status;
+			goto leave;
 		}
 
 		status = ocfs2_blkno_stringify(OCFS2_I(inode)->ip_blkno,

fs/ocfs2/namei.c int ocfs2_orphan_del(struct ocfs2_super *osb, · line 2367

Quoted 7 lines
fs/ocfs2/namei.c +1 -0
@@ -2357,6 +2361,7 @@ int ocfs2_orphan_del(struct ocfs2_super *osb,
 	ocfs2_journal_dirty(handle, orphan_dir_bh);
 
 leave:
+	kfree(name);
 	ocfs2_free_dir_lookup_result(&lookup);
 
 	if (status)

I think I prefer my fix:

fs/ocfs2/namei.c renamed from fs/ocfs2/namei.c~ocfs2-add-functions-to-add-and-remove-inode-in-orphan-dir-fix +1 -2
@@ -2296,8 +2296,7 @@ int ocfs2_orphan_del(struct ocfs2_super
 		     struct buffer_head *orphan_dir_bh,
 		     bool dio)
 {
-	int namelen = dio ? OCFS2_DIO_ORPHAN_PREFIX_LEN + OCFS2_ORPHAN_NAMELEN :
-			OCFS2_ORPHAN_NAMELEN;
+	const int namelen = OCFS2_DIO_ORPHAN_PREFIX_LEN + OCFS2_ORPHAN_NAMELEN;
 	char name[namelen + 1];
 	struct ocfs2_dinode *orphan_fe;
 	int status = 0;

It means we use 20 bytes of stack all the time, instead of sometimes-20, sometimes-16.

author
#2 lore raw

PATCH v2 1/1 ocfs2: fix warning 'ocfs2_orphan_del' uses dynamic stack allocation

Hi Andrew,

Quoted 58 lines · Andrew Morton
Quoted 33 lines · On Mon, 26 Jan 2015 11:20:47 +0800 Joseph Qi <[email protected]> wrote:

In ocfs2_orphan_del it uses dynamic stack allocation for orphan entry name. Fix it by using dynamic heap allocation.

...

fs/ocfs2/namei.c +6 -2
@@ -2298,18 +2298,22 @@ int ocfs2_orphan_del(struct ocfs2_super *osb,
 {
 	int namelen = dio ? OCFS2_DIO_ORPHAN_PREFIX_LEN + OCFS2_ORPHAN_NAMELEN :
 			OCFS2_ORPHAN_NAMELEN;
-	char name[namelen + 1];
+	char *name;
 	struct ocfs2_dinode *orphan_fe;
 	int status = 0;
 	struct ocfs2_dir_lookup_result lookup = { NULL, };
 
+	name = kmalloc(namelen + 1, GFP_NOFS);
+	if (!name)
+		goto leave;
+
 	if (dio) {
 		status = snprintf(name, OCFS2_DIO_ORPHAN_PREFIX_LEN + 1, "%s",
 				OCFS2_DIO_ORPHAN_PREFIX);
 		if (status != OCFS2_DIO_ORPHAN_PREFIX_LEN) {
 			status = -EINVAL;
 			mlog_errno(status);
-			return status;
+			goto leave;
 		}
 
 		status = ocfs2_blkno_stringify(OCFS2_I(inode)->ip_blkno,

fs/ocfs2/namei.c int ocfs2_orphan_del(struct ocfs2_super *osb, · line 2367

Quoted 7 lines
fs/ocfs2/namei.c +1 -0
@@ -2357,6 +2361,7 @@ int ocfs2_orphan_del(struct ocfs2_super *osb,
 	ocfs2_journal_dirty(handle, orphan_dir_bh);
 
 leave:
+	kfree(name);
 	ocfs2_free_dir_lookup_result(&lookup);
 
 	if (status)

I think I prefer my fix:

fs/ocfs2/namei.c renamed from fs/ocfs2/namei.c~ocfs2-add-functions-to-add-and-remove-inode-in-orphan-dir-fix +1 -2
@@ -2296,8 +2296,7 @@ int ocfs2_orphan_del(struct ocfs2_super
 		     struct buffer_head *orphan_dir_bh,
 		     bool dio)
 {
-	int namelen = dio ? OCFS2_DIO_ORPHAN_PREFIX_LEN + OCFS2_ORPHAN_NAMELEN :
-			OCFS2_ORPHAN_NAMELEN;
+	const int namelen = OCFS2_DIO_ORPHAN_PREFIX_LEN + OCFS2_ORPHAN_NAMELEN;
 	char name[namelen + 1];
 	struct ocfs2_dinode *orphan_fe;
 	int status = 0;
Quoted 5 lines

It means we use 20 bytes of stack all the time, instead of sometimes-20, sometimes-16.

.

If so, the namelen being passed to ocfs2_find_entry should be the actual name length. Otherwise, it will fail because of mismatch. I'll send a patch to fix this.