Re: potential null dereference
- From
René Scharfe <rene.scharfe@lsrfire.ath.cx>
- Date
- Dec 17, 2009, 12:30 UTC
- Message-ID
- <4B2A247F.4070705@lsrfire.ath.cx>
- In-Reply-To
- <4B2783DD.5060301@gmail.com>
Am 15.12.2009 13:41, schrieb Jiri Slaby:
Show 31 quoted lines
> Hi,
>
> Stanse found the following error in unpack-trees.c:
> dereferencing NULL pointer here.[. * o src_index]
>
> int unpack_trees(unsigned len, struct tree_desc *t, struct
> unpack_trees_options *o)
> {
> int ret;
> static struct cache_entry *dfc;
> ...
> if (o->src_index) { <-- loc0
> o->result.timestamp.sec = o->src_index->timestamp.sec;
> o->result.timestamp.nsec = o->src_index->timestamp.nsec;
> }
> o->merge_size = len;
>
> if (!dfc)
> dfc = xcalloc(1, ((1 + (0) + 8) & ~7));
> o->df_conflict_entry = dfc;
>
> if (len) {
> ...
> }
>
> if (o->merge) {
> while (o->pos < o->src_index->cache_nr) { <-- here
>
> It triggers, because there is a test for o->src_index being NULL at
> loc0, but here, it is dereferenced without a check. Can this happen
> (e.g. does o->merge != NULL imply o->src_index != NULL)?Running "git grep -w -B70 unpack_trees" and looking for "src_index" using less' search command showed me that src_index is never NULL when unpack_trees() is called.
Show 28 quoted lines
> Further, there is a warning in log-tree.c:
> pointer always points to valid memory here, but checking for not
> NULL.[parents]
>
> static int log_tree_diff(struct rev_info *opt, struct commit *commit,
> struct log_info *log)
> {
> int showed_log;
> struct commit_list *parents;
> unsigned const char *sha1 = commit->object.sha1;
>
> if (!opt->diff && !((&opt->diffopt)->flags & (1 << 14)))
> return 0;
>
>
> parents = commit->parents;
> if (!parents) { <-- loc0
> if (opt->show_root_diff) {
> diff_root_tree_sha1(sha1, "", &opt->diffopt);
> log_tree_diff_flush(opt);
> }
> return !opt->loginfo; <-- loc1
> }
>
> if (parents && parents->next) { <-- here
>
> I.e. if parents was NULL at loc0, we escaped at loc1. But we check
> parents against NULL here again.The check may be duplicate, but I suspect removing it won't change the resulting object code -- the compiler should be smart enough to come to the same conclusion.
Thanks, René