[linux-cifs-client] [PATCH 2/3] cifs: Make cifs_strlcpy_to_host
use this helper and fix incorrect NULL termination
Jeff Layton
jlayton at redhat.com
Tue Apr 21 16:15:38 GMT 2009
On Tue, 21 Apr 2009 17:55:15 +0530
Suresh Jayaraman <sjayaraman at suse.de> wrote:
> Make cifs_strlcpy_to_host use UniStrnlenBytes(). Also fix a bug
> introduced by a previous patch. The previous patch while fixing the
> buffer size, left out the NULL termination that is based on length of
> src buffer(in UCS characters) as it is. Fix this by using the length of
> dst buffer got from cifs_strfromUCS_le() for NULL termination.
>
>
> Signed-off-by: Suresh Jayaraman <sjayaraman at suse.de>
> ---
> fs/cifs/cifssmb.c | 29 +++++++++++++++++------------
> 1 files changed, 17 insertions(+), 12 deletions(-)
>
> diff --git a/fs/cifs/cifssmb.c b/fs/cifs/cifssmb.c
> index a02c43b..aab1d32 100644
> --- a/fs/cifs/cifssmb.c
> +++ b/fs/cifs/cifssmb.c
> @@ -91,26 +91,31 @@ static int
> cifs_strlcpy_to_host(char **dst, const char *src, const int maxlen,
> const bool is_unicode, const struct nls_table *nls_codepage)
> {
> - int plen;
> + int src_len, dst_len;
> + size_t nbytes;
>
> if (is_unicode) {
> - plen = UniStrnlen((wchar_t *)src, maxlen);
> - *dst = kmalloc((4 * plen) + 2, GFP_KERNEL);
> + nbytes = UniStrnlenBytes((wchar_t *)src, maxlen, &src_len,
> + nls_codepage);
> + *dst = kmalloc(nbytes + 2, GFP_KERNEL);
> if (!*dst)
> - goto cifs_strlcpy_to_host_ErrExit;
> - cifs_strfromUCS_le(*dst, (__le16 *)src, plen, nls_codepage);
> - (*dst)[plen] = 0;
> - (*dst)[plen+1] = 0; /* needed for Unicode */
> + goto err_exit;
> + dst_len = cifs_strfromUCS_le(*dst, (__le16 *)src, src_len,
> + nls_codepage);
> + /*
> + * cifs_strfromUCS_le() ensures single byte NULL termination
> + */
> + (*dst)[dst_len + 1] = 0; /* needed for Unicode, to be safe */
This is fine for now, but maybe we should just fix cifs_strfromUCS_le
to do the double-byte termination? Is a single-byte terminator ever
useful?
Fixing that will of course mean auditing all of the callers, but I
think they all have big enough buffers for this now. Right?
> } else {
> - plen = strnlen(src, maxlen);
> - *dst = kmalloc(plen + 2, GFP_KERNEL);
> + src_len = strnlen(src, maxlen);
> + *dst = kmalloc(src_len + 1, GFP_KERNEL);
> if (!*dst)
> - goto cifs_strlcpy_to_host_ErrExit;
> - strlcpy(*dst, src, plen);
> + goto err_exit;
> + strlcpy(*dst, src, src_len);
^^^^^^^
should be src_len+1.
> }
> return 0;
>
> -cifs_strlcpy_to_host_ErrExit:
> +err_exit:
> cERROR(1, ("Failed to allocate buffer for string\n"));
> return -ENOMEM;
> }
> _______________________________________________
> linux-cifs-client mailing list
> linux-cifs-client at lists.samba.org
> https://lists.samba.org/mailman/listinfo/linux-cifs-client
>
--
Jeff Layton <jlayton at redhat.com>
More information about the linux-cifs-client
mailing list