smbd failed in host when both lxd container and host have smbd

Bug #1792400 reported by Markus on 2018-09-13
14
This bug affects 1 person
Affects Status Importance Assigned to Milestone
samba (Ubuntu)
High
Unassigned
Trusty
Low
Unassigned
Xenial
High
Unassigned

Bug Description

[Impact]

 * Issue: the current init script
   * won't start samba related services on the host if there is a process
     of the same binary in a container
   * might on stop affect a process that it was not intended to stop

 * Solution: Fix init scripts to
   * start action to have a safer process detection with containers around
  * stop action to not affect unintended processes due to stale pidfiles

[Test Case]

 * 1. Start a container
 * 2. Start samba in the Container (or winbind or nmbd)
 * 3. Start samba in the host (or winbind or nmbd)
  => it will not start as such a binary is already running
 * #2 and #3 can be switched, and then as 4. restart smbd in the host
  => it will shut down but not re-start

Fixed: The container process should have no influence

 This also fixes issues where the pidfile would not be updated
 * install and start smbd
 * "Simulate" a corrupted pidfile by putting the PID of a different
   process in it
 * stop the sambd service
  => without the fixes this will drag down the other process you put in
     the pidfile

Fixed: a stale pidfile entry should not let non-smbd (or winbind, nmbd) processes be affected

[Regression Potential]

 * We tried to think of all edge cases of these start/stop actions but
   didn't come up with one that is broken. Aside from missing one of those
   cases there might be non-archive scripts that expect the old behavior.
   But even for thse no critical ones came to my mind so far.
   Worst case there'd be a combination that leads to the service
   no(re-)starting after the SRU - so thinking about potential cases is
   important.

[Other Info]

 * n/a

---

Setup: install smbd in host and lxd-container.

Now restart smbd in host:

service smbd restart
All is OK.
Problem: nmap shows "closed" on ports 139 and 445. And users cannot use smbd server in host.

  ● smbd.service - LSB: start Samba SMB/CIFS daemon (smbd)
   Loaded: loaded (/etc/init.d/smbd; bad; vendor preset: enabled)
   Active: active (exited) since Die 2016-10-18 17:35:23 CEST; 2s ago
     Docs: man:systemd-sysv-generator(8)
  Process: 24218 ExecStop=/etc/init.d/smbd stop (code=exited, status=0/SUCCESS)
  Process: 21980 ExecReload=/etc/init.d/smbd reload (code=exited, status=0/SUCCESS)
  Process: 25190 ExecStart=/etc/init.d/smbd start (code=exited, status=0/SUCCESS)

Okt 18 17:35:22 speedy systemd[1]: Starting LSB: start Samba SMB/CIFS daemon (smbd)...
Okt 18 17:35:23 speedy smbd[25190]: * Starting SMB/CIFS daemon smbd
Okt 18 17:35:23 speedy smbd[25190]: ...done.
Okt 18 17:35:23 speedy systemd[1]: Started LSB: start Samba SMB/CIFS daemon (smbd).

ps axf | grep smbd:

25356 pts/2 S+ 0:00 | \_ grep --color=auto smbd
19915 ? Ss 0:08 \_ /usr/sbin/smbd -D
19919 ? S 0:00 \_ /usr/sbin/smbd -D

However, netstat -tpln | grep "smbd" returns nothing and also nmap shows "closed" on ports 139 and 445.

Workaround [1]:
change /etc/init.d/smbd:
 if ! start-stop-daemon --start --quiet --oknodo --exec /usr/sbin/smbd -- -D ; then

to

 if ! start-stop-daemon --start --quiet --oknodo --pidfile /var/run/samba/smbd.pid --exec /usr/sbin/smbd -- -D ; then

I reported this to:
https://discuss.linuxcontainers.org/t/samba-in-host-and-container/2523

apt-cache policy samba
samba:
  Installed: 2:4.3.11+dfsg-0ubuntu0.16.04.15
  Candidate: 2:4.3.11+dfsg-0ubuntu0.16.04.16
  Version table:
     2:4.3.11+dfsg-0ubuntu0.16.04.16 500
        500 http://de.archive.ubuntu.com/ubuntu xenial-updates/main amd64 Packages
 *** 2:4.3.11+dfsg-0ubuntu0.16.04.15 500
        500 http://security.ubuntu.com/ubuntu xenial-security/main amd64 Packages
        100 /var/lib/dpkg/status
     2:4.3.8+dfsg-0ubuntu1 500
        500 http://de.archive.ubuntu.com/ubuntu xenial/main amd64 Packages

