All of lore.kernel.org
 help / color / mirror / Atom feed
* [Ocfs2-devel] [PATCH v2] ocfs2: fix warning 'ocfs2_orphan_del' uses dynamic stack allocation
@ 2015-01-26  3:20 Joseph Qi
  2015-01-26 22:38 ` Andrew Morton
  0 siblings, 1 reply; 3+ messages in thread
From: Joseph Qi @ 2015-01-26  3:20 UTC (permalink / raw)
  To: ocfs2-devel

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 <joseph.qi@huawei.com>
Reviewed-by: Xuejiufei <xuejiufei@huawei.com>
Reviewed-by: alex chen <alex.chen@huawei.com>
---
 fs/ocfs2/namei.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/fs/ocfs2/namei.c b/fs/ocfs2/namei.c
index 873b40a..5fe3af9 100644
--- a/fs/ocfs2/namei.c
+++ b/fs/ocfs2/namei.c
@@ -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)
-- 
1.8.4.3

^ permalink raw reply related	[flat|nested] 3+ messages in thread

* [Ocfs2-devel] [PATCH v2] ocfs2: fix warning 'ocfs2_orphan_del' uses dynamic stack allocation
  2015-01-26  3:20 [Ocfs2-devel] [PATCH v2] ocfs2: fix warning 'ocfs2_orphan_del' uses dynamic stack allocation Joseph Qi
@ 2015-01-26 22:38 ` Andrew Morton
  2015-03-11  6:16   ` Joseph Qi
  0 siblings, 1 reply; 3+ messages in thread
From: Andrew Morton @ 2015-01-26 22:38 UTC (permalink / raw)
  To: ocfs2-devel

On Mon, 26 Jan 2015 11:20:47 +0800 Joseph Qi <joseph.qi@huawei.com> wrote:

> In ocfs2_orphan_del it uses dynamic stack allocation for orphan entry
> name. Fix it by using dynamic heap allocation.
> 
> ...
>
> --- a/fs/ocfs2/namei.c
> +++ b/fs/ocfs2/namei.c
> @@ -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)

I think I prefer my fix:

--- a/fs/ocfs2/namei.c~ocfs2-add-functions-to-add-and-remove-inode-in-orphan-dir-fix
+++ a/fs/ocfs2/namei.c
@@ -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.

^ permalink raw reply	[flat|nested] 3+ messages in thread

* [Ocfs2-devel] [PATCH v2] ocfs2: fix warning 'ocfs2_orphan_del' uses dynamic stack allocation
  2015-01-26 22:38 ` Andrew Morton
@ 2015-03-11  6:16   ` Joseph Qi
  0 siblings, 0 replies; 3+ messages in thread
From: Joseph Qi @ 2015-03-11  6:16 UTC (permalink / raw)
  To: ocfs2-devel

Hi Andrew,

On 2015/1/27 6:38, Andrew Morton wrote:
> On Mon, 26 Jan 2015 11:20:47 +0800 Joseph Qi <joseph.qi@huawei.com> wrote:
> 
>> In ocfs2_orphan_del it uses dynamic stack allocation for orphan entry
>> name. Fix it by using dynamic heap allocation.
>>
>> ...
>>
>> --- a/fs/ocfs2/namei.c
>> +++ b/fs/ocfs2/namei.c
>> @@ -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)
> 
> I think I prefer my fix:
> 
> --- a/fs/ocfs2/namei.c~ocfs2-add-functions-to-add-and-remove-inode-in-orphan-dir-fix
> +++ a/fs/ocfs2/namei.c
> @@ -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.
> 
> .
> 
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.

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2015-03-11  6:16 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-01-26  3:20 [Ocfs2-devel] [PATCH v2] ocfs2: fix warning 'ocfs2_orphan_del' uses dynamic stack allocation Joseph Qi
2015-01-26 22:38 ` Andrew Morton
2015-03-11  6:16   ` Joseph Qi

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.