On Tue, 2012-06-12 at 07:15 +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. > > Changes since -v1 > > - removed the wrong strlen() micro optimization. > > Signed-off-by: Suresh Jayaraman <[email protected]> > Cc: Sachin Prabhu <[email protected]> > --- > fs/cifs/connect.c | 32 +++++++++++++++++--------------- > 1 files changed, 17 insertions(+), 15 deletions(-) > > diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c > index 78db68a..5b38407 100644 > --- a/fs/cifs/connect.c > +++ b/fs/cifs/connect.c > @@ -1653,24 +1653,26 @@ cifs_parse_mount_options(const char *mountdata, const > char *devname, > * 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);
Another way would have been to leave the earlier optimisation in place but simply reset temp_len within the if condition. This means the second strlen was called only in case we had encountered a double delimiter in the password. In any case, this code is called only once at mount time and hence the optimisation is not terribly important in this case. Acked-by: Sachin Prabhu <[email protected]> > -- > 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 -- 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
