[PATCH] Terminate hugetlbfs mount argument list

[PATCH] Terminate hugetlbfs mount argument list

[Andi Kleen]

Terminate hugetlbfs mount argument list

[2.6.25 candidate I believe]

The match_table_t for the mount arguments in hugetlbfs wasn't 
terminated as match_tokens expect. I didn't see a crash just code
audit, but it's still safer to terminate it in case the variables after
that in .data are not NULL.

Signed-off-by: Andi Kleen <ak@suse.de>

Index: linux/fs/hugetlbfs/inode.c
===================================================================
--- linux.orig/fs/hugetlbfs/inode.c
+++ linux/fs/hugetlbfs/inode.c
@@ -63,6 +63,7 @@ static match_table_t tokens = {
 	{Opt_uid,	"uid=%u"},
 	{Opt_gid,	"gid=%u"},
 	{Opt_err,	NULL},
+	{},
 };
 
 static void huge_pagevec_release(struct pagevec *pvec)

Re: [PATCH] Terminate hugetlbfs mount argument list

[Roland Dreier]

 > The match_table_t for the mount arguments in hugetlbfs wasn't 
 > terminated as match_tokens expect. I didn't see a crash just code
 > audit, but it's still safer to terminate it in case the variables after
 > that in .data are not NULL.

I think you're misunderstanding the match_token() interface.  The
comment before match_token() says:

 * @table: match_table_t describing the set of allowed option tokens and the
 * arguments that may be associated with them. Must be terminated with a
 * &struct match_token whose pattern is set to the NULL pointer.

and that's exactly what already exists here:

 	{Opt_gid,	"gid=%u"},
 	{Opt_err,	NULL},

So your patch is effectively an obfuscated NOP.

 - R.

Re: [PATCH] Terminate hugetlbfs mount argument list

[Andi Kleen]

On Tue, Mar 11, 2008 at 03:48:46PM -0700, Roland Dreier wrote:
>  > The match_table_t for the mount arguments in hugetlbfs wasn't 
>  > terminated as match_tokens expect. I didn't see a crash just code
>  > audit, but it's still safer to terminate it in case the variables after
>  > that in .data are not NULL.
> 
> I think you're misunderstanding the match_token() interface.  The
> comment before match_token() says:

Hmm indeed I did. To my defense it's a weird unusual coding pattern
that tricked me.  What good is it to give terminator elements own enum values?

-Andi

Re: [PATCH] Terminate hugetlbfs mount argument list

[Roland Dreier]

 > Hmm indeed I did. To my defense it's a weird unusual coding pattern
 > that tricked me.  What good is it to give terminator elements own enum values?

It's a little odd.  But I guess it does let you do

	token = match_token(...table...);
	switch (token) {
	case ERR_VALUE:...

and get ERR_VALUE back for anything that doesn't match one of the real
token patterns.

 - R.