Sleep hook in a subdirectory ignored but causes double execution of previous hook

Bug #1548486 reported by Dirk F on 2016-02-22
14
This bug affects 1 person
Affects Status Importance Assigned to Milestone
pm-utils
Won't Fix
Medium
pm-utils (Ubuntu)
Medium
Unassigned

Bug Description

The set of sleep hooks provided in /usr/lib/pm-utils/sleep.d includes one (95packagekit/95packagekit) that is stored in a subdirectory. This has two effects:

1 the hook is not run;

2 the previous hook, currently /usr/lib/pm-utils/sleep.d/95led, is run for a second time instead.

Presumably #1 is wrong, and results from an installation fault in packagekit, separately reported as bug 1548480.

#2 occurs because the function run_hooks() in /usr/lib/pm-utils/pm-functions computes a list of to-be-executed hooks that includes directories but neither fully validates each such hook as a regular file nor handles a set of to-be-executed hooks in a subdirectory.

At lines 243 on, there is a code path with no else clause through which the $hook from the previous iteration can be accidentally reused:

  if [ -f "$syshooks/$base" ]; then
   hook="$syshooks/$base"
  elif [ -f "$phooks/$base" ]; then
   hook="$phooks/$base"
  fi

An easy fix is to insert the missing else clause before the fi line so that the script skips the subdirectory or other non-regular file and carries on with the next correctly specified hook:

  if [ -f "$syshooks/$base" ]; then
   hook="$syshooks/$base"
  elif [ -f "$phooks/$base" ]; then
   hook="$phooks/$base"
  else
   continue
  fi

Observed in Lubuntu 14.04.3,4 with pm-utils 1.4.1-13ubuntu0.1,2. However the relevant pm-utils code dates back to 2008, pm-utils upstream version 1.1.0.

ProblemType: Bug
DistroRelease: Ubuntu 14.04
Package: pm-utils 1.4.1-13ubuntu0.2 [modified: usr/lib/pm-utils/pm-functions]
ProcVersionSignature: Ubuntu 4.2.0-29.34~14.04.1-generic 4.2.8-ckt3
Uname: Linux 4.2.0-29-generic i686
ApportVersion: 2.14.1-0ubuntu3.19
Architecture: i386
Date: Mon Feb 22 17:10:53 2016
InstallationDate: Installed on 2016-02-21 (0 days ago)
InstallationMedia: Lubuntu 14.04.4 LTS "Trusty Tahr" - Release i386 (20160217.1)
PackageArchitecture: all
SourcePackage: pm-utils
UpgradeStatus: No upgrade log present (probably fresh install)

Dirk F (fieldhouse) wrote :

Created attachment 122012
patch taken from running pm-functions; also fixes comment typo

The function run_hooks() in pm-functions proposes to-be-executed hooks that includes subdirectories in the hook directories but neither fully validates each such hook as a regular file nor handles a set of to-be-executed hooks in a subdirectory. This can cause a hook to be executed more than once.

At lines 241 on, there is a code path with no else clause:

  if [ -f "$syshooks/$base" ]; then
   hook="$syshooks/$base"
  elif [ -f "$phooks/$base" ]; then
   hook="$phooks/$base"
  fi

If the $base proposed by the for statement at line 229 doesn't match either of the -f tests (which will happen eg if a hook is configured in a subdirectory of the hook directory), the $hook from the previous iteration can be accidentally reused.

An easy fix is to insert the missing else clause before the fi line so that the script skips the subdirectory or other non-regular file and carries on with the next correctly specified hook, as in the attached patch.

Created attachment 122013
patch taken from running pm-functions; also fixes comment typo

(attached wrong file)

Dirk F (fieldhouse) wrote :

While not a totally critical bug it is sad that upstream did not respond at all yet which I guess everyone here was waiting for as well which made nothing move at all :-/.
At this point it might be worth considering to take that as a delta - subscribing a few to hope one finds time to pick it up.

