* null pointer dereference in refs/file-backend
@ 2019-01-16 9:18 Carlo Arenas
2019-01-16 15:49 ` René Scharfe
0 siblings, 1 reply; 2+ messages in thread
From: Carlo Arenas @ 2019-01-16 9:18 UTC (permalink / raw)
To: git; +Cc: mhagger, sandals
while running HEAD cppcheck against git HEAD got the following error,
that seem to be in the code all the way to maint:
[refs/files-backend.c:2681] -> [refs.c:1044] -> [cache.h:1075]:
(error) Null pointer dereference: src
the code that uses NULL as the source OID was introduced with
b0ca411051 ("files_transaction_prepare(): don't leak flags to packed
transaction", 2017-11-05) and doesn't seem to be a false positive,
hence why I am hoping someone else with a better understanding of it
could come out with a solution
Carlo
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: null pointer dereference in refs/file-backend
2019-01-16 9:18 null pointer dereference in refs/file-backend Carlo Arenas
@ 2019-01-16 15:49 ` René Scharfe
0 siblings, 0 replies; 2+ messages in thread
From: René Scharfe @ 2019-01-16 15:49 UTC (permalink / raw)
To: Carlo Arenas, git; +Cc: mhagger, sandals
Am 16.01.2019 um 10:18 schrieb Carlo Arenas:
> while running HEAD cppcheck against git HEAD got the following error,
> that seem to be in the code all the way to maint:
>
> [refs/files-backend.c:2681] -> [refs.c:1044] -> [cache.h:1075]:
> (error) Null pointer dereference: src
>
> the code that uses NULL as the source OID was introduced with
> b0ca411051 ("files_transaction_prepare(): don't leak flags to packed
> transaction", 2017-11-05) and doesn't seem to be a false positive,
> hence why I am hoping someone else with a better understanding of it
> could come out with a solution
Tl;dr: It's safe.
The statement at line 2681 ff. of refs/files-backend.c is:
ref_transaction_add_update(
packed_transaction, update->refname,
REF_HAVE_NEW | REF_NO_DEREF,
&update->new_oid, NULL,
NULL);
The function is defined in refs/files-backend.c; here are the lines
up to no. 1044:
struct ref_update *ref_transaction_add_update(
struct ref_transaction *transaction,
const char *refname, unsigned int flags,
const struct object_id *new_oid,
const struct object_id *old_oid,
const char *msg)
{
struct ref_update *update;
if (transaction->state != REF_TRANSACTION_OPEN)
BUG("update called for transaction that is not open");
FLEX_ALLOC_STR(update, refname, refname);
ALLOC_GROW(transaction->updates, transaction->nr + 1, transaction->alloc);
transaction->updates[transaction->nr++] = update;
update->flags = flags;
if (flags & REF_HAVE_NEW)
oidcpy(&update->new_oid, new_oid);
if (flags & REF_HAVE_OLD)
oidcpy(&update->old_oid, old_oid);
The "src" in the message "Null pointer dereference: src" refers to
the second parameter of oidcpy() in the line above, i.e. to old_oid.
That's the fifth parameter to ref_transaction_add_update(), and it
is NULL in the invocation mentioned at the top.
oidcopy() is only executed if the flag REF_HAVE_OLD is set, and that
caller passes only REF_HAVE_NEW and REF_NO_DEREF. So let's look at
their values:
$ git grep -E 'define (REF_HAVE_OLD|REF_HAVE_NEW|REF_NO_DEREF)'
refs.h:#define REF_NO_DEREF (1 << 0)
refs/refs-internal.h:#define REF_HAVE_NEW (1 << 2)
refs/refs-internal.h:#define REF_HAVE_OLD (1 << 3)
So these three flags don't overlap; oidcpy() in line 1044 is skipped
by the invocation of ref_transaction_add_update() that offended
cppcheck, i.e. NULL is not actually dereferenced.
I guess the function requires callers to indicate the presence of
non-NULL object ID pointers using flags instead of checking for
NULL directly because the new_oid and old_oid members of struct
ref_update are not nullable, yet they are used as (partial) input
for ref_transaction_add_update().
René
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2019-01-16 15:50 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-01-16 9:18 null pointer dereference in refs/file-backend Carlo Arenas
2019-01-16 15:49 ` René Scharfe
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).