[External Mail]Re: [PATCH] erofs-utils: avoid allocating large arrays on the stack

[Huang Jianan]

On 2024/10/25 0:48, Sandeep Dhavale wrote:
> 
> Hi Jianan,
> 
> On Thu, Oct 24, 2024 at 2:49 AM Jianan Huang via Linux-erofs
> <linux-erofs at lists.ozlabs.org> wrote:
>>
>> The default pthread stack size of bionic is 1M. Use malloc to avoid
>> stack overflow.
>>
>> Signed-off-by: Jianan Huang <huangjianan at xiaomi.com>
>> ---
>>   lib/compress.c | 31 +++++++++++++++++++++----------
>>   1 file changed, 21 insertions(+), 10 deletions(-)
>>
>> diff --git a/lib/compress.c b/lib/compress.c
>> index cbd4620..47ba662 100644
>> --- a/lib/compress.c
>> +++ b/lib/compress.c
>> @@ -451,31 +451,37 @@ static int z_erofs_fill_inline_data(struct erofs_inode *inode, void *data,
>>          return len;
>>   }
>>
>> -static void tryrecompress_trailing(struct z_erofs_compress_sctx *ctx,
>> -                                  struct erofs_compress *ec,
>> -                                  void *in, unsigned int *insize,
>> -                                  void *out, unsigned int *compressedsize)
>> +static int tryrecompress_trailing(struct z_erofs_compress_sctx *ctx,
>> +                                 struct erofs_compress *ec,
>> +                                 void *in, unsigned int *insize,
>> +                                 void *out, unsigned int *compressedsize)
>>   {
>>          struct erofs_sb_info *sbi = ctx->ictx->inode->sbi;
>> -       char tmp[Z_EROFS_PCLUSTER_MAX_SIZE];
>> +       char *tmp;
>>          unsigned int count;
>>          int ret = *compressedsize;
>>
>> +       tmp = malloc(Z_EROFS_PCLUSTER_MAX_SIZE);
>> +       if (!tmp)
>> +               return -ENOMEM;
>> +
>>          /* no need to recompress */
>>          if (!(ret & (erofs_blksiz(sbi) - 1)))
>> -               return;
>> +               return 0;
>>
> You can move allocation here instead of at the top to avoid malloc
> cost if we do not need the tmp.
> 
> Also need to free tmp on all the exit paths.

Thanks for your review, I will send out v2 to fix these issues.

Thanks,
Jianan.

> 
> Thanks,
> Sandeep.
> 
>>          count = *insize;
>>          ret = erofs_compress_destsize(ec, in, &count, (void *)tmp,
>>                                        rounddown(ret, erofs_blksiz(sbi)));
>>          if (ret <= 0 || ret + (*insize - count) >=
>>                          roundup(*compressedsize, erofs_blksiz(sbi)))
>> -               return;
>> +               return 0;
>>
>>          /* replace the original compressed data if any gain */
>>          memcpy(out, tmp, ret);
>>          *insize = count;
>>          *compressedsize = ret;
>> +
>> +       return 0;
>>   }
>>
>>   static bool z_erofs_fixup_deduped_fragment(struct z_erofs_compress_sctx *ctx,
>> @@ -631,9 +637,14 @@ frag_packing:
>>                          goto fix_dedupedfrag;
>>                  }
>>
>> -               if (may_inline && len == e->length)
>> -                       tryrecompress_trailing(ctx, h, ctx->queue + ctx->head,
>> -                                       &e->length, dst, &compressedsize);
>> +               if (may_inline && len == e->length) {
>> +                       ret = tryrecompress_trailing(ctx, h,
>> +                                                    ctx->queue + ctx->head,
>> +                                                    &e->length, dst,
>> +                                                    &compressedsize);
>> +                       if (ret)
>> +                               return ret;
>> +               }
>>
>>                  e->compressedblks = BLK_ROUND_UP(sbi, compressedsize);
>>                  DBG_BUGON(e->compressedblks * blksz >= e->length);
>> --
>> 2.43.0
>>