On Thu, May 17, 2012 at 11:44 AM, Sachin Prabhu <sprabhu@xxxxxxxxxx> wrote:
> On Fri, 2012-05-11 at 15:03 -0400, Jeff Layton wrote:
>> Traditionally, this ver= option was used to specify the "options
>> version" that we're passing in. It has always been set to '1' though
>> and we have never changed that.
>>
>> Eventually we want to have a ver= (or vers=) option that allows users
>> to specify the SMB version that they want to use to talk to the server.
>>
>> At that point, this option will just get in the way. Let's go ahead
>> and remove it now in preparation for that day.
>>
>
> Do we need 'ver=' mount option to specify the SMB version number? Isn't
> 'vers=' sufficient for this?
Yes - "vers" is sufficient (although I am fine with adding synonyms to
make it easier for nfs users - e.g. "smbvers=" or if we want to have
smb2 specific utility programs (e.g. for a phone or tablet) that call
do_mount automatically set vers=2.1.
I think that there will be times when we want (the kernel cifs.ko) to
know the version number of the user space utility (bugs? security
problems? debugging mount errors, at least when debugging set to
maximum level to log to dmesg?) so I think we should pass the real
mount.cifs version number in "ver=" as was the original intent (if for
nothing else it would help debugging mount bugs so we could log it to
dmesg in a few cases) - but since we have been sending "1" instead of
the mount.cifs version (and it is not currently used by the kernel
code) I can understand Jeff's point. In any case, we can't use "ver="
to mean "vers=" (ie the requested dialect the user wants to use) as if
so current mount.cifs will force cifs by sending "ver=1" and when we
want to move the default in the future it would get in the way
(cifs.ko would require an update of mount.cifs but we would have no
way of telling in kernel whether it was the old mount.cifs - kind of a
catch-22 due to the lack of version number)
>> Signed-off-by: Jeff Layton <jlayton@xxxxxxxxx>
>> ---
>> mount.cifs.c | 20 +++++++-------------
>> 1 files changed, 7 insertions(+), 13 deletions(-)
>>
>> diff --git a/mount.cifs.c b/mount.cifs.c
>> index 0408158..3041987 100644
>> --- a/mount.cifs.c
>> +++ b/mount.cifs.c
>> @@ -100,12 +100,6 @@
>> #define MAX_DOMAIN_SIZE 64
>>
>> /*
>> - * value of the ver= option that gets passed to the kernel. Used to indicate
>> - * behavioral changes introduced in the mount helper.
>> - */
>> -#define OPTIONS_VERSION "1"
>> -
>> -/*
>> * mount.cifs has been the subject of many "security" bugs that have arisen
>> * because of users and distributions installing it as a setuid root program
>> * before it had been audited for security holes. The default behavior is
>> @@ -1833,21 +1827,21 @@ assemble_mountinfo(struct parsed_mount_info *parsed_info,
>> goto assemble_exit;
>> }
>>
>> - /* copy in ver= string. It's not really needed, but what the hell */
>> - if (*parsed_info->options)
>> - strlcat(parsed_info->options, ",", sizeof(parsed_info->options));
>> - strlcat(parsed_info->options, "ver=", sizeof(parsed_info->options));
>> - strlcat(parsed_info->options, OPTIONS_VERSION, sizeof(parsed_info->options));
>> -
>> /* copy in user= string */
>> if (parsed_info->got_user) {
>> - strlcat(parsed_info->options, ",user=",
>> + if (*parsed_info->options)
>> + strlcat(parsed_info->options, ",",
>> + sizeof(parsed_info->options));
>> + strlcat(parsed_info->options, "user=",
>> sizeof(parsed_info->options));
>> strlcat(parsed_info->options, parsed_info->username,
>> sizeof(parsed_info->options));
>> }
>>
>> if (*parsed_info->domain) {
>> + if (*parsed_info->options)
>> + strlcat(parsed_info->options, ",",
>> + sizeof(parsed_info->options));
>> strlcat(parsed_info->options, ",domain=",
>> sizeof(parsed_info->options));
>> strlcat(parsed_info->options, parsed_info->domain,
--
Thanks,
Steve
--
To unsubscribe from this list: send the line "unsubscribe linux-cifs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
