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

[Arnd Bergmann]

On Tue, Jun 26, 2018 at 7:29 PM, Kees Cook <keescook@xxxxxxxxxxxx> 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@xxxxxxxxxxxxxx
>
> Cc: Anton Altaparmakov <anton@xxxxxxxxxx>
> Cc: linux-ntfs-dev@xxxxxxxxxxxxxxxxxxxxx
> Signed-off-by: Kees Cook <keescook@xxxxxxxxxxxx>
> ---
>  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