All three approaches seem to parse it in user space, which makes sense. Easier to do it in mount.cifs than in kernel fs_context parsing On Tue, Oct 11, 2022 at 1:57 PM Enzo Matsumiya <ematsumiya@xxxxxxx> wrote: > > 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 -- Thanks, Steve
