linux-kernel - Re: [PATCH] [media] s3c-camif: fix out-of-bounds array access

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-id: <ce741db2-dc57-7d23-90bc-319411006861@samsung.com>
Date:   Wed, 13 Sep 2017 17:54:45 +0200
From:   Sylwester Nawrocki <s.nawrocki@...sung.com>
To:     Arnd Bergmann <arnd@...db.de>
Cc:     Sylwester Nawrocki <snawrocki@...nel.org>,
        Mauro Carvalho Chehab <mchehab@...nel.org>,
        Linux Media Mailing List <linux-media@...r.kernel.org>,
        "moderated list:ARM/SAMSUNG EXYNOS ARM ARCHITECTURES" 
        <linux-samsung-soc@...r.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] [media] s3c-camif: fix out-of-bounds array access

On 09/13/2017 04:03 PM, Arnd Bergmann wrote:
> On Wed, Sep 13, 2017 at 11:25 AM, Sylwester Nawrocki
> <s.nawrocki@...sung.com>  wrote:
>> On 09/12/2017 10:09 PM, Arnd Bergmann wrote:
>>>    {
>>>        const struct s3c_camif_variant *variant = camif->variant;
>>>        const struct vp_pix_limits *pix_lim;
>>> -     int i = ARRAY_SIZE(camif_mbus_formats);
>>>
>>>        /* FIXME: constraints against codec or preview path ? */
>>>        pix_lim = &variant->vp_pix_limits[VP_CODEC];
>>>
>>> -     while (i-- >= 0)
>>> -             if (camif_mbus_formats[i] == mf->code)
>>> -                     break;
>>> -
>>> -     mf->code = camif_mbus_formats[i];
>>
>> Interesting finding... the function needs to ensure mf->code is set
>> to one of supported values by the driver, so instead of removing
>> how about changing the above line to:
>>
>>          if (i < 0)
>>                  mf->code = camif_mbus_formats[0];
>>
>> ?
> That would still have one of the two out-of-bounds accesses;-)

Ah, indeed :/

> maybe this
> 
> for (i = 0; i < ARRAY_SIZE(camif_mbus_formats); i++)
>          if (camif_mbus_formats[i] == mf->code)
>                 break;
> 
> if (i == ARRAY_SIZE(camif_mbus_formats))
>         mf->code = camif_mbus_formats[0];

Yes, it should work that way.

-- 
Thanks,
Sylwester