strlcpy() reads the entire source buffer first. This read may exceed the
destination size limit. This is both inefficient and can lead to linear
read overflows if a source string is not NUL-terminated.
Also, the strnlen() call does not avoid the read overflow in the strlcpy
function when a not NUL-terminated string is passed.
So, replace this block by a call to kstrndup() that avoids this type of
overflow and does the same.
Fixes: 066ce6899484d ("cifs: rename cifs_strlcpy_to_host and make it use new functions")
Signed-off-by: Len Baker <[email protected]>
---
fs/cifs/cifs_unicode.c | 9 ++-------
1 file changed, 2 insertions(+), 7 deletions(-)
diff --git a/fs/cifs/cifs_unicode.c b/fs/cifs/cifs_unicode.c
index 9bd03a231032..171ad8b42107 100644
--- a/fs/cifs/cifs_unicode.c
+++ b/fs/cifs/cifs_unicode.c
@@ -358,14 +358,9 @@ cifs_strndup_from_utf16(const char *src, const int maxlen,
if (!dst)
return NULL;
cifs_from_utf16(dst, (__le16 *) src, len, maxlen, codepage,
- NO_MAP_UNI_RSVD);
+ NO_MAP_UNI_RSVD);
} else {
- len = strnlen(src, maxlen);
- len++;
- dst = kmalloc(len, GFP_KERNEL);
- if (!dst)
- return NULL;
- strlcpy(dst, src, len);
+ dst = kstrndup(src, maxlen, GFP_KERNEL);
}
return dst;
--
2.25.1
Len Baker <[email protected]> writes:
> strlcpy() reads the entire source buffer first. This read may exceed the
> destination size limit. This is both inefficient and can lead to linear
> read overflows if a source string is not NUL-terminated.
>
> Also, the strnlen() call does not avoid the read overflow in the strlcpy
> function when a not NUL-terminated string is passed.
>
> So, replace this block by a call to kstrndup() that avoids this type of
> overflow and does the same.
>
> Fixes: 066ce6899484d ("cifs: rename cifs_strlcpy_to_host and make it use new functions")
> Signed-off-by: Len Baker <[email protected]>
> ---
> fs/cifs/cifs_unicode.c | 9 ++-------
> 1 file changed, 2 insertions(+), 7 deletions(-)
Reviewed-by: Paulo Alcantara (SUSE) <[email protected]>
tentatively merged into cifs-2.6.git for-next pending testing
On Tue, Aug 17, 2021 at 7:29 PM Paulo Alcantara <[email protected]> wrote:
>
> Len Baker <[email protected]> writes:
>
> > strlcpy() reads the entire source buffer first. This read may exceed the
> > destination size limit. This is both inefficient and can lead to linear
> > read overflows if a source string is not NUL-terminated.
> >
> > Also, the strnlen() call does not avoid the read overflow in the strlcpy
> > function when a not NUL-terminated string is passed.
> >
> > So, replace this block by a call to kstrndup() that avoids this type of
> > overflow and does the same.
> >
> > Fixes: 066ce6899484d ("cifs: rename cifs_strlcpy_to_host and make it use new functions")
> > Signed-off-by: Len Baker <[email protected]>
> > ---
> > fs/cifs/cifs_unicode.c | 9 ++-------
> > 1 file changed, 2 insertions(+), 7 deletions(-)
>
> Reviewed-by: Paulo Alcantara (SUSE) <[email protected]>
--
Thanks,
Steve
On Tue, 2021-08-17 at 12:27 +0200, Len Baker wrote:
> strlcpy() reads the entire source buffer first. This read may exceed the
> destination size limit. This is both inefficient and can lead to linear
> read overflows if a source string is not NUL-terminated.
>
> Also, the strnlen() call does not avoid the read overflow in the strlcpy
> function when a not NUL-terminated string is passed.
>
> So, replace this block by a call to kstrndup() that avoids this type of
> overflow and does the same.
>
> Fixes: 066ce6899484d ("cifs: rename cifs_strlcpy_to_host and make it use new functions")
> Signed-off-by: Len Baker <[email protected]>
> ---
> fs/cifs/cifs_unicode.c | 9 ++-------
> 1 file changed, 2 insertions(+), 7 deletions(-)
>
> diff --git a/fs/cifs/cifs_unicode.c b/fs/cifs/cifs_unicode.c
> index 9bd03a231032..171ad8b42107 100644
> --- a/fs/cifs/cifs_unicode.c
> +++ b/fs/cifs/cifs_unicode.c
> @@ -358,14 +358,9 @@ cifs_strndup_from_utf16(const char *src, const int maxlen,
> if (!dst)
> return NULL;
> cifs_from_utf16(dst, (__le16 *) src, len, maxlen, codepage,
> - NO_MAP_UNI_RSVD);
> + NO_MAP_UNI_RSVD);
> } else {
> - len = strnlen(src, maxlen);
> - len++;
> - dst = kmalloc(len, GFP_KERNEL);
> - if (!dst)
> - return NULL;
> - strlcpy(dst, src, len);
> + dst = kstrndup(src, maxlen, GFP_KERNEL);
> }
>
> return dst;
> --
> 2.25.1
>
Reviewed-by: Jeff Layton <[email protected]>
Added RB and repushed to cifs-2.6.git for-next
On Wed, Aug 18, 2021 at 8:22 AM Jeff Layton <[email protected]> wrote:
>
> On Tue, 2021-08-17 at 12:27 +0200, Len Baker wrote:
> > strlcpy() reads the entire source buffer first. This read may exceed the
> > destination size limit. This is both inefficient and can lead to linear
> > read overflows if a source string is not NUL-terminated.
> >
> > Also, the strnlen() call does not avoid the read overflow in the strlcpy
> > function when a not NUL-terminated string is passed.
> >
> > So, replace this block by a call to kstrndup() that avoids this type of
> > overflow and does the same.
> >
> > Fixes: 066ce6899484d ("cifs: rename cifs_strlcpy_to_host and make it use new functions")
> > Signed-off-by: Len Baker <[email protected]>
> > ---
> > fs/cifs/cifs_unicode.c | 9 ++-------
> > 1 file changed, 2 insertions(+), 7 deletions(-)
> >
> > diff --git a/fs/cifs/cifs_unicode.c b/fs/cifs/cifs_unicode.c
> > index 9bd03a231032..171ad8b42107 100644
> > --- a/fs/cifs/cifs_unicode.c
> > +++ b/fs/cifs/cifs_unicode.c
> > @@ -358,14 +358,9 @@ cifs_strndup_from_utf16(const char *src, const int maxlen,
> > if (!dst)
> > return NULL;
> > cifs_from_utf16(dst, (__le16 *) src, len, maxlen, codepage,
> > - NO_MAP_UNI_RSVD);
> > + NO_MAP_UNI_RSVD);
> > } else {
> > - len = strnlen(src, maxlen);
> > - len++;
> > - dst = kmalloc(len, GFP_KERNEL);
> > - if (!dst)
> > - return NULL;
> > - strlcpy(dst, src, len);
> > + dst = kstrndup(src, maxlen, GFP_KERNEL);
> > }
> >
> > return dst;
> > --
> > 2.25.1
> >
>
> Reviewed-by: Jeff Layton <[email protected]>
>
--
Thanks,
Steve