On Mon, 2012-06-11 at 21:02 +0530, Suresh Jayaraman wrote:
> The double delimiter check that allows a comma in the password parsing code is
> unconditional. We set "tmp_end" to the end of the string and we continue to
> check for double delimiter. In the case where the password doesn't contain a
> comma we end up setting tmp_end to NULL and eventually setting "options" to
> "end". This results in the premature termination of the options string and hence
> the values of UNCip and UNC are being set to NULL. This results in mount failure
> with "Connecting to DFS root not implemented yet" error.
>
> This error is usually not noticable as we have password as the last option in
> the superblock mountdata. But when we call expand_dfs_referral() from
> cifs_mount() and try to compose mount options for the submount, the resulting
> mountdata will be of the form
>
> ",ver=1,user=foo,pass=bar,ip=x.x.x.x,unc=\\server\share"
>
> and hence results in the above error. This bug has been seen with older NAS
> servers running Samba 3.0.24.
>
> Fix this by moving the double delimiter check inside the conditional loop.
> Also move the assignment of temp_len above so that we need not call
> strlen(value) twice.
>
> Signed-off-by: Suresh Jayaraman <sjayaraman@xxxxxxxx>
> ---
> fs/cifs/connect.c | 37 ++++++++++++++++++++-----------------
> 1 files changed, 20 insertions(+), 17 deletions(-)
>
> diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
> index 78db68a..fdbbb56 100644
> --- a/fs/cifs/connect.c
> +++ b/fs/cifs/connect.c
> @@ -1646,34 +1646,37 @@ cifs_parse_mount_options(const char *mountdata, const char *devname,
> value = strchr(data, '=');
> value++;
>
> + temp_len = strlen(value);
> +
> /* Set tmp_end to end of the string */
> - tmp_end = (char *) value + strlen(value);
> + tmp_end = (char *) value + temp_len;
>
> /* Check if following character is the deliminator
> * If yes, we have encountered a double deliminator
> * reset the NULL character to the deliminator
> */
> - if (tmp_end < end && tmp_end[1] == delim)
> + if (tmp_end < end && tmp_end[1] == delim) {
> tmp_end[0] = delim;
>
> - /* Keep iterating until we get to a single deliminator
> - * OR the end
> - */
> - while ((tmp_end = strchr(tmp_end, delim)) != NULL &&
> - (tmp_end[1] == delim)) {
> - tmp_end = (char *) &tmp_end[2];
> - }
> + /* Keep iterating until we get to a single
> + * deliminator OR the end
> + */
> + while ((tmp_end = strchr(tmp_end, delim))
> + != NULL && (tmp_end[1] == delim)) {
> + tmp_end = (char *) &tmp_end[2];
> + }
>
> - /* Reset var options to point to next element */
> - if (tmp_end) {
> - tmp_end[0] = '\0';
> - options = (char *) &tmp_end[1];
> - } else
> - /* Reached the end of the mount option string */
> - options = end;
> + /* Reset var options to point to next element */
> + if (tmp_end) {
> + tmp_end[0] = '\0';
> + options = (char *) &tmp_end[1];
> + } else
> + /* Reached the end of the mount option
> + * string */
> + options = end;
> + }
>
> /* Now build new password string */
> - temp_len = strlen(value);
The bug fix is correct but you cannot optimise the second strlen() call
in this manner since at this point if we had encountered a double
delimiter the end of the value string is shifted down further until we
hit a single instance of the delimiter. The length of the password at
this stage in this case is different to the one calculated above.
> vol->password = kzalloc(temp_len+1, GFP_KERNEL);
> if (vol->password == NULL) {
> printk(KERN_WARNING "CIFS: no memory "
Nack.
Sachin Prabhu
--
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
