linux-kernel - Re: [PATCH v6 02/10] Revert "drm/qxl: do not run release if qxl failed to init"

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]

Date:   Thu, 4 Feb 2021 15:21:08 -0500
From:   Tong Zhang <ztong0001@...il.com>
To:     Thomas Zimmermann <tzimmermann@...e.de>
Cc:     Gerd Hoffmann <kraxel@...hat.com>, dri-devel@...ts.freedesktop.org,
        David Airlie <airlied@...ux.ie>,
        open list <linux-kernel@...r.kernel.org>,
        "open list:DRM DRIVER FOR QXL VIRTUAL GPU" 
        <virtualization@...ts.linux-foundation.org>,
        "open list:DRM DRIVER FOR QXL VIRTUAL GPU" 
        <spice-devel@...ts.freedesktop.org>,
        Dave Airlie <airlied@...hat.com>
Subject: Re: [PATCH v6 02/10] Revert "drm/qxl: do not run release if qxl
 failed to init"



> On Feb 4, 2021, at 2:06 PM, Thomas Zimmermann <tzimmermann@...e.de> wrote:
> 
> Hi Tong
> 
>> I have posted an updated patch.
>> The new patch use the following logic
>> +	if (!qdev->ddev.mode_config.funcs)
>> +	  return;
> 
> This is again just papering over the issue. Better don't call qxl_drm_release() in the error path if qxl_device_init() fails.
> 
> I see two solutions: either roll-back manually, or use our new managed DRM interfaces. This is what the other drivers do.
> 
> Best regards
> Thomas


IMHO - qdev->ddev.mode_config.funcs is set only if the initialization is successful,
so using this as an indicator of error case looks ok to me.

The other two options you suggested would require some serious significant amount of work to be done,
which I don’t think I currently have such ability to do.

Thanks,
- Tong