On 06/11/2012 11:48 PM, Sachin Prabhu wrote: > 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 <[email protected]> >> --- >> 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
Doh, you're right. I'll respin the fixed one. -- To unsubscribe from this list: send the line "unsubscribe linux-cifs" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
