| Message ID | 49EC7785.2080304@suse.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, 20 Apr 2009 18:54:21 +0530 Suresh Jayaraman <sjayaraman@suse.de> wrote: > There is a possibility for the path_name and node_name buffers to > overflow if they contain charcters that are >2 bytes in the local > charset. Resize the buffer allocation so to avoid this possibility. > > Also, as pointed out by Jeff Layton, it would be appropriate to > rename the function to cifs_strlcpy_to_host to reflect the fact > that the copied string is always NULL terminated. > > > Signed-off-by: Suresh Jayaraman <sjayaraman@suse.de> > --- > fs/cifs/cifssmb.c | 20 ++++++++++---------- > 1 files changed, 10 insertions(+), 10 deletions(-) > > diff --git a/fs/cifs/cifssmb.c b/fs/cifs/cifssmb.c > index a0845dc..a02c43b 100644 > --- a/fs/cifs/cifssmb.c > +++ b/fs/cifs/cifssmb.c > @@ -88,29 +88,29 @@ static struct { > * on failure - errno > */ > static int > -cifs_strncpy_to_host(char **dst, const char *src, const int maxlen, > +cifs_strlcpy_to_host(char **dst, const char *src, const int maxlen, > const bool is_unicode, const struct nls_table *nls_codepage) > { > int plen; > > if (is_unicode) { > plen = UniStrnlen((wchar_t *)src, maxlen); > - *dst = kmalloc(plen + 2, GFP_KERNEL); > + *dst = kmalloc((4 * plen) + 2, GFP_KERNEL); > if (!*dst) > - goto cifs_strncpy_to_host_ErrExit; > + 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 */ > } else { > plen = strnlen(src, maxlen); > *dst = kmalloc(plen + 2, GFP_KERNEL); > if (!*dst) > - goto cifs_strncpy_to_host_ErrExit; > - strncpy(*dst, src, plen); > + goto cifs_strlcpy_to_host_ErrExit; > + strlcpy(*dst, src, plen); > } > - (*dst)[plen] = 0; > - (*dst)[plen+1] = 0; /* harmless for ASCII case, needed for Unicode */ > return 0; > > -cifs_strncpy_to_host_ErrExit: > +cifs_strlcpy_to_host_ErrExit: > cERROR(1, ("Failed to allocate buffer for string\n")); > return -ENOMEM; > } > @@ -4029,7 +4029,7 @@ parse_DFS_referrals(TRANSACTION2_GET_DFS_REFER_RSP *pSMBr, > /* copy DfsPath */ > temp = (char *)ref + le16_to_cpu(ref->DfsPathOffset); > max_len = data_end - temp; > - rc = cifs_strncpy_to_host(&(node->path_name), temp, > + rc = cifs_strlcpy_to_host(&(node->path_name), temp, > max_len, is_unicode, nls_codepage); > if (rc) > goto parse_DFS_referrals_exit; > @@ -4037,7 +4037,7 @@ parse_DFS_referrals(TRANSACTION2_GET_DFS_REFER_RSP *pSMBr, > /* copy link target UNC */ > temp = (char *)ref + le16_to_cpu(ref->NetworkAddressOffset); > max_len = data_end - temp; > - rc = cifs_strncpy_to_host(&(node->node_name), temp, > + rc = cifs_strlcpy_to_host(&(node->node_name), temp, > max_len, is_unicode, nls_codepage); > if (rc) > goto parse_DFS_referrals_exit; > _______________________________________________ > linux-cifs-client mailing list > linux-cifs-client@lists.samba.org > https://lists.samba.org/mailman/listinfo/linux-cifs-client > Acked-by: Jeff Layton <jlayton@redhat.com>
diff --git a/fs/cifs/cifssmb.c b/fs/cifs/cifssmb.c index a0845dc..a02c43b 100644 --- a/fs/cifs/cifssmb.c +++ b/fs/cifs/cifssmb.c @@ -88,29 +88,29 @@ static struct { * on failure - errno */ static int -cifs_strncpy_to_host(char **dst, const char *src, const int maxlen, +cifs_strlcpy_to_host(char **dst, const char *src, const int maxlen, const bool is_unicode, const struct nls_table *nls_codepage) { int plen; if (is_unicode) { plen = UniStrnlen((wchar_t *)src, maxlen); - *dst = kmalloc(plen + 2, GFP_KERNEL); + *dst = kmalloc((4 * plen) + 2, GFP_KERNEL); if (!*dst) - goto cifs_strncpy_to_host_ErrExit; + 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 */ } else { plen = strnlen(src, maxlen); *dst = kmalloc(plen + 2, GFP_KERNEL); if (!*dst) - goto cifs_strncpy_to_host_ErrExit; - strncpy(*dst, src, plen); + goto cifs_strlcpy_to_host_ErrExit; + strlcpy(*dst, src, plen); } - (*dst)[plen] = 0; - (*dst)[plen+1] = 0; /* harmless for ASCII case, needed for Unicode */ return 0; -cifs_strncpy_to_host_ErrExit: +cifs_strlcpy_to_host_ErrExit: cERROR(1, ("Failed to allocate buffer for string\n")); return -ENOMEM; } @@ -4029,7 +4029,7 @@ parse_DFS_referrals(TRANSACTION2_GET_DFS_REFER_RSP *pSMBr, /* copy DfsPath */ temp = (char *)ref + le16_to_cpu(ref->DfsPathOffset); max_len = data_end - temp; - rc = cifs_strncpy_to_host(&(node->path_name), temp, + rc = cifs_strlcpy_to_host(&(node->path_name), temp, max_len, is_unicode, nls_codepage); if (rc) goto parse_DFS_referrals_exit; @@ -4037,7 +4037,7 @@ parse_DFS_referrals(TRANSACTION2_GET_DFS_REFER_RSP *pSMBr, /* copy link target UNC */ temp = (char *)ref + le16_to_cpu(ref->NetworkAddressOffset); max_len = data_end - temp; - rc = cifs_strncpy_to_host(&(node->node_name), temp, + rc = cifs_strlcpy_to_host(&(node->node_name), temp, max_len, is_unicode, nls_codepage); if (rc) goto parse_DFS_referrals_exit;
There is a possibility for the path_name and node_name buffers to overflow if they contain charcters that are >2 bytes in the local charset. Resize the buffer allocation so to avoid this possibility. Also, as pointed out by Jeff Layton, it would be appropriate to rename the function to cifs_strlcpy_to_host to reflect the fact that the copied string is always NULL terminated. Signed-off-by: Suresh Jayaraman <sjayaraman@suse.de> --- fs/cifs/cifssmb.c | 20 ++++++++++---------- 1 files changed, 10 insertions(+), 10 deletions(-)