1. https://serverfault.com/questions/810544/samba-daemon-does-not-work-as-systemd-service-but-works-in-foreground

Related branches

Markus (markus212) on 2018-09-13
description: updated

This is "just one more" case of the common old init scripts didn't consider there could be more (=containers).

Interesting is that the stop already runs with the --pidfile so the stop will not kill it.

But the start will be blocked, as the existance of such a process will make --start be a no-op.

Man page:
Note: unless --pid or --pidfile are specified, start-stop-daemon behaves similar to killall(1). start-stop-daemon will scan the process table looking for any processes which match the process name, parent pid, uid, and/or gid (if specified). Any matching process will prevent --start from starting the daemon. All matching processes will be sent the TERM signal (or the one specified via --signal or --retry) if --stop is specified. For daemons which have long-lived children which need to live through a --stop, you must specify a pidfile.

-S, --start [--] arguments
Check for the existence of a specified process. If such a process exists, start-stop-daemon does nothing, and exits with error status 1 (0 if --oknodo is specified). If such a process does not exist, it starts an instance, using either the executable specified by --exec or, if specified, by --startas. Any arguments given after -- on the command line are passed unmodified to the program being started.

The --oknodo will make it a silent non fatal exit int hat case - as it is fine to run "start" if it is running already.

I'd recommend "--pidfile $SMBDPID" instead of the suggested path, but otherwise would agree to the fix.

It should be safe as that is essentially how later versions (Bionic) do it (via MAINPID tracking in systemd).

Changed in samba (Ubuntu):
status: New → Confirmed
tags: added: server-next
Changed in samba (Ubuntu Xenial):
status: New → Triaged
Changed in samba (Ubuntu):
status: Confirmed → Fix Released
Changed in samba (Ubuntu Trusty):
status: New → Triaged

This will eventually be interesting for Trusty as well (added a bug task), but lets discuss and fix Xenial first to avoid revising two branches all the time.

To avoid unintended processes to be acted on we should combine that with exec.
Man page:
-p, --pidfile pid-file
Check whether a process has created the file pid-file. Note: using this matching option alone might cause unintended processes to be acted on, if the old process terminated without being able to remove the pid-file.

Text would be like:
Let daemon start/stop actions only using --exec use --pidfile as well to avoid affecting or being affected by processed of the same name in containers. Let calls only using pidfile add --exec to avoid acting on processes reusing the PIDs.

There is some detail that I'm not sure it was intended or not and we have to be careful.
- debian/samba.samba-ad-dc.init
- debian/samba.smbd.init
On start they used --exec so they would block if "the other" was running
On stop they used --pidfile (so they only stop their own)
I wondered if that would be an issue, but those two init also share the PIDfile path, so it would not really behave differently after the change.

I opened an MP for review and discussions at [1].
In addition there is a xenial PPA to test from [2].

[1]: https://code.launchpad.net/~paelzer/ubuntu/+source/samba/+git/samba/+merge/355532
[2]: https://launchpad.net/~ci-train-ppa-service/+archive/ubuntu/3435

description: updated

Dep8 tests (rather basic) complete and ok.
Waiting for MP review and PPA testing now.

description: updated

The Xenial upload is in the SRU queue now.
Note: I'd start on Trusty after this completed in Xenial.

Hello Markus, or anyone else affected,

Accepted samba into xenial-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/samba/2:4.3.11+dfsg-0ubuntu0.16.04.17 in a few hours, and then in the -proposed repository.

Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation on how to enable and use -proposed.Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested and change the tag from verification-needed-xenial to verification-done-xenial. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed-xenial. In either case, details of your testing will help us make a better decision.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance!

Changed in samba (Ubuntu Xenial):
status: Triaged → Fix Committed
tags: added: verification-needed verification-needed-xenial
Download full text (8.9 KiB)

Without any containers it (re-)starts fine:

systemctl restart smbd; sleep 2s; systemctl status smbd
● smbd.service - LSB: start Samba SMB/CIFS daemon (smbd)
   Loaded: loaded (/etc/init.d/smbd; bad; vendor preset: enabled)
   Active: active (running) since Tue 2018-10-09 11:58:52 UTC; 2s ago
     Docs: man:systemd-sysv-generator(8)
  Process: 8925 ExecStop=/etc/init.d/smbd stop (code=exited, status=0/SUCCESS)
  Process: 8932 ExecStart=/etc/init.d/smbd start (code=exited, status=0/SUCCESS)
    Tasks: 3
   Memory: 21.7M
      CPU: 200ms
   CGroup: /system.slice/smbd.service
           ├─8943 /usr/sbin/smbd -D
           ├─8944 /usr/sbin/smbd -D
           └─8947 /usr/sbin/smbd -D

