2008-12-17 11:40:40

by Jeff Layton

Subject: [PATCH] cifs: fix buffer overrun in parse_DFS_referrals

While testing a kernel with memory poisoning enabled, I saw some warnings
about the redzone getting clobbered when chasing DFS referrals. The
buffer allocation for the unicode converted version of the searchName is
too small and needs to take null termination into account.

Signed-off-by: Jeff Layton <[email protected]>
Acked-by: Steve French <[email protected]>
---
fs/cifs/cifssmb.c | 3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/fs/cifs/cifssmb.c b/fs/cifs/cifssmb.c
index 9395928..824df14 100644
--- a/fs/cifs/cifssmb.c
+++ b/fs/cifs/cifssmb.c
@@ -3992,7 +3992,8 @@ parse_DFS_referrals(TRANSACTION2_GET_DFS_REFER_RSP *pSMBr,

node->flags = le16_to_cpu(pSMBr->DFSFlags);
if (is_unicode) {
- __le16 *tmp = kmalloc(strlen(searchName)*2, GFP_KERNEL);
+ __le16 *tmp = kmalloc(strlen(searchName)*2 + 2,
+ GFP_KERNEL);
cifsConvertToUCS((__le16 *) tmp, searchName,
PATH_MAX, nls_codepage, remap);
node->path_consumed = hostlen_fromUCS(tmp,
--
1.5.5.1

2008-12-17 15:41:19

by Renato S. Yamane

Subject: Re: [PATCH] cifs: fix buffer overrun in parse_DFS_referrals

Jeff Layton wrote:
> ---
> fs/cifs/cifssmb.c | 3 ++-
> 1 files changed, 2 insertions(+), 1 deletions(-)
>
> diff --git a/fs/cifs/cifssmb.c b/fs/cifs/cifssmb.c
> index 9395928..824df14 100644
> --- a/fs/cifs/cifssmb.c
> +++ b/fs/cifs/cifssmb.c
> @@ -3992,7 +3992,8 @@ parse_DFS_referrals(TRANSACTION2_GET_DFS_REFER_RSP *pSMBr,
>
> node->flags = le16_to_cpu(pSMBr->DFSFlags);
> if (is_unicode) {
> - __le16 *tmp = kmalloc(strlen(searchName)*2, GFP_KERNEL);
> + __le16 *tmp = kmalloc(strlen(searchName)*2 + 2,
> + GFP_KERNEL);
> cifsConvertToUCS((__le16 *) tmp, searchName,
> PATH_MAX, nls_codepage, remap);
> node->path_consumed = hostlen_fromUCS(tmp,

This patch can't be applied in -stable release:

yamane@mandachuva:~/kernel/linux-2.6.27.9$ patch -p1 < cifs.patch
patching file fs/cifs/cifssmb.c
patch unexpectedly ends in middle of line
Hunk #1 FAILED at 3992.
1 out of 1 hunk FAILED -- saving rejects to file fs/cifs/cifssmb.c.rej

Best regards,
Renato

2008-12-17 15:50:54

by Jeff Layton

Subject: Re: [PATCH] cifs: fix buffer overrun in parse_DFS_referrals

On Wed, 17 Dec 2008 13:40:56 -0200
"Renato S. Yamane" <[email protected]> wrote:

> Jeff Layton wrote:
> > ---
> > fs/cifs/cifssmb.c | 3 ++-
> > 1 files changed, 2 insertions(+), 1 deletions(-)
> >
> > diff --git a/fs/cifs/cifssmb.c b/fs/cifs/cifssmb.c
> > index 9395928..824df14 100644
> > --- a/fs/cifs/cifssmb.c
> > +++ b/fs/cifs/cifssmb.c
> > @@ -3992,7 +3992,8 @@ parse_DFS_referrals(TRANSACTION2_GET_DFS_REFER_RSP *pSMBr,
> >
> > node->flags = le16_to_cpu(pSMBr->DFSFlags);
> > if (is_unicode) {
> > - __le16 *tmp = kmalloc(strlen(searchName)*2, GFP_KERNEL);
> > + __le16 *tmp = kmalloc(strlen(searchName)*2 + 2,
> > + GFP_KERNEL);
> > cifsConvertToUCS((__le16 *) tmp, searchName,
> > PATH_MAX, nls_codepage, remap);
> > node->path_consumed = hostlen_fromUCS(tmp,
>
> This patch can't be applied in -stable release:
>
> yamane@mandachuva:~/kernel/linux-2.6.27.9$ patch -p1 < cifs.patch
> patching file fs/cifs/cifssmb.c
> patch unexpectedly ends in middle of line
> Hunk #1 FAILED at 3992.
> 1 out of 1 hunk FAILED -- saving rejects to file fs/cifs/cifssmb.c.rej
>

My apologies. The patch that introduced this problem isn't in stable
releases. You can drop this patch from stable queue. Sorry for false alarm.

It would be good for 2.6.28 kernels though since it's a regression and
possible memory corruptor.

Thanks,
--
Jeff Layton <[email protected]>