Re: A patch to implement the already documented "sep" option for the CIFS file system.

[Enzo Matsumiya <ematsumiya@suse.de>]

On 10/11, Steve French wrote:
>makes sense.
>
>Did anyone else review this yet?  (the mount.cifs version of the patch)

At a glance, the patch seems ok and solves a real problem.

However, I think a better approach would be to parse the string in user
space, i.e. it's much easier for mount.cifs to fetch what's the
UNC/password string if they're passed quoted (shell handles it), and
then use 0x1E (ASCII Record Separator) instead of a comma.  Then, in
the kernel, we'd only need to strsep() by 0x1E.  Since 0x1E is a
non-printable ASCII character, I have a hard assumption it's not allowed
as UNC/password in most systems.  Also since it would be only used
internally between mount.cifs and cifs.ko, users would not need to know
nor care about it.

Thoughts?


Enzo

>On Sat, Oct 8, 2022 at 2:41 PM Dmitry Telegin
><dmitry.s.telegin@gmail.com> wrote:
>>
>> 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:
>> https://github.com/dmitry-telegin/cifs-utils/commit/04325b842a82edf655a14174e763bc0b2a6870e1
>>
>> 2. For "mount" utility I did it in:
>> https://github.com/dmitry-telegin/util-linux in branch
>> "custom_option_separator". Commit:
>> https://github.com/dmitry-telegin/util-linux/commit/5e0ecd2498edae0bf0bcab4ba6a68a9803b34ccf
>>
>> 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.
>>
>> --
>> Dmitry Telegin
>
>
>
>-- 
>Thanks,
>
>Steve