linux-kernel - Re: [PATCH] fat: fix buffer overflow in vfat_create

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <8739zp6k9s.fsf@devron.myhome.or.jp>
Date:	Thu, 25 Mar 2010 14:30:07 +0900
From:	OGAWA Hirofumi <hirofumi@...l.parknet.co.jp>
To:	Nikolaus Schulz <microschulz@....de>
Cc:	Al Viro <viro@...iv.linux.org.uk>, Marton Balint <cus@...ekas.hu>,
	Alexey Dobriyan <adobriyan@...il.com>,
	Kevin Dankwardt <k@...mputing.com>,
	Christoph Hellwig <hch@....de>, linux-kernel@...r.kernel.org,
	stable@...nel.org
Subject: Re: [PATCH] fat: fix buffer overflow in vfat_create_shortname()

Nikolaus Schulz <microschulz@....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. 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.

> Signed-off-by: Nikolaus Schulz <microschulz@....de>
> Cc: stable@...nel.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@...l.parknet.co.jp>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/