Oct 09 11:58:52 x systemd[1]: Starting LSB: start Samba SMB/CIFS daemon (smbd)...
Oct 09 11:58:52 x smbd[8932]: * Starting SMB/CIFS daemon smbd
Oct 09 11:58:52 x smbd[8932]: ...done.
Oct 09 11:58:52 x systemd[1]: Started LSB: start Samba SMB/CIFS daemon (smbd).

But if I add a container with a smb binary it will fail to restart:

● smbd.service - LSB: start Samba SMB/CIFS daemon (smbd)
   Loaded: loaded (/etc/init.d/smbd; bad; vendor preset: enabled)
   Active: active (exited) since Tue 2018-10-09 12:02:41 UTC; 2s ago
     Docs: man:systemd-sysv-generator(8)
  Process: 12732 ExecStop=/etc/init.d/smbd stop (code=exited, status=0/SUCCESS)
  Process: 12739 ExecStart=/etc/init.d/smbd start (code=exited, status=0/SUCCESS)

Oct 09 12:02:41 x systemd[1]: Starting LSB: start Samba SMB/CIFS daemon (smbd)...
Oct 09 12:02:41 x smbd[12739]: * Starting SMB/CIFS daemon smbd
Oct 09 12:02:41 x smbd[12739]: ...done.
Oct 09 12:02:41 x systemd[1]: Started LSB: start Samba SMB/CIFS daemon (smbd).

This will block (as initially reported) and be non-obvious until one realizes the process from the container is what blocks this.

Installing from proposed

root@x:~# apt install samba
Reading package lists... Done
Building dependency tree
Reading state information... Done
The following additional packages will be installed:
  libwbclient0 python-samba samba-common samba-common-bin samba-libs
Suggested packages:
  bind9 bind9utils ctdb ldb-tools smbldap-tools winbind heimdal-clients
Recommended packages:
  samba-dsdb-modules samba-vfs-modules
The following packages will be upgraded:
  libwbclient0 python-samba samba samba-common samba-common-bin samba-libs
6 upgraded, 0 newly installed, 0 to remove and 23 not upgraded.
Need to get 7759 kB of archives.
After this operation, 0 B of additional disk space will be used.
Do you want to continue? [Y/n] Y
Get:1 http://archive.ubuntu.com/ubuntu xenial-proposed/main amd64 python-samba amd64 2:4.3.11+dfsg-0ubuntu0.16.04.17 [1059 kB]
Get:2 http://archive.ubuntu.com/ubuntu xenial-proposed/main amd64 samba-common-bin amd64 2:4.3.11+dfsg-0ubuntu0.16.04.17 [506 kB]
Get:3 http://archive.ubuntu.com/ubuntu xenial-proposed/main amd64 samba-libs amd64 2:4.3.11+dfsg-0ubuntu0.16.04.17 [5172 kB]
Get:4 http://archive.ubuntu.com/ubuntu xenial-proposed/main amd64 libwbclient0 amd64 2:4.3.11+dfsg-0ubuntu0.16.04.17 [30.3 kB]
Get:5 http://archive.ubuntu.com/ubuntu xenial-proposed/main amd64 samba amd64 2:4.3.11+dfsg-0ubuntu0.16.04.17 [908 kB]
Get:6 http:...

Read more...

tags: added: verification-done verification-done-xenial
removed: verification-needed verification-needed-xenial
Łukasz Zemczak (sil2100) wrote :

I think the testing and reviews performed on this fix are sufficient to let it in without any additional aging. The changes are easily revertible in the worst case.

The verification of the Stable Release Update for samba has completed successfully and the package has now been released to -updates. Subsequently, the Ubuntu Stable Release Updates Team is being unsubscribed and will not receive messages about this bug report. In the event that you encounter a regression using the package from -updates please report a new bug using ubuntu-bug and tag the bug report regression-update so we can easily find any regressions.

Launchpad Janitor (janitor) wrote :

This bug was fixed in the package samba - 2:4.3.11+dfsg-0ubuntu0.16.04.17

