Re: [PATCH] mm/slub.c: add parameter length checking for alloc_loc_track()

[Chen Gang F T <chen.gang.flying.transformer@gmail.com>]

On 07/18/2013 09:45 PM, Christoph Lameter wrote:
> On Thu, 18 Jul 2013, Chen Gang wrote:
> 
>> > Hmm... when anybody says "need respect original authors' willing and
>> > opinions", I think it often means we have found the direct issue, but
>> > none of us find the root issue.
> Is there an actual problem / failure being addressed by this patch?
> 

No, at least, this patch (add parameter length checking) is useless.

>> > e.g. for our this case:
>> >   the direct issue is:
>> >     "whether need check the length with 'max' parameter".
>> >   but maybe the root issue is:
>> >     "whether use 'size' as related parameter name instead of 'max'".
>> >     in alloc_loc_track(), 'max' just plays the 'size' role.
> "max" determines the size of the loc_track structure. So these can
> roughly mean the same thing.


Yes, "'max' can roughly mean the same thing", but they are still a
little different.

'max' also means: "the caller tells callee: I have told you the
maximize buffer length, so I need not check the buffer length to be
sure of no memory overflow, you need be sure of it".

'size' means: "the caller tells callee: you should use the size which I
give you, I am sure it is OK, do not care about whether it can cause
memory overflow or not".


The diff may like this:

--------------------------------diff begin------------------------------

diff --git a/mm/slub.c b/mm/slub.c
index 2b02d66..8564677 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -3988,12 +3988,12 @@ static void free_loc_track(struct loc_track *t)
 			get_order(sizeof(struct location) * t->max));
 }
 
-static int alloc_loc_track(struct loc_track *t, unsigned long max, gfp_t flags)
+static int alloc_loc_track(struct loc_track *t, unsigned long size, gfp_t flags)
 {
 	struct location *l;
 	int order;
 
-	order = get_order(sizeof(struct location) * max);
+	order = get_order(sizeof(struct location) * size);
 
 	l = (void *)__get_free_pages(flags, order);
 	if (!l)
@@ -4003,7 +4003,7 @@ static int alloc_loc_track(struct loc_track *t, unsigned long max, gfp_t flags)
 		memcpy(l, t->loc, sizeof(struct location) * t->count);
 		free_loc_track(t);
 	}
-	t->max = max;
+	t->max = size;
 	t->loc = l;
 	return 1;
 }

--------------------------------diff end--------------------------------

Thanks
-- 
Chen Gang

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>