On Wed, Oct 12, 2022 at 6:41 AM Leif Sahlberg <lsahlber@xxxxxxxxxx> wrote: > > On Wed, Oct 12, 2022 at 4:58 AM 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? > > The trouble with changing to 0x1e is that that would create a > dependency between mount.cifs and the kernel. > And that we would need to upgrade, or downgrade, them in lockstep > which will cause a lot of pain. > Old version of mount.cifs can not talk to new version of kernel and v.v. > > I think a better way to handle this would be to use an escape-character for ','. > (and remove all traces of "sep=") A good escape character would possibly be something i the range 0xf8 to 0xff as these byte values can not exist in utf8 encoded streams. > > > > > > > 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 > >