---------------
samba (2:4.3.11+dfsg-0ubuntu0.16.04.17) xenial; urgency=medium

  * d/samba.nmbd.init, d/samba.samba-ad-dc.init, d/samba.smbd.init, d/winbind.init
    avoid issues due to init scripts misdetecting services (LP: #1792400)
    - use --pidfile on --start to not block on same binaries running in
      containers
    - use --exec on --stop to not cause unintended processes to be acted on,
      if the old process terminated without being able to remove the pid-file.

 -- Christian Ehrhardt <email address hidden> Mon, 24 Sep 2018 12:08:45 +0200

Changed in samba (Ubuntu Xenial):
status: Fix Committed → Fix Released

Autopkgtests for trusty (on bileto ticket) complete, no problems going forward - waiting on MP review.

Actually MP completed since I last refreshed the page, so all complete uploading for SRU Team

Hello Markus, or anyone else affected,

Accepted samba into trusty-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/samba/2:4.3.11+dfsg-0ubuntu0.14.04.18 in a few hours, and then in the -proposed repository.

Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation on how to enable and use -proposed.Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested and change the tag from verification-needed-trusty to verification-done-trusty. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed-trusty. In either case, details of your testing will help us make a better decision.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance!

Changed in samba (Ubuntu Trusty):
status: Triaged → Fix Committed
tags: added: verification-needed verification-needed-trusty
removed: verification-done

Tested an upgrade from proposed.
- upgrade worked
- Primary on trusty the upstart jobs are important which are not affected
- If I force execution of the sysV code (hacky) I'm affected and the proposed change from proposed fixes it
- So that isn't an issue for the majority of users, but still an issue since the same code is delivered. Looking back we might have skipped it on Trusty entirely, but it is not "wrong" to fix it
- on the positive side we also have not heard an issue on Xenial which got the same change recently.

Setting verified.

Example showing that the default upstart isn't affected:

ubuntu@T-smb-nested:~$ ps axlf | grep smbd
0 1000 13315 3315 20 0 10504 892 pipe_w S+ pts/0 0:00 | \_ grep --color=auto smbd
0 165536 13311 13204 20 0 7604 576 - S pts/6 0:00 \_ /usr/sbin/smbd 1h
4 0 13291 1 20 0 316760 9108 - Ss ? 0:00 smbd -F
1 0 13300 13291 20 0 308676 2840 - S ? 0:00 \_ smbd -F
1 0 13302 13291 20 0 316760 3080 - S ? 0:00 \_ smbd -F

$ sudo service smbd status; echo ""; sudo service smbd restart; echo ""; sudo service smbd status
smbd start/running, process 13291

smbd stop/waiting
smbd start/running, process 13334

smbd start/running, process 13334

So as I outlined before, not that impacted on Trusty, but for the sake of fixing code that the packages deliver pushing the fix is still correct.

If the SRU Team disagrees we could as well cancel the SRU at this stage and remove it from proposed - sorry for the noise then.

tags: added: verification-done verification-done-trusty
removed: verification-needed verification-needed-trusty
Robie Basak (racb) wrote :

Thank you for noting the upstart interaction here. I agree that the fix is correct, but I'm not sure the SRU is justified then. I'm not keen on recommending users automatically download and install extra stuff when it's almost certain no user needs it. Therefore I think we should cancel this SRU. The Trusty task can be set to Invalid even perhaps, because the issue isn't believed to affect any Trusty users (upstart is the only supported init system on Ubuntu Trusty).

If an update is made to Trusty's samba package for any other reason (security update or a different SRU) then it's fine to bundle this one.

tags: added: verification-failed-trusty
removed: verification-done-trusty
Robie Basak (racb) wrote :

I see no particular reason to remove the package from trusty-proposed. It's tested to be correct and is what we'd want bundled anyway.

Changed in samba (Ubuntu Trusty):
status: Fix Committed → Invalid
Changed in samba (Ubuntu Trusty):
importance: Undecided → Low
Changed in samba (Ubuntu):
importance: Undecided → High
Changed in samba (Ubuntu Xenial):
importance: Undecided → High
Changed in samba (Ubuntu Trusty):
status: Invalid → Won't Fix

Thank you for taking the time to verify this stable release fix. We have noticed that you have used the verification-done tag for marking the bug as verified and would like to point out that due to a recent change in SRU bug verification policy fixes now have to be marked with per-release tags (i.e. verification-done-$RELEASE). Please remove the verification-done tag and add one for the release you have tested the package in. Thank you!

https://wiki.ubuntu.com/StableReleaseUpdates#Verification

tags: removed: verification-done
To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers