[xserver] dri3: cap the version returned to the client

Submitted by Emil Velikov on March 13, 2018, 6:38 p.m.

Details

Message ID 20180313183849.5840-1-emil.l.velikov@gmail.com
State Accepted
Commit ae5c0dd199a5fbfbdf7a2d6b8c1b28c410289106
Headers show
Series "dri3: cap the version returned to the client" ( rev: 1 ) in X.org (DEPRECATED - USE GITLAB)

Not browsing as part of any series.

Commit Message

Emil Velikov March 13, 2018, 6:38 p.m.
From: Emil Velikov <emil.velikov@collabora.com>

As per the protocol, the server should not return version greater than
the one supported by the client.

Add a spec quote and tweak the numbers accordingly.

Fixes: 563138298868 ("dri3: Add DRI3 extension")
Cc: Daniel Stone <daniels@collabora.com>
Cc: Keith Packard <keithp@keithp.com>
Signed-off-by: Emil Velikov <emil.velikov@collabora.com>
---
Note: we might want this in the stable releases?

Related: The DRI2 implementation has identical bug.
Although fixing that (unlike DRI3) might cause some ill written apps to
rightfully fall on their face.

Should we bother - yay, nay are appreciated.
---
 dri3/dri3_request.c | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

Patch hide | download patch | download mbox

diff --git a/dri3/dri3_request.c b/dri3/dri3_request.c
index 7f3f0d08c..fc258711b 100644
--- a/dri3/dri3_request.c
+++ b/dri3/dri3_request.c
@@ -45,7 +45,19 @@  proc_dri3_query_version(ClientPtr client)
     };
 
     REQUEST_SIZE_MATCH(xDRI3QueryVersionReq);
