[PATCH -next] hugetlbfs: a terminator for hugetlb_param_specs[]

[PATCH -next] hugetlbfs: a terminator for hugetlb_param_specs[]

[Qian Cai]

Booting up an arm64 server with CONFIG_VALIDATE_FS_PARSER=n triggers a
out-of-bounds error below, due to the commit 2284cf59cbce ("hugetlbfs:
Convert to fs_context") missed a terminator for hugetlb_param_specs[],
and causes this loop in fs_lookup_key(),

for (p = desc->specs; p->name; p++)

could not exit properly due to p->name is never be NULL.

[   91.575203] BUG: KASAN: global-out-of-bounds in fs_lookup_key+0x60/0x94
[   91.581810] Read of size 8 at addr ffff200010deeb10 by task mount/2461
[   91.597350] Call trace:
[   91.597357]  dump_backtrace+0x0/0x2b0
[   91.597361]  show_stack+0x24/0x30
[   91.597373]  dump_stack+0xc0/0xf8
[   91.623263]  print_address_description+0x64/0x2b0
[   91.627965]  kasan_report+0x150/0x1a4
[   91.627970]  __asan_report_load8_noabort+0x30/0x3c
[   91.627974]  fs_lookup_key+0x60/0x94
[   91.627977]  fs_parse+0x104/0x990
[   91.627986]  hugetlbfs_parse_param+0xc4/0x5e8
[   91.651081]  vfs_parse_fs_param+0x2e4/0x378
[   91.658118]  vfs_parse_fs_string+0xbc/0x12c
[   91.658122]  do_mount+0x11f0/0x1640
[   91.658125]  ksys_mount+0xc0/0xd0
[   91.658129]  __arm64_sys_mount+0xcc/0xe4
[   91.658137]  el0_svc_handler+0x28c/0x338
[   91.681740]  el0_svc+0x8/0xc

Fixes: 2284cf59cbce ("hugetlbfs: Convert to fs_context")
Signed-off-by: Qian Cai <cai@lca.pw>
---
 fs/hugetlbfs/inode.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/fs/hugetlbfs/inode.c b/fs/hugetlbfs/inode.c
index abf0c2eb834e..4f352743930f 100644
--- a/fs/hugetlbfs/inode.c
+++ b/fs/hugetlbfs/inode.c
@@ -81,6 +81,7 @@ static const struct fs_parameter_spec hugetlb_param_specs[] = {
 	fsparam_string("pagesize",	Opt_pagesize),
 	fsparam_string("size",		Opt_size),
 	fsparam_u32   ("uid",		Opt_uid),
+	{}
 };
 
 static const struct fs_parameter_description hugetlb_fs_parameters = {
-- 
2.17.2 (Apple Git-113)

Re: [PATCH -next] hugetlbfs: a terminator for hugetlb_param_specs[]

[Mike Kravetz]

On 2/4/19 5:22 PM, Qian Cai wrote:
> Booting up an arm64 server with CONFIG_VALIDATE_FS_PARSER=n triggers a
> out-of-bounds error below, due to the commit 2284cf59cbce ("hugetlbfs:
> Convert to fs_context") missed a terminator for hugetlb_param_specs[],
> and causes this loop in fs_lookup_key(),
> 
> for (p = desc->specs; p->name; p++)
> 
> could not exit properly due to p->name is never be NULL.
> 
> [   91.575203] BUG: KASAN: global-out-of-bounds in fs_lookup_key+0x60/0x94
> [   91.581810] Read of size 8 at addr ffff200010deeb10 by task mount/2461
> [   91.597350] Call trace:
> [   91.597357]  dump_backtrace+0x0/0x2b0
> [   91.597361]  show_stack+0x24/0x30
> [   91.597373]  dump_stack+0xc0/0xf8
> [   91.623263]  print_address_description+0x64/0x2b0
> [   91.627965]  kasan_report+0x150/0x1a4
> [   91.627970]  __asan_report_load8_noabort+0x30/0x3c
> [   91.627974]  fs_lookup_key+0x60/0x94
> [   91.627977]  fs_parse+0x104/0x990
> [   91.627986]  hugetlbfs_parse_param+0xc4/0x5e8
> [   91.651081]  vfs_parse_fs_param+0x2e4/0x378
> [   91.658118]  vfs_parse_fs_string+0xbc/0x12c
> [   91.658122]  do_mount+0x11f0/0x1640
> [   91.658125]  ksys_mount+0xc0/0xd0
> [   91.658129]  __arm64_sys_mount+0xcc/0xe4
> [   91.658137]  el0_svc_handler+0x28c/0x338
> [   91.681740]  el0_svc+0x8/0xc
> 
> Fixes: 2284cf59cbce ("hugetlbfs: Convert to fs_context")
> Signed-off-by: Qian Cai <cai@lca.pw>

Thanks for fixing this.  Looks like a simple oversight when 2284cf59cbce
was added.

Reviewed-by: Mike Kravetz <mike.kravetz@oracle.com>

I just started looking at the new mount API now.  It would be good if
David also took a look to make sure everything else is OK.

FYI David, the fs_parameter_spec example in the documentation (mount_api.txt)
is also missing a terminator.
-- 
Mike Kravetz

> ---
>  fs/hugetlbfs/inode.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/fs/hugetlbfs/inode.c b/fs/hugetlbfs/inode.c
> index abf0c2eb834e..4f352743930f 100644
> --- a/fs/hugetlbfs/inode.c
> +++ b/fs/hugetlbfs/inode.c
> @@ -81,6 +81,7 @@ static const struct fs_parameter_spec hugetlb_param_specs[] = {
>  	fsparam_string("pagesize",	Opt_pagesize),
>  	fsparam_string("size",		Opt_size),
>  	fsparam_u32   ("uid",		Opt_uid),
> +	{}
>  };
>  
>  static const struct fs_parameter_description hugetlb_fs_parameters = {
> 

Re: [PATCH -next] hugetlbfs: a terminator for hugetlb_param_specs[]

[David Howells]

Mike Kravetz <mike.kravetz@oracle.com> wrote:

> Thanks for fixing this.  Looks like a simple oversight when 2284cf59cbce
> was added.

I've already pushed a fix for this which Al should have folded in already.

> FYI David, the fs_parameter_spec example in the documentation (mount_api.txt)
> is also missing a terminator.

Thanks.

David

Re: [PATCH -next] hugetlbfs: a terminator for hugetlb_param_specs[]

[Al Viro]

On Fri, Feb 08, 2019 at 04:31:39PM +0000, David Howells wrote:
> Mike Kravetz <mike.kravetz@oracle.com> wrote:
> 
> > Thanks for fixing this.  Looks like a simple oversight when 2284cf59cbce
> > was added.
> 
> I've already pushed a fix for this which Al should have folded in already.

It has been in -next since Tuesday.