Re: client side decorations
I can't get one thing out of this discussion. So you are arguing about client side VS server side decorations, handling of moves/resizes, maybe even buttons scroll bars etc. But all wayland does is provide a communication channel that enables clients to draw in the GPU memory, and then compositors display this memory the way they want to on screen. So, in my understanding there can be compositors drawing stuff around the windows (because they have all screen's content and they can mix it in whatever way they want), handle common shortcuts for close, move etc. And there can be other compositors that don't do anything like that. And both of these will be wayland compositors. Or are you actually arguing to put some additional stuff to the wl protocol that will enable more compositor side actions (what is X currently doing)? And why is everybody talking about titlebars and close buttons? I think this is also part of the brain-damage, that years of using windows and mac has brought us. I'm using a tiling window manager, and I really can't understand why would anybody want titlebars buttons on them, unless he/she really enjoys spending half of the time moving windows around so he can see them (if they wobble it is so much fun, I agree). And how do the tiling WMs in X fit the client vs server discussion. If it was all client side I assume no one cares about tiling windows anymore (no application developer), so simply there would be no tiling at all? There are things that should be common for all windows. Like closing, moving, resizing. Of course, because I'm using tiling window manager, this is all done through kb shortcuts, implemented in the server (I'm not sure if this fits the client vs server discussion, because this does not involve drawing decorations but certainly puts some overhead in the server to manage windows). I cant imagine anybody using his computer if these operations, are done through GUI and are done differently depending on the window. Of course this can be solved outside wayland by making GTK+, Qt more alike or creating additional communication channels between the clients and the servers. But this then tends to another discussion we had about screen locking, security, and vnc :) which was basically weather wl should make the protocol handle these things, or hardcode the strange parts (like a locking app) inside the compositor and use dbus or similar to link client and server for those out-of-core-wayland issues. (The discussion led to nowhere, as this one is also headed to). So what are you actually arguing about :) I mean -- just both sides explain in more detail what they want implemented where (in client, in server, in 3rd party), because otherwise it is going nowhere. Cheers, Iskren ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: wayland screen locker and security in general
I don't think this lengthy discussion led to any concrete answers, but I do think that the questions are important and need such answers. I'll try to summarize the problems that need attention: 1. screen locking 1.1 who is going to implement it: compositor/compositor plugin/app 1.2 inhibit locking for movie players, slide-shows etc 1.2.1 what protocol to use to make sure screen saver wont stay inhibited forever because of broken app 1.2.2 what communication mechanism to use: compositor/dbus/other 2. Full screen apps -- need a way to specify that no other windows should be displayed on top of this one. What if multiple apps want to claim the screen? 3 Security issues 3.1 Protect from bad clients allocating too much pixmap space (and maybe other resources) 3.2 Make sure that the password requesting prompts are genuine, i.e. no app is going to look like another one (screen saver) and request the user pass in the same way so the user is tricked to enter it inside. I don't think any other OS tries to do this, but I might be wrong. 4. In case we choose to use apps to implement special features as screen locking (opposed to compositor[-plugins]) then I don't understand how can an app authorize itself for the compositor. For example the screen saver app should tell the compositor that it is that exact app, so the compositor will grant special privileges (or proxy or arbiter, whatever). So is this process going to involve asymmetric cryptography, and if yes where is the private key going to be stored. This may be a very stupid question, but I haven't seen anything of that sort so I'm wondering how its going to be made. I may be missing some, so please note them if you find any. Regards, Iskren On Tue, Apr 5, 2011 at 11:59 AM, Michal Suchanek hramr...@centrum.czwrote: Hello, what is the plan for screensave/screenlocker support in wayland? The support in X is a fail in several ways. First off there is no way to select all events. While you can spy on mouse position or key events you can't spy on mouse events so the screensaver activates when only mouse buttons/wheel are used. Second the way to lock the screen is to grab the X input (pointer and keyboard) so that other applications can't get them while the screen is locked. This fails when a grab already exists because X has only one grab and it is acquired on first-come first-serve basis. A technical issue with X protocol also requires the grab to be taken for pulldown menus and drag and drop operations in any application. Third is that you can't cover the screen. xscreensaver creates a fullscreen window to show the cool effects (or cool blackness) and raises it to top but subsequently created windows can get on top of it. xscreensaver works around it by raising the window every few minutes but there is no way to ensure that the screen is covered. There are other issues as well. In X any application can allocate any amount of pixmaps eventually crashing the X server. There should be a mechanism that allows limiting the damage a single application can do. There are several options here. One is to have some policy in the wayland server/compositor eg people suggest that applications should not be able to lock the screen, and the screenlocker binary be in a specific place, the wayland compositor implements a toolbar with a lock button, and whenever the button is pressed that binary is executed and granted the privilege to lock the screen. Another option is to use a proxy. The compositor and screenlocker would connect to the server directly while applications would connect through a proxy that denies some requests. The proxy can implement some policy like allowing up to 5MPix of screen data in up to 5 windows refreshed up to once per vsync. Yet another option is to have policy advisor to which the server talks through a separate connection. Whenever an application sends a request to the server it asks a policy advisor if the request should be honored. Compared to the proxy solution there are advantages and disadvantages. The disadvantage is that the server has to process even requests that are denied. The advantage is that the server can talk to the application directly. eg the application selects some events, the policy allows selecting them, and then the application can receive events directly. A non-trransparent protocol could have advantages of both. The application first talks to the policy server, and if the request is allowed it gets a handle which can be used to perform the request on the display server, input driver, or whatever other component implements it. With either the advisor or the proxy the policy can be dynamic and negotiated between the advisor and the application by arbitrary protocol independent of wayland protocols. With either the system fails when the policy component fails - either the proxy goes down and stops forwarding requests and repliesor the advisor goes down and
Re: handling unknown messages (pt 2 of TODO)
2011/4/11 Kristian Høgsberg k...@bitplanet.net On Sun, Apr 10, 2011 at 1:47 PM, Iskren Chernev iskren.cher...@gmail.com wrote: Hello, I think I can do the second point in the TODO file: The message format has to include information about number of fds in the message so we can skip a message correctly. Or we should just give up on trying to recover from unknown messages. We need to make sure you never get a message from an interface you don't know about (using per-client id space and subscribe) or include information on number of fds, so marshalling logic can skip. So one solution is to include the number of fds in the serialization of the message, maybe as a 3rd word in the header? Yeah, that's the straight forward idea. I was thinking that we could be a little smarter about the encoding and use the 16 bits we currently use for siz a little differently. Make the upper bit of the size field reserved, the next three the number of fds and the remaining 12 bits the size of the message. That means we're reducing the maximum message size from 64k to 4k bytes, but that's ok, the Wayland protocol isn't intended for bulk transfer (send a pipe fd for that). The reserved bit must be zero and I'm thinking we can use it to indicate the presence of optinal header fields. Ok, that makes sense. The idea about subscription also sounds reasonable, but I don't understand what is ment by per-client id space. After a global object is broadcasted, the client may choose to subscribe or not, but I don't see per-client id space here (except that the server must maintain for each client what ids is he listening to -- maybe that is meant by per client id space. The per-client id space has a couple of benefits. It greatly simplifies the id allocation (since now the client is in charge of the entire 32 bit id space) and it means that there's no way a client can possible address other clients objects. The subscription step is when the client assigns a client id to a global object. With this step in place, we could even change the type of ids used for global objects to be something else - strings, for example. Anyway, this is something I've been working on, I've just been distracted enough to not actually finish it for the previous one or two months... Also, somewhat related, how different versions of the same interface are going to be handled. For example a client only knows version 2 of Foo, but the server broadcasts version 3 of Foo at some point. So does the client subscribe? What if Foo's messages are crucial for that client? Maybe it should specify ver 2 in the subscription, and then the sever must make sure not to broadcast messages that are not included in ver 2, but then ver 3 must be backward compatible I guess. So what do you suggest? Yup, that's another benefit to the subscription step. The client can specify which version of the interface it understands. The idea is that interface should always be backwards compatible, clients are expected to ignore messages introduced in newer versions. So basically, even if you implement the subscription mechanism the number of file descriptors should be given -- except in the case where the server knows all previous versions and sends only the messages the client knows of. Maybe all messages should have the version number they were introduced in. Kristian So I can safely implement the passing of the number of fds the way that you describe it (safely -- I mean it wont be redundant after a few months :)). Regards, Iskren ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: fixed bug in wl_list
In my opinion the problem is this: The code to remove the element from the list is called every time, and the code to insert it is called only when 'surface'. So in case surface is NULL the item won't be added, but will be removed next time the function is entered. Isn't that the case? I don't have a detailed understanding of this exact code, but isn't the explanation as simple as that? Should I add some debugging info to see if this is indeed the case, or you are sure that the problem lies somewhere else? About the 'tagging' -- are you proposing to add a boolean flag to the listener to explicitly mark it as 'in a list' or 'out of a list'? Regards, Iskren On Sat, Mar 12, 2011 at 9:48 PM, Marty Jack marty...@comcast.net wrote: On 03/12/2011 02:20 PM, Bill Spitzak wrote: On 03/12/2011 04:28 AM, Marty Jack wrote: I have never encountered a system where it was believed to be desirable to allow something to be removed twice. It is important to keep data structures clean. If anything you would be more likely to see a debugging mode where the lists were fully checked after every insert or remove to make sure they are internally consistent, especially if they are important to keeping the system running. It's not that much different from memory allocation. A block is allocated, or it is free, and a double free is a bug. Making it crash at the moment the second remove is attempted is better than it leaving the data corrupted and crashing later. It makes it a lot easier to find out why it went wrong. I think that is what the newest version of the patch is doing, right? ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel Oh and no, the second version of the patch makes the second remove a no-op. ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: fixed bug in wl_list
Well, with your patch it doesn't seem to crash. There is also no list corruption, as far as I could test it :) Regards, Iskren 2011/3/13 Kristian Høgsberg k...@bitplanet.net On Fri, Mar 11, 2011 at 7:32 PM, Iskren Chernev iskren.cher...@gmail.com wrote: Hello, I found a bug and fixed it with the patch :) to reproduce: Hi, I wasn't able to reproduce it immediately, but I know that there's a crasher in there somewhere. I think the real fix is the patch I've attached, could you give it a try and see if it fixes it for you? thanks, Kristian run compositor on top of x11 repeat run flower drag drop it a little move the pointer in and out of the compositor/flower Ctrl+C the flower client it would break eventually problem: I found that the linked list surface-destroy_listener_list got corrupted at some point (it was not circular any more, strange next/prev etc), which causes the crash. solution: The problem was in wl_list_remove -- when you erase an element, you don't mark it as 'erased', by setting prev/next to NULL for example. Then if you erase it again the list becomes corrupt. I nullified the prev/next and check in the begining of wl_list_remove for not-in-list elements and just ignore them. That seems to fix it. Regards, Iskren ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
patches addressing error handling
Some resources were not freed if the create function exited earlier.
I'm not sure how important these patches are. If they are I can make
another pass on the code to fix similar errors. Also adding warn
unused result to the creation functions that may fail will help
spotting all places.
Regards,
Iskren
From 57f2ef62b7c6c01a4aa1e6944812b97f0ec7b9b4 Mon Sep 17 00:00:00 2001
From: Iskren Chernev iskren.cher...@gmail.com
Date: Fri, 11 Mar 2011 14:19:00 +0200
Subject: [PATCH 1/4] Better cleanup on display creation failure.
---
wayland/wayland-server.c |2 ++
1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/wayland/wayland-server.c b/wayland/wayland-server.c
index dece0d1..17b91ae 100644
--- a/wayland/wayland-server.c
+++ b/wayland/wayland-server.c
@@ -528,6 +528,7 @@ wl_display_create(void)
display-objects = wl_hash_table_create();
if (display-objects == NULL) {
+ wl_event_loop_destroy(display-loop);
free(display);
return NULL;
}
@@ -544,6 +545,7 @@ wl_display_create(void)
wl_display_add_object(display, display-object);
if (wl_display_add_global(display, display-object, NULL)) {
wl_event_loop_destroy(display-loop);
+ wl_hash_table_destroy(display-objects);
free(display);
return NULL;
}
--
1.7.4
From 08972227552d0d777862064ea9b17277042bf4ef Mon Sep 17 00:00:00 2001
From: Iskren Chernev iskren.cher...@gmail.com
Date: Fri, 11 Mar 2011 14:58:06 +0200
Subject: [PATCH 2/4] Better handling of creation errors in display.
---
wayland/wayland-client.c |6 ++
wayland/wayland-server.c |2 +-
2 files changed, 7 insertions(+), 1 deletions(-)
diff --git a/wayland/wayland-client.c b/wayland/wayland-client.c
index ad35c30..0be1c47 100644
--- a/wayland/wayland-client.c
+++ b/wayland/wayland-client.c
@@ -373,6 +373,11 @@ wl_display_connect(const char *name)
}
display-objects = wl_hash_table_create();
+ if (display-objects == NULL) {
+ close(display-fd);
+ free(display);
+ return NULL;
+ }
wl_list_init(display-global_listener_list);
display-proxy.object.interface = wl_display_interface;
@@ -397,6 +402,7 @@ WL_EXPORT void
wl_display_destroy(struct wl_display *display)
{
wl_connection_destroy(display-connection);
+ wl_hash_table_destroy(display-objects);
close(display-fd);
free(display);
}
diff --git a/wayland/wayland-server.c b/wayland/wayland-server.c
index 17b91ae..99b9f33 100644
--- a/wayland/wayland-server.c
+++ b/wayland/wayland-server.c
@@ -544,8 +544,8 @@ wl_display_create(void)
display-object.implementation = (void (**)(void)) display_interface;
wl_display_add_object(display, display-object);
if (wl_display_add_global(display, display-object, NULL)) {
- wl_event_loop_destroy(display-loop);
wl_hash_table_destroy(display-objects);
+ wl_event_loop_destroy(display-loop);
free(display);
return NULL;
}
--
1.7.4
From e535caf51fbf4823b082f1b382327d787f89b17e Mon Sep 17 00:00:00 2001
From: Iskren Chernev iskren.cher...@gmail.com
Date: Fri, 11 Mar 2011 14:43:10 +0200
Subject: [PATCH 3/4] Added wl_connection_create failure checks.
---
wayland/connection.c |2 ++
wayland/wayland-client.c |7 ++-
wayland/wayland-server.c |4
3 files changed, 12 insertions(+), 1 deletions(-)
diff --git a/wayland/connection.c b/wayland/connection.c
index 4a00470..0d705b5 100644
--- a/wayland/connection.c
+++ b/wayland/connection.c
@@ -160,6 +160,8 @@ wl_connection_create(int fd,
struct wl_connection *connection;
connection = malloc(sizeof *connection);
+ if (connection == NULL)
+ return NULL;
memset(connection, 0, sizeof *connection);
connection-fd = fd;
connection-update = update;
diff --git a/wayland/wayland-client.c b/wayland/wayland-client.c
index 0be1c47..1e9f915 100644
--- a/wayland/wayland-client.c
+++ b/wayland/wayland-client.c
@@ -394,7 +394,12 @@ wl_display_connect(const char *name)
display-connection = wl_connection_create(display-fd,
connection_update,
display);
-
+ if (display-connection == NULL) {
+ wl_hash_table_destroy(display-objects);
+ close(display-fd);
+ free(display);
+ return NULL;
+ }
return display;
}
diff --git a/wayland/wayland-server.c b/wayland/wayland-server.c
index 99b9f33..faa1e1a 100644
--- a/wayland/wayland-server.c
+++ b/wayland/wayland-server.c
@@ -226,6 +226,10 @@ wl_client_create(struct wl_display *display, int fd)
wl_client_connection_data, client);
client-connection =
wl_connection_create(fd, wl_client_connection_update, client);
+ if (client-connection == NULL) {
+ free(client);
+ return NULL;
+ }
wl_list_init(client-resource_list);
--
1.7.4
From adc5e6b17e102b7e16869f082364572fb6a51cac Mon Sep 17 00:00:00 2001
From: Iskren Chernev iskren.cher...@gmail.com
Date: Fri, 11 Mar 2011 16:30:59 +0200
Subject: [PATCH 4/4] Server socket creation error handling.
---
wayland/wayland-server.c | 26 --
1 files changed, 20 insertions(+), 6 deletions(-)
diff --git a/wayland/wayland-server.c
