lkml.org 
[lkml]   [2010]   [Mar]   [25]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH] fat: fix buffer overflow in vfat_create_shortname()
On Thu, Mar 25, 2010 at 02:30:07PM +0900, OGAWA Hirofumi wrote:
> Nikolaus Schulz <microschulz@web.de> writes:
>
> > When using the string representation of a random counter as part of the base
> > name, ensure that it is no longer than 4 bytes.
> >
> > Since we are repeatedly decrementing the counter in a loop until we have found a
> > unique base name, the counter may wrap around zero; therefore, it is not enough
> > to mask its higher bits before entering the loop, this must be done inside the
> > loop.
>
> This logic seems to still be strange after applying this patch.

Yeah, I find that code not terribly convincing either. :)

> However, anyway, your patch is much better off than current one. So,
> I'll apply this in the next merge window.
>
> Or should we apply this immediately?
>
> Thanks.

Given that this fixes a stack corruption which triggers the gcc stack
smashing protection and thus basically a crash, I vote for not
postponing it, but applying it immediately.

> > Signed-off-by: Nikolaus Schulz <microschulz@web.de>
> > Cc: stable@kernel.org
> > ---
> > fs/fat/namei_vfat.c | 6 +++---
> > 1 files changed, 3 insertions(+), 3 deletions(-)
> >
> > diff --git a/fs/fat/namei_vfat.c b/fs/fat/namei_vfat.c
> > index c1ef501..a448ee5 100644
> > --- a/fs/fat/namei_vfat.c
> > +++ b/fs/fat/namei_vfat.c
> > @@ -309,7 +309,7 @@ static int vfat_create_shortname(struct inode *dir, struct nls_table *nls,
> > {
> > struct fat_mount_options *opts = &MSDOS_SB(dir->i_sb)->options;
> > wchar_t *ip, *ext_start, *end, *name_start;
> > - unsigned char base[9], ext[4], buf[8], *p;
> > + unsigned char base[9], ext[4], buf[5], *p;
> > unsigned char charbuf[NLS_MAX_CHARSET_SIZE];
> > int chl, chi;
> > int sz = 0, extlen, baselen, i, numtail_baselen, numtail2_baselen;
> > @@ -467,7 +467,7 @@ static int vfat_create_shortname(struct inode *dir, struct nls_table *nls,
> > return 0;
> > }
> >
> > - i = jiffies & 0xffff;
> > + i = jiffies;
> > sz = (jiffies >> 16) & 0x7;
> > if (baselen > 2) {
> > baselen = numtail2_baselen;
> > @@ -476,7 +476,7 @@ static int vfat_create_shortname(struct inode *dir, struct nls_table *nls,
> > name_res[baselen + 4] = '~';
> > name_res[baselen + 5] = '1' + sz;
> > while (1) {
> > - sprintf(buf, "%04X", i);
> > + sprintf(buf, "%04X", i & 0xffff);
> > memcpy(&name_res[baselen], buf, 4);
> > if (vfat_find_form(dir, name_res) < 0)
> > break;
>
> --
> OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>
>


\
 
 \ /
  Last update: 2010-03-25 13:31    [W:0.079 / U:0.044 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site