[PATCH wayland] protocol: Bump seat to version 7 and require keymaps be private

[Derek Foreman]

Weston commit 76829fc4eaea329d2a525c3978271e13bd76c078 (and similar
commits for other compositors) protects the compositor's keyboard
mapping from client damage by duplicating the keymap for every
client.

On some systems there are other potential fixes for this - such as
using sealed memfds on linux - but we can't use them since
essentially all client code anywhere has mapped the keyboard map
with a MAP_SHARED mmap() call.

While we can't break years worth of code, we can require any future
clients to use MAP_PRIVATE if they use a seat version above 6.

If a compositor can't use sealing or a similar facility, it should
still protect itself with copied keymaps, but clients must always
assume shared mapping of a keymap will fail.

Signed-off-by: Derek Foreman <***@gmail.com>
---
protocol/wayland.xml | 11 +++++++----
1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/protocol/wayland.xml b/protocol/wayland.xml
index 141038b..802d433 100644
--- a/protocol/wayland.xml
+++ b/protocol/wayland.xml
@@ -1660,7 +1660,7 @@
</request>
</interface>

- <interface name="wl_seat" version="6">
+ <interface name="wl_seat" version="7">
<description summary="group of input devices">
A seat is a group of keyboards, pointer and touch devices. This
object is published as a global during start up, or when such a
@@ -1769,7 +1769,7 @@

</interface>

- <interface name="wl_pointer" version="6">
+ <interface name="wl_pointer" version="7">
<description summary="pointer input device">
The wl_pointer interface represents one or more input devices,
such as mice, which control the pointer location and pointer_focus
@@ -2092,7 +2092,7 @@
</event>
</interface>

- <interface name="wl_keyboard" version="6">
+ <interface name="wl_keyboard" version="7">
<description summary="keyboard input device">
The wl_keyboard interface represents one or more keyboards
associated with a seat.
@@ -2113,6 +2113,9 @@
<description summary="keyboard mapping">
This event provides a file descriptor to the client which can be
memory-mapped to provide a keyboard mapping description.
+
+ From version 7 onwards, the fd must be mapped with MAP_PRIVATE by
+ the recipient, as MAP_SHARED may fail.
</description>
<arg name="format" type="uint" enum="keymap_format" summary="keymap format"/>
<arg name="fd" type="fd" summary="keymap file descriptor"/>
@@ -2203,7 +2206,7 @@
</event>
</interface>

- <interface name="wl_touch" version="6">
+ <interface name="wl_touch" version="7">
<description summary="touchscreen input device">
The wl_touch interface represents a touchscreen
associated with a seat.

[Simon Ser]

Nice.

Reviewed-by: Simon Ser <***@emersion.fr>

[Daniel Stone]

On Thu, 1 Nov 2018 at 16:16, Derek Foreman

Post by Derek Foreman
Weston commit 76829fc4eaea329d2a525c3978271e13bd76c078 (and similar
commits for other compositors) protects the compositor's keyboard
mapping from client damage by duplicating the keymap for every
client.
On some systems there are other potential fixes for this - such as
using sealed memfds on linux - but we can't use them since
essentially all client code anywhere has mapped the keyboard map
with a MAP_SHARED mmap() call.
While we can't break years worth of code, we can require any future
clients to use MAP_PRIVATE if they use a seat version above 6.
If a compositor can't use sealing or a similar facility, it should
still protect itself with copied keymaps, but clients must always
assume shared mapping of a keymap will fail.

\o/

Reviewed-by: Daniel Stone <***@collabora.com>

[Philipp Kerling]

I second: \o/

Don't know if it means much at this point, but as original reporter of
the issue I claim this is

Reviewed-by: Philipp Kerling <***@casix.org>

[Philipp Kerling]

Actually... (below)

Post by Derek Foreman
Weston commit 76829fc4eaea329d2a525c3978271e13bd76c078 (and similar
commits for other compositors) protects the compositor's keyboard
mapping from client damage by duplicating the keymap for every
client.
On some systems there are other potential fixes for this - such as
using sealed memfds on linux - but we can't use them since
essentially all client code anywhere has mapped the keyboard map
with a MAP_SHARED mmap() call.
While we can't break years worth of code, we can require any future
clients to use MAP_PRIVATE if they use a seat version above 6.
If a compositor can't use sealing or a similar facility, it should
still protect itself with copied keymaps, but clients must always
assume shared mapping of a keymap will fail.
---
protocol/wayland.xml | 11 +++++++----
1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/protocol/wayland.xml b/protocol/wayland.xml
index 141038b..802d433 100644
--- a/protocol/wayland.xml
+++ b/protocol/wayland.xml
@@ -1660,7 +1660,7 @@
</request>
</interface>
- <interface name="wl_seat" version="6">
+ <interface name="wl_seat" version="7">
<description summary="group of input devices">
A seat is a group of keyboards, pointer and touch devices. This
object is published as a global during start up, or when such a
@@ -1769,7 +1769,7 @@
</interface>
- <interface name="wl_pointer" version="6">
+ <interface name="wl_pointer" version="7">

