Re: [PATCH 3/4] mdadm: Avoid array bounds check of gcc

[Xiao Ni <xni@redhat.com>]

On Thu, Sep 28, 2023 at 5:42 PM Mariusz Tkaczyk
<mariusz.tkaczyk@linux.intel.com> wrote:
>
> On Wed, 27 Sep 2023 10:52:18 +0800
> Xiao Ni <xni@redhat.com> wrote:
>
> > With gcc version 13.2.1 20230918 (Red Hat 13.2.1-3) (GCC), it reports error:
> > super-ddf.c:1988:58: error: array subscript -1 is below array bounds of
> > ‘struct phys_disk_entry[0]’ [-Werror=array-bounds=]
> > The subscrit is defined as int type. And it can be smaller than 0.
>
> If it can be smaller that 0 then it is something we need to fix.
> I think that it comes from here:
>                 info->disk.raid_disk = find_phys(ddf, ddf->dlist->disk.refnum);
>                 info->data_offset = be64_to_cpu(ddf->phys->
>                                                   entries[info->disk.raid_disk].
>                                                   config_size);
>
> find_phys can return -1.
> It is handled few lines bellow. I don't see reason why we cannot handle it here
> too.
>
>                 if (info->disk.raid_disk >= 0)
>                         pde = ddf->phys->entries + info->disk.raid_disk;
>
> I think that it will be fair to abort because metadata seems to be corrupted.
> We are referring to info->disk.raid_disk from many places. We cannot return
> error because it is void, we can just return.

Hi Mariusz

You mean something like this?

diff --git a/super-ddf.c b/super-ddf.c
index 7213284e0a59..b6e514042055 100644
--- a/super-ddf.c
+++ b/super-ddf.c
@@ -1984,6 +1984,9 @@ static void getinfo_super_ddf(struct supertype
*st, struct mdinfo *info, char *m
                info->disk.number = be32_to_cpu(ddf->dlist->disk.refnum);
                info->disk.raid_disk = find_phys(ddf, ddf->dlist->disk.refnum);

+               if (info->disk.raid_disk < 0)
+                       return;
+
                info->data_offset = be64_to_cpu(ddf->phys->
                                                  entries[info->disk.raid_disk].
                                                  config_size);

>
> > To avoid this error, add -Wno-array-bounds flag in Makefile.
>
> If you want do it this way please provide strong justification. We are
> disabling check in all code to hide particular case. It will not prevent us
> from similar mistakes during development in the future.

As Paul and you suggested, I'll not choose this way

Regards
Xiao
>
> Thanks,
> Mariusz
>