LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* [PATCH] CIFS: Fix a potencially linear read overflow
@ 2021-08-17 10:27 Len Baker
2021-08-18 0:28 ` Paulo Alcantara
2021-08-18 13:21 ` Jeff Layton
0 siblings, 2 replies; 5+ messages in thread
From: Len Baker @ 2021-08-17 10:27 UTC (permalink / raw)
To: Steve French, Jeff Layton, Suresh Jayaraman
Cc: Len Baker, linux-cifs, samba-technical, linux-kernel, Kees Cook,
linux-hardening
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 <len.baker@gmx.com>
---
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
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] CIFS: Fix a potencially linear read overflow
2021-08-17 10:27 [PATCH] CIFS: Fix a potencially linear read overflow Len Baker
@ 2021-08-18 0:28 ` Paulo Alcantara
2021-08-18 2:46 ` Steve French
2021-08-18 13:21 ` Jeff Layton
1 sibling, 1 reply; 5+ messages in thread
From: Paulo Alcantara @ 2021-08-18 0:28 UTC (permalink / raw)
To: Len Baker, Steve French, Jeff Layton, Suresh Jayaraman
Cc: Len Baker, linux-cifs, samba-technical, linux-kernel, Kees Cook,
linux-hardening
Len Baker <len.baker@gmx.com> 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 <len.baker@gmx.com>
> ---
> fs/cifs/cifs_unicode.c | 9 ++-------
> 1 file changed, 2 insertions(+), 7 deletions(-)
Reviewed-by: Paulo Alcantara (SUSE) <pc@cjr.nz>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] CIFS: Fix a potencially linear read overflow
2021-08-18 0:28 ` Paulo Alcantara
@ 2021-08-18 2:46 ` Steve French
0 siblings, 0 replies; 5+ messages in thread
From: Steve French @ 2021-08-18 2:46 UTC (permalink / raw)
To: Paulo Alcantara
Cc: Len Baker, Steve French, Jeff Layton, Suresh Jayaraman, CIFS,
samba-technical, LKML, Kees Cook, linux-hardening
tentatively merged into cifs-2.6.git for-next pending testing
On Tue, Aug 17, 2021 at 7:29 PM Paulo Alcantara <pc@cjr.nz> wrote:
>
> Len Baker <len.baker@gmx.com> 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 <len.baker@gmx.com>
> > ---
> > fs/cifs/cifs_unicode.c | 9 ++-------
> > 1 file changed, 2 insertions(+), 7 deletions(-)
>
> Reviewed-by: Paulo Alcantara (SUSE) <pc@cjr.nz>
--
Thanks,
Steve
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] CIFS: Fix a potencially linear read overflow
2021-08-17 10:27 [PATCH] CIFS: Fix a potencially linear read overflow Len Baker
2021-08-18 0:28 ` Paulo Alcantara
@ 2021-08-18 13:21 ` Jeff Layton
2021-08-20 5:58 ` Steve French
1 sibling, 1 reply; 5+ messages in thread
From: Jeff Layton @ 2021-08-18 13:21 UTC (permalink / raw)
To: Len Baker, Steve French, Suresh Jayaraman
Cc: linux-cifs, samba-technical, linux-kernel, Kees Cook, linux-hardening
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 <len.baker@gmx.com>
> ---
> 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 <jlayton@kernel.org>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] CIFS: Fix a potencially linear read overflow
2021-08-18 13:21 ` Jeff Layton
@ 2021-08-20 5:58 ` Steve French
0 siblings, 0 replies; 5+ messages in thread
From: Steve French @ 2021-08-20 5:58 UTC (permalink / raw)
To: Jeff Layton
Cc: Len Baker, Steve French, Suresh Jayaraman, CIFS, samba-technical,
LKML, Kees Cook, linux-hardening
Added RB and repushed to cifs-2.6.git for-next
On Wed, Aug 18, 2021 at 8:22 AM Jeff Layton <jlayton@kernel.org> 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 <len.baker@gmx.com>
> > ---
> > 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 <jlayton@kernel.org>
>
--
Thanks,
Steve
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2021-08-20 5:59 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-08-17 10:27 [PATCH] CIFS: Fix a potencially linear read overflow Len Baker
2021-08-18 0:28 ` Paulo Alcantara
2021-08-18 2:46 ` Steve French
2021-08-18 13:21 ` Jeff Layton
2021-08-20 5:58 ` Steve French
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).