Bug 103592

Summary:	dbus-daemon: Failing to set up a new connection is not logged
Product:	dbus	Reporter:	Simon McVittie <smcv>
Component:	core	Assignee:	Simon McVittie <smcv>
Status:	RESOLVED FIXED	QA Contact:	D-Bus Maintainers <dbus>
Severity:	normal
Priority:	medium	CC:	alexl, dbus, desrt, smcv
Version:	git master	Keywords:	patch
Hardware:	All
OS:	All
Whiteboard:	review+
i915 platform:		i915 features:
Attachments:	bus_connections_setup_connection: If we can't set it up, log why

Description Simon McVittie 2017-11-06 16:29:51 UTC

+++ This bug was initially created as a clone of Bug #101354 +++

(In reply to Philip Withnall on Bug #101354)
> Comment on attachment 133017 [details] [review]
> bus/containers: Set up new connections to join the bus
> 
> ::: bus/containers.c
> @@ +290,5 @@
> >  {
> > +  BusContainerInstance *instance = data;
> > +
> > +  if (!bus_context_add_incoming_connection (instance->context, new_connection))
> > +    return;
> 
> Do we want to log a warning on the OOM failure case? Otherwise it
> essentially gets silently ignored.

The OOM failure case is silently ignored in the current code too, so this isn't a regression in Bug #101354. However, we should probably log the suggested warning.

(It can fail as a result of OOM, AppArmor setup failing, or SELinux setup failing.)

Comment 1 Simon McVittie 2017-11-06 16:35:01 UTC

Created attachment 135261 [details] [review]
bus_connections_setup_connection: If we can't set it up, log why

While reviewing Bug #101354, Philip Withnall pointed out that if we
rejected a connection in the new code there, we didn't log why. It
turns out we didn't log that in the more normal code path either.
Redo the error handling so that failure to set up a connection
is logged.

Comment 2 Philip Withnall 2017-11-06 18:23:17 UTC

Comment on attachment 135261 [details] [review]
bus_connections_setup_connection: If we can't set it up, log why

Review of attachment 135261 [details] [review]:
-----------------------------------------------------------------

r+

Comment 3 Simon McVittie 2017-11-10 17:01:35 UTC

Thanks, fixed in git for 1.12.2 and 1.13.0

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.