Bug 57374 - Broken smb.conf during samba shares update
Summary: Broken smb.conf during samba shares update
Status: NEW
Alias: None
Product: UCS
Classification: Unclassified
Component: Samba4
Version: UCS 5.0
Hardware: Other Linux
: P5 normal
Target Milestone: ---
Assignee: Samba maintainers
QA Contact: Samba maintainers
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2024-06-03 14:11 CEST by Jürn Brodersen
Modified: 2025-02-24 11:08 CET (History)
1 user (show)

See Also:
What kind of report is it?: Bug Report
What type of bug is this?: 4: Minor Usability: Impairs usability in secondary scenarios
Who will be affected by this bug?: 1: Will affect a very few installed domains
How will those affected feel about the bug?: 2: A Pain – users won’t like this once they notice it
User Pain: 0.046
Enterprise Customer affected?:
School Customer affected?:
ISV affected?:
Waiting Support:
Flags outvoted (downgraded) after PO Review:
Ticket number:
Bug group (optional):
Customer ID:
Max CVSS v3 score:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Jürn Brodersen univentionstaff 2024-06-03 14:11:24 CEST
Broken smb.conf during samba shares update.

Found by Stefan: https://forge.univention.org/bugzilla/show_bug.cgi?id=57367#c0

The share configuration is deleted here:
https://git.knut.univention.de/univention/ucs/-/blob/5.0-7/services/univention-samba4/samba-shares.py?ref_type=heads#L137
and later rewritten here:
https://git.knut.univention.de/univention/ucs/-/blob/5.0-7/services/univention-samba4/samba-shares.py?ref_type=heads#L168

During that time frame, the include in shares.conf points to a non existing file and the following code fails:
```
>>> from samba.param import LoadParm
>>> lp = LoadParm()
>>> lp.load_default()
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
RuntimeError: Unable to load default file
```


I expect that time frame to be very short, so it might not be the reason for bug 57367. Though I think it still makes sense to fix this just to make sure it is not the problem.
Comment 1 Jürn Brodersen univentionstaff 2025-02-04 17:11:23 CET
Note: lp.load_default() doesn't throw an error in 5.2 anymore. It seems to be quite resilient now and just ignores any broken includes.
Comment 2 Sönke Schwardt-Krummrich univentionstaff 2025-02-24 11:08:17 CET
It's not only that the include file might be missing:
The code in samba-shares.py does not work with a temp file and atomic file handling. Therefore it's possible that due to concurrency the samba process reads an include file that is only halfway written to disk, which might also create followup problems (access control etc).

Suggestion:
- create temp file for new/updated include file
- use os.rename to move it atomically to final position