Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <da8874d4-66f1-d14e-c0ef-c3557e189cf4@redhat.com>
Date:   Wed, 4 May 2022 13:35:37 +0200
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101
 Thunderbird/91.8.0
Subject: Re: [PATCH v2] fbdev: Use helper to get fb_info in all file
 operations
Content-Language: en-US
To:     Thomas Zimmermann <tzimmermann@suse.de>,
        linux-kernel@vger.kernel.org, linux-fbdev@vger.kernel.org,
        Junxiao Chang <junxiao.chang@intel.com>,
        dri-devel@lists.freedesktop.org, Maxime Ripard <maxime@cerno.tech>,
        Sam Ravnborg <sam@ravnborg.org>
References: <20220503201934.681276-1-javierm@redhat.com>
 <YnJBGpvlViLV+0/a@phenom.ffwll.local>
 <038f8365-b23b-9d81-f7b2-8f8c6eb3a065@redhat.com>
 <YnJbvb5TlHs4ckPM@phenom.ffwll.local>
 <d47a3cab-4f21-3b8b-2834-030663677070@suse.de>
From:   Javier Martinez Canillas <javierm@redhat.com>
In-Reply-To: <d47a3cab-4f21-3b8b-2834-030663677070@suse.de>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
Precedence: bulk

Hello Thomas,

On 5/4/22 13:08, Thomas Zimmermann wrote:

[snip]

>>> So something similar to fb_file_fb_info() is needed to check if
>>> the vm_private_data is still valid. I guess that could be done
>>> by using the vmf->vma->vm_file and attempting the same trick that
>>> fb_file_fb_info() does ?
>>
>> Yeah should work, except if the ptes are set up already there's kinda not
>> much that this will prevent. We'd need to tear down mappings and SIGBUS or
>> alternatively have something else in place there so userspace doesn't blow
>> up in funny ways (which is what we're doing on the drm side, or at least
>> trying to).
>>
>> I'm also not sure how much we should care, since ideally for drm drivers
>> this is all taken care of by drm_dev_enter in the right places. It does
>> mean though that fbdev mmap either needs to have it's own memory or be
>> fully redirected to the drm gem mmap.
>>
>> And then we can afford to just not care to fix fbdev itself.
> 
> While the problem has been there ever since, the bug didn't happen until 
> we fixed hot-unplugging for fbdev. Not doing anything is probably not 
> the right thing.
>

Actually, this issue shouldn't happen if the fbdev drivers are not buggy
and do the proper cleanup at .fb_release() time rather than at .remove().

I'll post patches for simplefb and efifb which are the drivers that we
mostly care at this point. So we should be good and not need more fixes.

-- 
Best regards,

Javier Martinez Canillas
Linux Engineering
Red Hat