[Bug 1926298] Re: Segfault added in the recent changes

[Dan Bungert]

Requesting sponsorship for SRU of hirsute-update-
notifier-2-3.192.40.2.debdiff

** Patch added: "hirsute-update-notifier-2-3.192.40.2.debdiff"
   https://bugs.launchpad.net/ubuntu/+source/update-notifier/+bug/1926298/+attachment/5493992/+files/hirsute-update-notifier-2-3.192.40.2.debdiff

** Changed in: update-notifier (Ubuntu Hirsute)
       Status: New => Confirmed

** Tags added: patch

** Changed in: update-notifier (Ubuntu Hirsute)
     Assignee: Dan Bungert (dbungert) => (unassigned)

-- 
You received this bug notification because you are a member of Ubuntu
Sponsors Team, which is subscribed to the bug report.
https://bugs.launchpad.net/bugs/1926298

Title:
  Segfault added in the recent changes

Status in update-notifier package in Ubuntu:
  Fix Committed
Status in update-notifier source package in Hirsute:
  Confirmed

Bug description:
  [Impact]

  On hirsute/impish, update-notifier can crash when attempting to run hooks.
  This issue is the #2 crash on update-notifier in hirsute, presently (and
  maybe some of the others).  I would call this a severe regression.

  In the crash cases, cleanup steps are running on an uninitialized
  variable.  The fix resolves this by ensuring the relevant variable is
  initialized.

  [Test Case]

  Create a file 'bug'

  'Name: bug
  Priority: Low
  Command: "/bin/true"
  DisplayIf: /bin/true
  Description: update notifier bug'

  and copy it to /var/lib/update-notifier/user.d

  [Where problems could occur]

  This is not the most absolute minimal version of this fix.
  If the minimal version of the fix is desired, I can provide that, which
  should be a +2/-2 diff.  (I don't want to confuse the current proposal.)
  The fix changeset being proposed includes other glib usage improvements
  which sound really good from a maintainability perspective and may even
  close other yet unknown bugs but add their own risk of regressions if
  said maintainability was not done correctly.

  The original idea was to address a problem where we offer to run a
  command, and the user would click the button to do so, but then nothing
  would happen because the command wasn't on the system.  If the parsing
  of this command is invalid, then valid hooks may be filtered.

  It's possible for the command to contain arguments, and to be quoted (or
  not), or to not be the full command path.  Variables for such testing
  include:
  * rely on path lookup or fully qualified path supplied in command
  * quoted or unquoted or mixed quotes (quote the command and args
    outside of the quotes)
  * arguments or no
  * presence or absence of actual command
  See attached tarball for the resulting 20 tests (4 redundant tests pruned).
  All the 'present' hooks are valid, 'absent' hooks should be skipped.

  In an attempt to stave off further memory safety problems I've analyzed
  this version with valgrind.  valgrind is able to observe several
  problems in the unfixed version, whereas the fixed version only
  complains about some items in glib that appear to be unrelated to this
  change.

  I've got some unit test code as well but haven't integrated that yet to
  the update-notifier codebase.

  [Original Bug Report Description]

  The issue is new using
  https://code.launchpad.net/~dbungert/update-notifier/+git/update-notifier/+merge/397367
  , the previous revision didn't have the issue

  * Create a file 'bug'

  'Name: bug
  Priority: Low
  Command: "/bin/true"
  DisplayIf: /bin/true
  Description: update notifier bug'

  and copy it to /var/lib/update-notifier/user.d

  ->  update-notifier segfaults

  (gdb) bt
  #0  0x00007ffff718c769 in __GI___libc_free (mem=0x5550000aa7f9) at malloc.c:3288
  #1  0x00007ffff7365215 in g_strfreev (str_array=<optimized out>) at ../../../glib/gstrfuncs.c:2553
  #2  g_strfreev (str_array=0x5555555d9e00) at ../../../glib/gstrfuncs.c:2546
  #3  0x000055555555dd2d in hook_command_exists (cmd=0x555555702600 "\"/usr/bin/eog\"") at hooks.c:137
  #4  0x000055555555fd6e in is_hook_relevant (hook_file=0x555555829e7b "apt-file.update-notifier")
      at hooks.c:717
  #5  0x000055555555ffb1 in check_update_hooks (ta=0x5555556c9a20) at hooks.c:781
  #6  0x0000555555560715 in hook_tray_icon_init (ta=0x5555556c9a20) at hooks.c:969
  #7  0x000055555555cd77 in tray_icons_init (un=0x55555562c3e0,

  It seems likely to be the error on top of the weekly report for hirsute
  but that's missing a stacktrace

  One comment in addition on the change there, was the env split + iterate
  over pathdirs basically a re-implementation of
  https://developer.gnome.org/glib/stable/glib-Miscellaneous-Utility-Functions.html#g-find-program-in-path
  ? If there is particular reason to do that I would recommend just using
  the glib function instead (which might also fix the issue as a side
  effect, I didn't check)

To manage notifications about this bug go to:
https://bugs.launchpad.net/ubuntu/+source/update-notifier/+bug/1926298/+subscriptions