[linux-cifs-client] Null-Terminating bcc_ptr wrong?

[Jeff Layton]

On Fri, 10 Apr 2009 10:56:53 -0500
"Christopher R. Hertel" <crh at ubiqx.mn.org> wrote:

> Peter Hudec wrote:
> > Christopher R. Hertel wrote:
> >> This is a tangent, but...
> >>
> >> It shouldn't be UTF-8.  CIFS (the protocol) supports UCS-16LE Unicode
> >> encoding or 8-bit extended ASCII (OEM charset).
> >>
> >> Chris -)-----
> > The protocol SMB/CIFS uses UCS-2LE (to be correct), but the conversion
> > may be done to UTF-8.
> 
> So, I double checked.  :)
> 
> You are correct that there is no "UCS-16LE".  Ooops.  What I meant to write
> was UTF-16LE.  According to current docs ([MS-UCODEREF]), Microsoft supports
> full UTF-16 as opposed to UCS-2.
> 
> > The UCS-2LE-string from the packet can grow up to 4 bytes/character or
> > twice as big as UCS-2LE (it can - it does not necessarily do so!)
> 
> UCS-2 does not support surrogate pairs, so the encoding is actually fixed at
> 16 bits per code point.  UTF-16 does support surrogate pairs, which is where
> the expansion becomes possible.
> 
> > To determine the size of the target buffer without calculating it the
> > correct line should be:
> > 
> > tcon->nativeFileSystem = kzalloc(4 * (length + 1), GFP_KERNEL);
> 
> That'd be the safe way, though the terminating nul would still be only 2
> bytes so you could subtract two from the total.  ((4 * (length + 1)) - 2)
> 
> > tcon->nativeFileSystem is a buffer in the specific codepage. If it is
> > ISO 5598-1, e.g., it is fine, because it needs 8 bit (1 byte) per
> > character. If it is UTF-8, and only consists of ASCII (like "NTFS",
> > "FAT32"), it is fine too and only needs one byte per character.
> > But in a worse case (is there any file system containing special
> > characters? I don't know any) the buffer can grow up to 4 bytes per
> > character.
> 
> I know of one proposed file system that has a name that would require
> Unicode (and probably four-byte encoding).  It's only a dream at this point,
> however, so it's not relevant.
> 
> Fun.

Note that we've already been discussing similar problems on-list earlier
this week. See this thread:

[PATCH] cifs: Fix insufficient memory  allocation for nativeFileSystem field

The consensus so far is that the current scheme for handling CIFS
strings is a real mess, and we need some good helper functions that
make this less error prone. I believe Suresh J. is working on patches
and is auditing the existing code.

-- 
Jeff Layton <jlayton at redhat.com>