What is the reason for this version bump? The wl_pointer behavior is
unaffected by the wl_keyboard change, right? As far as I understood the
protocol versioning rules, bumping all child interfaces when updating
one child interface is not required. In fact that would make their
version counters quite superfluous.

(Same for wl_touch)

Post by Derek Foreman
<description summary="pointer input device">
The wl_pointer interface represents one or more input devices,
such as mice, which control the pointer location and
pointer_focus
@@ -2092,7 +2092,7 @@
</event>
</interface>
- <interface name="wl_keyboard" version="6">
+ <interface name="wl_keyboard" version="7">
<description summary="keyboard input device">
The wl_keyboard interface represents one or more keyboards
associated with a seat.
@@ -2113,6 +2113,9 @@
<description summary="keyboard mapping">
This event provides a file descriptor to the client which can be
memory-mapped to provide a keyboard mapping description.
+
+ From version 7 onwards, the fd must be mapped with MAP_PRIVATE by
+ the recipient, as MAP_SHARED may fail.
</description>
<arg name="format" type="uint" enum="keymap_format"
summary="keymap format"/>
<arg name="fd" type="fd" summary="keymap file descriptor"/>
@@ -2203,7 +2206,7 @@
</event>
</interface>
- <interface name="wl_touch" version="6">
+ <interface name="wl_touch" version="7">
<description summary="touchscreen input device">
The wl_touch interface represents a touchscreen
associated with a seat.

[Derek Foreman]

Post by Philipp Kerling
Actually... (below)

Yes, you are of course strictly correct.

However, the recent previous commits that bumped wl_seat version 9a18a8
and c5356e have also kept all three children in sync, and it's become
de-facto convention to do so.

Thanks,
Derek

Post by Philipp Kerling

Post by Derek Foreman
Weston commit 76829fc4eaea329d2a525c3978271e13bd76c078 (and similar
commits for other compositors) protects the compositor's keyboard
mapping from client damage by duplicating the keymap for every
client.
On some systems there are other potential fixes for this - such as
using sealed memfds on linux - but we can't use them since
essentially all client code anywhere has mapped the keyboard map
with a MAP_SHARED mmap() call.
While we can't break years worth of code, we can require any future
clients to use MAP_PRIVATE if they use a seat version above 6.
If a compositor can't use sealing or a similar facility, it should
still protect itself with copied keymaps, but clients must always
assume shared mapping of a keymap will fail.
---
protocol/wayland.xml | 11 +++++++----
1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/protocol/wayland.xml b/protocol/wayland.xml
index 141038b..802d433 100644
--- a/protocol/wayland.xml
+++ b/protocol/wayland.xml
@@ -1660,7 +1660,7 @@
</request>
</interface>
- <interface name="wl_seat" version="6">
+ <interface name="wl_seat" version="7">
<description summary="group of input devices">
A seat is a group of keyboards, pointer and touch devices. This
object is published as a global during start up, or when such a
@@ -1769,7 +1769,7 @@
</interface>
- <interface name="wl_pointer" version="6">
+ <interface name="wl_pointer" version="7">

What is the reason for this version bump? The wl_pointer behavior is
unaffected by the wl_keyboard change, right? As far as I understood the
protocol versioning rules, bumping all child interfaces when updating
one child interface is not required. In fact that would make their
version counters quite superfluous.
(Same for wl_touch)

Post by Derek Foreman
<description summary="pointer input device">
The wl_pointer interface represents one or more input devices,
such as mice, which control the pointer location and
pointer_focus
@@ -2092,7 +2092,7 @@
</event>
</interface>
- <interface name="wl_keyboard" version="6">
+ <interface name="wl_keyboard" version="7">
<description summary="keyboard input device">
The wl_keyboard interface represents one or more keyboards
associated with a seat.
@@ -2113,6 +2113,9 @@
<description summary="keyboard mapping">
This event provides a file descriptor to the client which can be
memory-mapped to provide a keyboard mapping description.
+
+ From version 7 onwards, the fd must be mapped with MAP_PRIVATE by
+ the recipient, as MAP_SHARED may fail.
</description>
<arg name="format" type="uint" enum="keymap_format"
summary="keymap format"/>
<arg name="fd" type="fd" summary="keymap file descriptor"/>
@@ -2203,7 +2206,7 @@
</event>
</interface>
- <interface name="wl_touch" version="6">
+ <interface name="wl_touch" version="7">
<description summary="touchscreen input device">
The wl_touch interface represents a touchscreen
associated with a seat.

