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@xxxxxxxxx> 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
