On Sun, 15 May 2011 20:38:24 -0500
Steve French <smfrench@xxxxxxxxx> wrote:
> Probably too late for 2.6.39 for a patch to a non-default mount option
> (but I agree that it should go to stable). Thoughts?
>
You're proposing to fix it in 2.6.38-stable, but leave it broken in
2.6.39? That doesn't make much sense to me...
> Jeff's suggestion below makes sense.
>
> On Fri, May 13, 2011 at 7:17 PM, Jeff Layton <jlayton@xxxxxxxxxx> wrote:
> > On Fri, 13 May 2011 05:32:35 +0200
> > Stefan Metzmacher <metze@xxxxxxxxx> wrote:
> >
> >> Commit "cifs: fix unaligned accesses in cifsConvertToUCS"
> >> (84cdf74e8096a10dd6acbb870dd404b92f07a756) does multiple steps
> >> in just one commit (moving the function and changing it without testing).
> >>
> >> put_unaligned_le16(temp, &target[j]); is never called for any codepoint
> >> the goes via the 'default' switch statement. As a result we put
> >> just zero (or maybe uninitialized) bytes into the target buffer,
> >>
> >> Signed-off-by: Stefan Metzmacher <metze@xxxxxxxxx>
> >> ---
> >> Âfs/cifs/cifs_unicode.c | Â 20 ++++++++++----------
> >> Â1 files changed, 10 insertions(+), 10 deletions(-)
> >>
> >> diff --git a/fs/cifs/cifs_unicode.c b/fs/cifs/cifs_unicode.c
> >> index fc0fd4f..b1ff0bd 100644
> >> --- a/fs/cifs/cifs_unicode.c
> >> +++ b/fs/cifs/cifs_unicode.c
> >> @@ -276,6 +276,7 @@ cifsConvertToUCS(__le16 *target, const char *source, int maxlen,
> >> Â Â Â Â Â Â Â return cifs_strtoUCS(target, source, PATH_MAX, cp);
> >>
> >> Â Â Â for (i = 0, j = 0; i < maxlen; j++) {
> >> + Â Â Â Â Â Â charlen = 1;
> >> Â Â Â Â Â Â Â src_char = source[i];
> >> Â Â Â Â Â Â Â switch (src_char) {
> >> Â Â Â Â Â Â Â case 0:
> >> @@ -315,18 +316,17 @@ cifsConvertToUCS(__le16 *target, const char *source, int maxlen,
> >> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â temp = 0x003f;
> >> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â charlen = 1;
> >> Â Â Â Â Â Â Â Â Â Â Â }
> >> - Â Â Â Â Â Â Â Â Â Â len_remaining -= charlen;
> >> - Â Â Â Â Â Â Â Â Â Â /*
> >> - Â Â Â Â Â Â Â Â Â Â Â* character may take more than one byte in the source
> >> - Â Â Â Â Â Â Â Â Â Â Â* string, but will take exactly two bytes in the
> >> - Â Â Â Â Â Â Â Â Â Â Â* target string
> >> - Â Â Â Â Â Â Â Â Â Â Â*/
> >> - Â Â Â Â Â Â Â Â Â Â i += charlen;
> >> - Â Â Â Â Â Â Â Â Â Â continue;
> >> + Â Â Â Â Â Â Â Â Â Â break;
> >> Â Â Â Â Â Â Â }
> >> + Â Â Â Â Â Â /*
> >> + Â Â Â Â Â Â Â* character may take more than one byte in the source
> >> + Â Â Â Â Â Â Â* string, but will take exactly two bytes in the
> >> + Â Â Â Â Â Â Â* target string
> >> + Â Â Â Â Â Â Â*/
> >> Â Â Â Â Â Â Â put_unaligned_le16(temp, &target[j]);
> >> - Â Â Â Â Â Â i++; /* move to next char in source string */
> >> - Â Â Â Â Â Â len_remaining--;
> >> + Â Â Â Â Â Â /* move to next char in source string */
> >> + Â Â Â Â Â Â i += charlen;
> >> + Â Â Â Â Â Â len_remaining -= charlen;
> >> Â Â Â }
> >>
> >> ÂctoUCS_out:
> >
> > Oof. Yeah, I broke it alright -- good catch...
> >
> > This patch probably won't apply to the current head of the tree,
> > however. Rather than have a special patch for 2.6.38 and .39, what may
> > be best is to go ahead and push 581ade4d in Steve's master branch to
> > stable, and then apply something like this untested patch on top of it.
> >
> > I'll plan to test it out when I get a chance, but I think that's
> > probably the best approach.
> >
> > Thoughts?
> >
> > -----------------[snip]--------------------
> >
> > cifs: fix cifsConvertToUCS() for the mapchars case
> >
> > As Metze pointed out, commit 84cdf74e broke mapchars case:
> >
> > Â ÂCommit "cifs: fix unaligned accesses in cifsConvertToUCS"
> > Â Â(84cdf74e8096a10dd6acbb870dd404b92f07a756) does multiple steps
> > Â Âin just one commit (moving the function and changing it without
> > Â Âtesting).
> >
> > Â Âput_unaligned_le16(temp, &target[j]); is never called for any
> > Â Âcodepoint the goes via the 'default' switch statement. As a result
> > Â Âwe put just zero (or maybe uninitialized) bytes into the target
> > Â Âbuffer.
> >
> > His proposed patch looks correct, but doesn't apply to the current head
> > of the tree. This patch should also fix it.
> >
> > Reported-by: Stefan Metzmacher <metze@xxxxxxxxx>
> > Signed-off-by: Jeff Layton <jlayton@xxxxxxxxxx>
> > ---
> > Âfs/cifs/cifs_unicode.c | Â 14 ++++++--------
> > Â1 files changed, 6 insertions(+), 8 deletions(-)
> >
> > diff --git a/fs/cifs/cifs_unicode.c b/fs/cifs/cifs_unicode.c
> > index 23d43cd..1b2e180 100644
> > --- a/fs/cifs/cifs_unicode.c
> > +++ b/fs/cifs/cifs_unicode.c
> > @@ -277,6 +277,7 @@ cifsConvertToUCS(__le16 *target, const char *source, int srclen,
> >
> > Â Â Â Âfor (i = 0, j = 0; i < srclen; j++) {
> > Â Â Â Â Â Â Â Âsrc_char = source[i];
> > + Â Â Â Â Â Â Â charlen = 1;
> > Â Â Â Â Â Â Â Âswitch (src_char) {
> > Â Â Â Â Â Â Â Âcase 0:
> > Â Â Â Â Â Â Â Â Â Â Â Âput_unaligned(0, &target[j]);
> > @@ -316,16 +317,13 @@ cifsConvertToUCS(__le16 *target, const char *source, int srclen,
> > Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Âdst_char = cpu_to_le16(0x003f);
> > Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Âcharlen = 1;
> > Â Â Â Â Â Â Â Â Â Â Â Â}
> > - Â Â Â Â Â Â Â Â Â Â Â /*
> > - Â Â Â Â Â Â Â Â Â Â Â Â* character may take more than one byte in the source
> > - Â Â Â Â Â Â Â Â Â Â Â Â* string, but will take exactly two bytes in the
> > - Â Â Â Â Â Â Â Â Â Â Â Â* target string
> > - Â Â Â Â Â Â Â Â Â Â Â Â*/
> > - Â Â Â Â Â Â Â Â Â Â Â i += charlen;
> > - Â Â Â Â Â Â Â Â Â Â Â continue;
> > Â Â Â Â Â Â Â Â}
> > + Â Â Â Â Â Â Â /*
> > + Â Â Â Â Â Â Â Â* character may take more than one byte in the source string,
> > + Â Â Â Â Â Â Â Â* but will take exactly two bytes in the target string
> > + Â Â Â Â Â Â Â Â*/
> > + Â Â Â Â Â Â Â i += charlen;
> > Â Â Â Â Â Â Â Âput_unaligned(dst_char, &target[j]);
> > - Â Â Â Â Â Â Â i++; /* move to next char in source string */
> > Â Â Â Â}
> >
> > ÂctoUCS_out:
> > --
> > 1.7.4.4
> >
> > --
> > 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
> >
>
>
>
--
Jeff Layton <jlayton@xxxxxxxxxx>
--
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
