Re: [PATCH] Fix segfault in diff-delta.c when FLEX_ARRAY is 1

From: Linus Torvalds <torvalds@linux-foundation.org>
Date: Dec 18, 2007, 04:46 UTC
Message-ID: <alpine.LFD.0.9999.0712172032090.21557@woody.linux-foundation.org>
In-Reply-To: <20071218014455.GB14981@artemis.madism.org>

On Tue, 18 Dec 2007, Pierre Habouzit wrote:

Show 5 quoted lines

> 
>   I wonder if we could teach sparse to prevent us from using pointer
> arithmetics on some types… because I obviously didn't read all the git
> code, and I wouldn't be surprised an instance of this still remains
> somehwere.

This should do it.

What this does is:
 - make flex structures not have a size at all (so "sizeof()" will fail)
 - add warnings for trying to add or subtract unsized pointers

so now you can try it on git with

	make CC=cgcc

and while it finds a fair number of "sizeof(..)" things and complains about them, the only invalid pointer arithmetic it finds is the

	mem = index + 1;

line in diff-delta.c.

Whether it is worth fixing all the "sizeof()" calls too, I dunno. They result in a slight waste of memory (ie we allocate too much memory), but I guess they should be harmless.

However, one indication that there may still be something wrong is that if you re-make git with FLEX_ARRAY set to some big insane value (say, 1234), then git will still fail the test-suite. So maybe there's a "sizeof()" that isn't just used for allocation sizes. I didn't check them all, there's something like 44 complaints like

	builtin-fetch.c:306:21: error: cannot size expression

from sparse with this patch.

		Linus

---
 evaluate.c |    8 ++++++++
 symbol.c   |    2 ++
 2 files changed, 10 insertions(+), 0 deletions(-)

diff --git a/evaluate.c b/evaluate.c
index 54fcd3f..cd816a8 100644
--- a/evaluate.c
+++ b/evaluate.c
@@ -576,6 +576,10 @@ static struct symbol *evaluate_ptr_add(struct expression *expr, struct symbol *i
 		expression_error(expr, "arithmetics on pointers to functions");
 		return NULL;
 	}
+	if (base->bit_size & 7) {
+		expression_error(expr, "arithmetic on unsized pointers");
+		return NULL;
+	}
 
 	/* Get the size of whatever the pointer points to */
 	multiply = base->bit_size >> 3;
@@ -820,6 +824,10 @@ static struct symbol *evaluate_ptr_sub(struct expression *expr)
 		expression_error(expr, "subtraction of functions? Share your drugs");
 		return NULL;
 	}
+	if (lbase->bit_size & 7) {
+		expression_error(expr, "subtracting unsized pointers");
+		return NULL;
+	}
 
 	expr->ctype = ssize_t_ctype;
 	if (lbase->bit_size > bits_in_char) {
diff --git a/symbol.c b/symbol.c
index 7539817..8b390ac 100644
--- a/symbol.c
+++ b/symbol.c
@@ -124,8 +124,10 @@ static void lay_out_struct(struct symbol *sym, struct struct_union_info *info)
 	 * structure size
 	 */
 	if (base_size < 0) {
+		info->bit_size = -1;
 		info->align_size = 0;
 		base_size = 0;
+		return;
 	}
 
 	align_bit_mask = (sym->ctype.alignment << 3) - 1;