On Wed, 13 Jan 2016 10:31:58 +0800
Jonas Ådahl <[email protected]> wrote:

> On Tue, Jan 12, 2016 at 10:58:16AM -0600, Derek Foreman wrote:
> > On 11/01/16 04:30 PM, Sung-Jin Park wrote:  
> > > This adds an API to get the socket fd for a client.
> > > The client socket fd can be used for a wayland compositor to validate a 
> > > request
> > > from a client.
> > > For instance, this will be helpful in some linux distributions, in which 
> > > SELinux
> > > or SMACK is enabled. In those environments, each file (including socket) 
> > > will have
> > > each security context in its inode as xattr member variable. A wayland 
> > > compositor
> > > can validate a client request by getting socket fd of the client and by 
> > > checking
> > > the security context associated with the socket fd.
> > > 
> > > Signed-off-by: Sung-Jin Park <[email protected]>
> > > ---
> > >  src/wayland-server-core.h |  3 +++
> > >  src/wayland-server.c      | 15 +++++++++++++++
> > >  2 files changed, 18 insertions(+)
> > > 
> > > diff --git a/src/wayland-server-core.h b/src/wayland-server-core.h
> > > index 1700cd3..0d5fbc1 100644
> > > --- a/src/wayland-server-core.h
> > > +++ b/src/wayland-server-core.h
> > > @@ -182,6 +182,9 @@ void
> > >  wl_client_get_credentials(struct wl_client *client,
> > >                     pid_t *pid, uid_t *uid, gid_t *gid);
> > >  
> > > +int
> > > +wl_client_get_socket_fd(struct wl_client *client);
> > > +
> > >  void
> > >  wl_client_add_destroy_listener(struct wl_client *client,
> > >                          struct wl_listener *listener);
> > > diff --git a/src/wayland-server.c b/src/wayland-server.c
> > > index 55c0cf9..973a71c 100644
> > > --- a/src/wayland-server.c
> > > +++ b/src/wayland-server.c
> > > @@ -491,6 +491,21 @@ wl_client_get_credentials(struct wl_client *client,
> > >           *gid = client->ucred.gid;
> > >  }
> > >  
> > > +/** Get the socket fd for the client  
> 
> s/fd/file descriptor/
> 
> > > + *
> > > + * \param client The display object
> > > + * \return fd The fd to use for the connection  
> > 
> > Should probably just be
> >  * \return The fd to use for the connection  
> 
> Maybe "file descriptor" here as well?
> 
> > 
> > Otherwise this looks good to me,
> > Reviewed-by: Derek Foreman <[email protected]>  
> 
> I would also prefer the API to be "wl_client_get_fd()" as that is that
> is what all the other similar API looks like (be it a socket fd or a
> non-socket fd).
> 
> Other than those issues, this looks reasonable to me, so consider it
> Reviewed-by: Jonas Ådahl <[email protected]>

Hi,

I agree with all of the above.

The fd may not always be from a socket file, it can also be from a call
to socketpair(2). Please refer to wl_client_get_credentials() for the
caveat there, and evaluate whether it applies to your use case.
wl_client_get_fd() doc should probably have a "see also
wl_client_get_credentials()" so that someone reading the doc finds out
about socketpair().

One should probably also document, that the fd you get from this
function is not duplicated, and it remains owned and used by
libwayland-server. Therefore if the caller does anything to the fd that
changes its state, it will likely cause problems. Getting the fd is for
inspection only, like the use case described in the commit message.

It might even make sense to add the example use case to the doc.

With these documentation additions, you get
Acked-by: Pekka Paalanen <[email protected]>


Thanks,
pq

> >   
> > > + *
> > > + * This function returns the client socket fd for the given client.
> > > + *
> > > + * \memberof wl_client
> > > + */
> > > +WL_EXPORT int
> > > +wl_client_get_socket_fd(struct wl_client *client)
> > > +{
> > > + return client->connection->fd;
> > > +}
> > > +
> > >  /** Look up an object in the client name space
> > >   *
> > >   * \param client The client object
> > >   
> > 
> > _______________________________________________
> > wayland-devel mailing list
> > [email protected]
> > http://lists.freedesktop.org/mailman/listinfo/wayland-devel  
> _______________________________________________
> wayland-devel mailing list
> [email protected]
> http://lists.freedesktop.org/mailman/listinfo/wayland-devel

Attachment: pgpg1zStjQLre.pgp
Description: OpenPGP digital signature

_______________________________________________
wayland-devel mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/wayland-devel

Reply via email to