linux-kernel - Re: [PATCH] [media] davinci/dm644x: work around ccdc_update_raw

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] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 20 Jun 2017 14:06:01 +0100
From:   "Lad, Prabhakar" <prabhakar.csengg@...il.com>
To:     Sekhar Nori <nsekhar@...com>
Cc:     Mauro Carvalho Chehab <mchehab@...nel.org>,
        Kevin Hilman <khilman@...nel.org>,
        linux-media <linux-media@...r.kernel.org>,
        LKML <linux-kernel@...r.kernel.org>,
        Arnd Bergmann <arnd@...db.de>
Subject: Re: [PATCH] [media] davinci/dm644x: work around ccdc_update_raw_params
 trainwreck

Hi Arnd,

Thanks for the patch.

On Fri, Jun 9, 2017 at 10:36 PM, Arnd Bergmann <arnd@...db.de> wrote:
> Now that the davinci drivers can be enabled in compile tests on other
> architectures, I ran into this warning on a 64-bit build:
>
> drivers/media/platform/davinci/dm644x_ccdc.c: In function 'ccdc_update_raw_params':
> drivers/media/platform/davinci/dm644x_ccdc.c:279:7: error: cast to pointer from integer of different size [-Werror=int-to-pointer-cast]
>
> While that looks fairly harmless (it would be fine on 32-bit), it was
> just the tip of the iceberg:
>
> - The function constantly mixes up pointers and phys_addr_t numbers
> - This is part of a 'VPFE_CMD_S_CCDC_RAW_PARAMS' ioctl command that is
>   described as an 'experimental ioctl that will change in future kernels',
>   but if we have users that probably won't happen.
> - The code to allocate the table never gets called after we copy_from_user
>   the user input over the kernel settings, and then compare them
>   for inequality.
> - We then go on to use an address provided by user space as both the
>   __user pointer for input and pass it through phys_to_virt to come up
>   with a kernel pointer to copy the data to. This looks like a trivially
>   exploitable root hole.
>
> This patch disables all the obviously broken code, by zeroing out the
> sensitive data provided by user space. I also fix the type confusion
> here. If we think the ioctl has no stable users, we could consider
> just removing it instead.
>
I suspect there shouldn’t  be possible users of this IOCTL, better of  removing
the IOCTL itself.

Sekhar your call, as the latest PSP releases for 644x use the media
controller framework.

Cheers,
--Prabhakar Lad