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

[Jeff Layton]

On Mon, 06 Apr 2009 12:35:58 +0530
Suresh Jayaraman <sjayaraman at suse.de> wrote:

> The upstream commit b363b3304bcf68c4541683b2eff70b29f0446a5b attempted
> to fix memory overwrite during tree connect response processing while
> mounting. However, the memory allocated may still be insufficient as
> UTF-8 string can be upto 4X times as UCS. So, would it be safe to
> allocate memory that is 4X instead of 2X?
> 
> Noticed by Marcus Meissner <meissner at suse.de>.
> 
> Signed-off-by: Suresh Jayaraman <sjayaraman at suse.de>
> ---
>  fs/cifs/connect.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
> index 0de3b56..b361be0 100644
> --- a/fs/cifs/connect.c
> +++ b/fs/cifs/connect.c
> @@ -3674,7 +3674,7 @@ CIFSTCon(unsigned int xid, struct cifsSesInfo *ses,
>  			    BCC(smb_buffer_response)) {
>  				kfree(tcon->nativeFileSystem);
>  				tcon->nativeFileSystem =
> -				    kzalloc(2*(length + 1), GFP_KERNEL);
> +				    kzalloc((4 * length) + 2, GFP_KERNEL);
>  				if (tcon->nativeFileSystem)
>  					cifs_strfromUCS_le(
>  						tcon->nativeFileSystem,

Wait...is this even enough? It looks like nls.h defines this:

/* this value hold the maximum octet of charset */
#define NLS_MAX_CHARSET_SIZE 6 /* for UTF-8 */

...it really looks like this needs to use the same constant.

There are other places in this code that make this sort of allocation.
Could you audit and fix them too? A better solution is really needed
here.

A helper function that basically does the allocation and buffer-length
limited conversion would be ideal. We have some functions that sort of
do this, but none of them seem to be quite right. Maybe the best thing
is just to fix cifs_strncpy_to_host() so that it's right and fix most
of the places that do this allocation manually to do it using that
function instead.

-- 
Jeff Layton <jlayton at redhat.com>