ppp-project/ppp

pppd/tdb.c Resource leak

BluesLion opened this issue · 5 comments

https://github.com/ppp-project/ppp/blob/master/pppd/tdb.c#L1360C1-L1377C3

	/* Is locked key the old key?  If so, traverse will be reliable. */
	if (tdb->travlocks.off) {
		if (tdb_lock(tdb,tdb->travlocks.hash,F_WRLCK))
			return tdb_null;
		if (rec_read(tdb, tdb->travlocks.off, &rec) == -1
		    || !(k = tdb_alloc_read(tdb,tdb->travlocks.off+sizeof(rec),
					    rec.key_len))
		    || memcmp(k, oldkey.dptr, oldkey.dsize) != 0) {
			/* No, it wasn't: unlock it and start from scratch */
			if (unlock_record(tdb, tdb->travlocks.off) != 0)
				return tdb_null;
			if (tdb_unlock(tdb, tdb->travlocks.hash, F_WRLCK) != 0)
				return tdb_null;
			tdb->travlocks.off = 0;
		}

		SAFE_FREE(k);
	}

k will be allocated after tdb_alloc_read, but it will true for either "if (unlock_record(tdb, tdb->travlocks.off) != 0)" or "if (tdb_unlock(tdb, tdb->travlocks.hash, F_WRLCK) != 0)".
Shouldn't we use "SAFE_FREE(k);" before "return tdb_null;" here to avoid a resource leak?

@paulusmack: Have you seen this ticket?