Skip to content

nspawn: don't hide --bind=/tmp/* mounts#4824

Merged
poettering merged 1 commit intosystemd:masterfrom
evverx:fix-4789
Dec 5, 2016
Merged

nspawn: don't hide --bind=/tmp/* mounts#4824
poettering merged 1 commit intosystemd:masterfrom
evverx:fix-4789

Conversation

@evverx
Copy link
Contributor

@evverx evverx commented Dec 5, 2016

Fixes #4789

@evverx
Copy link
Contributor Author

evverx commented Dec 5, 2016

[systemd-pr-build] $ /bin/sh -xe /tmp/hudson2438376146086078148.sh
+ /home/systemd/jenkins-glue.sh
Traceback (most recent call last):
  File "./slave-control.py", line 193, in <module>
    main()
  File "./slave-control.py", line 131, in main
    data = json.loads(json_data)
  File "/usr/lib64/python2.7/json/__init__.py", line 338, in loads
    return _default_decoder.decode(s)
  File "/usr/lib64/python2.7/json/decoder.py", line 365, in decode
    obj, end = self.raw_decode(s, idx=_w(s, 0).end())
  File "/usr/lib64/python2.7/json/decoder.py", line 383, in raw_decode
    raise ValueError("No JSON object could be decoded")
ValueError: No JSON object could be decoded

@zonque , please take a look.
json_data = duffy_cmd("/Node/get", params) is not working

{ "/proc/sysrq-trigger", "/proc/sysrq-trigger", NULL, NULL, MS_BIND, MOUNT_IN_USERNS|MOUNT_APPLY_APIVFS_RO }, /* Bind mount first ...*/
{ NULL, "/proc/sysrq-trigger", NULL, NULL, MS_BIND|MS_RDONLY|MS_NOSUID|MS_NOEXEC|MS_NODEV|MS_REMOUNT, MOUNT_IN_USERNS|MOUNT_APPLY_APIVFS_RO }, /* ... then, make it r/o */
{ "tmpfs", "/tmp", "tmpfs", "mode=1777", MS_STRICTATIME, MOUNT_FATAL|MOUNT_IN_USERNS },
{ "tmpfs", "/tmp", "tmpfs", "mode=1777", MS_STRICTATIME, MOUNT_FATAL },
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I should move this line to the "outer child"-part. Will fix

@martinpitt
Copy link
Contributor

Fedora test is broken, rest is okay.

@haraldh
Copy link
Member

haraldh commented Dec 5, 2016

@systemd-centos-ci
retest this please

@poettering
Copy link
Member

All looks good. merging.

@evverx i wonder though whether we can find a fix for the problem you tried to fix with 8492849? I mean, there was a good reason to make that change after all.

I wonder if there's a nice way to detect "foreign" mounts, so that PID 1 in the container could exclude them from umount.target Conflicts= lines. Or alternatively, we could add some logic that makes /tmp a user mount only when it is safe. I.e. basicely reuse your suggested logic, but conditionalize it on the whether there are any mounts configured with /tmp as prefix?

@poettering poettering merged commit c9fd987 into systemd:master Dec 5, 2016
@evverx evverx deleted the fix-4789 branch December 5, 2016 21:33
@evverx
Copy link
Contributor Author

evverx commented Dec 5, 2016

I wonder if there's a nice way to detect "foreign" mounts, so that PID 1 in the container could exclude them from umount.target Conflicts= lines

I think the only reliable way is to ask nspawn via some communication channel. But that's the nspawn-specific way and it doesn't fix something like #4810

@poettering
Copy link
Member

I think the only reliable way is to ask nspawn via some communication channel. But that's the nspawn-specific way and it doesn't fix something like #4810

I wonder if /proc/self/mountinfo in some way let's us know this...

@evverx
Copy link
Contributor Author

evverx commented Dec 6, 2016

Hm, that's a good question. I didn't think about it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

4 participants