linux-kernel - Re: [PATCH v3] drm/virtio: Add option to disable KMS support

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]

Message-ID: <a4bbfe5e-36b1-ed94-f805-42728a8b820d@suse.de>
Date:   Tue, 28 Feb 2023 13:47:18 +0100
From:   Thomas Zimmermann <tzimmermann@...e.de>
To:     Rob Clark <robdclark@...il.com>,
        Dmitry Osipenko <dmitry.osipenko@...labora.com>
Cc:     Rob Clark <robdclark@...omium.org>,
        open list <linux-kernel@...r.kernel.org>,
        dri-devel@...ts.freedesktop.org,
        Gurchetan Singh <gurchetansingh@...omium.org>,
        Gerd Hoffmann <kraxel@...hat.com>,
        Ryan Neph <ryanneph@...omium.org>,
        David Airlie <airlied@...hat.com>,
        "open list:VIRTIO GPU DRIVER" 
        <virtualization@...ts.linux-foundation.org>
Subject: Re: [PATCH v3] drm/virtio: Add option to disable KMS support



Am 28.02.23 um 13:34 schrieb Thomas Zimmermann:
> Hi
> 
> Am 27.02.23 um 19:15 schrieb Rob Clark:
>> On Mon, Feb 27, 2023 at 9:57 AM Dmitry Osipenko
>> <dmitry.osipenko@...labora.com> wrote:
>>>
>>> On 2/27/23 20:38, Rob Clark wrote:
>>> ...
>>>> +     if (IS_ENABLED(CONFIG_DRM_VIRTIO_GPU_KMS)) {
>>>> +             /* get display info */
>>>> +             virtio_cread_le(vgdev->vdev, struct virtio_gpu_config,
>>>> +                             num_scanouts, &num_scanouts);
>>>> +             vgdev->num_scanouts = min_t(uint32_t, num_scanouts,
>>>> +                                         VIRTIO_GPU_MAX_SCANOUTS);
>>>> +             if (!vgdev->num_scanouts) {
>>>> +                     /*
>>>> +                      * Having an EDID but no scanouts is 
>>>> non-sensical,
>>>> +                      * but it is permitted to have no scanouts and no
>>>> +                      * EDID (in which case DRIVER_MODESET and
>>>> +                      * DRIVER_ATOMIC are not advertised)
>>>> +                      */
>>>> +                     if (vgdev->has_edid) {
>>>> +                             DRM_ERROR("num_scanouts is zero\n");
>>>> +                             ret = -EINVAL;
>>>> +                             goto err_scanouts;
>>>> +                     }
>>>> +                     dev->driver_features &= ~(DRIVER_MODESET | 
>>>> DRIVER_ATOMIC);
>>>
>>> If it's now configurable by host, why do we need the
>>> CONFIG_DRM_VIRTIO_GPU_KMS?
>>
>> Because a kernel config option makes it more obvious that
>> modeset/atomic ioctls are blocked.  Which makes it more obvious about
>> where any potential security issues apply and where fixes need to get
>> backported to.  The config option is the only thing _I_ want,
>> everything else is just a bonus to help other people's use-cases.
> 
> I find this very vague. What's the security thread?
> 
> And if the config option is useful, shouldn't it be DRM-wide? The 
> modesetting ioctl calls are shared among all drivers.

For reference, here's an older discussion about render-only devices.

https://lore.kernel.org/dri-devel/20221011110437.15258-1-christian.koenig@amd.com/

> 
> Best regards
> Thomas
> 
>>
>> BR,
>> -R
> 

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Ivo Totev

Download attachment "OpenPGP_signature" of type "application/pgp-signature" (841 bytes)