-    (void) stuff;
+    /* From DRI3 proto:
+     *
+     * The client sends the highest supported version to the server
+     * and the server sends the highest version it supports, but no
+     * higher than the requested version.
+     */
+
+    if (rep.majorVersion > stuff->majorVersion ||
+        rep.minorVersion > stuff->minorVersion) {
+        rep.majorVersion = stuff->majorVersion;
+        rep.minorVersion = stuff->minorVersion;
+    }
+
     if (client->swapped) {
         swaps(&rep.sequenceNumber);
         swapl(&rep.length);

Comments

On 13 March 2018 at 18:38, Emil Velikov <emil.l.velikov@gmail.com> wrote:
> From: Emil Velikov <emil.velikov@collabora.com>
>
> As per the protocol, the server should not return version greater than
> the one supported by the client.
>
> Add a spec quote and tweak the numbers accordingly.
>
> Fixes: 563138298868 ("dri3: Add DRI3 extension")
> Cc: Daniel Stone <daniels@collabora.com>
> Cc: Keith Packard <keithp@keithp.com>
> Signed-off-by: Emil Velikov <emil.velikov@collabora.com>
> ---
> Note: we might want this in the stable releases?
>
> Related: The DRI2 implementation has identical bug.
> Although fixing that (unlike DRI3) might cause some ill written apps to
> rightfully fall on their face.
>
> Should we bother - yay, nay are appreciated.
> ---
>  dri3/dri3_request.c | 14 +++++++++++++-
>  1 file changed, 13 insertions(+), 1 deletion(-)
>
> diff --git a/dri3/dri3_request.c b/dri3/dri3_request.c
> index 7f3f0d08c..fc258711b 100644
> --- a/dri3/dri3_request.c
> +++ b/dri3/dri3_request.c
> @@ -45,7 +45,19 @@ proc_dri3_query_version(ClientPtr client)
>      };
>
>      REQUEST_SIZE_MATCH(xDRI3QueryVersionReq);
> -    (void) stuff;
> +    /* From DRI3 proto:
> +     *
> +     * The client sends the highest supported version to the server
> +     * and the server sends the highest version it supports, but no
> +     * higher than the requested version.
> +     */
> +
> +    if (rep.majorVersion > stuff->majorVersion ||
> +        rep.minorVersion > stuff->minorVersion) {
> +        rep.majorVersion = stuff->majorVersion;
> +        rep.minorVersion = stuff->minorVersion;
> +    }
> +
>      if (client->swapped) {
>          swaps(&rep.sequenceNumber);
>          swapl(&rep.length);
> --
Humble ping anyone?

-Emil
On Mon, 2018-03-19 at 12:04 +0000, Emil Velikov wrote:
> On 13 March 2018 at 18:38, Emil Velikov <emil.l.velikov@gmail.com> wrote:
> > From: Emil Velikov <emil.velikov@collabora.com>
> > 
> > As per the protocol, the server should not return version greater than
> > the one supported by the client.
> > 
> > Add a spec quote and tweak the numbers accordingly.
> > 
> > Fixes: 563138298868 ("dri3: Add DRI3 extension")
> > Cc: Daniel Stone <daniels@collabora.com>
> > Cc: Keith Packard <keithp@keithp.com>
> > Signed-off-by: Emil Velikov <emil.velikov@collabora.com>

Merged:

remote: I: patch #210343 updated using rev ae5c0dd199a5fbfbdf7a2d6b8c1b28c410289106.
remote: I: 1 patch(es) updated to state Accepted.
To ssh://git.freedesktop.org/git/xorg/xserver
   6a5d51e082..ae5c0dd199  master -> master

Though I think both this and the corresponding present patch are broken
if the client sends a version number of 0.42...

- ajax
On 19 March 2018 at 19:59, Adam Jackson <ajax@nwnk.net> wrote:
> On Mon, 2018-03-19 at 12:04 +0000, Emil Velikov wrote:
>> On 13 March 2018 at 18:38, Emil Velikov <emil.l.velikov@gmail.com> wrote:
>> > From: Emil Velikov <emil.velikov@collabora.com>
>> >
>> > As per the protocol, the server should not return version greater than
>> > the one supported by the client.
>> >
>> > Add a spec quote and tweak the numbers accordingly.
>> >
>> > Fixes: 563138298868 ("dri3: Add DRI3 extension")
>> > Cc: Daniel Stone <daniels@collabora.com>
>> > Cc: Keith Packard <keithp@keithp.com>
>> > Signed-off-by: Emil Velikov <emil.velikov@collabora.com>
>
> Merged:
>
> remote: I: patch #210343 updated using rev ae5c0dd199a5fbfbdf7a2d6b8c1b28c410289106.
> remote: I: 1 patch(es) updated to state Accepted.
> To ssh://git.freedesktop.org/git/xorg/xserver
>    6a5d51e082..ae5c0dd199  master -> master
>
> Though I think both this and the corresponding present patch are broken
> if the client sends a version number of 0.42...
>
True. We could bail out if the client sends a too old (garbage) version - WDYT?

In general handling of server_major != client_major could also be
improved, yet major won't be bumped in the foreseeable future.

Thanks
-Emil
On Mon, 2018-03-19 at 21:11 +0000, Emil Velikov wrote:
> On 19 March 2018 at 19:59, Adam Jackson <ajax@nwnk.net> wrote:
> > Merged:
> > 
> > remote: I: patch #210343 updated using rev ae5c0dd199a5fbfbdf7a2d6b8c1b28c410289106.
> > remote: I: 1 patch(es) updated to state Accepted.
> > To ssh://git.freedesktop.org/git/xorg/xserver
> >    6a5d51e082..ae5c0dd199  master -> master
> > 
> > Though I think both this and the corresponding present patch are broken
> > if the client sends a version number of 0.42...
> 
> True. We could bail out if the client sends a too old (garbage) version - WDYT?

We probably should. In practice it's not going to matter as no client
is that broken (and supporting two major versions of an extension is
nearly impossible in most cases), but technically correct is the best
kind of correct.

- ajax