[Pekka Paalanen]

On Thu, 1 Nov 2018 13:01:13 -0500

Post by Derek Foreman

Post by Philipp Kerling
Actually... (below)

Yes, you are of course strictly correct.
However, the recent previous commits that bumped wl_seat version 9a18a8
and c5356e have also kept all three children in sync, and it's become
de-facto convention to do so.

Yeah, either we bump them like this, or when someone then adds
something to e.g. wl_touch, he needs to know to take the new version
from wl_seat + 1, not wl_touch + 1.

Either way, the end result works exactly the same, and wl_touch etc.
will have some versions where nothing changed.


Thanks,
pq

Post by Derek Foreman

Post by Philipp Kerling

Post by Derek Foreman
Weston commit 76829fc4eaea329d2a525c3978271e13bd76c078 (and similar
commits for other compositors) protects the compositor's keyboard
mapping from client damage by duplicating the keymap for every
client.
On some systems there are other potential fixes for this - such as
using sealed memfds on linux - but we can't use them since
essentially all client code anywhere has mapped the keyboard map
with a MAP_SHARED mmap() call.
While we can't break years worth of code, we can require any future
clients to use MAP_PRIVATE if they use a seat version above 6.
If a compositor can't use sealing or a similar facility, it should
still protect itself with copied keymaps, but clients must always
assume shared mapping of a keymap will fail.
---
protocol/wayland.xml | 11 +++++++----
1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/protocol/wayland.xml b/protocol/wayland.xml
index 141038b..802d433 100644
--- a/protocol/wayland.xml
+++ b/protocol/wayland.xml
@@ -1660,7 +1660,7 @@
</request>
</interface>
- <interface name="wl_seat" version="6">
+ <interface name="wl_seat" version="7">
<description summary="group of input devices">
A seat is a group of keyboards, pointer and touch devices. This
object is published as a global during start up, or when such a
@@ -1769,7 +1769,7 @@
</interface>
- <interface name="wl_pointer" version="6">
+ <interface name="wl_pointer" version="7">

What is the reason for this version bump? The wl_pointer behavior is
unaffected by the wl_keyboard change, right? As far as I understood the
protocol versioning rules, bumping all child interfaces when updating
one child interface is not required. In fact that would make their
version counters quite superfluous.

[Pekka Paalanen]

On Thu, 1 Nov 2018 11:15:46 -0500

Post by Derek Foreman
Weston commit 76829fc4eaea329d2a525c3978271e13bd76c078 (and similar
commits for other compositors) protects the compositor's keyboard
mapping from client damage by duplicating the keymap for every
client.
On some systems there are other potential fixes for this - such as
using sealed memfds on linux - but we can't use them since
essentially all client code anywhere has mapped the keyboard map
with a MAP_SHARED mmap() call.
While we can't break years worth of code, we can require any future
clients to use MAP_PRIVATE if they use a seat version above 6.
If a compositor can't use sealing or a similar facility, it should
still protect itself with copied keymaps, but clients must always
assume shared mapping of a keymap will fail.
---
protocol/wayland.xml | 11 +++++++----
1 file changed, 7 insertions(+), 4 deletions(-)

Acked-by: Pekka Paalanen <***@collabora.co.uk>


Thanks,
pq

[Derek Foreman]

Post by Pekka Paalanen
On Thu, 1 Nov 2018 11:15:46 -0500

Post by Derek Foreman
Weston commit 76829fc4eaea329d2a525c3978271e13bd76c078 (and similar
commits for other compositors) protects the compositor's keyboard
mapping from client damage by duplicating the keymap for every
client.
On some systems there are other potential fixes for this - such as
using sealed memfds on linux - but we can't use them since
essentially all client code anywhere has mapped the keyboard map
with a MAP_SHARED mmap() call.
While we can't break years worth of code, we can require any future
clients to use MAP_PRIVATE if they use a seat version above 6.
If a compositor can't use sealing or a similar facility, it should
still protect itself with copied keymaps, but clients must always
assume shared mapping of a keymap will fail.
---
protocol/wayland.xml | 11 +++++++----
1 file changed, 7 insertions(+), 4 deletions(-)

And landed now.

Thanks,
Derek

Post by Pekka Paalanen
Thanks,
pq