Skip to contentSkip to conversation

Terminate hugetlbfs mount argument list

PATCH In review Andi Kleen 2 participants 4 messages Mar 11, 2008 HugeTLB

lore
Share & tools
lore

Andi Kleen

author
#0 lore raw

PATCH 1/1 Terminate hugetlbfs mount argument list

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.

fs/hugetlbfs/inode.c +1 -0
Index: 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)
#1 lore raw
Quoted 4 lines

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.

author
#2 lore raw
Quoted 7 lines · Roland Dreier
Quoted 4 lines

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

#3 lore raw
Quoted 2 lines

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.