[PATCH v2] erofs: Prevent entering an infinite loop when i is 0

[Gao Xiang]

Hi Sandeep,

On 2024/8/31 05:46, Sandeep Dhavale wrote:
> Hi Liujinbao,
> On Thu, Aug 29, 2024 at 5:24 AM liujinbao1 <jinbaoliu365 at gmail.com> wrote:
>>
>> From: liujinbao1 <liujinbao1 at xiaomi.com>
>>
>> When i=0 and err is not equal to 0,
>> the while(-1) loop will enter into an
>> infinite loop. This patch avoids this issue
>>
>> Signed-off-by: liujinbao1 <liujinbao1 at xiaomi.com>
>> ---
>>   fs/erofs/decompressor.c | 10 +++++-----
>>   1 file changed, 5 insertions(+), 5 deletions(-)
>>
>> diff --git a/fs/erofs/decompressor.c b/fs/erofs/decompressor.c
>> index c2253b6a5416..672f097966fa 100644
>> --- a/fs/erofs/decompressor.c
>> +++ b/fs/erofs/decompressor.c
>> @@ -534,18 +534,18 @@ int z_erofs_parse_cfgs(struct super_block *sb, struct erofs_super_block *dsb)
>>
>>   int __init z_erofs_init_decompressor(void)
>>   {
>> -       int i, err;
>> +       int i, err = 0;
>>
>>          for (i = 0; i < Z_EROFS_COMPRESSION_MAX; ++i) {
>>                  err = z_erofs_decomp[i] ? z_erofs_decomp[i]->init() : 0;
>> -               if (err) {
>> -                       while (--i)
>> +               if (err && i) {
>> +                       while (i--)
> Actually there is a subtle bug in this fix. We will never enter the if
> block here when i=0 and err is set which we were trying to fix.
> This will cause z_erofs_decomp[0]->init() error to get masked and we
> will continue the outer for loop (i.e. when i=0 and err is set).

Thanks for catching this!

Yes, that needs to be resolved, and I will replace the
patch to the new version.

Thanks,
Gao Xiang

> 
> Yes original code had the bug but probably simpler and readable fix could be
> 
> diff --git a/fs/erofs/decompressor.c b/fs/erofs/decompressor.c
> index c2253b6a5416..abf2db2ba10c 100644
> --- a/fs/erofs/decompressor.c
> +++ b/fs/erofs/decompressor.c
> @@ -539,7 +539,7 @@ int __init z_erofs_init_decompressor(void)
>          for (i = 0; i < Z_EROFS_COMPRESSION_MAX; ++i) {
>                  err = z_erofs_decomp[i] ? z_erofs_decomp[i]->init() : 0;
>                  if (err) {
> -                       while (--i)
> +                       while (i-- > 0)
>                                  if (z_erofs_decomp[i])
>                                          z_erofs_decomp[i]->exit();
>                          return err;
> 
> Let me know if you want me to send a fix with your Reported-by or you
> can send quick v3.
> 
> Thanks,
> Sandeep.
>