From b850a4f0cc89a5f465c2b76a6d01e405a98218fd Mon Sep 17 00:00:00 2001
From: Minghui Yang <myang@yugabyte.com>
Date: Fri, 10 May 2024 01:39:09 +0000
Subject: [PATCH] [#22342] YSQL: load_relcache_init_file should clear
 yb_table_properties

Summary:
1. Related data structures:

```
typedef struct YbTablePropertiesData* YbTableProperties;

typedef struct RelationData
{
    ...
    YbTableProperties yb_table_properties; /* NULL if not loaded */
    ...
} RelationData;
```

Note that `yb_table_properties` is a pointer.

2. In `write_relcache_init_file`

```
        /* first write the relcache entry proper */
        write_item(rel, sizeof(RelationData), fp);
```
This is going to write the contents of RelationData as raw bytes to the rel cache init file fp.

3. In `load_relcache_init_file`, we load the raw bytes back into RelationData:

```
        /* first read the relation descriptor length */
        nread = fread(&len, 1, sizeof(len), fp);
        if (nread != sizeof(len))
        {
            if (nread == 0)
                break;          /* end of file */
            goto read_failed;
        }

        /* safety check for incompatible relcache layout */
        if (len != sizeof(RelationData))
            goto read_failed;

        /* allocate another relcache header */
        if (num_rels >= max_rels)
        {
            max_rels *= 2;
            rels = (Relation *) repalloc(rels, max_rels * sizeof(Relation));
        }

        rel = rels[num_rels++] = (Relation) palloc(len);

        /* then, read the Relation structure */
        if (fread(rel, 1, len, fp) != len)
            goto read_failed;
```

4. The bug here is that `yb_table_properties` is a pointer, it is a private memory
address in the PG backend process that invoked `write_relcache_init_file`, and
then it will be interpreted within the PG backend process that invoked
`load_relcache_init_file`. It is not valid to do that because it can point to some
random bytes meant for other things. In the worst case the same raw pointer may
point to an address that is not even mmap'ed into the PG backend process that
loads the data and can lead to SEGV if accessed.

This diff fixes the bug by adding to `load_relcache_init_file`:
```
        /* YB properties will be loaded lazily */
        rel->yb_table_properties = NULL;
```
We are already clearing yb_table_properties in `AllocateRelationDesc`.

Note:

This appears to be a recent change. In the past when `write_relcache_init_file` is invoked, we have not set `yb_table_properties` but after commit `07db0b57cee0aee372a6820638fd07cb20c499a9` we are because
in `ybcSetupScanPlan` we now call `YbGetTableProperties`:

```
static void
ybcSetupScanPlan(bool xs_want_itup, YbScanDesc ybScan, YbScanPlan scan_plan)
{
    Relation relation = ybScan->relation;
    Relation index = ybScan->index;
    YbTableProperties yb_table_prop_relation = YbGetTableProperties(relation);
```
Jira: DB-11248

Test Plan: jenkins

Reviewers: kfranz, fizaa

Reviewed By: fizaa

Subscribers: yql

Differential Revision: https://phorge.dev.yugabyte.com/D34931
---
 src/postgres/src/backend/utils/cache/relcache.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/src/postgres/src/backend/utils/cache/relcache.c b/src/postgres/src/backend/utils/cache/relcache.c
index c8ba1fadcc84..7cd45eea65ed 100644
--- a/src/postgres/src/backend/utils/cache/relcache.c
+++ b/src/postgres/src/backend/utils/cache/relcache.c
@@ -7893,6 +7893,9 @@ load_relcache_init_file(bool shared)
 		rel->rd_amcache = NULL;
 		MemSet(&rel->pgstat_info, 0, sizeof(rel->pgstat_info));
 
+		/* YB properties will be loaded lazily */
+		rel->yb_table_properties = NULL;
+
 		/*
 		 * Recompute lock and physical addressing info.  This is needed in
 		 * case the pg_internal.init file was copied from some other database