Skip to content

network: fix criticals when updating connection

Michael Catanzaro requested to merge mcatanzaro/vpn-criticals into main
network: fix criticals when updating connection

If the operation is cancelled (because the dialog was closed, because
the Apply button was pressed), then trying to make further use of the
source_object is a use after free, which is bad. At first I tried to fix
this by simply avoiding the use after free when the operation is
canceled, but then I realized it is ridiculous to always try committing
connection changes when closing the dialog, then immediately cancel the
operation by destroying the dialog.

So instead I've decided to not pass the cancellable along to these
operations, and instead ref the dialog to keep it alive until the
operations complete. Instead, let's just hide the window.

This commit also removes an inaccurate comment /* Leave the editor open
*/ placed right before the call to the function that hides the editor.
There's no need to leave the editor open when updating the device fails.
The connection properties at least are still saved.

Fixes #2618

Also:

network: don't try to update device if it doesn't exist

All of the following code assumes that self->device is valid, so we need
to skip over it. It's confusing, but this is a multipurpose dialog and
self->device is optional when creating the dialog. E.g. when modifying
VPN configuration, we update just the configuration, not an NMDevice.

Also:

Don't warn about failure to apply changes on inactive device

If the NMDevice is not active (i.e. if we are editing a connection that
is not active) then don't warn when failing to reapply changes to the
device. That's expected and not something we should warn about.

We could perhaps not even try, but the device could become inactive
between the time of our check and this error, so better ignore the error
regardless.

(This third commit is a little unrelated, but I'll sneak it in here because the required code change would otherwise conflict with this merge request.)

Edited by Michael Catanzaro

Merge request reports