Resource id allocation on server side

[Kristian Høgsberg]

On Tue, Jul 17, 2012 at 11:52:02AM +0000, Fiedler, Mathias wrote:
> Hi,
> 

> I'm facing an issue when the client tries to create a new object
> with an id bigger than the current maximum id on server-side +1. Is
> it correct that those ids will be silently ignored?
> E.g. wl_client_add_resource() would call wl_map_insert_at() return
> -1 which isn't checked.
> 
> For example, this situation might happen when a client calls
> wl_seat.get_pointer() on a seat which has no pointer device
> assigned.  The result is that all the following object creations
> will silently fail. The client won't realize its failure until he
> tries to call a method on such object. Such error might be hard to
> find.
> 
> Is my analysis correct, or did i miss something? If not what would
> be a correct fix for this?

Yes, you are right, as long as the client doesn't realize that the
pointer object didn't get allocated, and keep using new IDs (as
opposed to reusing old freed up IDs), those new objects will get
silently ignored.

> Restricting the id allocation to the next higher id seems
> reasonable, so the compositor get in trouble at corrupt messages
> with a new id that are too big.  But should this restriction in
> wayland-server be relaxed?  Or shall the compositor keep track of
> current maximum id and allocate new ids even when the corresponding
> server-side resource is not created?

I want to keep the restriction in the server, so we need to fix these
cases where an object may not be created.  The display_sync handler is
similar in that it destroys the object immeidately.  That function
used to not even allocate the object, just send the event back without
ever inserting the object.  Now it creates the objects, inserts it and
then destroys it, because otherwise it causes the same problem you hit.

First we need to not silently ignore this error, and then we need to
insert and destroy and object with the id, even in cases where we
don't create the object.  A better option perhaps is to handle this
centrally in deref_new_objects(), which already pre-processes the
incoming request and touches all new IDs.  Or even better, handle it
in the 'n' case in wl_connection_demarshal(), since it should be
handle for both server and client.

Thanks for tracking this down,
Kristian