linux-kernel - Re: [PATCH 2/2] fbdev: Make fb_release() return -ENODEV if fbdev was unregistered

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:   Mon, 2 May 2022 15:39:13 +0200
From:   Javier Martinez Canillas <javierm@...hat.com>
To:     Thomas Zimmermann <tzimmermann@...e.de>,
        linux-kernel@...r.kernel.org
Cc:     linux-fbdev@...r.kernel.org, Helge Deller <deller@....de>,
        Zheyu Ma <zheyuma97@...il.com>,
        Changcheng Deng <deng.changcheng@....com.cn>,
        dri-devel@...ts.freedesktop.org, Maxime Ripard <maxime@...no.tech>,
        Zhen Lei <thunder.leizhen@...wei.com>,
        Alex Deucher <alexander.deucher@....com>,
        Sam Ravnborg <sam@...nborg.org>,
        Guenter Roeck <linux@...ck-us.net>
Subject: Re: [PATCH 2/2] fbdev: Make fb_release() return -ENODEV if fbdev was
 unregistered

Hello Thomas,

On 5/2/22 15:20, Thomas Zimmermann wrote:
> 
> 
> Am 02.05.22 um 15:09 schrieb Javier Martinez Canillas:
>> A reference to the framebuffer device struct fb_info is stored in the file
>> private data, but this reference could no longer be valid and must not be
>> accessed directly. Instead, the file_fb_info() accessor function must be
>> used since it does sanity checking to make sure that the fb_info is valid.
>>
>> This can happen for example if the fbdev driver was one that is using a
>> framebuffer provided by the system firmware. In that case, the fbdev core
>> could unregister the framebuffer device if a real video driver is probed.
>>
>> Reported-by: Maxime Ripard <maxime@...no.tech>
>> Signed-off-by: Javier Martinez Canillas <javierm@...hat.com>
> 
> Reviewed-by: Thomas Zimmermann <tzimmermann@...e.de>
>

Thanks.
 
> This seems like the correct thing to do in any case. Thanks for the 

Agreed, it's certainly a bug if not the same that was already reported.

> patch. Before merging, you should also add
> 
> Fixes: 27599aacbaef ("fbdev: Hot-unplug firmware fb devices on forced 
> removal")

I thought about that but I don't think that's accurate since the bug is
not related to that commit. That might make easier to reproduce it but
is something that would happen anyway if for example someone attempted
to remove a module or unbind the device using the sysfs entries.

Maybe I can comment in the commit message that this change made it more
likely to occur and for that reason I'm adding a fixes tag.

> Reported-by: Junxiao Chang <junxiao.chang@...el.com>
>

Indeed.
 -- 
Best regards,

Javier Martinez Canillas
Linux Engineering
Red Hat