Skip to content

Conversation

@bboozzoo
Copy link
Contributor

@bboozzoo bboozzoo commented Dec 11, 2025

When /tmp/snap-private-tmp does not exist, it will be created at runtime by snap-confine. Specifically, when more than one instance of snap-confine is active, both race to create the directory. Commit
ba1b2ff changed the code to use sc_ensure_mkdir() helper. The helper, creates the directory with 0000 mode bits, and proceeds to update its ownership
and mode to match what was requested. It is possible that when one instance of snap-confine is in the middle of creating the directory, another instance may observe an intermediate state in which either the ownership or permissions have not yet been updated. Instead of failing, let the other instance attempt to amend the ownership and permissions.

Related: SNAPDENG-36092
Fixes: LP#2134364

Thanks for helping us make a better snapd!
Have you signed the license agreement and read the contribution guide?

@bboozzoo bboozzoo added the Needs security review Can only be merged once security gave a :+1: label Dec 11, 2025
Copilot AI review requested due to automatic review settings December 11, 2025 13:44
@bboozzoo bboozzoo force-pushed the bboozzoo/snap-confine-race-private-tmp branch from 87e8e0c to 763ab4a Compare December 11, 2025 13:46
@github-actions
Copy link

github-actions bot commented Dec 11, 2025

Mon Dec 15 12:21:34 UTC 2025
The following results are from: https://github.com/canonical/snapd/actions/runs/20158938593

Failures:

Preparing:

  • openstack:opensuse-15.6-64:
  • openstack:opensuse-15.6-64:
  • openstack:opensuse-15.6-64:tests/main/
  • openstack:opensuse-15.6-64:tests/smoke/

Executing:

  • openstack:debian-sid-64:tests/main/interfaces-network-status-classic
  • openstack:opensuse-tumbleweed-selinux-64:tests/main/auto-refresh-gating-from-snap
  • openstack:opensuse-tumbleweed-selinux-64:tests/main/snap-debug-raa
  • openstack:opensuse-tumbleweed-selinux-64:tests/main/auto-refresh-pre-download:ignore
  • openstack:opensuse-tumbleweed-selinux-64:tests/main/auto-refresh-pre-download:close_mid_restart
  • openstack:opensuse-tumbleweed-selinux-64:tests/main/snap-refresh-hold
  • openstack:opensuse-tumbleweed-selinux-64:tests/main/auto-refresh-pre-download:restart
  • openstack:opensuse-tumbleweed-selinux-64:tests/main/auto-refresh:regular
  • openstack:opensuse-tumbleweed-selinux-64:tests/main/auto-refresh-retry
  • openstack:opensuse-tumbleweed-selinux-64:tests/main/refresh-app-awareness-notify
  • openstack:opensuse-tumbleweed-selinux-64:tests/main/auto-refresh-backoff
  • openstack:opensuse-tumbleweed-selinux-64:tests/main/auto-refresh-gating
  • openstack:opensuse-tumbleweed-selinux-64:tests/main/auto-refresh-pre-download:close
  • openstack:opensuse-tumbleweed-selinux-64:tests/main/auto-refresh:parallel

Restoring:

  • openstack:opensuse-15.6-64:
  • openstack:opensuse-15.6-64:
  • openstack:opensuse-tumbleweed-selinux-64:tests/main/snap-debug-raa
  • openstack:opensuse-tumbleweed-selinux-64:tests/main/refresh-app-awareness-notify

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR addresses a race condition in snap-confine when multiple instances concurrently attempt to create the /tmp/snap-private-tmp directory. The fix changes the approach from failing immediately when observing non-conforming directory attributes to attempting to fix the ownership and permissions, which may be in an intermediate state due to the race.

Key Changes

  • Refactored the single ownership/permission validation into separate checks for type, ownership, and mode
  • Added defensive code to attempt fixing ownership and mode if they don't match expected values
  • Enhanced comments to explain the race condition and the two-stage directory creation process in sc_ensure_mkdir()

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

…te-tmp

When /tmp/snap-private-tmp does not exist, it will be created at runtime
by snap-confine. Specifically, when more than one instance of
snap-confine is active, both race to create the directory. Commit
canonical@ba1b2ff
changed the code to use sc_ensure_mkdir() helper. The helper, creates
the directory with 0000 mode bits, and proceeds to update its ownership
and mode to match what was requested. It is possible that when one
instance of snap-confine is in the middle of creating the directory,
another instance may observe an intermediate state in which either the
ownership or permissions have not yet been updated. Instead of failing,
let the other instance attempt to amend the ownership and permissions.

Related: SNAPDENG-36092
Fixes: LP#2134364

Signed-off-by: Maciej Borzecki <maciej.borzecki@canonical.com>
@bboozzoo bboozzoo force-pushed the bboozzoo/snap-confine-race-private-tmp branch from 763ab4a to 2f35623 Compare December 11, 2025 13:59
@codecov
Copy link

codecov bot commented Dec 11, 2025

Codecov Report

❌ Patch coverage is 0% with 10 lines in your changes missing coverage. Please review.
✅ Project coverage is 77.47%. Comparing base (7e4a7f6) to head (2f35623).
⚠️ Report is 8 commits behind head on master.

Files with missing lines Patch % Lines
cmd/snap-confine/mount-support.c 0.00% 8 Missing and 2 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master   #16367      +/-   ##
==========================================
- Coverage   77.48%   77.47%   -0.01%     
==========================================
  Files        1339     1338       -1     
  Lines      182935   182929       -6     
  Branches     2438     2442       +4     
==========================================
- Hits       141740   141732       -8     
- Misses      32610    32611       +1     
- Partials     8585     8586       +1     
Flag Coverage Δ
unittests 77.47% <0.00%> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@bboozzoo bboozzoo added the cross-distro Runs all spread systems in parallel label Dec 12, 2025
@bboozzoo bboozzoo closed this Dec 12, 2025
@bboozzoo bboozzoo reopened this Dec 12, 2025
Copy link
Collaborator

@pedronis pedronis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1

Copy link
Contributor

@jslarraz jslarraz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM considering that all instances will be racing toward the same final state, and no intermediate change makes the state to diverge from the target

@bboozzoo bboozzoo added the Auto rerun spread Auto reruns spread up to 4 times in non-draft PRs w/ >=1 approval and <20 fails in any fund. system label Dec 15, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Auto rerun spread Auto reruns spread up to 4 times in non-draft PRs w/ >=1 approval and <20 fails in any fund. system cross-distro Runs all spread systems in parallel Needs security review Can only be merged once security gave a :+1:

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants