Skip to content

s3:passdb: smbpasswd reset permissions only if not 0600

Browsing files or download files from samba server, smbd would check user's 
id to decide whether this user could access these files, by lookup user's 
information from the password file (e.g. /usr/local/samba/private/smbpasswd).
smbd might goes through startsmbfilepwent(), this api calls [f]chmod() to 
make sure the password file has valid permissions 0600.

Consider a scenario: we are doing a read performance benchmark about 
downloading a bunch of files (e.g. a thousand files) from a samba server,
monitoring file system i/o activities counters, and expecting that should 
be only read operations on file system because this is just downloading, no 
uploading is involved. But actually found that still write operations on file 
system, because smbd lookup user and always reset 0600 permissions on password 
file while access each file, it makes dirty pages (inode modification) in ram, 
later triggered a kernel journal daemon to sync dirty pages into back storage 
(e.g. ext3 kjournald, or ext4 jbd2).
This looks like not friendly for read performance benchmark if it happened on
an entry-level systems with much less memory and limited computation power,
because dirty pages syncing in the meantime slows down read performance.

This patch adds fstat() before [f]chmod(), it would check whether password
file has valid permissions 0600 or not. If 0600 smbd would bypass [f]chmod()
to avoid making dirty pages on file systems. If not 0600 smbd would warn and
go through [f]chmod() to set valid permissions 0600 to password file as
earlier days.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=15555

Checklist

  • Commits have Signed-off-by: with name/author being identical to the commit author
  • (optional) This MR is just one part towards a larger feature.
  • (optional, if backport required) Bugzilla bug filed and BUG: tag added
  • Test suite updated with functionality tests
  • Test suite updated with negative tests
  • Documentation updated
  • CI timeout is 3h or higher (see Settings/CICD/General pipelines/ Timeout)

Reviewer's checklist:

  • There is a test suite reasonably covering new functionality or modifications
  • Function naming, parameters, return values, types, etc., are consistent and according to README.Coding.md
  • This feature/change has adequate documentation added
  • No obvious mistakes in the code

Merge request reports