linux-kernel - Re: [PATCH] [RFC] ntfs: disable for 64KB because of stack overflow risk

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: <1B89CC23-0CB4-4DA3-BA84-3DD628675351@tuxera.com>
Date:   Mon, 27 Sep 2021 23:21:53 +0000
From:   Anton Altaparmakov <anton@...era.com>
To:     Arnd Bergmann <arnd@...nel.org>
CC:     Kees Cook <keescook@...omium.org>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Arnd Bergmann <arnd@...db.de>,
        "linux-ntfs-dev@...ts.sourceforge.net" 
        <linux-ntfs-dev@...ts.sourceforge.net>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] [RFC] ntfs: disable for 64KB because of stack overflow
 risk

Hi Arnd,

Thanks for the patch but what is the problem with the stack usage exceeding 2048 bytes?

I am not aware of any architectures that implements kernel stack size (THREAD_SIZE) less than page size and by default most architectures with 4kiB page size even use two pages to make the stack 8kiB.

Thus on a 64kiB page size system the stack size is minimum 64kiB so using just over 2kiB seems to totally fine to me so why apply your patch?

Seems to me it would be better to fix the warning message you are seeing instead - it is a really stupid warning on a system with 64kiB stack size!

Best regards,

	Anton

> On 27 Sep 2021, at 15:18, Arnd Bergmann <arnd@...nel.org> wrote:
> 
> From: Arnd Bergmann <arnd@...db.de>
> 
> On ARM64 randconfig builds, we occasionally get warnings for NTFS:
> 
> fs/ntfs/aops.c: In function 'ntfs_write_mst_block':
> fs/ntfs/aops.c:1328:1: error: the frame size of 2224 bytes is larger than 2048 bytes [-Werror=frame-larger-than=]
> 
> The problem here is that with 64KB pages, we get two arrays on the
> stack that each have 128 pointers, for a total of 2KB. Before
> the VLA change, this could already happen with 512-byte blocks,
> however in practice NTFS should usually have 4KB blocks and not
> be affected by this (see link).
> 
> Now the stack usage is always > 2KB on any architecture with 64KB
> pages. Since both NTFS and 64KB page support are fairly rare,
> we may get away with just marking the combination as disallowed
> in Kconfig and see if anyone complains before we find a different
> way to address it.
> 
> Fixes: ac4ecf968acb ("ntfs: aops: remove VLA usage")
> Link: https://support.microsoft.com/en-us/help/140365/default-cluster-size-for-ntfs-fat-and-exfat
> Signed-off-by: Arnd Bergmann <arnd@...db.de>
> ---
> fs/ntfs/Kconfig | 1 +
> fs/ntfs/aops.c  | 3 +++
> 2 files changed, 4 insertions(+)
> 
> diff --git a/fs/ntfs/Kconfig b/fs/ntfs/Kconfig
> index 1667a7e590d8..b770f0209b9c 100644
> --- a/fs/ntfs/Kconfig
> +++ b/fs/ntfs/Kconfig
> @@ -2,6 +2,7 @@
> config NTFS_FS
> 	tristate "NTFS file system support"
> 	select NLS
> +	depends on !PAGE_SIZE_64KB && !ARM64_64K_PAGES
> 	help
> 	  NTFS is the file system of Microsoft Windows NT, 2000, XP and 2003.
> 
> diff --git a/fs/ntfs/aops.c b/fs/ntfs/aops.c
> index bb0a43860ad2..76d59bd4c1eb 100644
> --- a/fs/ntfs/aops.c
> +++ b/fs/ntfs/aops.c
> @@ -914,6 +914,9 @@ static int ntfs_write_mst_block(struct page *page,
> 	bool sync, is_mft, page_is_dirty, rec_is_dirty;
> 	unsigned char bh_size_bits;
> 
> +	/* Two arrays of MAX_BUF_PER_PAGE on the stack risks an overrun with 64K pages */
> +	BUILD_BUG_ON(PAGE_SIZE >= 65536);
> +
> 	if (WARN_ON(rec_size < NTFS_BLOCK_SIZE))
> 		return -EINVAL;
> 
> -- 
> 2.29.2
>