If you want you might ping on the upstream bug once more.

Changed in pm-utils (Ubuntu):
status: New → Confirmed
importance: Undecided → Medium

Given the slow (=no) movement of upstream we should expect the Delta to be a long term one.
That combined with the fact that this bug is present in Debian too, and Ubuntu currently doesn't make any changes over the Debian package. So this bug would be best fixed directly in Debian, and then Ubuntu will pick up the fix automatically.

I linked up the upstream bug here for now

Dirk F (fieldhouse) wrote :

Christian, thanks for looking at this.

I'd definitely encourage someone with a relevant Debian installation to reportbug this, and indeed any of the bugs that I've reported in pm-utils but remain ignored upstream.

I revisit dormant bugs on a regular scheduling following our triaging policy.
It seems nothing happened upstream still.

For the Debian reporting you can do so through the mail interface - you don't need a Debian system.
Usually it is preferred that the reporter does so, but if you need help or want me to do that please ping here and I'll do so.

On 2017-07-14 09:58, Christian Ehrhardt wrote:
> I revisit dormant bugs on a regular scheduling following our triaging policy.
> It seems nothing happened upstream still.
>
> For the Debian reporting you can do so through the mail interface - you don't need a Debian system.
> Usually it is preferred that the reporter does so, but if you need help or want me to do that please ping here and I'll do so.

Happy for you to do that, as Debian still seems to be interested in
pm-utils. Not sure what stops Debian package maintainers from checking
the upstream bug tracker, however.

/df

Andreas Hasenack (ahasenack) wrote :

Upstream bug still not touched, and patch still not applied not even in the ubuntu cosmic package, which is still a sync.

Andreas Hasenack (ahasenack) wrote :

Reproduced on trusty. In xenial the sleed.d packagekit hook is gone.

sudo apt install pm-utils packagekit

I hacked /usr/lib/pm-utils/pm-functions to log which hook it was running, and forced a suspend with "pm-suspend". In my case I got multiple runs of 98video-quirk-db-handler right after the incorrect packagekit directory was hit:

hook var is
base is 99video
runing hook /usr/lib/pm-utils/sleep.d/99video
hook var is /usr/lib/pm-utils/sleep.d/99video
base is 98video-quirk-db-handler
runing hook /usr/lib/pm-utils/sleep.d/98video-quirk-db-handler
hook var is /usr/lib/pm-utils/sleep.d/98video-quirk-db-handler
base is 95packagekit
runing hook /usr/lib/pm-utils/sleep.d/98video-quirk-db-handler
hook var is /usr/lib/pm-utils/sleep.d/98video-quirk-db-handler
base is 95led
runing hook /usr/lib/pm-utils/sleep.d/95led
hook var is /usr/lib/pm-utils/sleep.d/95led
base is 95hdparm-apm
...

With the proposed fix, the packagekit hook isn't run, and we also don't get double runs of other hooks.

So the fix could be what is proposed, or/and moving the packagekit hook to the right place.

Dirk F (fieldhouse) wrote :

"So the fix could be what is proposed, or/and moving the packagekit hook to the right place."

I'd say both.

I understand that, per https://bugs.launchpad.net/ubuntu/+source/packagekit/+bug/1548480/comments/8, the PackageKit side of the issue would be fixed if Ubuntu shipped version 0.8.14 or later, which has been true since Xenial as Andreas observes in comment #10.

Regarding the pm-utils fix, the problems seem to be that no-one is prepared to own and update pm-utils, and the systemd versions of Ubuntu don't need pm-utils (in fact, could be seriously confused by it). Consequently the fix should be implemented in Debian, as Christian says in comment #7, where pm-utils has been recognised as orphaned and is still being updated despite Debian using systemd.

pm-utils hasn't been touched in eight years, none of this is likely to get addressed. Closing bugs and disabling the bz product.

Changed in pm-utils:
importance: Unknown → Medium
status: Unknown → Won't Fix
To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.