Bug 255870 - [PATCH] rpc/rpcsec_gss: Fix a double free in rpc_gss_marshal
Summary: [PATCH] rpc/rpcsec_gss: Fix a double free in rpc_gss_marshal
Status: Closed Not A Bug
Alias: None
Product: Base System
Classification: Unclassified
Component: kern (show other bugs)
Version: CURRENT
Hardware: Any Any
: --- Affects Many People
Assignee: freebsd-bugs (Nobody)
Depends on:
Reported: 2021-05-14 12:03 UTC by lylgood
Modified: 2021-06-16 16:37 UTC (History)
2 users (show)

See Also:

assigns verf.oa_base to checksum.value back (525 bytes, patch)
2021-05-14 12:03 UTC, lylgood
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description lylgood 2021-05-14 12:03:58 UTC
Created attachment 224934 [details]
assigns verf.oa_base to checksum.value back

Bug File: sys/rpc/rpcsec_gss/rpcsec_gss.c

In function rpc_gss_marshal, checksum.value is assigned to verf.oa_base at line 591. Then verf.oa_base is freed via xdr_opaque_auth()->xdr_bytes()->mem_free(),
and verf.oa_base is set to NULL. Notice that, checksum.value is a dangling pointer now which points to a freed memory object.

Then gss_release_buffer() at line 595 is called, and the memory object pointed by checksum.value is freed via free() again.

As verf.oa_base is set to NULL if verf.oa_base is freed, so, my patch assigns verf.oa_base to checksum.value back. If the verf.oa_base is freed, the value of checksum.value will be NULL and no double free happens.
Comment 1 Mark Johnston freebsd_committer 2021-05-26 14:19:46 UTC
I believe this bug can't be triggered today.  XDR_ENCODE is the only operation that rpc_gss_marshal() will handle, based on my reading of the callers.  I'm not very certain though.  Rick, do you see anything that should be changed here?
Comment 2 Rick Macklem freebsd_committer 2021-06-16 16:37:56 UTC
I think your analysis is correct.

rpc_gss_marshal() is only called by
the AUTH_MARSHALL() macros and
they are only used during encoding
(x_op == XDR_ENCODE).
--> See all uses of AUTH_MARSHALL().

Since mem_free() is not called in
xdr_bytes() for the XDR_ENCODE case,
there is no "double free".

Also, since it is on the main "always
executed" code path, any such bug would
have been detected during testing.