linux-kernel - Re: [PATCH 1/3] NTFS: aops: Remove VLA usage

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: <CAK8P3a1r41cbLmWrd3fLfYfX4NRekDSS2P1N7xV-zYQ-2P0x0w@mail.gmail.com>
Date:   Thu, 5 Jul 2018 22:44:09 +0200
From:   Arnd Bergmann <arnd@...db.de>
To:     Kees Cook <keescook@...omium.org>
Cc:     Andrew Morton <akpm@...ux-foundation.org>,
        Anton Altaparmakov <anton@...era.com>,
        linux-ntfs-dev@...ts.sourceforge.net,
        Al Viro <viro@...iv.linux.org.uk>, Jan Kara <jack@...e.cz>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 1/3] NTFS: aops: Remove VLA usage

On Tue, Jun 26, 2018 at 7:29 PM, Kees Cook <keescook@...omium.org> wrote:
> In the quest to remove all stack VLA usage from the kernel[1], this
> uses the maximum size needed on the stack and adds a sanity check for
> robustness: index.block_size cannot be larger than PAGE_SIZE nor less
> than NTFS_BLOCK_SIZE.
>
> [1] https://lkml.kernel.org/r/CA+55aFzCG-zNmZwX4A2FQpadafLfEzK6CC=qPXydAacU1RqZWA@mail.gmail.com
>
> Cc: Anton Altaparmakov <anton@...era.com>
> Cc: linux-ntfs-dev@...ts.sourceforge.net
> Signed-off-by: Kees Cook <keescook@...omium.org>
> ---
>  fs/ntfs/aops.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/fs/ntfs/aops.c b/fs/ntfs/aops.c
> index 3a2e509c77c5..58dadff3e0e0 100644
> --- a/fs/ntfs/aops.c
> +++ b/fs/ntfs/aops.c
> @@ -926,7 +926,7 @@ static int ntfs_write_mst_block(struct page *page,
>         ntfs_volume *vol = ni->vol;
>         u8 *kaddr;
>         unsigned int rec_size = ni->itype.index.block_size;
> -       ntfs_inode *locked_nis[PAGE_SIZE / rec_size];
> +       ntfs_inode *locked_nis[PAGE_SIZE / NTFS_BLOCK_SIZE];
>         struct buffer_head *bh, *head, *tbh, *rec_start_bh;
>         struct buffer_head *bhs[MAX_BUF_PER_PAGE];
>         runlist_element *rl;

This has uncovered what looks like a preexisting bug on architectures
with large page size, this is what I get with 64K pages on arm64:

fs/ntfs/aops.c: In function 'ntfs_write_mst_block':
fs/ntfs/aops.c:1328:1: error: the frame size of 2432 bytes is larger
than 2048 bytes [-Werror=frame-larger-than=]

Since both ntfs and 64k pages are fairly obscure features, we might
get away with just disabling the combination of the two in Kconfig.

Using dynamic allocation might be tricky here, since I assume this
could be called during writeback in order to free memory, and I can't
immediately see any better fix.

        Arnd