Skip to content

Added support of custom option separator for the cifs filesystem. #1836

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Added support of custom option separator for the cifs filesystem. #1836

wants to merge 1 commit into from

Conversation

dmitry-telegin
Copy link

DESCRIPTION OF THE PROBLEM

Some users are accustomed to using shared folders in Windows with a comma in the name, for example: "//server3/Block 3,4".
When they try to migrate to Linux, they cannot mount such paths.

An example of the line generated by "mount.cifs" for the kernel when mounting "//server3/Block 3,4":
"ip=10.0.2.212,unc=\server3\Block 3,4,iocharset=utf8,user=user1,domain=AD"
Accordingly, due to the extra comma, we have an error:
"Sep 7 21:57:18 S1 kernel: [ 795.604821] CIFS: Unknown mount option "4""

DOCUMENTATION

https://www.kernel.org/doc/html/latest/admin-guide/cifs/usage.html
The "sep" parameter is described here to specify a new delimiter instead of a comma.
I quote:
"sep
if first mount option (after the -o), overrides the comma as the separator between the mount parms. e.g.:
-o user=myname,password=mypassword,domain=mydom
could be passed instead with period as the separator by:
-o sep=.user=myname.password=mypassword.domain=mydom
this might be useful when comma is contained within username or password or domain."

RESEARCH WORK

I looked at the "mount.cifs" code. There is no provision for the use of a comma by the user, since the comma is used to form the parameter string to the kernel (man 2 mount). This line can be seen by adding the "--verbose" flag to the mount.
"mount.cifs --help" lists "sep" as a possible option, but does not implement it in the code and does not describe it in "man 8 mount.cifs".

I looked at the "pam-mount" code - the mount options are assembled with a wildcard comma. The result is a text line: "mount -t cifs ...".

The handling of options in the "mount" utility is based on the "libmount" library, which is hardcoded to use only a comma as a delimiter.

I tried to mount "//server3/Block 3,4" with my own program (man 2 mount) by specifying "sep=!" - successfully.

SOLUTION

It would be nice if we add in "mount.cifs", in "mount" utility and in "pam-mount" the ability to set a custom mount option separator. In other words, we need to implement the already documented "sep" option.

  1. For "mount.cifs" I did it in: https://github.com/dmitry-telegin/cifs-utils in branch "custom_option_separator". Commit: dmitry-telegin/cifs-utils@04325b8

  2. For "mount" utility I did it in: https://github.com/dmitry-telegin/util-linux in branch "custom_option_separator". Commit: dmitry-telegin@5e0ecd2

  3. For "pam-mount" I did it in: https://sourceforge.net/u/dmitry-t/pam-mount/ci/master/tree/ in branch "custom_option_separator". Commit: https://sourceforge.net/u/dmitry-t/pam-mount/ci/9860f9234977f1110230482b5d87bdcb8bc6ce03/

I checked the work on the Linux 5.10 kernel.

@karelzak
Copy link
Collaborator

For years we support commas in mount options as non-delimiters; a nice example is SELinux mount options. The value has to be quoted, for example:

 -o  'context="system_u:object_r:tmp_t:s0:c127,c456",noexec'

So, for your use case, you can try the following:

 -o 'ip=10.0.2.212,unc="\server3\Block 3,4",iocharset=utf8,user=user1,domain=AD'

but I have no clue if mount.cifs supports it, but libmount is ready for this.

@dmitry-telegin
Copy link
Author

I checked your suggestion. The Linux kernel does not accept it.

My test program:

#include <stdio.h>
#include <string.h>
#include <errno.h>
#include <sys/mount.h>

static const char *cifs_fstype = "cifs";

int main(void)
{
    int rc = 0;
    unsigned long flags = 0;
    char const *orig_dev = "//server3/test,3,3,";
    /* char const *options = "sep=;ip=10.0.2.212;unc=\\\\server3\\test,3,3,;iocharset=utf8;vers=1.0;uid=1000;user=user1;domain=AD;pass=Passw0rd"; */
    char const *options = "ip=10.0.2.212,unc=\"\\\\server3\\test,3,3,\",iocharset=utf8,vers=1.0,uid=1000,user=user1,domain=AD,pass=Passw0rd";

    printf("orig_dev: %s\n", orig_dev);
    printf("options: %s\n", options);
    rc = mount(orig_dev, "/mnt", cifs_fstype, flags, options);
    if (rc)
      printf("mount error(%d): %s\n", errno, strerror(errno));
    else
      printf("mount Ok\n");
  return 0;
}

Result:

$ sudo ./mount_cifs 
orig_dev: //server3/test,3,3,
options: ip=10.0.2.212,unc="\\server3\test,3,3,",iocharset=utf8,vers=1.0,uid=1000,user=user1,domain=AD,pass=Passw0rd
mount error(22): Invalid argument

$ sudo tail /var/log/syslog | grep CIFS
Oct 11 14:57:49 S1 kernel: [ 1317.682345] CIFS: Unknown mount option "3"

$ uname -a
Linux S1 4.19.0-20-amd64 #1 SMP Debian 4.19.235-1 (2022-03-17) x86_64 GNU/Linux

If I use the "sep" option, the mount succeeds.
In other words, the kernel already now supports the solution I proposed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@dmitry-telegin @karelzak