diff mbox

drm: Fix uabi regression by allowing garbage mode->type from userspace

Message ID 20180321211246.10152-1-ville.syrjala@linux.intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Ville Syrjälä March 21, 2018, 9:12 p.m. UTC
From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Apparently xf86-video-vmware leaves the mode->type uninitialized
when feeding the mode to the kernel. Thus we have no choice but
to accept the garbage in. We'll just ignore any of the bits we
don't want. The mode type is just a hint anyway, and more
useful for the kernel->userspace direction.

Reported-by: Thomas Hellstrom <thomas@shipmail.org>
CC: Thomas Hellstrom <thomas@shipmail.org>
Cc: Adam Jackson <ajax@redhat.com>
Cc: Alex Deucher <alexander.deucher@amd.com>
Fixes: c6ed6dad5cfb ("drm/uapi: Validate the mode flags/type")
References: https://lists.freedesktop.org/archives/dri-devel/2018-March/170213.html
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/drm_modes.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

Comments

Thomas Hellström (VMware) March 22, 2018, 7:42 a.m. UTC | #1
On 03/21/2018 10:12 PM, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Apparently xf86-video-vmware leaves the mode->type uninitialized
> when feeding the mode to the kernel. Thus we have no choice but
> to accept the garbage in. We'll just ignore any of the bits we
> don't want. The mode type is just a hint anyway, and more
> useful for the kernel->userspace direction.
>
> Reported-by: Thomas Hellstrom <thomas@shipmail.org>
> CC: Thomas Hellstrom <thomas@shipmail.org>
> Cc: Adam Jackson <ajax@redhat.com>
> Cc: Alex Deucher <alexander.deucher@amd.com>
> Fixes: c6ed6dad5cfb ("drm/uapi: Validate the mode flags/type")
> References: https://lists.freedesktop.org/archives/dri-devel/2018-March/170213.html
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>   drivers/gpu/drm/drm_modes.c | 8 +++++++-
>   1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
> index f6b7c0e36a1a..e82b61e08f8c 100644
> --- a/drivers/gpu/drm/drm_modes.c
> +++ b/drivers/gpu/drm/drm_modes.c
> @@ -1611,7 +1611,13 @@ int drm_mode_convert_umode(struct drm_device *dev,
>   	out->vscan = in->vscan;
>   	out->vrefresh = in->vrefresh;
>   	out->flags = in->flags;
> -	out->type = in->type;
> +	/*
> +	 * Old xf86-video-vmware (possibly others too) used to
> +	 * leave 'type' unititialized. Just ignore any bits we
> +	 * don't like. It's a just hint after all, and more
> +	 * useful for the kernel->userspace direction anyway.
> +	 */
> +	out->type = in->type & DRM_MODE_TYPE_ALL;
>   	strncpy(out->name, in->name, DRM_DISPLAY_MODE_LEN);
>   	out->name[DRM_DISPLAY_MODE_LEN-1] = 0;
>   

Tested-by: Thomas Hellstrom <thellstrom@vmware.com>

Thanks,

Thomas
Maarten Lankhorst March 23, 2018, 11:04 a.m. UTC | #2
Op 22-03-18 om 08:42 schreef Thomas Hellstrom:
> On 03/21/2018 10:12 PM, Ville Syrjala wrote:
>> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>>
>> Apparently xf86-video-vmware leaves the mode->type uninitialized
>> when feeding the mode to the kernel. Thus we have no choice but
>> to accept the garbage in. We'll just ignore any of the bits we
>> don't want. The mode type is just a hint anyway, and more
>> useful for the kernel->userspace direction.
>>
>> Reported-by: Thomas Hellstrom <thomas@shipmail.org>
>> CC: Thomas Hellstrom <thomas@shipmail.org>
>> Cc: Adam Jackson <ajax@redhat.com>
>> Cc: Alex Deucher <alexander.deucher@amd.com>
>> Fixes: c6ed6dad5cfb ("drm/uapi: Validate the mode flags/type")
>> References: https://lists.freedesktop.org/archives/dri-devel/2018-March/170213.html
>> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>> ---
>>   drivers/gpu/drm/drm_modes.c | 8 +++++++-
>>   1 file changed, 7 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
>> index f6b7c0e36a1a..e82b61e08f8c 100644
>> --- a/drivers/gpu/drm/drm_modes.c
>> +++ b/drivers/gpu/drm/drm_modes.c
>> @@ -1611,7 +1611,13 @@ int drm_mode_convert_umode(struct drm_device *dev,
>>       out->vscan = in->vscan;
>>       out->vrefresh = in->vrefresh;
>>       out->flags = in->flags;
>> -    out->type = in->type;
>> +    /*
>> +     * Old xf86-video-vmware (possibly others too) used to
>> +     * leave 'type' unititialized. Just ignore any bits we
>> +     * don't like. It's a just hint after all, and more
>> +     * useful for the kernel->userspace direction anyway.
>> +     */
>> +    out->type = in->type & DRM_MODE_TYPE_ALL;
>>       strncpy(out->name, in->name, DRM_DISPLAY_MODE_LEN);
>>       out->name[DRM_DISPLAY_MODE_LEN-1] = 0;
>>   
>
> Tested-by: Thomas Hellstrom <thellstrom@vmware.com>
>
> Thanks,
>
> Thomas
>
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Daniel Stone March 23, 2018, 11:05 a.m. UTC | #3
On 21 March 2018 at 21:12, Ville Syrjala <ville.syrjala@linux.intel.com> wrote:
> Apparently xf86-video-vmware leaves the mode->type uninitialized
> when feeding the mode to the kernel. Thus we have no choice but
> to accept the garbage in. We'll just ignore any of the bits we
> don't want. The mode type is just a hint anyway, and more
> useful for the kernel->userspace direction.

Reviewed-by: Daniel Stone <daniels@collabora.com>
Ville Syrjälä March 23, 2018, 11:52 a.m. UTC | #4
On Thu, Mar 22, 2018 at 08:42:11AM +0100, Thomas Hellstrom wrote:
> On 03/21/2018 10:12 PM, Ville Syrjala wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > Apparently xf86-video-vmware leaves the mode->type uninitialized
> > when feeding the mode to the kernel. Thus we have no choice but
> > to accept the garbage in. We'll just ignore any of the bits we
> > don't want. The mode type is just a hint anyway, and more
> > useful for the kernel->userspace direction.
> >
> > Reported-by: Thomas Hellstrom <thomas@shipmail.org>
> > CC: Thomas Hellstrom <thomas@shipmail.org>
> > Cc: Adam Jackson <ajax@redhat.com>
> > Cc: Alex Deucher <alexander.deucher@amd.com>
> > Fixes: c6ed6dad5cfb ("drm/uapi: Validate the mode flags/type")
> > References: https://lists.freedesktop.org/archives/dri-devel/2018-March/170213.html
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >   drivers/gpu/drm/drm_modes.c | 8 +++++++-
> >   1 file changed, 7 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
> > index f6b7c0e36a1a..e82b61e08f8c 100644
> > --- a/drivers/gpu/drm/drm_modes.c
> > +++ b/drivers/gpu/drm/drm_modes.c
> > @@ -1611,7 +1611,13 @@ int drm_mode_convert_umode(struct drm_device *dev,
> >   	out->vscan = in->vscan;
> >   	out->vrefresh = in->vrefresh;
> >   	out->flags = in->flags;
> > -	out->type = in->type;
> > +	/*
> > +	 * Old xf86-video-vmware (possibly others too) used to
> > +	 * leave 'type' unititialized. Just ignore any bits we
> > +	 * don't like. It's a just hint after all, and more
> > +	 * useful for the kernel->userspace direction anyway.
> > +	 */
> > +	out->type = in->type & DRM_MODE_TYPE_ALL;
> >   	strncpy(out->name, in->name, DRM_DISPLAY_MODE_LEN);
> >   	out->name[DRM_DISPLAY_MODE_LEN-1] = 0;
> >   
> 
> Tested-by: Thomas Hellstrom <thellstrom@vmware.com>

Thanks for the testing and reviews. And sorry for the extra hassle.

Pushed to drm-misc-next-fixes.
diff mbox

Patch

diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
index f6b7c0e36a1a..e82b61e08f8c 100644
--- a/drivers/gpu/drm/drm_modes.c
+++ b/drivers/gpu/drm/drm_modes.c
@@ -1611,7 +1611,13 @@  int drm_mode_convert_umode(struct drm_device *dev,
 	out->vscan = in->vscan;
 	out->vrefresh = in->vrefresh;
 	out->flags = in->flags;
-	out->type = in->type;
+	/*
+	 * Old xf86-video-vmware (possibly others too) used to
+	 * leave 'type' unititialized. Just ignore any bits we
+	 * don't like. It's a just hint after all, and more
+	 * useful for the kernel->userspace direction anyway.
+	 */
+	out->type = in->type & DRM_MODE_TYPE_ALL;
 	strncpy(out->name, in->name, DRM_DISPLAY_MODE_LEN);
 	out->name[DRM_DISPLAY_MODE_LEN-1] = 0;